mvllow / modes.nvim

Prismatic line decorations for the adventurous vim user
550 stars 13 forks source link

RFC: Handling line number highlights #13

Closed shaeinst closed 2 years ago

shaeinst commented 2 years ago

as you can see in SS, changing mode also changes the line highlight. either there should be option to change line color or this plugin should not affect the line number
image

mvllow commented 2 years ago

Hmm, modes only sets the number foreground, your theme may be linking the background to an existing group. Either way, I agree it would be nice to choose which sections are highlighted 😌

Screen Shot 2022-02-28 at 4 14 09 PM
fitrh commented 2 years ago

@mvllow An option to enable CursorLineNr override is a good idea

Another option is that we can detect whether CursorLineNr has set background or not

We have to think of a better approach for this option but a dirty hack like the following works

diff --git a/lua/modes/init.lua b/lua/modes/init.lua
index 8e264ba..424552f 100644
--- a/lua/modes/init.lua
+++ b/lua/modes/init.lua
@@ -15,13 +15,17 @@ end
 function modes.set_highlights(style)
    if style == 'init' then
        vim.cmd('hi CursorLine guibg=' .. init_colors.cursor_line)
-       vim.cmd('hi CursorLineNr guifg=' .. init_colors.cursor_line_nr)
+       if not vim.api.nvim_get_hl_by_name('CursorLineNr', true).background then
+           vim.cmd('hi CursorLineNr guifg=' .. init_colors.cursor_line_nr)
+       end
        vim.cmd('hi ModeMsg guifg=' .. init_colors.mode_msg)
    end

    if style == 'copy' then
        vim.cmd('hi CursorLine guibg=' .. dim_colors.copy)
-       vim.cmd('hi CursorLineNr guifg=' .. colors.copy)
+       if not vim.api.nvim_get_hl_by_name('CursorLineNr', true).background then
+           vim.cmd('hi CursorLineNr guifg=' .. colors.copy)
+       end
        vim.cmd('hi ModeMsg guifg=' .. colors.copy)
        vim.cmd('hi ModesOperator guifg=NONE guibg=NONE')
        vim.cmd('hi! link ModesOperator ModesCopy')
@@ -29,7 +33,9 @@ function modes.set_highlights(style)

    if style == 'delete' then
        vim.cmd('hi CursorLine guibg=' .. dim_colors.delete)
-       vim.cmd('hi CursorLineNr guifg=' .. colors.delete)
+       if not vim.api.nvim_get_hl_by_name('CursorLineNr', true).background then
+           vim.cmd('hi CursorLineNr guifg=' .. colors.delete)
+       end
        vim.cmd('hi ModeMsg guifg=' .. colors.delete)
        vim.cmd('hi ModesOperator guifg=NONE guibg=NONE')
        vim.cmd('hi! link ModesOperator ModesDelete')
@@ -37,13 +43,17 @@ function modes.set_highlights(style)

    if style == 'insert' then
        vim.cmd('hi CursorLine guibg=' .. dim_colors.insert)
-       vim.cmd('hi CursorLineNr guifg=' .. colors.insert)
+       if not vim.api.nvim_get_hl_by_name('CursorLineNr', true).background then
+           vim.cmd('hi CursorLineNr guifg=' .. colors.insert)
+       end
        vim.cmd('hi ModeMsg guifg=' .. colors.insert)
    end

    if style == 'visual' then
        vim.cmd('hi CursorLine guibg=' .. dim_colors.visual)
-       vim.cmd('hi CursorLineNr guifg=' .. colors.visual)
+       if not vim.api.nvim_get_hl_by_name('CursorLineNr', true).background then
+           vim.cmd('hi CursorLineNr guifg=' .. colors.visual)
+       end
        vim.cmd('hi ModeMsg guifg=' .. colors.visual)
    end
 end

https://user-images.githubusercontent.com/55179750/158295732-3d6f326e-71fc-4e97-9953-81a4eea1abac.mp4

No more CursorLineNr color change

mvllow commented 2 years ago

Oh that makes sense now, if line number has a background we can avoid changing the foreground because the contrast will probably not be usable. I like that solution rather than having an option. Good suggestion 😌

It could be beneficial to also check that CursorLineNr does not equal Normal background because I assume some themes would set it just to set it.

mvllow commented 2 years ago

The above solution is what I'm leaning towards at the moment but welcome any other feedback.

Another option could be to set the background of CursorLineNr to match the cursorline but unfortunately this also introduces potential contrast issues.

shaeinst commented 2 years ago

i thinks this should be an option, something like smart_cursorlinenr=true/false. ie, if smart_cursorlinenr is true, then mode.vim would override the CursorLineNr else, @fitrh 's method applied.

Another option could be to set the background of CursorLineNr to match the cursorline