inccommand: nvim_buf_set_text causes cursor flickering

[smjonas]

Problem

When typing an argument of a previewable command in command line mode (only on some key presses), the cursor is briefly moved to the end of the text. This only occurs when the buffer text is changed using vim.api.nvim_buf_set_text in the preview callback.

Initially reported here (the recording might be useful): https://github.com/smjonas/inc-rename.nvim/issues/63

Steps to reproduce

• nvim --clean
• :e test.lua

Paste the following Lua code:

local function replace_first_line_preview(opts, preview_ns, preview_buf)
  local replacement = opts.args
  vim.api.nvim_buf_set_text(0, 0, 0, 1, -1, { replacement })
  return 2
end

vim.api.nvim_create_user_command(
  "ReplaceFirstLine",
  function() end,
  { nargs = 1, preview = replace_first_line_preview }
)

• :so %
• start typing :ReplaceFirstLine a
• move cursor to the left so that it is no longer at the end of the command argument in the command line
• type multiple a's (hold the a key if the flicker is not immediately visible)

Observe that while typing some a's, the cursor is briefly moved to the right of the last a before being reset to the current position. position.

Expected behavior

Cursor should not flicker.

Nvim version (nvim -v)

NVIM v0.11.0-dev-911+ga2008118a

Vim (not Nvim) behaves the same?

n/a

Operating system/version

Linux Mint 21.2

Terminal name/version

Kitty 0.21.2

$TERM environment variable

xterm-kitty

Installation

appimage

[justinmk]

Flicker caused by non-trivial functions, especially RPC/API (though this case is not RPC), is kind of a general, known topic, but I couldn't find tracking issue.

[zeertzjq]

This only happens with treesitter because treesitter uses nvim__redraw. Related: #28668

[zeertzjq]

Hmm, there is a difference between different options of nvim__redraw regarding how the redraw can be reflected immediately:

• For range, both update_screen() and ui_flush() are needed to reflect the redraw immediately, so calling ui_flush() without update_screen() won't work (and leads to the flicker here).
• For cursor, statuscolumn, statusline and winbar, only a ui_flush() is needed to reflect the redraw immediately.

[luukvbaal]

This only happens with treesitter because treesitter uses nvim__redraw. Related: #28668

Unrelated but I found that it still happens even after vim.treesitter.stop(). This is because stop() does not actually stop the highlighter's on_bytes callback.

The issue also happens when calling anything else that flushes the cursor position at msg_row/col still at the end of the newly changed cmdline. I.e. the following reproduces the flickering also without treesitter enabled:

local function replace_first_line_preview(opts, preview_ns, preview_buf)
  local replacement = opts.args
  vim.api.nvim_buf_set_text(0, 0, 0, 1, -1, { replacement })
  vim.cmd.sleep("1m")
  -- vim.cmd.redrawtabline()
  return 2
end

vim.treesitter.stop()
vim.api.nvim_create_user_command(
  "ReplaceFirstLine",
  function() end,
  { nargs = 1, preview = replace_first_line_preview }
)

Unlikely that any of those commands make sense to call inside the cmdpreview callback but I think it does indicate that a proper solution would be to ensure that the cursor is at the correct position before doing cmdpreview: #31040.

Hmm, there is a difference between different options of nvim__redraw regarding how the redraw can be reflected immediately:

True, do you think that presents a problem intrinsically? Currently nvim__redraw tries to not bother the caller with this and do what's needed implicitly.

For cursor, statuscolumn, statusline and winbar, only a ui_flush() is needed to reflect the redraw immediately.

True except for statuscolumn which does require update_screen().

For range, both update_screen() and ui_flush() are needed to reflect the redraw immediately, so calling ui_flush() without update_screen() won't work (and leads to the flicker here).

Yeah nvim__redraw->range() currently does more than what the old nvim__buf_redraw_range() did, and that the ui_flush() doesn't actually reflect what's intended to be redrawn for the TS changedtree callback (updated highlights). I raised this previously in the original PR:https://github.com/neovim/neovim/pull/28101#issuecomment-209105796 (without response). Currently it basically serves as a "flush pressed keys because TS callback may take a long time" (unintended).

We can either keep the ui_flush() for range (which was established to have improved editor responsiveness when added to the TS highlighter path), in which case it should probably also update_screen() (one doesn't make sense without the other as you say although I don't think it's what leads to the flicker per se).

Or we can special case range and omit it the ui_flush() to keep it working like in 0.9 if there is a valid reason for doing so.

Further findings:

On top of the on_bytes callback still being active after vim.treesitter.stop(), it seems entirely redundant in the first place? AFAIU an on_bytes callback should mean that the buffer range is already marked for redraw by change.c. Removing the callback altogether seems to confirm that, which doesn't yield any failed tests. I raised #31041 to do away with it.

So I think calling update_screen() and ui_flush() implicitly for range would make sense: #31042. But only if we get rid of the on_bytes callback as there it would just yield double redraws since the usual pattern is extmark_splice (emits on_bytes) -> changed_() marking the buffer to be redrawn again.