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

some feedback on Code Lens API defaults #206

Open
pickx opened this issue Apr 15, 2024 · 8 comments
Open

some feedback on Code Lens API defaults #206

pickx opened this issue Apr 15, 2024 · 8 comments

Comments

@pickx
Copy link

pickx commented Apr 15, 2024

so this is a cool feature (thank you for implementing this, and of course everyone involved in maintaining this extension)

that said, coming from my Rust + rust-analyzer setup with completely stock Error Lens, I found the stock experience a bit jarring:

  1. default errorLens.codeLensTemplate is $severity $message which doesn't match my default errorLens.messageTemplate of $message. this has the effect of suddenly putting emojis all over the editor (due to defaults of errorLens.severityText). this just looks quite weird. is the intention to make it easier to distinguish severity, because Error Lens has no colors? then I think emojis should not be the default - I tried "ERROR:", "WARNING:", "INFO:", "HINT:" instead and I actually like it. either that or make errorLens.codeLensTemplate default to $message.
  2. with the default errorLens.codeLensLength.max of 100, most of my messages were truncated. with the small font size of Code Lens, I don't see a reason not to use a larger value like 300.
  3. maybe the term "message" could be changed, since it's a little opaque. how about something like "inline message"?
  4. I think the settings errorLens.messageEnabled and errorLens.codeLensEnabled should be merged as this will improve discoverability, and if possible, make it the top setting in VSCode GUI config editor. maybe also add errorLens.statusBarMessageEnabled in there, and make the title "show error messages in:" or something like that.
@duncanawoods
Copy link

duncanawoods commented Apr 15, 2024

Hello fellow rust-analyzer user :)

1. Yep, the lack of colours.

I'm half and half. Personally I really need the colour cue to spot errors without reading the lens text. A single coding error can generate a few errors, a few warnings and half a dozen hints. I'm not an emoji fan but error/warning as emoji has helped me.

I do find the green apple for hints too much though. I don't want green blobs everywhere for potentially dozens of hints. Something without colour and understated like ↘ for hints works nicely for me. I wouldn't want rust-analyzer users to be incentivised to suppress hints because they are super helpful for e.g. tracking borrowing through a function.

2. I agree on the message length.

I have mostly been using 999 but I can also recommend something a bit shorter than your typical window like 200 which is enough to get the gist but still see if there are other diagnostics on the same line warranting mouse-hovering over the lens. There can be multiple errors on the same line and the first one isn't always the most revealing.

Perhaps there should be a subtle indication of the number of errors on a line at the start of the lens.

3.

4. On the sorting.. it's alphabetic. It could be forced with hacky names but.. eh, probably a useful vscode feature request because even with the search and filtering, I spend a lot of painful time scrolling/hunting options. I end up using settings json when I'm experimenting with options because the settings UI can also reload and lose your position.

@pickx
Copy link
Author

pickx commented Apr 15, 2024

  1. On the sorting.. it's alphabetic. It could be forced with hacky names but.. eh, probably a useful vscode feature request because even with the search and filtering, I spend a lot of painful time scrolling/hunting options. I end up using settings json when I'm experimenting with options because the settings UI can also reload and lose your position.

this segues to another point: playing with the settings of this extension today for the first time (because until today it Just Worked™), it feels like the extension's settings could use some consolidation/reorganization. it's great that it exposes so much customization, don't get me wrong, it's just difficult to navigate.

(again, I hope this doesn't come off the wrong way, since this is a good extension and I've been getting a lot of mileage out of it)

@usernamehw
Copy link
Owner

  1. I like the default as it is $severity $message. Would it be less jarring if code lens only showed 1 problem instead of multiple?
  2. errorLens.codeLensLength.max and errorLens.codeLensLength.min should probably be applied to each message, not differently as they are now. I'll change the default of max to 200 or 300 or 500.
  3. Don't want to make any breaking changes.
  4. errorLens.messageEnabled and errorLens.codeLensEnabled are different features. I think some people might use extensions to toggle settings in which case it would work better as they are now. Assigning "order" property would probably be the best way to sort them in GUI.

@usernamehw
Copy link
Owner

usernamehw commented Apr 20, 2024

I do find the green apple for hints too much though. I don't want green blobs everywhere for potentially dozens of hints.

Code lens should respect setting errorLens.enabledDiagnosticLevels which has hints disabled by default. If someone enables messages for "hint" - they can change another setting for which emoji to show.

@usernamehw
Copy link
Owner

it's great that it exposes so much customization, don't get me wrong, it's just difficult to navigate.

It would probably be easier if this extension had documentation.

@usernamehw
Copy link
Owner

usernamehw commented Apr 20, 2024

Emoji don't even render properly depending on OS or fallback font.

emoji_demo

Example of fallback for emoji:

"editor.codeLensFontFamily": "'Apple Color Emoji', 'Segoe UI Emoji', 'Noto Color Emoji'",

demo4

@usernamehw
Copy link
Owner

usernamehw commented Apr 20, 2024

🐛 Code lens doesn't respect errorLens.enabled

@usernamehw
Copy link
Owner

🐛 Code lens doesn't respect errorLens.enabledDiagnosticLevels

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

No branches or pull requests

3 participants