New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide "terminal.integrated.enableBold" setting connected to #22422 #22465

Merged
merged 12 commits into from Mar 20, 2017

Conversation

Projects
None yet
4 participants
@lifez
Contributor

lifez commented Mar 12, 2017

Fixes #22422

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Mar 12, 2017

@lifez, thanks for your PR! By analyzing the history of the files in this pull request, we identified @alexandrudima and @Tyriar to be potential reviewers.

mention-bot commented Mar 12, 2017

@lifez, thanks for your PR! By analyzing the history of the files in this pull request, we identified @alexandrudima and @Tyriar to be potential reviewers.

@msftclas

This comment has been minimized.

Show comment
Hide comment
@msftclas

msftclas Mar 12, 2017

@lifez,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

msftclas commented Mar 12, 2017

@lifez,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

@msftclas

This comment has been minimized.

Show comment
Hide comment
@msftclas

msftclas Mar 12, 2017

@lifez, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

msftclas commented Mar 12, 2017

@lifez, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

@Tyriar Tyriar added this to the March 2017 milestone Mar 12, 2017

@lifez

This comment has been minimized.

Show comment
Hide comment
@lifez

lifez Mar 12, 2017

Contributor

I have rethink about it.It would be great if we can set font-weight of terminal

The reason is when I use vscode on osx with bold font in terminal

the integrate terminal text is not bold

What do you think? @Tyriar

Contributor

lifez commented Mar 12, 2017

I have rethink about it.It would be great if we can set font-weight of terminal

The reason is when I use vscode on osx with bold font in terminal

the integrate terminal text is not bold

What do you think? @Tyriar

@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Mar 13, 2017

Member

So you want all bold text? I wonder if you can set the font family to the be the bold variant? Ie. font-family: "Courier New Bold" or something?

Member

Tyriar commented Mar 13, 2017

So you want all bold text? I wonder if you can set the font family to the be the bold variant? Ie. font-family: "Courier New Bold" or something?

@lifez

This comment has been minimized.

Show comment
Hide comment
@lifez

lifez Mar 13, 2017

Contributor

Oh, ok I miss this point :)

Contributor

lifez commented Mar 13, 2017

Oh, ok I miss this point :)

@lifez

This comment has been minimized.

Show comment
Hide comment
@lifez

lifez Mar 17, 2017

Contributor

Could you guide me a little about event listener on IConfigurationService I don't see some code about append css ?

Contributor

lifez commented Mar 17, 2017

Could you guide me a little about event listener on IConfigurationService I don't see some code about append css ?

@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Mar 17, 2017

Member

@lifez sure, you will want to listen to the IConfigurationService within terminalPanel.ts and add the class disable-bold to this._parentDomElement. Listening to config updates can be seen here:

this._register(this._configurationService.onDidUpdateConfiguration(() => this._updateFont()));

Then you will want to add the styles to terminal.css, font-weight: normal !important is probably fine.

Member

Tyriar commented Mar 17, 2017

@lifez sure, you will want to listen to the IConfigurationService within terminalPanel.ts and add the class disable-bold to this._parentDomElement. Listening to config updates can be seen here:

this._register(this._configurationService.onDidUpdateConfiguration(() => this._updateFont()));

Then you will want to add the styles to terminal.css, font-weight: normal !important is probably fine.

@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Mar 17, 2017

Member

@lifez I tried to resolve the conflicts using GitHub's web UI for the first time to help you out, hopefully I didn't break compilation or anything 😄

Member

Tyriar commented Mar 17, 2017

@lifez I tried to resolve the conflicts using GitHub's web UI for the first time to help you out, hopefully I didn't break compilation or anything 😄

@lifez

This comment has been minimized.

Show comment
Hide comment
@lifez

lifez Mar 17, 2017

Contributor

@Tyriar Thank you

Then i will do something like this DOM.toggleClass(this._parentDomElement, 'disable-bold') ?

Contributor

lifez commented Mar 17, 2017

@Tyriar Thank you

Then i will do something like this DOM.toggleClass(this._parentDomElement, 'disable-bold') ?

@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Mar 17, 2017

Member

@lifez yes, but prefer the explicit value instead of toggle:

DOM.toggleClass(this._parentDomElement, 'disable-bold', !isEnabled);
Member

Tyriar commented Mar 17, 2017

@lifez yes, but prefer the explicit value instead of toggle:

DOM.toggleClass(this._parentDomElement, 'disable-bold', !isEnabled);
@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Mar 20, 2017

Member

Thanks @lifez, I tweaked things a bit and this is good to go 👍

Member

Tyriar commented Mar 20, 2017

Thanks @lifez, I tweaked things a bit and this is good to go 👍

@Tyriar Tyriar merged commit 8dc5a60 into Microsoft:master Mar 20, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@lifez lifez deleted the lifez:terminal-enable-bold branch Mar 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment