mvllow / modes.nvim

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

fix: do not overwrite user's `guicursor` shape #56

Closed zahimeen closed 3 months ago

zahimeen commented 4 months ago

fixes #54. removed the cursor shape options (e.g. block, ver25. hor20) from being set in the guicursor. now this plugin only handles the highlighting.

for example, the following line:

vim.opt.guicursor:append('v-sm:block-ModesVisual')

was changed to:

vim.opt.guicursor:append('v-sm:ModesVisual')
mvllow commented 4 months ago

Oo excited to test this. Thanks for the contributions, I'll be able to review soon 💜

colout commented 3 months ago

Been testing millow's fork/branch with no issues. Works as advertised 👍

mvllow commented 3 months ago

Appreciate the feedback :) I just got back from a trip last night so still playing catch up

fitrh commented 3 months ago

This change breaks the current behavior

From the linked issue, what we want is for the guicursor shape to be configurable

zahimeen commented 3 months ago

This change breaks the current behavior

From the linked issue, what we want is for the guicursor shape to be configurable

I argue that this plugin has no need to be able to configure the guicursor shape when neovim literally provides you the ability to set the cursor completely by yourself independent of this plugin in a very easy way.

I made that issue and worded exactly what I wanted. I wanted this plugin to take its hands off of my guicursor shape configurations. Respect user's guicursor settings [not]() Add guicursor shape configuration options

colout commented 3 months ago

Respect user's guicursor settings not Add guicursor shape configuration options

^^^ Agreed. This plugin makes it super-simple to customize colors per-mode which otherwise requires bespoke autocmds. I can't think of a problem with the default guicursor implementation of cursor shapes that needs a plugin to solve.

:h guicursor has a clear examples that that even work in old versions of vanilla vim.

mvllow commented 3 months ago

This new behaviour seems ideal, and is how I would have implemented initially if I had a better understanding of what I was actually doing.

I'm a little confused about the debate going on now though, is there a disagreement on whether this plugin should change the cursor shape or not?

colout commented 3 months ago

imho I can't think of a use case for a plugin to manage the cursor shape during mode switching (it's a single line option config), but of course that doesn't mean one doesn't exist.

I'm curious if @fitrh's issue is that users might rely on this bug to set their cursor shape to the ones hardcoded in the plugin. I doubt this the case though, since the cursor shape isn't configurable and I think you just copied the default nvim cursor shape anyway (so shouldn't be a change even if they are) .

Either way, the ask seems out of scope from this issue (and OP confirmed this above).

If @fitrh has a use case for why the builtin option isn't sufficient, maybe they can open a new issue with their use case to discuss as a new feature request?