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

feat: attempt to hide cursor when leaping #143

Closed
wants to merge 1 commit into from

Conversation

mitchellwrosen
Copy link

This PR partially addresses #70 by applying a workaround to neovim/neovim#20793 suggested by @folke: just hide and restore the cursor.

The implementation is... weird :/. I've tried to explain what's going on in a comment. Suggestions for wording and/or alternative implementation(s) welcome.

@ggandor
Copy link
Owner

ggandor commented Apr 27, 2023

Nice, thank you! I'm totally fine with this for now, one can always set blend back to a value other than 0 on LeapLeave. It's enough to give the explanation in the commit message I guess (the comments are great btw), and the highlight stuff directly in the lambdas (a function would be captured in create_autocmd anyway, so macros are really overkill here).

@folke
Copy link

folke commented Apr 28, 2023

It's unfortunately not as straightforward. There's some weird bugs in Neovim where cursor updates are delayed. Check the Noice code for more details:

https://github.com/folke/noice.nvim/blob/main/lua/noice/util/hacks.lua#L255-L283

Having Noice with Leap will also lead to issues with this PR since there will likely be race conditions where the wrong cursor is restored.

Would it be possible to set a vim.g.... global that I can set to false from within Noice, to prevent Leap doing the cursor hiding? Alternatively, you could skip hiding the cursor when package.loaded["noice"], but I'm happy to do the opposite from Noice as well.

@ggandor
Copy link
Owner

ggandor commented Apr 30, 2023

Would it be possible to set a vim.g.... global that I can set to false from within Noice, to prevent Leap doing the cursor hiding? Alternatively, you could skip hiding the cursor when package.loaded["noice"], but I'm happy to do the opposite from Noice as well.

Any of the above is fine, but I'm not sure I understand the implications correctly (sorry, I'm a bit low on mental energy): does this mean that cursor hiding will only work for one of the two plugins?

@mitchellwrosen
Copy link
Author

Hrm... I think if either plugin has to know about each other, that's probably worse than doing nothing in leap, for now - assuming the issue will be addressed in neovim core some time ;).

A third option would be some documentation in the leap README about how one might hide the cursor, with some additional caveats for Noice users.

(But I'm cool with the above suggestions, too - just LMK which to implement, in addition to that macro -> fn change I still have to make)

@folke
Copy link

folke commented May 2, 2023

@ggandor Noice disables the cursor whenever some code does vim.fn.getchar, vim.fn.getcharstr, vim.fn.inputlist, so the fixes I did were not specifically for leap, but basically for all plugins.

This means that the leap cursor issue doesn't exist for users that also use Noice.

What Noice (and with this PR leap does) when the cursor needs to hide:

  1. save the cursor state
  2. set cursor to invisible cursor
  3. restore cursor state

Now, when both leap or Noice would be doing this, there's a very likely race-condition, where for example:

  1. Noice saves cursor state
  2. Noice sets cursor to NoiceDisabled
  3. Leap saves cursor state (which is now the Noice one)
  4. Noice restores cursor state (back to normal)
  5. Leap restores cursor state => will set it to NoiceDisabled

The easiest way would be to skip hiding the cursor in leap when package.loaded['noice'].

@ggandor
Copy link
Owner

ggandor commented May 2, 2023

Noice disables the cursor whenever some code does vim.fn.getchar, vim.fn.getcharstr, vim.fn.inputlist, so the fixes I did were not specifically for leap, but basically for all plugins.

Ah, thanks for the clarification, great.

he easiest way would be to skip hiding the cursor in leap when package.loaded['noice'].

This is what we'll do then.

@mitchellwrosen
Copy link
Author

Ok, I think I've made the requested changes

@ggandor
Copy link
Owner

ggandor commented May 9, 2023

But what about the delaying issue mentioned above? @folke could you give us a TLDR on that? :)

@mitchellwrosen
Copy link
Author

@ggandor Ah sorry, I had missed that. Ok, after reading the code, and the issue that lead to some of it (folke/noice.nvim#114), I'm left feeling like I just don't really understand what's going on. I'm happy to drop this feature and say Hiding Cursor in Neovim Considered Too Complicated.

Is anyone else interested in picking up this PR from here?

@ggandor
Copy link
Owner

ggandor commented Mar 30, 2024

Rendered obsolete by neovim/neovim#27858.

@ggandor ggandor closed this Mar 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.

3 participants