-
Notifications
You must be signed in to change notification settings - Fork 64
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
feat: add inlay hint support #70
base: main
Are you sure you want to change the base?
Conversation
themes/gruvbox-dark-hard.json
Outdated
"editorInlayHint.background": "#3c383680", | ||
"editorInlayHint.foreground": "#928374", | ||
"editorInlayHint.typeForeground": "#d79921", | ||
"editorInlayHint.paramForeground": "#458588", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be parameterForeground
:
https://github.com/microsoft/vscode/blob/2f582604fbdcedf1deff493ff7afa996d0d68c0f/src/vs/platform/theme/common/colorRegistry.ts#L391-L396
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
(Note: I am just a user of this theme, not a maintainer) I think your changes greatly improve the visuals of inlay hints compared to master. However, I would personally prefer a bit more contrast between the background of the inlay hints and the overall background of the theme, because with this version they blend in with the "real" code a bit. |
For me the current background works just fine, but if adding a bit of contrast makes it better I'll do that. You can test out the idea yourself by changing the line "editorInlayHint.background": "#3c383680", into "editorInlayHint.background": "#3c3836", which makes the background opaque instead of 50% transparent. I'm looking forward to your test results! |
@lynzrand I personally prefer the version with |
Thanks @lynzrand , and apologies for not being able to review this sooner 🙏 I agree with @Minnozz that I would appreciate more contrast for the inlay background, so please use the one without the alpha. I'm keen to hear your feedback on this suggestion though: just set the inlay background, and remove the setting for all foregrounds - this will use similar color to code comments for all foregrounds. The reason is that these inlays are more "helper" type instead of real code, so I think treating them as such might make more senses? |
Added inlay hint color for the theme.
I'm not very sure about the color selection -- I'm currently using the color 'gray' in the color scheme as the text color for regular inlay hints, and the
{color}1
version of the type and parameter color for type/parameter inlay hints.Before: #68
After:
(closes #68)