helix-editor / helix

A post-modern modal text editor.
https://helix-editor.com
Mozilla Public License 2.0
32.92k stars 2.43k forks source link

Improve/fix some built in themes #3314

Closed ChrHorn closed 5 months ago

ChrHorn commented 2 years ago

Some themes that come with helix need some polish.

acme: statusline, menu, maybe errors

acme

base_16_transparent: menu bg same as standard bg (intentional, add border once it's possible)

base16_transparent

nord_light: ui menu selected bg very similar to standard bg

nord_light

rose_pine_dawn: ui menu selected bg same as standard bg

rose_pine_dawn
FIXED dark_plus: indent guide bg dark_plus ayu_light: statusline fg, menu fg ayu_light
AlexanderBrevig commented 2 years ago

I agree, and made a tool for helping out #3234. A lot of the issues you point out are not yet checked against, but they could :)

GreasySlug commented 2 years ago

About base16_transparent, it is intentionally the same as the background. The reason for this is to maintain the uniformity of transparency. I agree that it is hard to see, but it is not possible to change the transparency #2751 or add a border of the pop-up background.

ChrHorn commented 2 years ago

Some of these might be intentional and fixes would be a bit opinionated. Pinging

@two-six for Acme, Nord Light @enkodr for Ayu Light @chunghha for Rose Pine Dawn

About base16_transparent, it is intentionally the same as the background. The reason for this is to maintain the uniformity of transparency. I agree that it is hard to see, but it is not possible to change the transparency #2751 or add a border of the pop-up background.

Makes sense, added a note.

xcdkz commented 2 years ago

Thanks for the ping, I'll look into fixing acme. The main point of the nord light is the low contrast between elements of the ui, so in my opinion it looks just like it should although I'll try experimenting a little with it.

AlexanderBrevig commented 2 years ago

Thanks for the ping, I'll look into fixing acme. The main point of the nord light is the low contrast between elements of the ui, so in my opinion it looks just like it should although I'll try experimenting a little with it.

It is very hard to read the undefined ["warning","error","info","hint","diagnostic"].

image

ChrHorn commented 2 years ago

Thanks for the ping, I'll look into fixing acme. The main point of the nord light is the low contrast between elements of the ui, so in my opinion it looks just like it should although I'll try experimenting a little with it.

I think the main issue is that the code and the selected element blend into each other a bit too much. The contrast looks fine for the standard nord theme:

nord

Looks also fine in the VSCode Nord Light theme: image

xcdkz commented 2 years ago

Are the colors terminal-dependent? I have no problems in Konsole, can you tell me what terminal do you use? scrot helix

AlexanderBrevig commented 2 years ago

It maybe/probably falls back to some base16 color set for the terminal? I use a gruvbox theme.

| alacritty | wezterm |
|   Foot    |  kitty  |

image

ChrHorn commented 2 years ago

Acme uses a mix of hard coded colors and colors derived from the terminal theme. That's why it can look pretty bad when a dark terminal theme is used.

The issue for the Nord Light errors is that you used diagnostic.error instead of error. error will color the gutter and the overlaid text, diagnostic.error will only color the text.

yelfathi commented 1 year ago

Using Acme theme on iterm2 (acme theme also), and I got this white background for errors don't know if it is related to this issue. Thanks!

EDIT: also the buffer line background.

image
xcdkz commented 1 year ago

@olbitset I have already fixed this in #5019, it just wasn't included in the latest release. You would need to compile helix from source or update $HOME/.config/helix/runtime/themes/acme.toml file to the current version.

yelfathi commented 1 year ago

Thanks @two-six I included as a personal theme. What about the TOP bufferline (as the selection one), is it expected to have a gray background? If so what is the parameter in the toml config to adapt it to my need?

xcdkz commented 1 year ago

Thanks @two-six I included as a personal theme. What about the TOP bufferline (as the selection one), is it expected to have a gray background? If so what is the parameter in the toml config to adapt it to my need?

Do you mean foreground? I see background is light blue in your screenshot, not gray.

Set "ui.bufferline" = { fg = "indent", bg = "acme_bar_bg" } to "ui.bufferline" = { fg = "black", bg = "acme_bar_bg" }. If you want you can put any color you want there instead of black, like this: "ui.bufferline" = { fg = "#000000", bg = "acme_bar_bg" }

I've never used bufferline before, but as you mentioned current configuration doesn't really fit this theme really well. I will open a pull request to change it from gray to black.

yelfathi commented 1 year ago

Sorry I was talking about the bg. Thanks for the inputs I will adapt to my preferences!