- Notifications
You must be signed in to change notification settings - Fork33.8k
Add colorized bracket highlighting colours#132494
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Add colorized bracket highlighting colours#132494
Uh oh!
There was an error while loading.Please reload this page.
Conversation
hediet commentedSep 6, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks! Can you post a before/after screenshot of the same code region, so that it becomes very clear what the difference is? |
Ah, I've already added those screenshots to the issue that I raised, with the bracket colorization off and on too. |
hediet commentedSep 6, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@roblourens what do you think (I think it is your theme)? |
Not me :) |
roblourens commentedSep 9, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I don't think that individual themes have owners. You could run it by @misolori if you want another opinion. Looks fine to me though. |
@misolori What do you think? |
miguelsolorio commentedSep 10, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Here's a snapshot of the theme colors + bracket colors with the original vs the proposed colors: I'd probably keep the bracket colors closer to the original so this is what I would suggest: "editorBracketHighlight.foreground1":"#b58800","editorBracketHighlight.foreground2":"#d33682","editorBracketHighlight.foreground3":"#0B75BF", BeforeAfter |
Doesn't this conflict with the feature? I think we'd want to mirror the original as close as possible while still matching the theme
Fair point, I think an alternate color can work here as well |
I think it is actually not a bad idea to use white as one of three (or more) colors for specific themes: This feature is all about being able to see matching bracket pairs, not neccessarily about highlighting brackets from text. |
@misolori Is there anything I can do to get this over the line? What's the blocker here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for the ping, LGTM let's get this merged and see how it looks like on Insiders. Thank you!
Uh oh!
There was an error while loading.Please reload this page.
This PRfixes#132493
As suggested inthis comment by@hediet.
This shows an preview of what these colours would look like (generated from adding these settings to my local settings.json file.
edit by@hediet:
With default colors:

Without bracket pair colorization:
