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

Introduce minimap section headers, a la Xcode #190759

Merged
merged 28 commits into from Mar 18, 2024

Conversation

dgileadi
Copy link
Contributor

This addresses #74843. It adds section headers to the minimap, sourced from region markers and MARK: comments:

vscode-minimap

MARK: comments behave the same way as they do in Xcode's minimap—if the comment contains a hyphen before the header text then it displays a horizontal separator line above the section header.

This PR adds three settings to control the appearance of the section headers, or to turn them off:

vscode-section-header-settings

Feedback, testing and code fixes are very welcome!

@andyjeffries
Copy link

I've downloaded this and the patch works absolutely lovely with plain Ruby files:

Screenshot 2023-08-31 at 08 50 43

I don't know if this is a problem with the patch though or the CraigMaslowski.erb extension, but it doesn't work for either comment style in .html.erb files:

Screenshot 2023-08-31 at 08 51 54

@dgileadi
Copy link
Contributor Author

I don't know if this is a problem with the patch though or the CraigMaslowski.erb extension, but it doesn't work for either comment style in .html.erb files:

It seems to be a problem with the extension. The code for detecting section headers uses the region markers and comment markers from the language configuration file. That extension's language configuration file defines /* and */ for its block comments, as well as // for its single line comments. If you make a comment using that style, you'll see the section header:
Screenshot 2023-08-31 at 9 20 05 AM

@andyjeffries
Copy link

Thanks @dgileadi , I've raised an issue craigmaslowski/vscode-erb#5 in the extension repo.

@andyjeffries
Copy link

That extension is deprecated (but I still had it installed and thought VS Code). Just as a follow up, I'm actually using Shopify's Ruby LSP extension and iit's working perfectly. Hopefully soon this can be merged...

Screenshot 2023-09-03 at 11 23 52

@andyjeffries
Copy link

Hey @alexdima is there anything we can do as the community to help get this merged in and released? Thanks.

Copy link
Member

@alexdima alexdima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this looks amazing!

The problem is that this is not implemented in a performant manner, so I can't accept this in the current shape. We need that the view is very fast in rendering a frame, or in reacting to a change event. The goal is to have the event reaction time + rendering time of the entire editor be under 10ms, to leave some browser slack time and allow for 60FPS.

The problem with the proposed change is that there are heavy computations done during view event handling/during rendering. These are easy to spot by looking for the compute calls which pass in the entire file. These will grind rendering to a halt for a very large file (100k lines, etc). The computation, by calling tokenizeIfCheap with incrementing line numbers, will also basically trigger synchronous tokenization of the entire buffer. tokenizeIfCheap will tokenize a line if the previous line is tokenized.

For similar problems, we have taken a different approach. For example, for detecting links in the editor, we have a link contribution which reacts to change events with a debounce pattern (so not every file change triggers a recomputation). The actual computation of detecting links is then done on a web worker, and the results are then collected and transformed into decorations which are set on the text model. The decorations are stored in a bunch of efficient interval trees which allows updating them while typing efficiently and also allows for querying them efficiently based on reading patterns (decorations that go in the overview ruler are stored in a different tree than those that don't, because the overview ruler always fetches them all, while the others are usually fetched for the viewport).

I would also recommend that here we implement this using pattern:

  • we would have a piece of code (probably in editor/contrib) which reacts to when an editor gets set a new text model or to when the text model changes in a debounced way.
  • it then can scan for these markers either on the UI thread with a time cap or on the web worker.
  • once it has computed the locations, it sets decorations with those ranges on the text model
  • the decorations need a new field to indicate the extra special minimap rendering
  • at each frame, the minimap reads decorations from the text model and renders them as possible
  • in the rendering code, we need to be as minimalistic as possible to repaint only when necessary, etc.

@dgileadi
Copy link
Contributor Author

Thanks for the feedback, @alexdima. Your recommended changes are probably best implemented by someone familiar with the vscode code base (and of course with dedicated vscode development time). In the mean time, could you point me to the code in vscode that implements the link detection? That would help me better understand how more efficient section header detection could work.

@alexdima
Copy link
Member

Sure

The link computation might appear a bit too complex because it goes through the language feature registry because there's vscode API for extensions to contribute link providers. Maybe a simpler example is the UnicodeHighlighter:

@dgileadi
Copy link
Contributor Author

dgileadi commented Oct 5, 2023

Okay @alexdima I think I've implemented what you suggested. Let me know what you think. Thanks!

@pouyakary
Copy link
Contributor

@andyjeffries you're very welcome, and yes! while this @dgileadi has made an exceptional job on this; your championing also counts. Thank you very much for this!

@dgileadi, again! Thanks a lot! I too really wished to have this!

@andyjeffries

This comment was marked as spam.

@pouyakary

This comment was marked as spam.

@MaxLikesCode

This comment was marked as spam.

@dgileadi
Copy link
Contributor Author

Thanks @alexdima for your attention and helpful changes!

Copy link
Member

@alexdima alexdima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Excellent PR!

I did one small conceptual tweak where we now parse all mark headers and then eliminate the ones not in comments on the UI side, where we have good tokenization information available.

@alexdima alexdima enabled auto-merge (squash) March 18, 2024 07:53
@alexdima alexdima added this to the March 2024 milestone Mar 18, 2024
@alexdima alexdima merged commit c800bf9 into microsoft:main Mar 18, 2024
6 checks passed
chen-ky pushed a commit to chen-ky/vscode that referenced this pull request Mar 18, 2024
* WIP for adding minimap section headers for microsoft#74843

* Get section headers rendering

* Fix default value of section header font size

* Fix tests

* Improve section header position

* Fix separator display, update after config change

* Split too-long headers with an ellipsis

* Render section headers on the decorations canvas

* Support MARK with just a separator line

* Calculate minimap section headers asynchronously

* Simplify change

* Avoid font variable duplication

* Fix issue introduced earlier

* Recompute section headers when the language configuration changes

* Fix problem in constructing region header range

* Parse mark headers in the entire file and then filter out the ones not appearing in comments on the UI side, where tokens info is available

---------

Co-authored-by: Alexandru Dima <alexdima@microsoft.com>
@pouyakary
Copy link
Contributor

@dgileadi I just wanted to thank you and also @alexdima, @andyjeffries, and @sandy081. I wished for this feature to exist for so many years and finally having it means the world to me. Thank you so much for all the efforts ❤️, I just opened vscode and got the update and it made my day.

@detzt
Copy link

detzt commented Apr 7, 2024

Such a pleasant surprise to see this after updating VSCode. I didn't even know I wanted this, but now I already feel how it improves my life. Thank you ❤️

@NHUJI
Copy link

NHUJI commented Apr 9, 2024

This is a feature I've been dreaming of! I've added MARK to my codebase before as a way to manually differentiate between codes, and now it's actually useful!❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants