fix(terminal): avoid more `busy_start` lacking `busy_stop` #32509

seandewar · 2025-02-18T15:18:58Z

Problem: after #32458, it may still be possible for busy_start UI events to be emitted without matching busy_stops in the terminal.

Solution: do terminal_enter's cursor visibility check immediately after setting/restoring State so it occurs before events. This ensures that if pending escape sequences are processed while in terminal_enter, the cursor's initial visibility is set before is_focused is checked by term_settermprop.

As a result, we can move the call to showmode back to where it was originally.

Problem: after neovim#32458, it may still be possible for `busy_start` UI events to be emitted without matching `busy_stop`s in the terminal. Solution: do `terminal_enter`'s cursor visibility check immediately after setting/restoring State so it occurs before events. This ensures that if pending escape sequences are processed while in `terminal_enter`, the cursor's initial visibility is set before `is_focused` is checked by `term_settermprop`. As a result, we can move the call to `showmode` back to where it was originally.

phanen · 2025-02-21T15:19:14Z

This problem still exist for me... I can stably reproduce with:
~~nvim --clean -u minimal.lua +term bash, then press a<c-x><c-e>.~~

VIMRUNTIME=./runtime VISUAL='./build/bin/nvim --clean -u minimal.lua' ./build/bin/nvim --clean -u minimal.lua +'term bash' (then a<c-x><c-e>).

minimal.lua:

-- Run this file as `nvim --clean -u minimal.lua`
for name, url in pairs {
  ['flatten.nvim'] = 'https://github.com/willothy/flatten.nvim',
  ['gitsigns.nvim'] = 'https://github.com/lewis6991/gitsigns.nvim',
  ['nvim-treesitter-context'] = 'https://github.com/nvim-treesitter/nvim-treesitter-context',
} do
  local install_path = vim.fn.fnamemodify('nvim_issue/' .. name, ':p')
  if vim.fn.isdirectory(install_path) == 0 then
    vim.fn.system { 'git', 'clone', '--depth=1', url, install_path }
  end
  vim.opt.runtimepath:append(install_path)
end

vim.cmd([[au BufReadPost * sil! norm! g`"zv']])
vim.o.clipboard = 'unnamedplus'

require('flatten').setup {}
require('treesitter-context').setup {}
require('gitsigns').setup {}

Bisected, not happened before https://github.com/neovim/neovim/commit/0dd933265ff2e64786fd30f949e767e10f401519...

seandewar · 2025-02-21T18:32:05Z

@phanen Hmm, that doesn't reproduce for me. Maybe the inner Nvim (that's momentarily spawned by <C-X><C-E> before flatten does its magic) is still using your personal config there, as I had to
modify minimal.lua to set $VISUAL to build/bin/nvim --clean -u minimal.lua so flatten works for me (otherwise the inner Nvim used my personal config which doesn't have flatten; both the inner and outer Nvim need it to work). 🤔

phanen · 2025-02-22T01:58:45Z

@seandewar Sorry for missing that. I just updated my reproduce. (also rm -rf ~/.config/nvim), hope this can reproduce..

seandewar · 2025-02-22T02:43:49Z

Thanks! That repro'd it for me; this seems to be a unique case.

From a quick GDBing, it seems nvim_set_current_buf is called from an event while in terminal_execute, where the destination buffer has no terminal. That causes terminal_execute to return early here.

Although s->term remains unchanged here, multiple events including escape codes to change cursor visibility can be processed within the same terminal_execute call, causing term_settermprop to not call ui_busy_start/stop here if cursor visibility changes after the buffer switch, but before the epilogue of terminal_enter (as is_focused now returns false).

I don't have any time left today; I'll try to address that later. 👍

MariaSolOs · 2025-02-23T05:32:33Z

I'm also still having cursor issues :(

github-actions bot added the terminal built-in :terminal or :shell label Feb 18, 2025

seandewar marked this pull request as ready for review February 18, 2025 16:07

seandewar merged commit f3ce675 into neovim:master Feb 19, 2025
38 checks passed

seandewar deleted the i-hate-cursors branch February 19, 2025 10:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(terminal): avoid more `busy_start` lacking `busy_stop` #32509

fix(terminal): avoid more `busy_start` lacking `busy_stop` #32509

fix(terminal): avoid more busy_start lacking busy_stop #32509

fix(terminal): avoid more busy_start lacking busy_stop #32509

Conversation

fix(terminal): avoid more `busy_start` lacking `busy_stop` #32509

fix(terminal): avoid more `busy_start` lacking `busy_stop` #32509