microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
160.52k stars 28.1k forks source link

More than 6 colors in Bracket Pair Colorization? (Rainbow?) #139777

Open FatalMerlin opened 2 years ago

FatalMerlin commented 2 years ago

Hi!

First of all I appreciate the work done to improve this feature and make it so much more performant :)

Today I wanted to migrate to the new built-in bracket colorization feature but realized it doesn't support the color options of the original plugin. As far as I understand the VS Code implementation supports up to 6 colors, but is there any reason not to support the array configuration of the original plugin?

It would be beautiful, so I can keep using my rainbow brackets with the added performance improvement! 😄

Here's my config:

    "bracketPairColorizer.colorMode": "Consecutive",
    "bracketPairColorizer.showBracketsInRuler": true,
    "bracketPairColorizer.showBracketsInGutter": true,
    "bracketPairColorizer.consecutivePairColors": [
        "()",
        "[]",
        "{}",
        [
            "#ff0000",
            "#ff8000",
            "#ffbf00",
            "#ffff00",
            "#bfff00",
            "#00ff00",
            "#00ffbf",
            "#00ffff",
            "#00bfff",
            "#0080ff",
            "#0066ff",
            "#0000ff",
            "#8000ff",
            "#bf00ff",
            "#ff00ff",
            "#ff00bf",
            "#ff0080"
        ],
        "White"
    ],

And here's the beautiful result: image

I would really appreciate this 👍🏻

ArturoDent commented 2 years ago

Duplicate: https://github.com/microsoft/vscode/issues/134200

FatalMerlin commented 2 years ago

Duplicate: #134200

Duplicate has been closed due to lack of votes, however this is not requesting a new feature but rather feature parity with the extension this is supposed to replace. The extension now has a deprecation notice, which lead me to attempt an upgrade. However I can not migrate if the replacement is not feature-complete.

hediet commented 2 years ago

And here's the beautiful result:

While this screenshot looks beautiful indeed, brackets usually don't occur like that in common source code.

Are that many colors useful when reading common code? I doubt the colors can be distinguished easily when the brackets are separated by syntax-highlighted text.

FatalMerlin commented 2 years ago

@hediet while I agree that my showcase obviously won't appear in any common source code (excluding Lisp) it's rather helpful if you are using low indentation values (e.g. 2 spaces) and multiple layers of nested declarations, e.g. more common in JavaScript / TypeScript with nested objects, anonymous functions / lambdas etc.

Also I find it hard to limit the argument about the usefulness of this feature to code readability when this is also a cosmetic customization of the editor, especially with the theming support, thus leaning into personal taste IMO.

And since the original extension allowed for this customization, I don't understand why the superseding implementation does not.

But yes, I do find it quite useful, and so do some of my colleagues who adopted my config, especially in combination with the brackets shown in the ruler and / or gutter, since they help to distinguish closely intertwined scopes and are also pleasing to look at.

Please also note that this is making use of the "bracketPairColorizer.colorMode": "Consecutive" option, thus incrementing the color even with alternating bracket types, e.g.:

image

Or, small example that more closely resembles real-world code:

image

hediet commented 2 years ago

Would 8 colors be enough? I think that would cover the maximum depth in the screenshot.

The problem is that these colors are themable and themes don't support color lists. Thus, there has to be bracketPairColor1, bracketPairColor2, ... and so on.

FatalMerlin commented 2 years ago

The screenshot is not meant as a full depth example, merely to show how it looks in use. Even if I were using 8 colors, what about the next person that used 9, and then 10?

This was working with the original extension, and the implementation of the theme-support is now breaking the original features.

Is there any solution that actually allows for lists in any way? I almost thought this problem would be an implementation specific limitation.

Would the color list feature be beneficial to features beyond the bracket pair colorization? Could you jury-rig it so that you allow multiple colors in one value? E.g. bracketPairColors="#0000FF;#00FF00;#FF0000" ?

That would add flexibility and possibly limit property pollution for the themes.

Couldn't the colors be extended by support for arrays? It's based off of JSON as far as I know so the de-serialization should not be a problem, and if now possibly having more than one color might be incompatible you could default to the first color maybe?

If possible I could also try looking into this, though I haven't worked on the VS Code project yet so I'd be happy if you could point me to a good starting point :)

FatalMerlin commented 2 years ago

@hediet is there any update on when this might get implemented from your side?

hediet commented 2 years ago

No updates. Supporting lists in themes would be a major undertaking and is currently not on the roadmap.

JosephChomboM commented 2 years ago

I am new to this. is that code with a brackets library or is it defaults brackets code? and how can i put it in my vs code? @FatalMerlin

avicularian commented 2 years ago

The colour scheme in the OP is beautiful—props for sharing!

Programming in highly nested languages like any Lisp, or Dart with Flutter, is painful without rainbow bracket colour schemes. More than 6 colours is a necessity. I usually use 20—and it makes the code much, much easier to write. This needless, arbitrary limitation has had so many of us tearing our hair out in frustration.

My kakoune editor can do this in a plug-in that takes roughly a page of code, why can't the immensely popular VSCode?

Fix it. And please don't overcomplicate things.

Add the following settings:

editorBracketHighlight.foreground1
editorBracketHighlight.foreground2
editorBracketHighlight.foreground3
editorBracketHighlight.foreground4
editorBracketHighlight.foreground5
editorBracketHighlight.foreground6
editorBracketHighlight.foreground7
editorBracketHighlight.foreground8
...
editorBracketHighlight.foreground18
editorBracketHighlight.foreground19
editorBracketHighlight.foreground20

Resolve the (tiresome, interminably complicated) issue about lists in setting files later. Add this simple, easy-to-implement functionality that users of nested languages are lacking now.

I'll send the PR if that's a problem.

FatalMerlin commented 2 years ago

@YoguiBearPY there used to be an extension for this that was also the foundation for this feature in VS Code, they merely migrated the feature into the VS Code code-base and the extension is now deprecated AFAIK. Unfortunately the extension had way more configuration options which were dropped during the integration into VS Code.

FatalMerlin commented 2 years ago

@avicularian thanks for the positive feedback, I almost felt like I was alone in needing this. It's not just cosmetic, it really makes a difference, but it depends on your language and coding style.

Maybe your suggestion would be easier to approve than my implementation to support arrays for the color properties. Can you give any feedback @hediet ?

hediet commented 2 years ago

Can you share a screenshot of a real world situation where these colors really shine?

FatalMerlin commented 2 years ago

@hediet we already had this discussion and I have previously supplied examples, where you then already proposed this solution.

Is this sufficient or do you need us to supply another use-case?

Quote from you:

Would 8 colors be enough? I think that would cover the maximum depth in the screenshot.

The problem is that these colors are themable and themes don't support color lists. Thus, there has to be bracketPairColor1, bracketPairColor2, ... and so on.

hediet commented 2 years ago

Oh right, sorry, I missed that. I agree this is a valid feature request and I'll keep this issue in the backlog.

avicularian commented 1 year ago

Thanks, @hediet.

The problem for me, I think, is that once you start using a lot of colours on your brackets, the presence of said brackets provides a great psychological relief.

(Another thing to add is that human grades of colour perception vary quite significantly by individual — what feels like a big waste of the colour spectrum to me might not for some).

Anyway, the thing is, being 17 levels deep in nested brackets is extremely psychologically stressing — but when you have the colours set up to handle them, you suddenly feel "at home" knowing which level of the hierarchy you're at by the colour of the brackets.

Yet with the VSCode default setup, my brain quickly goes into panic mode once that second or third pair of red brackets is opened — I can no longer tell at a glance which of a pair goes with which.

It's easy to dismiss this as a trivial feature request, but it's a shame to lose the old customizability, and anyway once you've tried using this setup, you don't want to go back from it. (Of course, I generally try not to go 17 bracket pairs deep, but to be able to do so, without stress, makes for quick prototyping in languages like Clojure before I take a step back and split everything into smaller functions).

Here's my proposed solution:

This doesn't tread on any toes, gets the problem out of the way quickly, sends the bracket colour fans wild with appreciation, and can even be reverted at a future point without breaking anything too dear.

(And it also — in a roundabout way — enables the ability to have your 20+ brackets colours which change according to theme, as the VSCode API opens up the option of reactive setting changes for a clever extension developer)

avicularian commented 1 year ago

(https://github.com/microsoft/vscode/blob/82a0be36fee33b866fb5788d9bb10b32a11a812e/src/vs/editor/common/model/bracketPairsTextModelPart/colorizedBracketPairsDecorationProvider.ts#L122-L126)

What's the performance hit, on theme change, for dynamically changing the number of style collector rules if we go, say, to 7, 9, or 17 colours (and removing this modulo, of course)?

hediet commented 1 year ago

Anyway, the thing is, being 17 levels deep in nested brackets is extremely psychologically stressing

Well, I have to admit, this is extremely stressing, thus you should really avoid having code that is nested that deep!

What's the performance hit, on theme change, for dynamically changing the number of style collector rules if we go, say, to 7, 9, or 17 colours (and removing this modulo, of course)?

Performance is not the problem here, it is the amount of newly introduced colors.

avicularian commented 1 year ago

Again:

Oh well, due to the complete non-configurability of VSCode, and constant bad aesthetic decisions, I'll be dropping it.

FatalMerlin commented 1 year ago

Oh right, sorry, I missed that. I agree this is a valid feature request and I'll keep this issue in the backlog.

@hediet you already agreed multiple times that you can see the validity of this request, yet the discussion keeps looping around this point.

It's simple, really:

Therefore I kindly ask you to reconsider this request for implementation.

As previously mentioned I already submitted a POC PR that was rejected due to coding style issues by a senior engineer of yours. Please feel free to adapt my solution such that it fits your engineering requirements of VS Code.

The effort for this implementation is very minimal, and much more time already went into this discussion than would have been needed for the implementation.

Hopeful, Merlin

noenapply commented 1 year ago

Great work @FatalMerlin, I hope it was not in vain. I still use the deprecated extension due to this limitation.

textzenith commented 1 year ago

Me too, still using the "deprecated" extension, probably going to have to make my own little VSCodium fork 😂. Ridiculous!

FatalMerlin commented 1 year ago

@hediet instead of my current proposed PR #180586, do you think it would make sense for me to invest the time to properly implement color list support?

This could for example come in handy in themes beyond the scope of this requeest, as it could be used to allow themes to provide e.g. a list of accent colors that users can utilize to quickly customize given themes without overwriting theme values by hand.

Or would you rather take a look at the current PR and discuss this seperately? Happy to have a talk about this.

rakkarage commented 8 months ago

image

Screenshot from 2023-10-31 23-03-07

deity-jeroen commented 6 months ago

Looking forward to the fix here being merged: https://github.com/microsoft/vscode/pull/180586

Going by this thread everyone seems to agree that there is a valid need. If it's not currently worth implementing support for color lists, then it ought to be acceptable to introduce the 18 extra colors to address this need.

FatalMerlin commented 6 months ago

@deity-jeroen , thanks to your comment I just checked & updated the PR so that it stays ready for review. One day :) It is at least in the backlog.

Saturnine-Softworks commented 4 days ago

Just chiming in as a developer using TS/JS + Svelte. Good gods, 6 colors is not enough. What an absurd oversight, which I can only see as being motivated by somebody wanting to impose their opinionated view of proper code structure on other developers.

deity-jeroen commented 4 days ago

@Saturnine-Softworks I'm sure it wasn't any sort of malicious to start with but by this point the maintainers need (a multitude of) clear demonstrations of circumstances where this would be necessary to prioritise the change high enough. If you can post some good example screenshots with context I think that would help our cause.

Saturnine-Softworks commented 3 days ago

@deity-jeroen I agree, I don't think it's necessarily malicious. And while I can understand why this might not be a priority, what baffles me is that it's something trivial to address which others have already proposed solutions for. I don't manage any software nearly so large as VSCode, so I may not understand the pipeline and schedule these developers stick to, of course. Even so, from the consumer side of VSCode, this just really seems like an easy to change, arbitrary limitation, which we didn't have when this feature was a plugin.

Anyways, a couple of screenshots of some Typescript code my app uses. The pinkish-reddish color is the 6th one. You can see in some places here I almost cycle around twice, so ~12 layers. If I were to scour my code I could probably find some deeper nesting. I won't claim this is the most well-written code in the world, but as others have pointed out, that's out of scope here.

Screenshot 2024-07-03 at 22 04 37 Screenshot 2024-07-03 at 22 05 58
Saturnine-Softworks commented 3 days ago

In a Svelte file, using the EditorJS API, around 12 layers nesting

Screenshot 2024-07-03 at 22 25 40

Here I even reach 7 layers nesting in Rust in some code for handling JS/Rust interop. It's only one color recycled, but I think the fact that this level of nesting occurs in the first place is enough of an argument.

Screenshot 2024-07-03 at 22 32 19
Saturnine-Softworks commented 3 days ago

Using a parser library called nom in Rust. Easily ~8 layers nested.

Screenshot 2024-07-03 at 22 39 12

And some more casual typescript in a svelte file, ~10 layers nested. The repeating colors really start to throw me off at around this depth.

Screenshot 2024-07-03 at 22 49 41