From 5e521c3b5ac3905ba4b34fead3dcd9b69e5196ed Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Sun, 23 Feb 2025 15:58:16 +0000 Subject: [PATCH] fix(terminal): avoid tailed cursor in focused terminal in events Problem: in terminal mode, adjust_topline moves curwin's cursor to the last row so set_topline tails the non-scrollback area. This may result in the observed cursor position remaining tailed in events within the focused terminal, rather than reflecting the actual cursor position. Solution: use the focused terminal's actual cursor position immediately, rather than relying on the next terminal_check_cursor call in terminal_check to set it. Note: Maybe also possible for terminal mode cursor position to be stale (reporting the normal mode position) when switching buffers in events to another terminal until the next terminal_check call? (or until refresh_terminal is called for it) Maybe not worth fixing that, though. --- src/nvim/mbyte.c | 2 +- src/nvim/terminal.c | 9 ++++- test/functional/terminal/cursor_spec.lua | 49 ++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/nvim/mbyte.c b/src/nvim/mbyte.c index 80b94d5887..b9bac8c947 100644 --- a/src/nvim/mbyte.c +++ b/src/nvim/mbyte.c @@ -2185,7 +2185,7 @@ void mb_adjust_cursor(void) } /// Checks and adjusts cursor column. Not mode-dependent. -/// @see check_cursor_col_win +/// @see check_cursor_col /// /// @param win_ Places cursor on a valid column for this window. void mb_check_adjust_col(void *win_) diff --git a/src/nvim/terminal.c b/src/nvim/terminal.c index 0e58541f26..1690e7f6d3 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -2238,8 +2238,9 @@ static void adjust_topline(Terminal *term, buf_T *buf, int added) if (wp->w_buffer == buf) { linenr_T ml_end = buf->b_ml.ml_line_count; bool following = ml_end == wp->w_cursor.lnum + added; // cursor at end? + bool focused = wp == curwin && is_focused(term); - if (following || (wp == curwin && is_focused(term))) { + if (following || focused) { // "Follow" the terminal output wp->w_cursor.lnum = ml_end; set_topline(wp, MAX(wp->w_cursor.lnum - wp->w_view_height + 1, 1)); @@ -2247,7 +2248,11 @@ static void adjust_topline(Terminal *term, buf_T *buf, int added) // Ensure valid cursor for each window displaying this terminal. wp->w_cursor.lnum = MIN(wp->w_cursor.lnum, ml_end); } - mb_check_adjust_col(wp); + if (focused) { + terminal_check_cursor(); + } else { + mb_check_adjust_col(wp); + } } } } diff --git a/test/functional/terminal/cursor_spec.lua b/test/functional/terminal/cursor_spec.lua index 0be8321d24..4835b939f5 100644 --- a/test/functional/terminal/cursor_spec.lua +++ b/test/functional/terminal/cursor_spec.lua @@ -5,10 +5,13 @@ local tt = require('test.functional.testterm') local feed, clear = n.feed, n.clear local testprg, command = n.testprg, n.command local eq, eval = t.eq, n.eval +local api = n.api +local exec_lua = n.exec_lua local matches = t.matches local call = n.call local hide_cursor = tt.hide_cursor local show_cursor = tt.show_cursor +local retry = t.retry local is_os = t.is_os local skip = t.skip @@ -460,6 +463,52 @@ describe(':terminal cursor', function() eq(0, screen._mode_info[terminal_mode_idx].blinkon) eq(0, screen._mode_info[terminal_mode_idx].blinkoff) end) + + it('position correct within events', function() + local term, term_unfocused = exec_lua(function() + vim.cmd 'bwipeout!' + local term_unfocused = vim.api.nvim_open_term(0, {}) + vim.cmd.vnew() + vim.cmd.wincmd '|' + local term = vim.api.nvim_open_term(0, {}) + -- We'll use this keymap to pause the main loop while we send events, as we want the test to + -- run within the same terminal_execute call (while using test suite facilities like retry). + vim.keymap.set('t', '', 'let g:sleepy = 1 | sleep 5000 | let g:sleepy = 0') + return term, term_unfocused + end) + feed('i') + + local function check_pos(expected_pos, expected_virtcol, chan, data) + api.nvim_chan_send(chan, data) -- Using nvim_chan_send so terminal_receive is immediate. + + -- Results won't be visible until refresh_terminal is called, which happens on a timer. + retry(nil, nil, function() + eq(expected_pos, eval("getpos('.')[1:]")) + end) + eq(expected_virtcol, eval("virtcol('.', 1)")) + eq(1, eval('g:sleepy')) -- :sleep shouldn't have timed out. + end + + check_pos({ 1, 4, 0 }, { 4, 4 }, term, 'foo') + -- double-width char at end (3 bytes) + check_pos({ 2, 13, 0 }, { 12, 12 }, term, '\r\nbarbaaaar哦') + -- Move to 1,12 (beyond eol; sets coladd) + check_pos({ 1, 4, 8 }, { 12, 12 }, term, '\27[1;12H') + -- Move to 4,1 + check_pos({ 4, 1, 0 }, { 1, 1 }, term, '\27[4;1H') + -- Move to 4,5 (beyond eol; sets coladd) + check_pos({ 4, 1, 4 }, { 5, 5 }, term, '\27[4;5H') + -- Move to 2,10 (head of wide char) + check_pos({ 2, 10, 0 }, { 10, 11 }, term, '\27[2;10H') + -- Move to 2,11 (non-head of wide char) + check_pos({ 2, 10, 0 }, { 10, 11 }, term, '\27[2;11H') + -- Move to 2,12 (after wide char) + check_pos({ 2, 13, 0 }, { 12, 12 }, term, '\27[2;12H') + -- Move to 2,13 (beyond eol; sets coladd) + check_pos({ 2, 13, 1 }, { 13, 13 }, term, '\27[2;13H') + -- Cursor movement in unfocused terminal shouldn't affect us + check_pos({ 2, 13, 1 }, { 13, 13 }, term_unfocused, 'amogus') + end) end) describe('buffer cursor position is correct in terminal without number column', function()