Change built-in themes to use curly underlines #5419
Conversation
the-mikedavis
added
A-theme
|
I think this is definitely an improvement. Adding separate colored underlines for diagnostics provides additional information to the user and style wise curly also seems the most appropriate for diagnostics to me. However I think we usually get the theme author to sign off on theme changes. It's not a hard requirement but it would be good to ping the authors of all themes changed so they have a chance to take a look at the colors you used/offer their input. |
|
I think it might also be a good idea to add this to the themelint aswell (found in |
|
@enkodr @vv9k @bootradev @ChrHorn @nogden @Rinnray @Yevgnen @CptPotato @AlexanderBrevig @krfl @jbaa @irishmaestro @PORTALSURFER @intarga @zetashift @ramojus @kvrohit @kirawi @WindSoilder @n0s4 @jhscheer @0rphee @jharrilim @raygervais @two-six @nuid32 @erasin @uncomfyhalomacro @vlmutolo @workingj @VuiMuich @p4ymak @atog @paulgraydon @kamek-pf Hey everyone! Do you mind if I modify your themes to make use of curly and colored underlines for diagnostic messages? (Silence will be interpreted as as "Ok") |
|
Fleet Dark is currently using |
|
As I said,
But now I'll add change Fleet Dark as well |
| "diagnostic.warning" = { underline = { color = "yellow", style = "curl" } } # Diagnostics warning (editing area) | ||
| "diagnostic.error" = { underline = { color = "red", style = "curl" } } # Diagnostics error (editing area) | ||
| "diagnostic.info" = { underline = { color = "blue", style = "curl" } } # Diagnostics info (editing area) | ||
| "diagnostic.hint" = { underline = { color = "green", style = "curl" } } # Diagnostics hint (editing area) |
There was a problem hiding this comment.
I'm on helix 22.12 (96ff64a8) and with these changes the underline gets the same color of the element it is underlining, e.g. white under white, yellow under yellow. However, it should be red for error, yellow for warning, etc.
There was a problem hiding this comment.
Does this happen only for this theme?
I don't mind, nor do I think |
Please go ahead and thanks talking this chore 🙏🏻 |
|
Hi! Thanks a lot for doing this change, looks good to me! |
I don't mind at all! |
|
Hi, just go ahead and make the change :-D |
|
Sure! I won't mind |
based on the changes made by the default versions of the themes in helix-editor/helix#5419
|
Sure! Please do. Thank you. |
| @@ -87,7 +87,10 @@ | |||
| "diff.minus" = "red" | |||
|
|
|||
| # make diagnostic underlined, to distinguish with selection text. | |||
| diagnostic = { modifiers = ["underlined"] } | |||
| "diagnostic.warning" = { underline = { color = "orange", style = "curl" } } | |||
| "diagnostic.error" = { underline = { color = "magenta", style = "curl" } } # maybe should be red? | |||
There was a problem hiding this comment.
Oh, I didn't notice that magenta is not defined, just used whatever "error" used
There was a problem hiding this comment.
No problem. It's fixed now.
* Change built-in themes to use curly underlines * Change fleet_dark to use curly underlines
Fixes #5361.
I didn't touch themes that don't underline their diagnostics messages and themes that use
underline.style = "line"instead ofmodifiers = ["underlined"]