ggandor / flit.nvim

Enhanced f/t motions for Leap
The Unlicense
366 stars 15 forks source link

Blinking Cursor left in place where I started motion #3

Closed pBorak closed 7 months ago

pBorak commented 2 years ago

Hey @ggandor - let me start by appreciating the flawless work you have done with this and other plugins of yours. Thanks!

I've noticed there's a blinking cursor left at the position where I started the 'f/t' motion - this makes it confusing for me where my actual cursor position is.

https://user-images.githubusercontent.com/45797239/193772506-2db2cf26-219e-4b69-911d-4f84ef995089.mov

FYI. I am running neovim v.0.8 with the following flit config.

        {
          'ggandor/flit.nvim',
          config = function()
            require('flit').setup({
              multiline = false,
            })
          end,
        },
ggandor commented 2 years ago

That is the actual cursor blinking there - it does not move to the command line when it should (on the getchar call). I'm noticing the same bug since updating to the stable release of 0.8 (how ironic). This is not flit-specific, so it would make sense to create an issue in the Lightspeed or Leap repo instead.

I'd appreciate any help in at least tracking down the core commit that caused this, so that we can move on to fixing this. I'll have pretty limited time in the coming days unfortunately.

let me start by appreciating the flawless work you have done with this and other plugins of yours.

Thank you very much, as always!

ggandor commented 2 years ago

An unrelated issue, but I couldn't help noticing that the backdrop highlight is off by 1 character on your recording (the immediate next character is not greyed out). I cannot reproduce this though.

pBorak commented 2 years ago

May this one be related? https://github.com/neovim/neovim/issues/20309#issuecomment-1256731277

ggandor commented 2 years ago

Yes, it seems so.

folke commented 2 years ago

@ggandor for Noice, I was able to simply hide the real cursor with https://github.com/folke/noice.nvim/blob/main/lua/noice/util/hacks.lua#L251

And here's the hl for the hidden cursor:

  vim.api.nvim_set_hl(0, "NoiceHiddenCursor", { blend = 100, nocombine = true })
folke commented 2 years ago

Also worth mentioning is that https://github.com/echasnovski/mini.nvim/blob/main/lua/mini/jump.lua doesnt suffer the same problem

folke commented 2 years ago

You can probably also just update the cursor position before doing the getchar and not use a fake cursor?

ggandor commented 2 years ago

Hey, thanks very much for the helpful hints!

You can probably also just update the cursor position before doing the getchar and not use a fake cursor?

This is what we do. The position is updated, the screen is :redraw-n, but the cursor stays in place! Mysterious.

for Noice, I was able to simply hide the real cursor with https://github.com/folke/noice.nvim/blob/main/lua/noice/util/hacks.lua#L251

Yeah I guess I'll try something like that, as a last resort.

https://github.com/echasnovski/mini.nvim/blob/main/lua/mini/jump.lua doesnt suffer the same problem

Really? Now this is interesting. Problem is, I don't have the mental energy to go through the code now :(

folke commented 2 years ago

@ggandor no worries, works fine as it is :)

The difference with mini is that mini doesn't wait for input in a loop. They simply have a mapping for the f key, highlight other matches and jump to the first match. No waiting for input here.

Whenever you move or do something, the highlights are removed.

Pressing f again, starts the same mapping and there they check if this is a continuation fo the last search, or a new search.

zeertzjq commented 2 years ago

I'd appreciate any help in at least tracking down the core commit that caused this, so that we can move on to fixing this. I'll have pretty limited time in the coming days unfortunately.

This is introduced by https://github.com/neovim/neovim/pull/20188

To restore the previous behavior, remove this if:

diff --git a/src/nvim/getchar.c b/src/nvim/getchar.c
index f76107c40..a60cbb5d8 100644
--- a/src/nvim/getchar.c
+++ b/src/nvim/getchar.c
@@ -1655,10 +1655,8 @@ static void getchar_common(typval_T *argvars, typval_T *rettv)
   no_mapping++;
   allow_keys++;
   for (;;) {
-    if (msg_col > 0) {
-      // Position the cursor. Needed after a message that ends in a space.
-      ui_cursor_goto(msg_row, msg_col);
-    }
+    // Position the cursor. Needed after a message that ends in a space.
+    ui_cursor_goto(msg_row, msg_col);

     if (argvars[0].v_type == VAR_UNKNOWN) {
       // getchar(): blocking wait.
EtiamNullam commented 1 year ago

Not sure if it's applicable here, but I usually use vim.schedule as a workaround for such scenario:

  -- instructions after which redraw is needed

- -- some blocking action, like waiting for input, which will prevent redraw
+ vim.schedule(function()
+   -- some blocking action, like waiting for input, which will prevent redraw
+ end)
folke commented 1 year ago

@EtiamNullam that would't help here, since this has to be a blocking event. Flit is waiting for you to enter a character

EtiamNullam commented 1 year ago

@folke I'm using your which-key.nvim extensively in my workflow (thank you for this amazing plugin!) and wrapping require('which-key').show() in vim.schedule() worked for in the past to let nvim redraw the screen before require('which-key').show() blocks it.

Short example, very close to what I've used before, doesn't seem to be needed anymore:

increase_font_size()

vim.schedule(function()
  require('which-key').show(keys_used_to_get_here)
end)

I wish there was a natural to achieve this "sticky" behavior in your plugin, but I don't want to get too much off-topic here.

Maybe I'm still missing something. I see it's not really trivial to try it here in flit.

folke commented 1 year ago

again, that would NOT work for keymaps with expr=true

AlexSWall commented 1 year ago

As quick addendums in case it saves anyone time, I only get this behaviour if I'm using LuaLine; if I remove it, I no longer get this issue. Furthermore, if I have vim.opt.lazyredraw = true, the previous cursor location/match is highlighted instead of the original location (which I find much more confusing). And finally, this should probably be an issue for leap as opposed to flit, as it holds for 'go to the next match' functionality in leap.

EtiamNullam commented 1 year ago

@AlexSWall: I believe it should not matter if you use lualine or lazyredraw, it's an upstream bug introduced in Neovim 0.8 and @ggandor has decided to just wait for an upstream fix.

AlexSWall commented 1 year ago

@EtiamNullam I realize it's an upstream issue, I'm adding the above to help replicate the issue and understand the different ways it appears, both for when it comes to ensuring it's fixed and for those confused by either having a seemingly-different-but-similar bug where the cursor is moving or not having the bug at all. The behaviour certainly depends on lualine and lazyredraw for me; without lualine, I do not experience the issue.

folke commented 1 year ago

@EtiamNullam you should NOT set lazyredraw. You're literally telling Neovim to not redraw when needed, so obviously you'll have issues with that. See :h lazyredraw image

kohane27 commented 1 year ago

I can also verify that when I turned off lualine, this multiple cursor issue is gone.

I have also tested lazyredraw. It doesn't affect the result:

  1. lazyredraw off + lualine off = no issue
  2. lazyredraw on + lualine on = issue comes back
  3. lazyredraw off + lualine on = issue comes back
  4. lazyredraw on + lualine off = no issue

As long as lualine is disabled, this "multiple cursor" issue is gone.

ggandor commented 7 months ago

Fixed by https://github.com/neovim/neovim/pull/27858 and https://github.com/ggandor/flit.nvim/commit/04f744bbb2b91fb2ad2c702b5eb8e23d17924fa6 for nightly builds.