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

Add menu item to toggle code lenses #2351

Merged
merged 3 commits into from
Nov 5, 2023
Merged

Add menu item to toggle code lenses #2351

merged 3 commits into from
Nov 5, 2023

Conversation

rchl
Copy link
Member

@rchl rchl commented Nov 1, 2023

Code Lens is one of those features that is IMO too annoying to have always enabled but can be useful to use once in a while to catch un-referenced symbols and such so I think it deserves its own toggle like that.

Threw it in very quickly based on Inlay Hints and Hover Popups toggles. Might have missed something.

Main.sublime-menu Outdated Show resolved Hide resolved
plugin/code_lens.py Outdated Show resolved Hide resolved
@predrag-codetribe
Copy link

predrag-codetribe commented Nov 2, 2023

based on the PR title,
I would say that you explicitly just added the command in the menu.

Currently for inlay hints there is a command in the command palette,
do you think that it should be added there, for consistency?

Also by looking at the name "LSP: Show Code Lens"
it is not obvous that this command can be toggled? "LSP: Toggle Code Lens" makes more sense to me

But I get why "LSP: Toggle Code Lens" was choosen, because inlay hints toggle also uses "LSP: Show Inlay Hint"

@rchl
Copy link
Member Author

rchl commented Nov 2, 2023

Also by looking at the name "LSP: Show Code Lens"
it is not obvous that this command can be toggled? "LSP: Toggle Code Lens" makes more sense to me

consistency

Screenshot 2023-11-02 at 23 43 50

As far as whether it's obvious or not that it can be toggled... this is about naming conventions for checkboxes. If it's clear that it's a checkbox then it would be even enough to just say "LSP: Code Lenses" since it would be clear from the context that it's a toggle. On Mac it's not that obvious though when checkbox is disabled so maybe that's why "Show..." is better.

And of course we should try to be consistent with ST but it seems like ST uses checkboxes very sparingly. It prefers to instead change the item title based on the state (showing "Show..." or "Hide...").

@rchl rchl merged commit 104e4eb into main Nov 5, 2023
4 checks passed
@rchl rchl deleted the feat/code-lens-toggle branch November 5, 2023 20:26
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

3 participants