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

Selection backgrounds are clobbered by rulers. #7371

Open
7ombie opened this issue Jun 18, 2023 · 8 comments
Open

Selection backgrounds are clobbered by rulers. #7371

7ombie opened this issue Jun 18, 2023 · 8 comments
Labels
C-bug Category: This is a bug

Comments

@7ombie
Copy link
Contributor

7ombie commented Jun 18, 2023

Summary

Whenever a selection crosses over a ruler, the foreground color gets updated correctly, but the background color remains the same (the ruler color).

Reproduction Steps

Enable rulers (adding something like rulers = [80] in the [editor] section of your config.toml). Then, create a selection that spans a ruler.

The background color of the ruler-column does not change to the selection color (while the foreground color updates correctly). This includes the cursor, so when you only have a cursor (a single-character selection), your cursor seems to disappear behind the ruler.

Note: I also reported that my cursor never blinks in Helix (this was in a separate issue that somebody else opened). That issue may be contributing to the issue I'm having here.

Helix log

n/a

Platform

macOS Ventura 13.4

Terminal Emulator

iTerm2 3.4.19

Helix Version

helix 23.05 (7f5940b)

@7ombie 7ombie added the C-bug Category: This is a bug label Jun 18, 2023
@kirawi
Copy link
Member

kirawi commented Jun 18, 2023

Seems like a duplicate of #5675?

@7ombie
Copy link
Contributor Author

7ombie commented Jun 18, 2023

I'm not sure it's the same. Maybe, but I don't think so. This seems to be related to an oversight with the rulers specifically, which take precedence over the theme's background color (as they should), but also take precedence over the selection/cursor color (incorrectly).

#5675 changes the cursor color over whitespace, while this issue fails to adopt the selection color when the selection spans a ruler. I'll try and upload a screenshot (I'm on holiday, in a caravan).

@7ombie
Copy link
Contributor Author

7ombie commented Jun 18, 2023

Below are two screenshots which represent an animation (I cannot upload video at the moment).

I have two rulers (20 columns apart), and I'm in Select Mode, moving the cursor to the right...

Screenshot 2023-06-18 at 20 39 08 Screenshot 2023-06-18 at 20 38 33

Note that the foreground color is always correct, including turning black when the cursor is over (under) the ruler (my theme sets the cursor foreground color to black).

P.S. Ignore the apostrophe in ruler's. I don't know what happened there.

@gabydd
Copy link
Member

gabydd commented Jun 19, 2023

yeah this is the same problem as in #5675 it happens because the selection(and the cursor) is rendered before the rulers, see how rulers is called after selections

Self::doc_selection_highlights(
editor.mode(),
doc,
view,
theme,
&config.cursor_shape,
),
Self::render_rulers(editor, doc, view, inner, surface, theme);

@pascalkuthe
Copy link
Member

pascalkuthe commented Jun 19, 2023

We could render the rulers before the text to fix this line we do for the cursorsline/cursorcolumn. That would mean that themes that define background color for their text would clobber the rules but we are sort of leaning towards forbidding that anyways since the same caveats apply to any background decoration like cursor line/current debug breakpoint,...

@the-mikedavis
Copy link
Member

There was #3599 along those lines but foreground rulers would completely disappear. I think we could render the background and modifier parts of the ruler Style before the text highlights and then the foreground part afterwards so it would layer correctly.

@the-mikedavis
Copy link
Member

Or I suppose we would want the background part of the ruler to be rendered on top of the text highlights but below the selection highlights.

I wonder if we should also not allow the reversed modifier to affect rulers and other decorations like indent guides? I think we could swap the foreground/backgrounds for those Styles when the existing Style has the reversed modifier.

@7ombie
Copy link
Contributor Author

7ombie commented Jun 19, 2023

Thanks for looking into this. Much appreciated.

nilium added a commit to nilium/helix that referenced this issue Aug 16, 2024
Render rulers before the cursor to ensure that the cursor, when over
a ruler, is not hidden from view. Without this, you typically end up
with 1) foreground text that is the same as the background if the
ruler doesn't already have a foreground and 2) no visible cursor,
because the ruler's background color took precedence. By moving the
rulers before the cursor, this ensures that the theme is still rendered
more or less the way one would visually expect things to turn out.

Related to helix-editor#7371. Not submitting this as a patch
because it doesn't seem like this is the desired solution, but it
thankfully works perfectly for me.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug
Projects
None yet
Development

No branches or pull requests

5 participants