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

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

Merged
merged 1 commit into from
May 30, 2024
Merged

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

merged 1 commit into from
May 30, 2024

Conversation

zahimeen
Copy link
Contributor

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
Copy link
Owner

mvllow commented May 13, 2024

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

@colout
Copy link

colout commented May 26, 2024

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

@mvllow
Copy link
Owner

mvllow commented May 26, 2024

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

@fitrh
Copy link
Collaborator

fitrh commented May 27, 2024

This change breaks the current behavior

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

@zahimeen
Copy link
Contributor Author

zahimeen commented May 27, 2024

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
Copy link

colout commented May 27, 2024

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
Copy link
Owner

mvllow commented May 27, 2024

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
Copy link

colout commented May 27, 2024

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?

@mvllow mvllow merged commit 0a279a5 into mvllow:main May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Respect user's guicursor settings
4 participants