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

Feature Request: Colored line scopes #131001

Closed
hediet opened this issue Aug 17, 2021 · 20 comments
Closed

Feature Request: Colored line scopes #131001

hediet opened this issue Aug 17, 2021 · 20 comments
Assignees
Labels
bracket-pair-colorization feature-request Request for new features or functionality on-testplan

Comments

@hediet
Copy link
Member

hediet commented Aug 17, 2021

Now that bracket pair colorization landed, we can do more advanced stuff like this:

(Idea by @CoenraadS)

@hediet hediet added the feature-request Request for new features or functionality label Aug 17, 2021
@hediet hediet added this to the Backlog milestone Aug 17, 2021
@hediet hediet self-assigned this Aug 17, 2021
@hediet
Copy link
Member Author

hediet commented Aug 17, 2021

Also see #130379 for a related feature.

@CoenraadS
Copy link
Contributor

CoenraadS commented Aug 17, 2021

I will include this screenshot also, as it shows more 'smart' behavior that would be nice to have.

image
image
image
image

This is a more general smart indent line feature, and not actually related to the coloring though. Just putting it out there. I believe it is one of the reasons my extension so popular. I even got emails from teachers about how much it helped their students.

@Stanzilla
Copy link

Any plans to also add the "consecutive/independent color mode" and "force unique opening color" settings?

@hediet
Copy link
Member Author

hediet commented Aug 17, 2021

@Stanzilla please open a separate issue for these two features!
In these feature requests, please provide examples that demonstrate the usefulness of the feature.

@Stanzilla
Copy link

Done!

#131027
#131029

@byehack
Copy link

byehack commented Aug 17, 2021

Any plans to also add the "show Brackets In Ruler" settings?

it's really usefull when brackets are so long. we can see first/end of bracket in ruller with specified color in lef/middle/right of ruler.

@hediet
Copy link
Member Author

hediet commented Aug 18, 2021

@byehack please file an issue request!

@hediet
Copy link
Member Author

hediet commented Aug 18, 2021

Maybe this feature could even be implemented in an extension if some version of #131062 (comment) lands:
On every cursor change, query all brackets pairs that intersect with the cursor position and create these line decorations for the inner-most bracket pair.


Also, I think it might make sense to only show these colored line scopes when the distance of opening/closing bracket is less than 500 lines apart (it is very cheap to compute the distance).
In this case, you can quickly iterate over all lines in between, find the smallest indentation and setup all decorations.

For a performant implementation that spans thousands of lines, you should not walk all lines in between on every cursor move operation. In this case, the tree data structure needs to be extended so that you can cheaply compute the smallest indentation in logarithmic time.
However, this probably will prevent reusing text nodes.
Also, you would need to compute the line decorations on demand for the current viewport.

@sanket-bhalerao
Copy link

@hediet is this feature separate from the editor.bracketPairColorization.enabled?
I don't feel comfortable(at least at this point in time) with the brackets being colored differently. However, I would love to see the line scopes as shown by @CoenraadS. If it's a possibility I would love to enable the line scopes and keep the bracket pair colorization disabled.

@hediet
Copy link
Member Author

hediet commented Aug 20, 2021

If implemented, this feature could be enabled independently, yes.

@JustinGrote
Copy link
Contributor

This is the only thing preventing me from removing the BPC2 extension.
image

@MatthewCushing
Copy link

I will include this screenshot also, as it shows more 'smart' behavior that would be nice to have.

image
image
image
image

This is a more general smart indent line feature, and not actually related to the coloring though. Just putting it out there. I believe it is one of the reasons my extension so popular. I even got emails from teachers about how much it helped their students.

I personally can't live without this feature after the years I've been using it with your extension. Like @JustinGrote said, it is the only thing preventing me from moving to VS Code's native version. Thanks for all the work you've done on your extension and helping getting it into native @CoenraadS and the VSCode team.

@omer-bar
Copy link

omer-bar commented Sep 2, 2021

I upgraded to the native VSCode bracket pair colorization and when i realized that this feature isn't implemented i turned it off and reinstalled BPC2, I think this feature is one of the best and most helpful features in the extension ecosystem.
I also feel this is important for us developers to say this here, so the implementation of this feature will happen and not get thrown away to the end of the line. there is a real and i suspect large audience of developers who need this feature.

@hediet
Copy link
Member Author

hediet commented Sep 27, 2021

Implemented in #133509.

We will iterate on the feature we implemented and differs a little bit from the requested feature.

Mainly, scope lines for all bracket pairs are shown, not only for the active one.
But indentation is not considered and there are no horizontal lines.

@hediet hediet closed this as completed Sep 27, 2021
@jjspace
Copy link

jjspace commented Sep 27, 2021

@hediet Just looking at the screenshots in that PR I don't personally feel this feature has been implemented and this issue should remain open to track it. At the very least follow up issues should be created so it doesn't get forgotten.

scope lines for all bracket pairs are shown

This is not how BPC2 worked and I could easily see people thinking this adds too much visual clutter distracting from the point of highlighting the current block/bracket pair you're in. To @CoenraadS's comment "I even got emails from teachers about how much it helped their students". If every block's guide is shown then this can't happen as easily

there are no horizontal lines

However more importantly, this is the biggest benefit of this feature and the whole reason to use it in my opinion. Just look at the difference it makes in @JustinGrote's comment #131001 (comment) or the main screenshots shared by @CoenraadS #131001 (comment). I already use editor.renderIndentGuides which gives much the same effect as what is shown in the PR so the changes you made would not help me much. However if the horizontal lines are added then I think I (and others) would actually switch away from BPC2

@JustinGrote
Copy link
Contributor

@hediet agree with jjspace on both the visual clutter and the horizontal lines, I'm ok with an additive setting as a future iteration, but this is not sufficient to move me from BPC2 (your hard work and efforts are appreciated however!)

@Nahor
Copy link

Nahor commented Sep 27, 2021

And what happens when there are multiple brackets on the same line? e.g. last line of:
image

Bracket guides can overlap badly formatted text (fixing this is a non-trivial follow up feature and this case should not happen often anyway):

For C/C++, this will happen for macros and goto labels. Both could be quite common:
image

@silkfire
Copy link

silkfire commented Sep 27, 2021

I have to say that it looks like a very poor implementation, not even close to the original.

image

@JustinGrote
Copy link
Contributor

@silkfire as they said they are going to iterate on this, it may just be baby steps and they have to carefully consider performance and stability. I'm willing to give them the benefit of the doubt, they've been clear this is just a first iteration.

@hediet
Copy link
Member Author

hediet commented Sep 27, 2021

@JustinGrote thanks for creating the follow-up issue!

I fully understand that the implemented feature currently is a strict subset of the BP2 functionality.
Because we cannot easily take features back once they are generally available, we rather try to start with a small feature-set, incrementally iterate on it and see what the community-feedback is.

If we would have directly implemented the full BP2 feature of colored line scopes, we would not have learnt if/why a simpler version of the feature isn't enough.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bracket-pair-colorization feature-request Request for new features or functionality on-testplan
Projects
None yet
Development

No branches or pull requests