Skip to content
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

Add a setting for disabling rendering "intense" text as bold #10648

Closed
wants to merge 4 commits into from

Conversation

zadjii-msft
Copy link
Member

Summary of the Pull Request

This adds a new setting disableIntenseIsBold. It's a per-appearance, control setting, defaulting to false.

  • When set to false, we'll render text formatted with ^[[1m as bold. (the 1.10+ default)
  • When set to true, we won't. (the pre-1.10 behavior)

PR Checklist

Validation Steps Performed

image

Yea that works. Printed some bold text, toggled it on, the text was no longer bold. hooray.

@ghost ghost added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Jul 13, 2021
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

I'm approving, but please don't forget to...

  • update the schema
  • update the docs

I don't like that the setting is called disableIntenseIsBold. I wish we could remove the "disable" part, but then we would have a setting that defaults to true, and that's not great either. So imo, it's fine.

@@ -252,6 +254,7 @@ namespace Microsoft::Console::Render
// Preferences and overrides
bool _softwareRendering;
bool _forceFullRepaintRendering;
bool _intenseIsBold{ true };
Copy link
Member

Choose a reason for hiding this comment

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

nit: looks like all of the other ones are initialized in the ctor. We should probably move this there for consistency

Copy link
Member

Choose a reason for hiding this comment

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

We should start moving these things into the header honestly.

@@ -402,6 +402,7 @@
IsChecked="{x:Bind IsBellStyleFlagSet(4), BindBack=SetBellStyleTaskbar, Mode=TwoWay}" />
</StackPanel>
</local:SettingContainer>

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see you thought you had to edit Profiles.xaml haha. You can definitely undo this one.

@skyline75489
Copy link
Collaborator

Thanks @zadjii-msft for this!

Off: this seems to be a good place to also add “intense as bright” option. But I never really know where bright is enabled in DX engine.

@zadjii-msft
Copy link
Member Author

I'm gonna close this PR, as to not pollute this PR that's already got green checks. From discussion in sync today - we're not gonna implement the setting this way right now. We're instead gonna do the EnumFlagMapper thing, and do "intenseAs": "none"|"bold"|"bright"|"all" (naming tbd). I'm not gonna touch the "bright" setting for now, and just slam in the "bold"/"none" one.

ghost pushed a commit that referenced this pull request Aug 16, 2021
…10759)

## Summary of the Pull Request

This adds a new setting `intenseTextStyle`. It's a per-appearance, control setting, defaulting to `"all"`.
* When set to `"all"` or `["bold", "bright"]`, then we'll render text as both **bold** and bright (1.10 behavior)
* When set to `"bold"`, `["bold"]`, we'll render text formatted with `^[[1m` as **bold**, but not bright
* When set to `"bright"`, `["bright"]`, we'll render text formatted with `^[[1m` as bright, but not bold. This is the pre 1.10 behavior
* When set to `"none"`, we won't do anything special for it at all. 

## references
* I last did this in #10648. This time it's an enum, so we can add bright in the future. It's got positive wording this time.
* ~We will want to add `"bright"` as a value in the future, to disable the auto intense->bright conversion.~ I just did that now.
* #5682 is related

## PR Checklist
* [x] Closes #10576 
* [x] I seriously don't think we have an issue for "disable intense is bright", but I'm not crazy, people wanted that, right? #2916 (comment) was the closest
* [x] I work here
* [x] Tests added/passed
* [x] MicrosoftDocs/terminal#381

## Validation Steps Performed

<!-- ![image](https://user-images.githubusercontent.com/18356694/125480327-07f6b711-6bca-4c1b-9a76-75fc978c702d.png) -->
![image](https://user-images.githubusercontent.com/18356694/128929228-504933ee-cf50-43a2-9982-55110ba39191.png)


Yea that works. Printed some bold text, toggled it on, the text was no longer bold. hooray.


### EDIT, 10 Aug

```json
"intenseTextStyle": "none",
"intenseTextStyle": "bold",
"intenseTextStyle": "bright",
"intenseTextStyle": "all",
"intenseTextStyle": ["bold", "bright"],
```

all work now. Repro script:
```sh
printf "\e[1m[bold]\e[m[normal]\e[34m[blue]\e[1m[bold blue]\e[m\n"
```
DHowett pushed a commit that referenced this pull request Aug 25, 2021
…10759)

This adds a new setting `intenseTextStyle`. It's a per-appearance, control setting, defaulting to `"all"`.
* When set to `"all"` or `["bold", "bright"]`, then we'll render text as both **bold** and bright (1.10 behavior)
* When set to `"bold"`, `["bold"]`, we'll render text formatted with `^[[1m` as **bold**, but not bright
* When set to `"bright"`, `["bright"]`, we'll render text formatted with `^[[1m` as bright, but not bold. This is the pre 1.10 behavior
* When set to `"none"`, we won't do anything special for it at all.

* I last did this in #10648. This time it's an enum, so we can add bright in the future. It's got positive wording this time.
* ~We will want to add `"bright"` as a value in the future, to disable the auto intense->bright conversion.~ I just did that now.
* #5682 is related

* [x] Closes #10576
* [x] I seriously don't think we have an issue for "disable intense is bright", but I'm not crazy, people wanted that, right? #2916 (comment) was the closest
* [x] I work here
* [x] Tests added/passed
* [x] MicrosoftDocs/terminal#381

<!-- ![image](https://user-images.githubusercontent.com/18356694/125480327-07f6b711-6bca-4c1b-9a76-75fc978c702d.png) -->
![image](https://user-images.githubusercontent.com/18356694/128929228-504933ee-cf50-43a2-9982-55110ba39191.png)

Yea that works. Printed some bold text, toggled it on, the text was no longer bold. hooray.

```json
"intenseTextStyle": "none",
"intenseTextStyle": "bold",
"intenseTextStyle": "bright",
"intenseTextStyle": "all",
"intenseTextStyle": ["bold", "bright"],
```

all work now. Repro script:
```sh
printf "\e[1m[bold]\e[m[normal]\e[34m[blue]\e[1m[bold blue]\e[m\n"
```
@DHowett DHowett deleted the dev/migrie/f/10576-disable-intense-is-bold branch October 26, 2021 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a setting for disabling "intense is bold"
5 participants