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

Adds editorUnnecessaryCode.foreground. #53519

Closed

Conversation

benjaminjackman
Copy link

This commit allows users to set a color for unused code which had been an
unconfigurable behavior (defaulting to a shade of gray) until #51104 / #52218 when
it was replaced with setting that allows for either decreasing the unused
token's opacity while preserving its color and/or setting the color of
a 2px dashed border that "underlines" the unused token.

The previous behavior avoided some contrast issues introduced by #51104 / #52218
when reducing the opacity of some syntax colors that were already at
or just over the low-contrast frontier into squintland, the unlegible
country, a place no programmer should be forced gaze.

Conversely, tokens that don't contrast enough with themselves won't scan
well when the programmer is looking for unused tokens.

@msftclas
Copy link

msftclas commented Jul 4, 2018

CLA assistant check
All CLA requirements met.

@jrieken
Copy link
Member

jrieken commented Jul 4, 2018

@benjaminjackman fyi - if you want unused code to be more visible you can configure it to be an error/warning. That will combine both styles - the squiggle and the fading out.

@benjaminjackman benjaminjackman force-pushed the unusedColor branch 2 times, most recently from 442ad2d to 01200fa Compare July 4, 2018 13:55
This commit allows users to set a color for unused code which had been an
unconfigurable behavior (defaulting to a shade of gray) until microsoft#51104 / microsoft#52218 when
it was replaced with setting that allows for either decreasing the unused
token's opacity while preserving its color and/or setting the color of
a 2px dashed border that "underlines" the unused token.

The previous behavior avoided some contrast issues introduced by microsoft#51104 / microsoft#52218
when reducing the opacity of some syntax colors that were already at
or just over the low-contrast frontier into squintland, the unlegible
country, a place no programmer should be forced gaze.

Conversely, tokens that don't contrast enough with themselves won't scan
well when the programmer is looking for unused tokens.
@mjbvz
Copy link
Contributor

mjbvz commented Jul 18, 2018

Graying out was added as a temporary solution when this feature was first added. Once we moved to having an official API for unused variables, we went with fading the code out as overriding the color generally makes tokenization look busted. High contrast themes add an underline instead of fading the code

@jabacchetta
Copy link

jabacchetta commented Jan 6, 2019

I personally still prefer the grayed out foreground color over all of the options presented here (there's a good chance most developers coming from WebStorm will agree).

I'm not sure I understand the logic behind disabling this feature because of the way it makes syntax highlighting look. If a theme explicitly sets the unused code to be grayed out, users have the option to continue using that theme or install a different one.

Additionally, there are a lot of theme settings that can be changed to make things "look busted".

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants