Skip to content

Add numeric values support for terminal.integrated.fontWeight#106072

Merged
Tyriar merged 2 commits intomicrosoft:masterfrom
IllusionMH:terminal-numeric-font-weight-101467
Sep 9, 2020
Merged

Add numeric values support for terminal.integrated.fontWeight#106072
Tyriar merged 2 commits intomicrosoft:masterfrom
IllusionMH:terminal-numeric-font-weight-101467

Conversation

@IllusionMH
Copy link
Copy Markdown
Contributor

@IllusionMH IllusionMH commented Sep 3, 2020 •
edited by Tyriar
Loading

Fixes #106347
This PR supersedes #101468
Depends on xtermjs/xterm.js#3062

Incorporates feedback from #101467 (comment) and #105880 (comment)

I've simplified FontWeight to skip strings with numeric values and use number instead. This obviously leads to type error in terminal constructor call but works as expected and can be merged when Xterm.js updated with number type support. No type errors after xterm@4.9.0-beta.32 update.

/assign @Tyriar

@IllusionMH IllusionMH mentioned this pull request Sep 3, 2020
Closed
@Tyriar Tyriar added this to the September 2020 milestone Sep 3, 2020
Tyriar added a commit that referenced this pull request Sep 8, 2020
@Tyriar
xterm@4.9.0-beta.32, xterm-addon-webgl@0.9.0-beta.4
ec870ee 
Fixes #106248
Fixes #105979
Part of #101467
Part of #106072
@Tyriar
Copy link
Copy Markdown
Contributor

Tyriar commented Sep 8, 2020

I pulled the xterm.js change in ec870ee

@IllusionMH IllusionMH force-pushed the terminal-numeric-font-weight-101467 branch from c9f642a to cd3c52b Compare September 8, 2020 16:22
@IllusionMH
Copy link
Copy Markdown
Contributor Author

IllusionMH commented Sep 8, 2020

@Tyriar thank you for Xterm PR review and update in this repo!

I've rebased on top of master - TS error is gone and continues to works as expected in different ranges (also removed unused import).

PR is ready for review and merge.

@Tyriar Tyriar mentioned this pull request Sep 9, 2020
Closed
Tyriar
Tyriar approved these changes Sep 9, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, works great!

@Tyriar Tyriar merged commit f4833ee into microsoft:master Sep 9, 2020
@IllusionMH
Copy link
Copy Markdown
Contributor Author

IllusionMH commented Sep 9, 2020

Thanks for review!

@IllusionMH IllusionMH deleted the terminal-numeric-font-weight-101467 branch September 9, 2020 13:02
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@Tyriar Tyriar Tyriar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Tyriar Tyriar

Labels

None yet

Projects

None yet

Milestone

September 2020

Development

Successfully merging this pull request may close these issues.

Allow number values for terminal.integrated.fontWeight and fontWeightBold

2 participants

@IllusionMH @Tyriar