Grdowns/inactive region opacity #2061
Conversation
…into grdowns/inactive-region-opacity
| let settings: CppSettings = new CppSettings(this.RootUri); | ||
|
|
||
| let decoration: vscode.TextEditorDecorationType = vscode.window.createTextEditorDecorationType({ | ||
| opacity: String(settings.inactiveRegionOpacity) |
There was a problem hiding this comment.
Do we need any error validation here?
There was a problem hiding this comment.
Yeah, should be a number between 0 and 100...although maybe 0 is too low if the text becomes invisible.
There was a problem hiding this comment.
We don't do it elsewhere in the codebase. I believe the line of thought is CppSettings should always be constructible, as this.RootUri should always be valid. @bobbrow?
There was a problem hiding this comment.
I think Andrew is referring to the inactiveRegionOpacity setting
Extension/package.json
Outdated
| "C_Cpp.inactiveRegionOpacity": { | ||
| "type:": "number", | ||
| "default": 0.5, | ||
| "Description": "Controls the opacity of inactive preprocessor blocks. Scales between 0.0 and 1.0. This setting is overridden by dimInactiveRegions.", |
There was a problem hiding this comment.
I think it would be more clear if we said "This setting only applies when inactive region dimming is enabled."
Extension/package.json
Outdated
| }, | ||
| "C_Cpp.inactiveRegionOpacity": { | ||
| "type:": "number", | ||
| "default": 0.5, |
There was a problem hiding this comment.
Can you add "minimum": 0 and "maximum": 1 to the schema so that VS Code will squiggle values outside the valid range?
| let settings: CppSettings = new CppSettings(this.RootUri); | ||
|
|
||
| let decoration: vscode.TextEditorDecorationType = vscode.window.createTextEditorDecorationType({ | ||
| opacity: String(settings.inactiveRegionOpacity) |
There was a problem hiding this comment.
you shouldn't need to make this a string since you already enforced the inactiveRegionOpacity setting to be a string in settings.ts.
| let renderOptions: vscode.DecorationRenderOptions = { | ||
| light: { color: "rgba(175,175,175,1.0)" }, | ||
| dark: { color: "rgba(155,155,155,1.0)" }, | ||
| rangeBehavior: vscode.DecorationRangeBehavior.ClosedOpen |
There was a problem hiding this comment.
do we need to keep the ClosedOpen rangeBehavior?
Extension/package.json
Outdated
| }, | ||
| "C_Cpp.inactiveRegionOpacity": { | ||
| "type:": "number", | ||
| "default": 0.5, |
There was a problem hiding this comment.
Can this be 0.55 to match Visual Studio's default opacity?
| let settings: CppSettings = new CppSettings(this.RootUri); | ||
|
|
||
| let decoration: vscode.TextEditorDecorationType = vscode.window.createTextEditorDecorationType({ | ||
| opacity: String(settings.inactiveRegionOpacity) |
There was a problem hiding this comment.
Yeah, should be a number between 0 and 100...although maybe 0 is too low if the text becomes invisible.
| let renderOptions: vscode.DecorationRenderOptions = { | ||
| light: { color: "rgba(175,175,175,1.0)" }, | ||
| dark: { color: "rgba(155,155,155,1.0)" }, | ||
| rangeBehavior: vscode.DecorationRangeBehavior.ClosedOpen |
|
@bobbrow Can you unblock this now? |
|
Is it possible to restore the greyed text and leave users with the option to choose either grey inactive regions OR dimmed but coloured inactive regions? Some people do prefer greyed sections to dimmed-but-still-coloured. |
|
@olr666 It's possible, but I don't think we were planning to implement that unless we got more user feedback. We could accept a pull request for some bool grayInactiveRegions setting that defaulted to false...maybe the inactiveRegionOpacity could be used to change the gray color as well? |
|
I think adding an |
Add opacity setting for inactive preprocessor blocks to replace text greying