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

Not all themes display rulers #5721

Closed
5 tasks done
non-descriptive opened this issue Jan 29, 2023 · 9 comments · Fixed by #10309
Closed
5 tasks done

Not all themes display rulers #5721

non-descriptive opened this issue Jan 29, 2023 · 9 comments · Fixed by #10309
Labels
A-theme Area: Theme and appearence related C-enhancement Category: Improvements

Comments

@non-descriptive
Copy link

non-descriptive commented Jan 29, 2023

Summary

themes which do not display rulers

  • default
  • kanagawa
  • rasmus
  • solarized_light
  • solarized_dark

Basically theme doesn't have proper bg color for ui.virtual.ruler or fg color for ui.virtual.rulers. Usually they should have either gray color or the same color as ui.cursorline.primary

Reproduction Steps

  1. setup editor.rulers value.
  2. set theme to one in the list above

Helix log

No response

Platform

Windows

Terminal Emulator

Windows Terminal

Helix Version

helix 22.12 (4d548a0)

@non-descriptive non-descriptive added the C-bug Category: This is a bug label Jan 29, 2023
@pascalkuthe pascalkuthe added the A-theme Area: Theme and appearence related label Jan 30, 2023
@the-mikedavis
Copy link
Member

Except for kanagawa, these all use foreground rulers. The default and rasmus set ui.virtual to a foreground color which makes sense for some scopes like ui.virtual.indent-guide or ui.virtual.whitespace. I think those are probably oversights and could be updated.

Solarized light and dark set ui.virtual.ruler to a foreground color explicitly which I think is a stylistic choice: foreground rulers are visible if you have lines that cross them. Foreground rulers can be useful because you can see when lines are too long without the ruler constantly being visible.

Kanagawa has a background ruler though. It looks to me like it's displaying a ruler.

@non-descriptive
Copy link
Author

non-descriptive commented Jan 31, 2023

I fixed some themes locally and didn't test others, just noticed there's no rulers were displayed. Also it's kinda weird way to separate rulers - themes differentiate ruler and rulers keywords. One of it works with background color and another one with foreground color.

@tqwewe
Copy link

tqwewe commented Jun 28, 2023

Updated list of themes which do not display rulers:

  • adwaita-dark
  • default
  • github_dark
  • github_dark_colorblind
  • github_dark_dimmed
  • github_dark_high_contrast
  • github_dark_tritanopia
  • github_light
  • github_light_colorblind
  • github_light_high_contrast
  • github_light_tritanopia
  • mellow
  • rasmus
  • solarized_dark
  • solarized_light

@pieqq
Copy link

pieqq commented Jul 20, 2023

I think at the very least, the default theme should display both foreground and background rulers. I wanted to display rulers the way I was used to in vim (what is called background rulers in Helix apparently), so I added

[editor]
rulers = [80]

in my config.toml, and yet nothing showed up. Sure, if lines go beyond 80 characters, the character placed at column 80 will be in a different color, but:

  • this does not work if the character is a space
  • this only shows for lines that go through the ruler limit

Only this morning did I figure this out when trying other themes! :)

(by the way, the live preview when running :theme <tab><tab> is amazing!!)

@pascalkuthe pascalkuthe added C-enhancement Category: Improvements and removed C-bug Category: This is a bug labels Jul 20, 2023
@ardrigh
Copy link

ardrigh commented Dec 3, 2023

I just noticed this on the default theme in Helix. I had set a ruler for 100 characters, and when I was typing the character changed colour, but the vertical bar isn't visible.

There is no indication the ruler has been set correctly based on the configuration.

This then looks like a rendering bug to the user in Helix displaying one character in a different colour.

I am not sure why there needs to be fancy foreground and background separation, but in at least the default theme, Helix should act the same as other editors using background colour visible vertical ruler lines on screen.

If people wanted to have smart foreground colours about text beyond rulers, they should really theme all text beyond the ruler in the different colour, not a single character.

@non-descriptive
Copy link
Author

There are foreground and background colors, you probably set only foreground which changes font color.

@pieqq
Copy link

pieqq commented Dec 4, 2023

There are foreground and background colors, you probably set only foreground which changes font color.

That's the issue here: we are talking about the default theme. I think at the very least the default theme should have both background and foreground set up for the rulers in order to have a nice default experience. User can then move on to another theme if they wish so :)

@ulmer-a
Copy link

ulmer-a commented Dec 18, 2023

Is anything holding back this issue? Otherwise, I could probably fix this. Should the respective themes just be fixed or should helix just assume a default of, e.g., ui.cursorline.primary if a theme doesn't specify a ui.virtual.ruler?

@the-mikedavis
Copy link
Member

We just need to figure out if these themes are using foreground rulers intentionally or if it'd be ok to switch to background. If you'd like to make a PR that switches these to background rulers we can tag the theme authors and ask

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-theme Area: Theme and appearence related C-enhancement Category: Improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants