Toggle for mode dependent cursor block color / cursor block color theming rewrite

[ivenw]

I really like the colored mode indicator in the statusline toggled by editor.color-mode. I am desiring a similar option for coloring the cursor block according to the mode (i.e. editor.cursor-color-mode). So I went about looking at implementing this to potentially make a PR for this down the line.

In researching if a similar effort is already worked on I stumbled upon #1337 and the tangentially related #1833. To my surprise the feature is essentially implemented but is difficult to discover or to turn into an option since many themes implement the overwriting ui.cursor.primary setting, as pointed out in #1337.

I propose to deprecate ui.cursor.primary, with ui.cursor taking its place for theming the primary cursor block. The current role of ui.cursor of theming the secondary cursor block would then be taken by the new ui.cursor.secondary theme parameter. (For consistency, this will also deprecate ui.selection.primary and introduce ui.selection.secondary following the same logic).

While this is a breaking change, I think it comes with a number of advantages that will pay off going forward.

1. It makes logically more sense to declare the colors of the primary selection and cursor first. If the theme desires, it can set colors for the less common secondary selection as well.
2. From this follows also a clearer fallback behavior in the absence of certain cursor theming parameters.
3. We can easily implement mode-dependent cursor theming as an option.

With this proposed change the cursor theming logic would as follows:

• ui.selection sets the primary selection color. Mandatory.
• ui.cursor sets the primary cursor block color. Sets the primary cursor block color in NORMAL mode if editor.cursor-color-mode true. Fallback to ui.cursor. Fallback to ui.selection.
• ui.cursor.insert sets the primary cursor block color in INSERT mode if editor.cursor-color-mode true. Fallback to ui.cursor.
• ui.cursor.select sets the primary cursor block color in SELECT mode if editor.cursor-color-mode true. Fallback to ui.cursor.
• ui.selection.secondary sets the secondary selection color. Fallback to ui.selection
• ui.cursor.secondary sets the secondary cursor block color. Sets the secondary cursor block color in NORMAL mode if editor.cursor-color-mode true. Fallback to ui.cursor.
• ui.cursor.secondary.insert sets the secondary cursor block color in INSERT mode if editor.cursor-color-mode true. Fallback to ui.cursor.secondary.
• ui.cursor.secondary.select sets the secondary cursor block color in SELECT mode if editor.cursor-color-mode true. Fallback to ui.cursor.secondary.

Below is the same control flow in a flowchart. Arrows denote the traversal order of the parameters. For fallback behavior follow them in the opposite direction.

Now again, I realize this breaks the current theming implementation. So if you are interested in this rewrite, I’d be happy to update all currently implemented themes as part of the PR. Feel free to check out my proof-of-concept implementation. The changes are largely limited to fn doc_selection_highlights

flowchart TB;
    A(ui.selection);
    B(ui.cursor);
    C(ui.cursor.insert);
    D(ui.cursor.select);
    E(ui.cursor.secondary);
    F(ui.cursor.secondary.insert);
    G(ui.cursor.secondary.select);
    H(ui.selection.secondary);
    I{editor.cursor-color-mode};
    J{editor.cursor-color-mode};
    A-->H;
    A-->B;
    B-->I;
    I--true-->C;
    I--true-->D;
    B-->E;
    E-->J;
    J--true-->F;
    J--true-->G;

[the-mikedavis]

This would be inconsistent with the default fallback behavior for themes (a.b.c falls back to a.b which falls back to a)

[ivenw]

I think there is a good point @the-mikedavis. To which part are you referring to exactly? If I understand correctly it's the fallback of ui.cursor to ui.selection but that is current behaviour.

[the-mikedavis]

Oh I see, I misread the part on eliminating ui.cursor.primary, ignore me.

I would prefer to keep the .primary but I think you're right about it being simpler to eliminate it and use ui.cursor for primary and ui.cursor.secondary for secondaries. Also see #2366 which attempts to solve this while keeping the .primary.

[ivenw]

I did see #2366 but didn't consider it too closely since it didn't seem to get much traction.

I also found that having ui.cursor.insert change what it is themeing based on whether ui.cursor.primary is present feels inconsistent and confusing. (But I get where the author is coming from in not wanting to mess with things too much.)

Before I realized that ui.cursor.insert et al. exists my original implementation actually just introduced ui.cursor.primary.insert, which I also find clearer and preferable over the ui.cursor.insert.primary solution proposed in #2366.

Having thought about it for a while, switching from .primary to .secondary makes the most sense (to me) from a theming hierarchy.

I think the question is largely how mature you consider Helix to be, whether my proposed change is acceptable or not. It does break current behavior but the biult-in themes can be migrated, so the question how user-facing the theming system is remains.

Also to be clear, my main desire with this issue was to implement the editor.cursor-color-mode toggle. The cursor theming rewrite was more a side effect that I thought could bring additional benefit.

[ivenw]

Is this something you are still interested in?

[the-mikedavis]

I think I would need to see this as a PR to fully understand the change - could you open one up?

It does break current behavior

That's not such a big deal - there isn't really a guarantee of stability since we use calendar versioning (although we try to only make breaking changes when there's a solid advantage to it). Migrating the in-repo themes would help a bunch with that 👍

[ivenw]

@the-mikedavis sounds good, will do when I get around to it. Migrating the in-repo themes I would then do if the PR is otherwise in principle accepted.

[archseer]

Closed by #5130