From 8c5bc4920af27e6289e8c999a1c05d2b1b4c580f 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 01/10] 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 5b94a47f74..e60c1ce267 100644 --- a/src/nvim/mbyte.c +++ b/src/nvim/mbyte.c @@ -2187,7 +2187,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 5f4ff44a32..ac903f9adb 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -2251,8 +2251,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_height_inner + 1, 1)); @@ -2260,7 +2261,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 93e55bb103..406d3ffad6 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 @@ -470,6 +473,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() From 7f5427b857273e0eef8df2102d67a4bc46e3f18c Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Thu, 27 Feb 2025 12:46:56 +0000 Subject: [PATCH 02/10] fix(terminal): add various missing redraws Problem: missing redraws when restoring saved cursorline/column, plus missing statusline and mode redraws when not updating the screen in terminal mode. Solution: schedule the redraws in a similar manner to other modes and remove some now unnecessary redrawing logic. Redraw if cursorline-related options change from entering terminal mode. This fixes test failures in later commits. WTF: TextChangedT triggers based on must_redraw, which is... fun...? Try to preserve its behaviour as much as we can for now. --- src/nvim/terminal.c | 31 +++-- test/functional/terminal/window_spec.lua | 143 +++++++++++++++++++++++ test/functional/ui/title_spec.lua | 17 ++- 3 files changed, 177 insertions(+), 14 deletions(-) diff --git a/src/nvim/terminal.c b/src/nvim/terminal.c index ac903f9adb..6e5f48aded 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -710,18 +710,19 @@ bool terminal_enter(void) } else { curwin->w_p_cul = false; } - if (curwin->w_p_cuc) { - redraw_later(curwin, UPD_SOME_VALID); - } curwin->w_p_cuc = false; curwin->w_p_so = 0; curwin->w_p_siso = 0; + if (curwin->w_p_cuc != save_w_p_cuc) { + redraw_later(curwin, UPD_SOME_VALID); + } else if (curwin->w_p_cul != save_w_p_cul + || (curwin->w_p_cul && curwin->w_p_culopt_flags != save_w_p_culopt_flags)) { + redraw_later(curwin, UPD_VALID); + } s->term->pending.cursor = true; // Update the cursor shape table adjust_topline(s->term, buf, 0); // scroll to end showmode(); - curwin->w_redr_status = true; // For mode() in statusline. #8323 - redraw_custom_title_later(); ui_cursor_shape(); apply_autocmds(EVENT_TERMENTER, NULL, NULL, false, curbuf); may_trigger_modechanged(); @@ -749,6 +750,12 @@ bool terminal_enter(void) (void)parse_shape_opt(SHAPE_CURSOR); if (save_curwin == curwin->handle) { // Else: window was closed. + if (save_w_p_cuc != curwin->w_p_cuc) { + redraw_later(curwin, UPD_SOME_VALID); + } else if (save_w_p_cul != curwin->w_p_cul + || (save_w_p_cul && save_w_p_culopt_flags != curwin->w_p_culopt_flags)) { + redraw_later(curwin, UPD_VALID); + } curwin->w_p_cul = save_w_p_cul; if (save_w_p_culopt) { free_string_option(curwin->w_p_culopt); @@ -813,10 +820,18 @@ static int terminal_check(VimState *state) assert(s->term == curbuf->terminal); terminal_check_cursor(); validate_cursor(curwin); + const bool text_changed = must_redraw != 0; + show_cursor_info_later(false); if (must_redraw) { update_screen(); - + } else { + redraw_statuslines(); + if (clear_cmdline || redraw_cmdline || redraw_mode) { + showmode(); // clear cmdline and show mode + } + } + if (text_changed) { // Make sure an invoked autocmd doesn't delete the buffer (and the // terminal) under our fingers. curbuf->b_locked++; @@ -832,10 +847,6 @@ static int terminal_check(VimState *state) may_trigger_win_scrolled_resized(); - if (need_maketitle) { // Update title in terminal-mode. #7248 - maketitle(); - } - setcursor(); refresh_cursor(s->term, &s->cursor_visible); ui_flush(); diff --git a/test/functional/terminal/window_spec.lua b/test/functional/terminal/window_spec.lua index a65d18de70..0fc5e9e7f1 100644 --- a/test/functional/terminal/window_spec.lua +++ b/test/functional/terminal/window_spec.lua @@ -189,6 +189,149 @@ describe(':terminal window', function() ]]) end) end) + + it('redrawn when restoring cursorline/column', function() + screen:set_default_attr_ids({ + [1] = { bold = true }, + [2] = { foreground = 130 }, + [3] = { foreground = 130, underline = true }, + [12] = { underline = true }, + [19] = { background = 7 }, + }) + + feed([[]]) + command('setlocal cursorline') + screen:expect([[ + tty ready | + {12:^ }| + |*5 + ]]) + feed('i') + screen:expect([[ + tty ready | + ^ | + |*4 + {1:-- TERMINAL --} | + ]]) + feed([[]]) + screen:expect([[ + tty ready | + {12:^ }| + |*5 + ]]) + + command('setlocal number') + screen:expect([[ + {2: 1 }tty ready | + {3: 2 }{12:^rows: 6, cols: 46 }| + {2: 3 } | + {2: 4 } | + {2: 5 } | + {2: 6 } | + | + ]]) + feed('i') + screen:expect([[ + {2: 1 }tty ready | + {2: 2 }rows: 6, cols: 46 | + {3: 3 }^ | + {2: 4 } | + {2: 5 } | + {2: 6 } | + {1:-- TERMINAL --} | + ]]) + feed([[]]) + screen:expect([[ + {2: 1 }tty ready | + {2: 2 }rows: 6, cols: 46 | + {3: 3 }{12:^ }| + {2: 4 } | + {2: 5 } | + {2: 6 } | + | + ]]) + + command('setlocal nonumber nocursorline cursorcolumn') + screen:expect([[ + {19:t}ty ready | + {19:r}ows: 6, cols: 46 | + ^rows: 6, cols: 50 | + {19: } |*3 + | + ]]) + feed('i') + screen:expect([[ + tty ready | + rows: 6, cols: 46 | + rows: 6, cols: 50 | + ^ | + |*2 + {1:-- TERMINAL --} | + ]]) + feed([[]]) + screen:expect([[ + {19:t}ty ready | + {19:r}ows: 6, cols: 46 | + {19:r}ows: 6, cols: 50 | + ^ | + {19: } |*2 + | + ]]) + end) + + it('redraws cursor info in terminal mode', function() + skip(is_os('win'), '#31587') + command('file AMOGUS | set laststatus=2 ruler') + screen:expect([[ + tty ready | + rows: 5, cols: 50 | + ^ | + |*2 + {17:AMOGUS 3,0-1 All}| + {3:-- TERMINAL --} | + ]]) + feed_data('you are the imposter') + screen:expect([[ + tty ready | + rows: 5, cols: 50 | + you are the imposter^ | + |*2 + {17:AMOGUS 3,21 All}| + {3:-- TERMINAL --} | + ]]) + feed([[]]) + screen:expect([[ + tty ready | + rows: 5, cols: 50 | + you are the imposte^r | + |*2 + {17:AMOGUS 3,20 All}| + | + ]]) + end) + + it('redraws stale statuslines and mode when not updating screen', function() + command('file foo | set ruler | vsplit') + screen:expect([[ + tty ready │tty ready | + rows: 5, cols: 25 │rows: 5, cols: 25 | + ^ │ | + │ |*2 + {17:foo 3,0-1 All }{18:foo 2,0-1 Top}| + {3:-- TERMINAL --} | + ]]) + command("call win_execute(win_getid(winnr('#')), 'call cursor(1, 1)')") + screen:expect([[ + tty ready │tty ready | + rows: 5, cols: 25 │rows: 5, cols: 25 | + ^ │ | + │ |*2 + {17:foo 3,0-1 All }{18:foo 1,1 All}| + {3:-- TERMINAL --} | + ]]) + command('echo ""') + screen:expect_unchanged() + end) end) describe(':terminal with multigrid', function() diff --git a/test/functional/ui/title_spec.lua b/test/functional/ui/title_spec.lua index 2de1e71457..5e7c9c74b9 100644 --- a/test/functional/ui/title_spec.lua +++ b/test/functional/ui/title_spec.lua @@ -79,18 +79,27 @@ describe('title', function() it('is updated in Terminal mode', function() api.nvim_set_option_value('title', true, {}) - api.nvim_set_option_value('titlestring', '(%{mode(1)}) | nvim', {}) + api.nvim_set_option_value('titlestring', '%t (%{mode(1)}) | nvim', {}) fn.jobstart({ n.testprg('shell-test'), 'INTERACT' }, { term = true }) + api.nvim_buf_set_name(0, 'shell-test') screen:expect(function() - eq('(nt) | nvim', screen.title) + eq('shell-test (nt) | nvim', screen.title) end) feed('i') screen:expect(function() - eq('(t) | nvim', screen.title) + eq('shell-test (t) | nvim', screen.title) + end) + api.nvim_set_option_value('titlelen', 1, {}) + screen:expect(function() + eq(']]) screen:expect(function() - eq('(nt) | nvim', screen.title) + eq('shell-test (nt) | nvim', screen.title) end) end) From f3f6705075676a812ed4ddb7b3409d77ed499861 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Fri, 28 Feb 2025 10:15:25 +0000 Subject: [PATCH 03/10] fix(terminal): avoid events messing up topline of focused terminal Problem: topline of a focused terminal window may not tail to terminal output if events scroll the window. Solution: set the topline in terminal_check_cursor. --- src/nvim/terminal.c | 20 ++++-- test/functional/terminal/window_spec.lua | 82 ++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 7 deletions(-) diff --git a/src/nvim/terminal.c b/src/nvim/terminal.c index 6e5f48aded..e83ab5bd94 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -800,6 +800,11 @@ static void terminal_check_cursor(void) curwin->w_wcol = term->cursor.col + win_col_off(curwin); curwin->w_cursor.lnum = MIN(curbuf->b_ml.ml_line_count, row_to_linenr(term, term->cursor.row)); + const linenr_T topline = MAX(curbuf->b_ml.ml_line_count - curwin->w_height_inner + 1, 1); + // Don't update topline if unchanged to avoid unnecessary redraws. + if (topline != curwin->w_topline) { + set_topline(curwin, topline); + } // Nudge cursor when returning to normal-mode. int off = is_focused(term) ? 0 : (curwin->w_p_rl ? 1 : -1); coladvance(curwin, MAX(0, term->cursor.col + off)); @@ -2260,11 +2265,16 @@ static void adjust_topline(Terminal *term, buf_T *buf, int added) { FOR_ALL_TAB_WINDOWS(tp, wp) { if (wp->w_buffer == buf) { + if (wp == curwin && is_focused(term)) { + // Move window cursor to terminal cursor's position and "follow" output. + terminal_check_cursor(); + continue; + } + 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 || focused) { + if (following) { // "Follow" the terminal output wp->w_cursor.lnum = ml_end; set_topline(wp, MAX(wp->w_cursor.lnum - wp->w_height_inner + 1, 1)); @@ -2272,11 +2282,7 @@ 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); } - if (focused) { - terminal_check_cursor(); - } else { - mb_check_adjust_col(wp); - } + mb_check_adjust_col(wp); } } } diff --git a/test/functional/terminal/window_spec.lua b/test/functional/terminal/window_spec.lua index 0fc5e9e7f1..34088bb699 100644 --- a/test/functional/terminal/window_spec.lua +++ b/test/functional/terminal/window_spec.lua @@ -3,8 +3,10 @@ local n = require('test.functional.testnvim')() local tt = require('test.functional.testterm') local feed_data = tt.feed_data +local feed_csi = tt.feed_csi local feed, clear = n.feed, n.clear local poke_eventloop = n.poke_eventloop +local exec_lua = n.exec_lua local command = n.command local retry = t.retry local eq = t.eq @@ -332,6 +334,86 @@ describe(':terminal window', function() command('echo ""') screen:expect_unchanged() end) + + it('has correct topline if scrolled by events', function() + skip(is_os('win'), '#31587') + local lines = {} + for i = 1, 10 do + table.insert(lines, 'cool line ' .. i) + end + feed_data(lines) + feed_csi('1;1H') -- Cursor to 1,1 (after any scrollback) + + -- :sleep (with leeway) until the refresh_terminal uv timer event triggers before we move the + -- cursor. Check that the next terminal_check tails topline correctly. + command('set ruler | sleep 20m | call nvim_win_set_cursor(0, [1, 0])') + screen:expect([[ + ^cool line 5 | + cool line 6 | + cool line 7 | + cool line 8 | + cool line 9 | + cool line 10 | + {3:-- TERMINAL --} 6,1 Bot | + ]]) + command('call nvim_win_set_cursor(0, [1, 0])') + screen:expect_unchanged() + + feed_csi('2;5H') -- Cursor to 2,5 (after any scrollback) + screen:expect([[ + cool line 5 | + cool^ line 6 | + cool line 7 | + cool line 8 | + cool line 9 | + cool line 10 | + {3:-- TERMINAL --} 7,5 Bot | + ]]) + -- Check topline correct after leaving terminal mode. + -- The new cursor position is one column left of the terminal's actual cursor position. + command('stopinsert | call nvim_win_set_cursor(0, [1, 0])') + screen:expect([[ + cool line 5 | + coo^l line 6 | + cool line 7 | + cool line 8 | + cool line 9 | + cool line 10 | + 7,4 Bot | + ]]) + end) + + it('not unnecessarily redrawn by events', function() + eq('t', eval('mode()')) + exec_lua(function() + _G.redraws = {} + local ns = vim.api.nvim_create_namespace('test') + vim.api.nvim_set_decoration_provider(ns, { + on_start = function() + table.insert(_G.redraws, 'start') + end, + on_win = function(_, win) + table.insert(_G.redraws, 'win ' .. win) + end, + on_end = function() + table.insert(_G.redraws, 'end') + end, + }) + -- Setting a decoration provider typically causes an initial redraw. + vim.cmd.redraw() + _G.redraws = {} + end) + + -- The event we sent above to set up the test shouldn't have caused a redraw. + -- For good measure, also poke the event loop. + poke_eventloop() + eq({}, exec_lua('return _G.redraws')) + + -- Redraws if we do something useful, of course. + feed_data('foo') + screen:expect { any = 'foo' } + eq({ 'start', 'win 1000', 'end' }, exec_lua('return _G.redraws')) + end) end) describe(':terminal with multigrid', function() From 934d28558d48a3714261d51c86aba48ecdeb67eb Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Mon, 3 Mar 2025 14:19:32 +0000 Subject: [PATCH 04/10] fix(terminal): check size when creating new tabpage Problem: when creating a new tabpage with a terminal window, terminal size is not updated if there is no statusline. Solution: do not rely on last_status to implicitly call terminal_check_size as a side effect of making room for a statusline; call it directly. --- src/nvim/window.c | 4 +++ test/functional/terminal/window_spec.lua | 32 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/nvim/window.c b/src/nvim/window.c index 3f4c24bf17..65012f8564 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -4245,6 +4245,10 @@ int win_new_tabpage(int after, char *filename) newtp->tp_topframe = topframe; last_status(false); + if (curbuf->terminal) { + terminal_check_size(curbuf->terminal); + } + redraw_all_later(UPD_NOT_VALID); tabpage_check_windows(old_curtab); diff --git a/test/functional/terminal/window_spec.lua b/test/functional/terminal/window_spec.lua index 34088bb699..2dcaf203d4 100644 --- a/test/functional/terminal/window_spec.lua +++ b/test/functional/terminal/window_spec.lua @@ -1,5 +1,6 @@ local t = require('test.testutil') local n = require('test.functional.testnvim')() +local Screen = require('test.functional.ui.screen') local tt = require('test.functional.testterm') local feed_data = tt.feed_data @@ -383,6 +384,37 @@ describe(':terminal window', function() ]]) end) + it('in new tabpage has correct terminal size', function() + screen:set_default_attr_ids({ + [1] = { reverse = true }, + [3] = { bold = true }, + [17] = { background = 2, foreground = Screen.colors.Grey0 }, + [18] = { background = 2, foreground = 8 }, + [19] = { underline = true, foreground = Screen.colors.Grey0, background = 7 }, + [20] = { underline = true, foreground = 5, background = 7 }, + }) + + command('file foo | vsplit') + screen:expect([[ + tty ready │tty ready | + rows: 5, cols: 25 │rows: 5, cols: 25 | + ^ │ | + │ |*2 + {17:foo }{18:foo }| + {3:-- TERMINAL --} | + ]]) + command('tab split') + screen:expect([[ + {19: }{20:2}{19: foo }{3: foo }{1: }{19:X}| + tty ready | + rows: 5, cols: 25 | + rows: 5, cols: 50 | + ^ | + | + {3:-- TERMINAL --} | + ]]) + end) + it('not unnecessarily redrawn by events', function() eq('t', eval('mode()')) exec_lua(function() From 2eea65fe689d55bdf8cb5ff7e953fbb4c1475a98 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Mon, 3 Mar 2025 15:53:03 +0000 Subject: [PATCH 05/10] fix(terminal): update winopts and focus when switching terminals Problem: window options and terminal focus notifications not updated when switching terminals without leaving terminal mode. Solution: update them. --- src/nvim/terminal.c | 135 +++++++++++------- test/functional/terminal/buffer_spec.lua | 67 +++++++++ test/functional/terminal/window_spec.lua | 167 +++++++++++++++++++++++ 3 files changed, 323 insertions(+), 46 deletions(-) diff --git a/src/nvim/terminal.c b/src/nvim/terminal.c index e83ab5bd94..78ebd72e4d 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -112,6 +112,16 @@ typedef struct { bool got_bsl; ///< if the last input was bool got_bsl_o; ///< if left terminal mode with bool cursor_visible; ///< cursor's current visibility; ensures matched busy_start/stop UI events + + // These fields remember the prior values of window options before entering terminal mode. + // Valid only when save_curwin_handle != 0. + handle_T save_curwin_handle; + bool save_w_p_cul; + char *save_w_p_culopt; + uint8_t save_w_p_culopt_flags; + int save_w_p_cuc; + OptInt save_w_p_so; + OptInt save_w_p_siso; } TerminalState; #ifdef INCLUDE_GENERATED_DECLARATIONS @@ -671,6 +681,75 @@ void terminal_check_size(Terminal *term) invalidate_terminal(term, -1, -1); } +static void set_terminal_winopts(TerminalState *const s) + FUNC_ATTR_NONNULL_ALL +{ + assert(s->save_curwin_handle == 0); + + // Disable these options in terminal-mode. They are nonsense because cursor is + // placed at end of buffer to "follow" output. #11072 + s->save_curwin_handle = curwin->handle; + s->save_w_p_cul = curwin->w_p_cul; + s->save_w_p_culopt = NULL; + s->save_w_p_culopt_flags = curwin->w_p_culopt_flags; + s->save_w_p_cuc = curwin->w_p_cuc; + s->save_w_p_so = curwin->w_p_so; + s->save_w_p_siso = curwin->w_p_siso; + + if (curwin->w_p_cul && curwin->w_p_culopt_flags & kOptCuloptFlagNumber) { + if (!strequal(curwin->w_p_culopt, "number")) { + s->save_w_p_culopt = curwin->w_p_culopt; + curwin->w_p_culopt = xstrdup("number"); + } + curwin->w_p_culopt_flags = kOptCuloptFlagNumber; + } else { + curwin->w_p_cul = false; + } + curwin->w_p_cuc = false; + curwin->w_p_so = 0; + curwin->w_p_siso = 0; + + if (curwin->w_p_cuc != s->save_w_p_cuc) { + redraw_later(curwin, UPD_SOME_VALID); + } else if (curwin->w_p_cul != s->save_w_p_cul + || (curwin->w_p_cul && curwin->w_p_culopt_flags != s->save_w_p_culopt_flags)) { + redraw_later(curwin, UPD_VALID); + } +} + +static void unset_terminal_winopts(TerminalState *const s) + FUNC_ATTR_NONNULL_ALL +{ + assert(s->save_curwin_handle != 0); + + win_T *const wp = handle_get_window(s->save_curwin_handle); + if (!wp) { + free_string_option(s->save_w_p_culopt); + s->save_curwin_handle = 0; + return; + } + + if (win_valid(wp)) { // No need to redraw if window not in curtab. + if (s->save_w_p_cuc != wp->w_p_cuc) { + redraw_later(wp, UPD_SOME_VALID); + } else if (s->save_w_p_cul != wp->w_p_cul + || (s->save_w_p_cul && s->save_w_p_culopt_flags != wp->w_p_culopt_flags)) { + redraw_later(wp, UPD_VALID); + } + } + + wp->w_p_cul = s->save_w_p_cul; + if (s->save_w_p_culopt) { + free_string_option(wp->w_p_culopt); + wp->w_p_culopt = s->save_w_p_culopt; + } + wp->w_p_culopt_flags = s->save_w_p_culopt_flags; + wp->w_p_cuc = s->save_w_p_cuc; + wp->w_p_so = s->save_w_p_so; + wp->w_p_siso = s->save_w_p_siso; + s->save_curwin_handle = 0; +} + /// Implements MODE_TERMINAL state. :help Terminal-mode bool terminal_enter(void) { @@ -692,33 +771,7 @@ bool terminal_enter(void) mapped_ctrl_c |= MODE_TERMINAL; // Always map CTRL-C to avoid interrupt. RedrawingDisabled = false; - // Disable these options in terminal-mode. They are nonsense because cursor is - // placed at end of buffer to "follow" output. #11072 - handle_T save_curwin = curwin->handle; - bool save_w_p_cul = curwin->w_p_cul; - char *save_w_p_culopt = NULL; - uint8_t save_w_p_culopt_flags = curwin->w_p_culopt_flags; - int save_w_p_cuc = curwin->w_p_cuc; - OptInt save_w_p_so = curwin->w_p_so; - OptInt save_w_p_siso = curwin->w_p_siso; - if (curwin->w_p_cul && curwin->w_p_culopt_flags & kOptCuloptFlagNumber) { - if (strcmp(curwin->w_p_culopt, "number") != 0) { - save_w_p_culopt = curwin->w_p_culopt; - curwin->w_p_culopt = xstrdup("number"); - } - curwin->w_p_culopt_flags = kOptCuloptFlagNumber; - } else { - curwin->w_p_cul = false; - } - curwin->w_p_cuc = false; - curwin->w_p_so = 0; - curwin->w_p_siso = 0; - if (curwin->w_p_cuc != save_w_p_cuc) { - redraw_later(curwin, UPD_SOME_VALID); - } else if (curwin->w_p_cul != save_w_p_cul - || (curwin->w_p_cul && curwin->w_p_culopt_flags != save_w_p_culopt_flags)) { - redraw_later(curwin, UPD_VALID); - } + set_terminal_winopts(s); s->term->pending.cursor = true; // Update the cursor shape table adjust_topline(s->term, buf, 0); // scroll to end @@ -749,25 +802,7 @@ bool terminal_enter(void) // Restore the terminal cursor to what is set in 'guicursor' (void)parse_shape_opt(SHAPE_CURSOR); - if (save_curwin == curwin->handle) { // Else: window was closed. - if (save_w_p_cuc != curwin->w_p_cuc) { - redraw_later(curwin, UPD_SOME_VALID); - } else if (save_w_p_cul != curwin->w_p_cul - || (save_w_p_cul && save_w_p_culopt_flags != curwin->w_p_culopt_flags)) { - redraw_later(curwin, UPD_VALID); - } - curwin->w_p_cul = save_w_p_cul; - if (save_w_p_culopt) { - free_string_option(curwin->w_p_culopt); - curwin->w_p_culopt = save_w_p_culopt; - } - curwin->w_p_culopt_flags = save_w_p_culopt_flags; - curwin->w_p_cuc = save_w_p_cuc; - curwin->w_p_so = save_w_p_so; - curwin->w_p_siso = save_w_p_siso; - } else if (save_w_p_culopt) { - free_string_option(save_w_p_culopt); - } + unset_terminal_winopts(s); // Tell the terminal it lost focus terminal_focus(s->term, false); @@ -954,11 +989,19 @@ static int terminal_execute(VimState *state, int key) if (curbuf->terminal == NULL) { return 0; } + if (s->save_curwin_handle != curwin->handle) { + // Terminal window changed, update window options. + unset_terminal_winopts(s); + set_terminal_winopts(s); + } if (s->term != curbuf->terminal) { // Active terminal buffer changed, flush terminal's cursor state to the UI + terminal_focus(s->term, false); + s->term = curbuf->terminal; s->term->pending.cursor = true; invalidate_terminal(s->term, -1, -1); + terminal_focus(s->term, true); } return 1; } diff --git a/test/functional/terminal/buffer_spec.lua b/test/functional/terminal/buffer_spec.lua index 943e445c25..16736db484 100644 --- a/test/functional/terminal/buffer_spec.lua +++ b/test/functional/terminal/buffer_spec.lua @@ -343,6 +343,73 @@ describe(':terminal buffer', function() |*4 ]]) end) + + it('reports focus notifications when requested', function() + feed([[]]) + exec_lua(function() + local function new_test_term() + local chan = vim.api.nvim_open_term(0, { + on_input = function(_, term, buf, data) + if data == '\27[I' then + vim.api.nvim_chan_send(term, 'focused\n') + elseif data == '\27[O' then + vim.api.nvim_chan_send(term, 'unfocused\n') + end + end, + }) + vim.api.nvim_chan_send(chan, '\27[?1004h') -- Enable focus reporting + end + + vim.cmd 'edit bar' + new_test_term() + vim.cmd 'vnew foo' + new_test_term() + vim.cmd 'vsplit' + end) + screen:expect([[ + ^ │ │ | + │ │ |*4 + {17:foo }{18:foo bar }| + | + ]]) + + feed('i') + screen:expect([[ + focused │focused │ | + ^ │ │ | + │ │ |*3 + {17:foo }{18:foo bar }| + {3:-- TERMINAL --} | + ]]) + -- Next window has the same terminal; no new notifications. + command('wincmd w') + screen:expect([[ + focused │focused │ | + │^ │ | + │ │ |*3 + {18:foo }{17:foo }{18:bar }| + {3:-- TERMINAL --} | + ]]) + -- Next window has a different terminal; expect new unfocus and focus notifications. + command('wincmd w') + screen:expect([[ + focused │focused │focused | + unfocused │unfocuse│^ | + │ │ |*3 + {18:foo foo }{17:bar }| + {3:-- TERMINAL --} | + ]]) + -- Leaving terminal mode; expect a new unfocus notification. + feed([[]]) + screen:expect([[ + focused │focused │focused | + unfocused │unfocuse│unfocused | + │ │^ | + │ │ |*2 + {18:foo foo }{17:bar }| + | + ]]) + end) end) describe(':terminal buffer', function() diff --git a/test/functional/terminal/window_spec.lua b/test/functional/terminal/window_spec.lua index 2dcaf203d4..8d07323b45 100644 --- a/test/functional/terminal/window_spec.lua +++ b/test/functional/terminal/window_spec.lua @@ -415,6 +415,173 @@ describe(':terminal window', function() ]]) end) + it('restores window options when switching terminals', function() + -- Make this a screen test to also check for proper redrawing. + screen:set_default_attr_ids({ + [1] = { bold = true }, + [2] = { foreground = Screen.colors.Gray0, background = 7, underline = true }, + [3] = { foreground = 5, background = 7, underline = true }, + [4] = { reverse = true }, + [5] = { bold = true, foreground = 5 }, + [6] = { foreground = 12 }, + [7] = { reverse = true, bold = true }, + [12] = { underline = true }, + [17] = { foreground = Screen.colors.Gray0, background = 2 }, + [18] = { foreground = 8, background = 2 }, + [19] = { background = 7 }, + }) + + feed([[]]) + command([[ + file foo + setlocal cursorline + vsplit + setlocal nocursorline cursorcolumn + ]]) + screen:expect([[ + {19:t}ty ready │tty ready | + ^rows: 5, cols: 25 │{12:rows: 5, cols: 25 }| + {19: } │ |*3 + {17:foo }{18:foo }| + | + ]]) + + feed('i') + screen:expect([[ + tty ready │tty ready | + rows: 5, cols: 25 │{12:rows: 5, cols: 25 }| + ^ │ | + │ |*2 + {17:foo }{18:foo }| + {1:-- TERMINAL --} | + ]]) + command('wincmd p') + screen:expect([[ + {19:t}ty ready │tty ready | + {19:r}ows: 5, cols: 25 │rows: 5, cols: 25 | + │^ | + {19: } │ |*2 + {18:foo }{17:foo }| + {1:-- TERMINAL --} | + ]]) + feed([[]]) + screen:expect([[ + {19:t}ty ready │tty ready | + {19:r}ows: 5, cols: 25 │rows: 5, cols: 25 | + │{12:^ }| + {19: } │ |*2 + {18:foo }{17:foo }| + | + ]]) + + -- Ensure things work when switching tabpages. + command('tab split | setlocal cursorline cursorcolumn') + screen:expect([[ + {2: }{3:2}{2: foo }{1: foo }{4: }{2:X}| + {19:t}ty ready | + {19:r}ows: 5, cols: 25 | + {12:^rows: 5, cols: 50 }| + {19: } |*2 + | + ]]) + feed('i') + screen:expect([[ + {2: }{3:2}{2: foo }{1: foo }{4: }{2:X}| + tty ready | + rows: 5, cols: 25 | + rows: 5, cols: 50 | + ^ | + | + {1:-- TERMINAL --} | + ]]) + command('tabprevious') + screen:expect([[ + {1: }{5:2}{1: foo }{2: foo }{4: }{2:X}| + {19:r}ows: 5, cols: 25 │rows: 5, cols: 25 | + rows: 5, cols: 50 │rows: 5, cols: 50 | + {19: } │^ | + {19: } │ | + {18:foo }{17:foo }| + {1:-- TERMINAL --} | + ]]) + feed([[]]) + screen:expect([[ + {1: }{5:2}{1: foo }{2: foo }{4: }{2:X}| + {19:r}ows: 5, cols: 25 │rows: 5, cols: 25 | + rows: 5, cols: 50 │rows: 5, cols: 50 | + {19: } │{12: }| + {19: } │^ | + {18:foo }{17:foo }| + | + ]]) + command('tabnext') + screen:expect([[ + {2: }{3:2}{2: foo }{1: foo }{4: }{2:X}| + {19:t}ty ready | + {19:r}ows: 5, cols: 25 | + {19:r}ows: 5, cols: 50 | + {12:^ }| + {19: } | + | + ]]) + + -- Closing windows shouldn't break things. + command('tabprevious') + feed('i') + screen:expect([[ + {1: }{5:2}{1: foo }{2: foo }{4: }{2:X}| + {19:r}ows: 5, cols: 25 │rows: 5, cols: 25 | + rows: 5, cols: 50 │rows: 5, cols: 50 | + {19: } │ | + {19: } │^ | + {18:foo }{17:foo }| + {1:-- TERMINAL --} | + ]]) + command('quit') + screen:expect([[ + {1: foo }{2: foo }{4: }{2:X}| + tty ready | + rows: 5, cols: 25 | + rows: 5, cols: 50 | + ^ | + | + {1:-- TERMINAL --} | + ]]) + feed([[]]) + screen:expect([[ + {1: foo }{2: foo }{4: }{2:X}| + {19:t}ty ready | + {19:r}ows: 5, cols: 25 | + {19:r}ows: 5, cols: 50 | + ^ | + {19: } | + | + ]]) + + -- Switching to a non-terminal. + command('vnew') + feed([[pi]]) + screen:expect([[ + {1: }{5:2}{1: foo }{2: foo }{4: }{2:X}| + │rows: 5, cols: 25 | + {6:~ }│rows: 5, cols: 50 | + {6:~ }│ | + {6:~ }│^ | + {4:[No Name] }{17:foo }| + {1:-- TERMINAL --} | + ]]) + command('wincmd p') + screen:expect([[ + {1: }{5:2}{1: [No Name] }{2: foo }{4: }{2:X}| + ^ │{19:r}ows: 5, cols: 25 | + {6:~ }│{19:r}ows: 5, cols: 50 | + {6:~ }│ | + {6:~ }│{19: } | + {7:[No Name] }{18:foo }| + | + ]]) + end) + it('not unnecessarily redrawn by events', function() eq('t', eval('mode()')) exec_lua(function() From 46d68e5290fc40e2ea87c46e60fdfb22b4703d08 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Mon, 3 Mar 2025 20:43:07 +0000 Subject: [PATCH 06/10] fix(terminal): patch various autocommand-related holes Problem: autocommands can cause various problems in terminal mode, which can lead to crashes, for example. Solution: fix found issues. Move some checks to terminal_check and guard against autocommands messing with things. Trigger TermEnter/Leave after terminal mode has changed/restored most state. Wipeout the correct buffer if TermLeave switches buffers and fix a UAF if it or WinScrolled/Resized frees the terminal prematurely. These changes also allow us to remove the buffer restrictions on TextChangedT; they were inadequate in stopping some issues, and WinScrolled/Resized was lacking them anyway. --- src/nvim/terminal.c | 116 +++++++++++++---------- test/functional/autocmd/termxx_spec.lua | 68 +++++++++---- test/functional/terminal/buffer_spec.lua | 73 ++++++++++++++ test/functional/terminal/window_spec.lua | 3 +- 4 files changed, 193 insertions(+), 67 deletions(-) diff --git a/src/nvim/terminal.c b/src/nvim/terminal.c index 78ebd72e4d..ad16a9a0dc 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -606,14 +606,14 @@ void terminal_close(Terminal **termpp, int status) // If this was called by close_buffer() (status is -1), or if exiting, we // must inform the buffer the terminal no longer exists so that // close_buffer() won't call this again. - // If inside Terminal mode K_EVENT handling, setting buf_handle to 0 also + // If inside Terminal mode event handling, setting buf_handle to 0 also // informs terminal_enter() to call the close callback before returning. term->buf_handle = 0; if (buf) { buf->terminal = NULL; } if (!term->refcount) { - // Not inside Terminal mode K_EVENT handling. + // Not inside Terminal mode event handling. // We should not wait for the user to press a key. term->destroy = true; term->opts.close_cb(term->opts.data); @@ -777,12 +777,13 @@ bool terminal_enter(void) adjust_topline(s->term, buf, 0); // scroll to end showmode(); ui_cursor_shape(); - apply_autocmds(EVENT_TERMENTER, NULL, NULL, false, curbuf); - may_trigger_modechanged(); // Tell the terminal it has focus terminal_focus(s->term, true); + apply_autocmds(EVENT_TERMENTER, NULL, NULL, false, curbuf); + may_trigger_modechanged(); + s->state.execute = terminal_execute; s->state.check = terminal_check; state_enter(&s->state); @@ -797,8 +798,6 @@ bool terminal_enter(void) ui_busy_stop(); } - apply_autocmds(EVENT_TERMLEAVE, NULL, NULL, false, curbuf); - // Restore the terminal cursor to what is set in 'guicursor' (void)parse_shape_opt(SHAPE_CURSOR); @@ -816,12 +815,19 @@ bool terminal_enter(void) unshowmode(true); } ui_cursor_shape(); + + // If we're to close the terminal, don't let TermLeave autocommands free it first! if (s->close) { - bool wipe = s->term->buf_handle != 0; + s->term->refcount++; + } + apply_autocmds(EVENT_TERMLEAVE, NULL, NULL, false, curbuf); + if (s->close) { + s->term->refcount--; + const handle_T buf_handle = s->term->buf_handle; // Callback may free s->term. s->term->destroy = true; s->term->opts.close_cb(s->term->opts.data); - if (wipe) { - do_cmdline_cmd("bwipeout!"); + if (buf_handle != 0) { + do_buffer(DOBUF_WIPE, DOBUF_FIRST, FORWARD, buf_handle, true); } } @@ -845,24 +851,68 @@ static void terminal_check_cursor(void) coladvance(curwin, MAX(0, term->cursor.col + off)); } -// Function executed before each iteration of terminal mode. -// Return: -// 1 if the iteration should continue normally -// 0 if the main loop must exit +static bool terminal_check_focus(TerminalState *const s) + FUNC_ATTR_NONNULL_ALL +{ + if (curbuf->terminal == NULL) { + return false; + } + + if (s->save_curwin_handle != curwin->handle) { + // Terminal window changed, update window options. + unset_terminal_winopts(s); + set_terminal_winopts(s); + } + if (s->term != curbuf->terminal) { + // Active terminal buffer changed, flush terminal's cursor state to the UI. + terminal_focus(s->term, false); + + s->term = curbuf->terminal; + s->term->pending.cursor = true; + invalidate_terminal(s->term, -1, -1); + terminal_focus(s->term, true); + } + return true; +} + +/// Function executed before each iteration of terminal mode. +/// +/// @return: +/// 1 if the iteration should continue normally +/// 0 if the main loop must exit static int terminal_check(VimState *state) { TerminalState *const s = (TerminalState *)state; - if (stop_insert_mode) { + if (stop_insert_mode || !terminal_check_focus(s)) { return 0; } - assert(s->term == curbuf->terminal); + // Validate topline and cursor position for autocommands. Especially important for WinScrolled. terminal_check_cursor(); validate_cursor(curwin); - const bool text_changed = must_redraw != 0; - show_cursor_info_later(false); + // Don't let autocommands free the terminal from under our fingers. + s->term->refcount++; + if (must_redraw) { + // TODO(seandewar): above changes will maybe change the behaviour of this more; untrollify this + apply_autocmds(EVENT_TEXTCHANGEDT, NULL, NULL, false, curbuf); + } + may_trigger_win_scrolled_resized(); + s->term->refcount--; + if (s->term->buf_handle == 0) { + s->close = true; + return 0; + } + + // Autocommands above may have changed focus, scrolled, or moved the cursor. + if (!terminal_check_focus(s)) { + return 0; + } + terminal_check_cursor(); + validate_cursor(curwin); + + show_cursor_info_later(false); if (must_redraw) { update_screen(); } else { @@ -871,21 +921,6 @@ static int terminal_check(VimState *state) showmode(); // clear cmdline and show mode } } - if (text_changed) { - // Make sure an invoked autocmd doesn't delete the buffer (and the - // terminal) under our fingers. - curbuf->b_locked++; - - // save and restore curwin and curbuf, in case the autocmd changes them - aco_save_T aco; - aucmd_prepbuf(&aco, curbuf); - apply_autocmds(EVENT_TEXTCHANGEDT, NULL, NULL, false, curbuf); - aucmd_restbuf(&aco); - - curbuf->b_locked--; - } - - may_trigger_win_scrolled_resized(); setcursor(); refresh_cursor(s->term, &s->cursor_visible); @@ -986,23 +1021,6 @@ static int terminal_execute(VimState *state, int key) terminal_send_key(s->term, key); } - if (curbuf->terminal == NULL) { - return 0; - } - if (s->save_curwin_handle != curwin->handle) { - // Terminal window changed, update window options. - unset_terminal_winopts(s); - set_terminal_winopts(s); - } - if (s->term != curbuf->terminal) { - // Active terminal buffer changed, flush terminal's cursor state to the UI - terminal_focus(s->term, false); - - s->term = curbuf->terminal; - s->term->pending.cursor = true; - invalidate_terminal(s->term, -1, -1); - terminal_focus(s->term, true); - } return 1; } diff --git a/test/functional/autocmd/termxx_spec.lua b/test/functional/autocmd/termxx_spec.lua index 950ef2f58a..0ec1a54d32 100644 --- a/test/functional/autocmd/termxx_spec.lua +++ b/test/functional/autocmd/termxx_spec.lua @@ -5,6 +5,7 @@ local uv = vim.uv local clear, command, testprg = n.clear, n.command, n.testprg local eval, eq, neq, retry = n.eval, t.eq, t.neq, t.retry +local exec_lua = n.exec_lua local matches = t.matches local ok = t.ok local feed = n.feed @@ -197,30 +198,63 @@ it('autocmd TermEnter, TermLeave', function() }, eval('g:evs')) end) -describe('autocmd TextChangedT', function() - local screen - before_each(function() - clear() - screen = tt.setup_screen() - end) +describe('autocmd TextChangedT,WinResized', function() + before_each(clear) - it('works', function() + it('TextChangedT works', function() command('autocmd TextChangedT * ++once let g:called = 1') + tt.setup_screen() tt.feed_data('a') retry(nil, nil, function() eq(1, api.nvim_get_var('called')) end) end) - it('cannot delete terminal buffer', function() - command('autocmd TextChangedT * bwipe!') - tt.feed_data('a') - screen:expect({ any = 'E937: ' }) - feed('') - command('autocmd! TextChangedT') - matches( - '^E937: Attempt to delete a buffer that is in use: term://', - api.nvim_get_vvar('errmsg') - ) + it('no crash when deleting terminal buffer', function() + -- Using nvim_open_term over :terminal as the former can free the terminal immediately on + -- close, causing the crash. + + -- WinResized + local buf1, term1 = exec_lua(function() + vim.cmd.new() + local buf = vim.api.nvim_get_current_buf() + local term = vim.api.nvim_open_term(0, { + on_input = function() + vim.cmd.wincmd '_' + end, + }) + vim.api.nvim_create_autocmd('WinResized', { + once = true, + command = 'bwipeout!', + }) + return buf, term + end) + feed('ii') + eq(false, api.nvim_buf_is_valid(buf1)) + eq('n', eval('mode()')) + eq({}, api.nvim_get_chan_info(term1)) -- Channel should've been cleaned up. + + -- TextChangedT + local buf2, term2 = exec_lua(function() + vim.cmd.new() + local buf = vim.api.nvim_get_current_buf() + local term = vim.api.nvim_open_term(0, { + on_input = function(_, chan) + vim.api.nvim_chan_send(chan, 'sup') + end, + }) + vim.api.nvim_create_autocmd('TextChangedT', { + once = true, + command = 'bwipeout!', + }) + return buf, term + end) + feed('ii') + -- refresh_terminal is deferred, so TextChangedT may not trigger immediately. + retry(nil, nil, function() + eq(false, api.nvim_buf_is_valid(buf2)) + end) + eq('n', eval('mode()')) + eq({}, api.nvim_get_chan_info(term2)) -- Channel should've been cleaned up. end) end) diff --git a/test/functional/terminal/buffer_spec.lua b/test/functional/terminal/buffer_spec.lua index 16736db484..77fae6087c 100644 --- a/test/functional/terminal/buffer_spec.lua +++ b/test/functional/terminal/buffer_spec.lua @@ -351,12 +351,15 @@ describe(':terminal buffer', function() local chan = vim.api.nvim_open_term(0, { on_input = function(_, term, buf, data) if data == '\27[I' then + vim.b[buf].term_focused = true vim.api.nvim_chan_send(term, 'focused\n') elseif data == '\27[O' then + vim.b[buf].term_focused = false vim.api.nvim_chan_send(term, 'unfocused\n') end end, }) + vim.b.term_focused = false vim.api.nvim_chan_send(chan, '\27[?1004h') -- Enable focus reporting end @@ -373,6 +376,21 @@ describe(':terminal buffer', function() | ]]) + -- TermEnter/Leave happens *after* entering/leaving terminal mode, so focus should've changed + -- already by the time these events run. + exec_lua(function() + _G.last_event = nil + vim.api.nvim_create_autocmd({ 'TermEnter', 'TermLeave' }, { + callback = function(args) + _G.last_event = args.event + .. ' ' + .. vim.fs.basename(args.file) + .. ' ' + .. tostring(vim.b[args.buf].term_focused) + end, + }) + end) + feed('i') screen:expect([[ focused │focused │ | @@ -381,6 +399,8 @@ describe(':terminal buffer', function() {17:foo }{18:foo bar }| {3:-- TERMINAL --} | ]]) + eq('TermEnter foo true', exec_lua('return _G.last_event')) + -- Next window has the same terminal; no new notifications. command('wincmd w') screen:expect([[ @@ -409,6 +429,7 @@ describe(':terminal buffer', function() {18:foo foo }{17:bar }| | ]]) + eq('TermLeave bar false', exec_lua('return _G.last_event')) end) end) @@ -751,6 +772,58 @@ describe(':terminal buffer', function() unchanged = true, }) end) + + it('does not wipeout unrelated buffer after channel closes', function() + local screen = Screen.new(50, 7) + screen:set_default_attr_ids({ + [1] = { foreground = Screen.colors.Blue1, bold = true }, + [2] = { reverse = true }, + [31] = { background = Screen.colors.DarkGreen, foreground = Screen.colors.White, bold = true }, + }) + + local old_buf = api.nvim_get_current_buf() + command('new') + fn.chanclose(api.nvim_open_term(0, {})) + local term_buf = api.nvim_get_current_buf() + screen:expect([[ + ^ | + [Terminal closed] | + {31:[Scratch] }| + | + {1:~ }| + {2:[No Name] }| + | + ]]) + + -- Autocommand should not result in the wrong buffer being wiped out. + command('autocmd TermLeave * ++once wincmd p') + feed('ii') + screen:expect([[ + ^ | + {1:~ }|*5 + | + ]]) + eq(old_buf, api.nvim_get_current_buf()) + eq(false, api.nvim_buf_is_valid(term_buf)) + + term_buf = api.nvim_get_current_buf() + fn.chanclose(api.nvim_open_term(term_buf, {})) + screen:expect([[ + ^ | + [Terminal closed] | + |*5 + ]]) + + -- Autocommand should not result in a heap UAF if it frees the terminal prematurely. + command('autocmd TermLeave * ++once bwipeout!') + feed('ii') + screen:expect([[ + ^ | + {1:~ }|*5 + | + ]]) + eq(false, api.nvim_buf_is_valid(term_buf)) + end) end) describe('on_lines does not emit out-of-bounds line indexes when', function() diff --git a/test/functional/terminal/window_spec.lua b/test/functional/terminal/window_spec.lua index 8d07323b45..878b514e33 100644 --- a/test/functional/terminal/window_spec.lua +++ b/test/functional/terminal/window_spec.lua @@ -495,12 +495,13 @@ describe(':terminal window', function() {1:-- TERMINAL --} | ]]) command('tabprevious') + -- TODO(seandewar): w_wrow's wrong if the terminal doesn't match the window size... screen:expect([[ {1: }{5:2}{1: foo }{2: foo }{4: }{2:X}| {19:r}ows: 5, cols: 25 │rows: 5, cols: 25 | rows: 5, cols: 50 │rows: 5, cols: 50 | - {19: } │^ | {19: } │ | + {19: } │^ | {18:foo }{17:foo }| {1:-- TERMINAL --} | ]]) From 5ee9e3f2587a0661333a07d9f8a29e9b1e282246 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Fri, 22 Aug 2025 13:54:05 +0100 Subject: [PATCH 07/10] fix(terminal): possibly wrong wrow/wcol in active terminal Problem: w_wrow/col calculation in terminal_check_cursor is wrong when the terminal is smaller than the window. Common when there's a larger window open with the same terminal, or just after a resize (as refresh_size is deferred). Solution: don't calculate it; validate_cursor will correct it later if it's out-of-date. Note that the toplines set for the terminal (also before this PR) assume 'nowrap' (which is set by default for terminal windows), and that no weird stuff like filler lines are around. That means, for example, it's possible for the cursor to be moved off-screen if there's wrapped lines. If this happens, it's likely update_topline will move the cursor back on screen via validate_cursor or similar, but maybe this should be handled more elegantly in the future? --- src/nvim/terminal.c | 2 -- test/functional/terminal/window_spec.lua | 11 +++++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/nvim/terminal.c b/src/nvim/terminal.c index ad16a9a0dc..8640b977df 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -837,8 +837,6 @@ bool terminal_enter(void) static void terminal_check_cursor(void) { Terminal *term = curbuf->terminal; - curwin->w_wrow = term->cursor.row; - curwin->w_wcol = term->cursor.col + win_col_off(curwin); curwin->w_cursor.lnum = MIN(curbuf->b_ml.ml_line_count, row_to_linenr(term, term->cursor.row)); const linenr_T topline = MAX(curbuf->b_ml.ml_line_count - curwin->w_height_inner + 1, 1); diff --git a/test/functional/terminal/window_spec.lua b/test/functional/terminal/window_spec.lua index 878b514e33..6b7074b1c2 100644 --- a/test/functional/terminal/window_spec.lua +++ b/test/functional/terminal/window_spec.lua @@ -495,13 +495,12 @@ describe(':terminal window', function() {1:-- TERMINAL --} | ]]) command('tabprevious') - -- TODO(seandewar): w_wrow's wrong if the terminal doesn't match the window size... screen:expect([[ {1: }{5:2}{1: foo }{2: foo }{4: }{2:X}| {19:r}ows: 5, cols: 25 │rows: 5, cols: 25 | rows: 5, cols: 50 │rows: 5, cols: 50 | - {19: } │ | {19: } │^ | + {19: } │ | {18:foo }{17:foo }| {1:-- TERMINAL --} | ]]) @@ -510,8 +509,8 @@ describe(':terminal window', function() {1: }{5:2}{1: foo }{2: foo }{4: }{2:X}| {19:r}ows: 5, cols: 25 │rows: 5, cols: 25 | rows: 5, cols: 50 │rows: 5, cols: 50 | - {19: } │{12: }| - {19: } │^ | + {19: } │{12:^ }| + {19: } │ | {18:foo }{17:foo }| | ]]) @@ -533,8 +532,8 @@ describe(':terminal window', function() {1: }{5:2}{1: foo }{2: foo }{4: }{2:X}| {19:r}ows: 5, cols: 25 │rows: 5, cols: 25 | rows: 5, cols: 50 │rows: 5, cols: 50 | - {19: } │ | {19: } │^ | + {19: } │ | {18:foo }{17:foo }| {1:-- TERMINAL --} | ]]) @@ -566,8 +565,8 @@ describe(':terminal window', function() {1: }{5:2}{1: foo }{2: foo }{4: }{2:X}| │rows: 5, cols: 25 | {6:~ }│rows: 5, cols: 50 | - {6:~ }│ | {6:~ }│^ | + {6:~ }│ | {4:[No Name] }{17:foo }| {1:-- TERMINAL --} | ]]) From e4db5edb8a21c6506d095b872420b6d9a4ad5e1e Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue, 19 Aug 2025 15:03:40 +0100 Subject: [PATCH 08/10] fix(terminal): don't trigger TextChangedT for unrelated redraws Problem: TextChangedT fires depending on whether Nvim needs to update_screen while in terminal mode. This makes little sense as redraws can be completely unrelated to the terminal. Also, TextChanged could be fired from changes in terminal mode after returning to normal mode. Solution: trigger it when b:changedtick changes, like other such events. Happens when invalid cells are refreshed, though is no longer affected by cursor changes. Don't fire TextChanged from changes in terminal mode after leaving. Unlike the other TextChanged* events, I've elected to not have it be influenced by typeahead. Plus, unlike when leaving insert mode when no TextChangedI events are defined, I don't trigger TextChanged when returning to normal mode from changes in terminal mode (is that a Vim bug?) Curiously, Vim's TextChangedT is different; it's tied to its terminal cursor redraws, which triggers pretty eagerly (but is unaffected by unrelated redraws) - usually *twice* when data is sent to the terminal (regardless of whether it causes any visible changes, like incomplete escape codes; wasn't true for Nvim). Not clear to me how this event was actually intended to work, but this seems to make the most sense to me. --- src/nvim/buffer_defs.h | 2 +- src/nvim/terminal.c | 9 ++- test/functional/autocmd/termxx_spec.lua | 81 +++++++++++++++++++++++-- 3 files changed, 83 insertions(+), 9 deletions(-) diff --git a/src/nvim/buffer_defs.h b/src/nvim/buffer_defs.h index 1dde2c5101..bb22c76cae 100644 --- a/src/nvim/buffer_defs.h +++ b/src/nvim/buffer_defs.h @@ -408,7 +408,7 @@ struct file_buffer { varnumber_T b_last_changedtick; // b:changedtick when TextChanged was // last triggered. - varnumber_T b_last_changedtick_i; // b:changedtick for TextChangedI + varnumber_T b_last_changedtick_i; // b:changedtick for TextChangedI/T varnumber_T b_last_changedtick_pum; // b:changedtick for TextChangedP bool b_saving; // Set to true if we are in the middle of diff --git a/src/nvim/terminal.c b/src/nvim/terminal.c index 8640b977df..74efd51cfd 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -780,6 +780,8 @@ bool terminal_enter(void) // Tell the terminal it has focus terminal_focus(s->term, true); + // Don't fire TextChangedT from changes in Normal mode. + curbuf->b_last_changedtick_i = buf_get_changedtick(curbuf); apply_autocmds(EVENT_TERMENTER, NULL, NULL, false, curbuf); may_trigger_modechanged(); @@ -805,6 +807,8 @@ bool terminal_enter(void) // Tell the terminal it lost focus terminal_focus(s->term, false); + // Don't fire TextChanged from changes in terminal mode. + curbuf->b_last_changedtick = buf_get_changedtick(curbuf); if (curbuf->terminal == s->term && !s->close) { terminal_check_cursor(); @@ -892,9 +896,10 @@ static int terminal_check(VimState *state) // Don't let autocommands free the terminal from under our fingers. s->term->refcount++; - if (must_redraw) { - // TODO(seandewar): above changes will maybe change the behaviour of this more; untrollify this + if (has_event(EVENT_TEXTCHANGEDT) + && curbuf->b_last_changedtick_i != buf_get_changedtick(curbuf)) { apply_autocmds(EVENT_TEXTCHANGEDT, NULL, NULL, false, curbuf); + curbuf->b_last_changedtick_i = buf_get_changedtick(curbuf); } may_trigger_win_scrolled_resized(); s->term->refcount--; diff --git a/test/functional/autocmd/termxx_spec.lua b/test/functional/autocmd/termxx_spec.lua index 0ec1a54d32..c5bc5d928e 100644 --- a/test/functional/autocmd/termxx_spec.lua +++ b/test/functional/autocmd/termxx_spec.lua @@ -1,6 +1,6 @@ local t = require('test.testutil') local n = require('test.functional.testnvim')() -local tt = require('test.functional.testterm') +local Screen = require('test.functional.ui.screen') local uv = vim.uv local clear, command, testprg = n.clear, n.command, n.testprg @@ -202,12 +202,81 @@ describe('autocmd TextChangedT,WinResized', function() before_each(clear) it('TextChangedT works', function() - command('autocmd TextChangedT * ++once let g:called = 1') - tt.setup_screen() - tt.feed_data('a') - retry(nil, nil, function() - eq(1, api.nvim_get_var('called')) + local screen = Screen.new(50, 7) + screen:set_default_attr_ids({ + [1] = { bold = true }, + [31] = { foreground = Screen.colors.Gray100, background = Screen.colors.DarkGreen }, + [32] = { + foreground = Screen.colors.Gray100, + bold = true, + background = Screen.colors.DarkGreen, + }, + }) + + local term, term_unfocused = exec_lua(function() + -- Split windows before opening terminals so TextChangedT doesn't fire an additional time due + -- to the inner terminal being resized (which is usually deferred too). + vim.cmd.vnew() + local term_unfocused = vim.api.nvim_open_term(0, {}) + vim.cmd.wincmd 'p' + local term = vim.api.nvim_open_term(0, {}) + vim.cmd.startinsert() + return term, term_unfocused end) + eq('t', eval('mode()')) + + exec_lua(function() + _G.n_triggered = 0 + vim.api.nvim_create_autocmd('TextChanged', { + callback = function() + _G.n_triggered = _G.n_triggered + 1 + end, + }) + _G.t_triggered = 0 + vim.api.nvim_create_autocmd('TextChangedT', { + callback = function() + _G.t_triggered = _G.t_triggered + 1 + end, + }) + end) + + api.nvim_chan_send(term, 'a') + retry(nil, nil, function() + eq(1, exec_lua('return _G.t_triggered')) + end) + api.nvim_chan_send(term, 'b') + retry(nil, nil, function() + eq(2, exec_lua('return _G.t_triggered')) + end) + + -- Not triggered by changes in a non-current terminal. + api.nvim_chan_send(term_unfocused, 'hello') + screen:expect([[ + hello │ab^ | + │ |*4 + {31:[Scratch] [-] }{32:[Scratch] [-] }| + {1:-- TERMINAL --} | + ]]) + eq(2, exec_lua('return _G.t_triggered')) + + -- Not triggered by unflushed redraws. + api.nvim__redraw({ valid = false, flush = false }) + eq(2, exec_lua('return _G.t_triggered')) + + -- Not triggered when not in terminal mode. + command('stopinsert') + eq('n', eval('mode()')) + eq(2, exec_lua('return _G.t_triggered')) + eq(0, exec_lua('return _G.n_triggered')) -- Nothing we did was in Normal mode yet. + + api.nvim_chan_send(term, 'c') + screen:expect([[ + hello │a^bc | + │ |*4 + {31:[Scratch] [-] }{32:[Scratch] [-] }| + | + ]]) + eq(1, exec_lua('return _G.n_triggered')) -- Happened in Normal mode. end) it('no crash when deleting terminal buffer', function() From a2d4a0fda9ee0c6d032e834104e205e3eebc0dd3 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Wed, 29 Oct 2025 06:50:14 +0800 Subject: [PATCH 09/10] vim-patch:9.1.1885: Wrong restored cursor pos when re-entering buffer after changes Problem: Wrong restored cursor position when re-entering a buffer previously viewed in a window after making changes to the same buffer in another window. Solution: Adjust per-window "last cursor" positions on buffer changes. (zeertzjq) closes: vim/vim#18655 https://github.com/vim/vim/commit/b2e6b328dafe1cdfff10f1e811355e514da8071d --- src/nvim/mark.c | 38 ++++++++++++++++++++------------ test/old/testdir/test_buffer.vim | 31 ++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 14 deletions(-) diff --git a/src/nvim/mark.c b/src/nvim/mark.c index 50520584a5..d1054ea72e 100644 --- a/src/nvim/mark.c +++ b/src/nvim/mark.c @@ -1158,6 +1158,23 @@ void ex_changes(exarg_T *eap) } \ } +// Like ONE_ADJUST_NODEL(), but if the position is within the deleted range, +// move it to the start of the line before the range. +#define ONE_ADJUST_CURSOR(pp) \ + { \ + pos_T *posp = pp; \ + if (posp->lnum >= line1 && posp->lnum <= line2) { \ + if (amount == MAXLNUM) { /* line with cursor is deleted */ \ + posp->lnum = MAX(line1 - 1, 1); \ + posp->col = 0; \ + } else { /* keep cursor on the same line */ \ + posp->lnum += amount; \ + } \ + } else if (amount_after && posp->lnum > line2) { \ + posp->lnum += amount_after; \ + } \ + } + // Adjust marks between "line1" and "line2" (inclusive) to move "amount" lines. // Must be called before changed_*(), appended_lines() or deleted_lines(). // May be called before or after changing the text. @@ -1316,20 +1333,7 @@ void mark_adjust_buf(buf_T *buf, linenr_T line1, linenr_T line2, linenr_T amount } } if (!by_api && (by_term ? win->w_cursor.lnum < buf->b_ml.ml_line_count : win != curwin)) { - if (win->w_cursor.lnum >= line1 && win->w_cursor.lnum <= line2) { - if (amount == MAXLNUM) { // line with cursor is deleted - if (line1 <= 1) { - win->w_cursor.lnum = 1; - } else { - win->w_cursor.lnum = line1 - 1; - } - win->w_cursor.col = 0; - } else { // keep cursor on the same line - win->w_cursor.lnum += amount; - } - } else if (amount_after && win->w_cursor.lnum > line2) { - win->w_cursor.lnum += amount_after; - } + ONE_ADJUST_CURSOR(&(win->w_cursor)); } if (adjust_folds) { @@ -1340,6 +1344,12 @@ void mark_adjust_buf(buf_T *buf, linenr_T line1, linenr_T line2, linenr_T amount // adjust diffs diff_mark_adjust(buf, line1, line2, amount, amount_after); + + // adjust per-window "last cursor" positions + for (size_t i = 0; i < kv_size(buf->b_wininfo); i++) { + WinInfo *wip = kv_A(buf->b_wininfo, i); + ONE_ADJUST_CURSOR(&(wip->wi_mark.mark)); + } } // This code is used often, needs to be fast. diff --git a/test/old/testdir/test_buffer.vim b/test/old/testdir/test_buffer.vim index 19799a5478..1e4f19eb69 100644 --- a/test/old/testdir/test_buffer.vim +++ b/test/old/testdir/test_buffer.vim @@ -619,17 +619,48 @@ func Test_switch_to_previously_viewed_buffer() vsplit call cursor(100, 3) + call assert_equal('100', getline('.')) edit Xotherbuf buffer Xviewbuf call assert_equal([0, 100, 3, 0], getpos('.')) + call assert_equal('100', getline('.')) + edit Xotherbuf + wincmd p + normal! gg10dd + wincmd p + buffer Xviewbuf + call assert_equal([0, 90, 3, 0], getpos('.')) + call assert_equal('100', getline('.')) + + edit Xotherbuf + wincmd p + normal! ggP + wincmd p + buffer Xviewbuf + call assert_equal([0, 100, 3, 0], getpos('.')) + call assert_equal('100', getline('.')) + + edit Xotherbuf + wincmd p + normal! 96gg10ddgg + wincmd p + buffer Xviewbuf + " The original cursor line was deleted, so cursor is restored to the start + " of the line before the deleted range. + call assert_equal([0, 95, 1, 0], getpos('.')) + call assert_equal('95', getline('.')) + + normal! u exe win_id2win(oldwin) .. 'close' setlocal bufhidden=hide call cursor(200, 3) + call assert_equal('200', getline('.')) edit Xotherbuf buffer Xviewbuf call assert_equal([0, 200, 3, 0], getpos('.')) + call assert_equal('200', getline('.')) bwipe! Xotherbuf bwipe! Xviewbuf From a6d8f40b647112c4eb8604effb408c8aad94ec67 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Tue, 28 Oct 2025 08:10:58 +0800 Subject: [PATCH 10/10] fix(terminal): keep last cursor if it's on the last row --- src/nvim/mark.c | 7 +- src/nvim/terminal.c | 21 +- test/functional/autocmd/termxx_spec.lua | 4 +- test/functional/terminal/scrollback_spec.lua | 233 +++++++++++++------ 4 files changed, 182 insertions(+), 83 deletions(-) diff --git a/src/nvim/mark.c b/src/nvim/mark.c index d1054ea72e..78cd4e7e5d 100644 --- a/src/nvim/mark.c +++ b/src/nvim/mark.c @@ -1236,7 +1236,8 @@ void mark_adjust_buf(buf_T *buf, linenr_T line1, linenr_T line2, linenr_T amount ONE_ADJUST(&(buf->b_last_change.mark.lnum)); // last cursor position, if it was set - if (!equalpos(buf->b_last_cursor.mark, initpos)) { + if (!equalpos(buf->b_last_cursor.mark, initpos) + && (!by_term || buf->b_last_cursor.mark.lnum < buf->b_ml.ml_line_count)) { ONE_ADJUST(&(buf->b_last_cursor.mark.lnum)); } @@ -1348,7 +1349,9 @@ void mark_adjust_buf(buf_T *buf, linenr_T line1, linenr_T line2, linenr_T amount // adjust per-window "last cursor" positions for (size_t i = 0; i < kv_size(buf->b_wininfo); i++) { WinInfo *wip = kv_A(buf->b_wininfo, i); - ONE_ADJUST_CURSOR(&(wip->wi_mark.mark)); + if (!by_term || wip->wi_mark.mark.lnum < buf->b_ml.ml_line_count) { + ONE_ADJUST_CURSOR(&(wip->wi_mark.mark)); + } } } diff --git a/src/nvim/terminal.c b/src/nvim/terminal.c index 74efd51cfd..1b9fa6b1b7 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -774,7 +774,7 @@ bool terminal_enter(void) set_terminal_winopts(s); s->term->pending.cursor = true; // Update the cursor shape table - adjust_topline(s->term, buf, 0); // scroll to end + adjust_topline_cursor(s->term, buf, 0); // scroll to end showmode(); ui_cursor_shape(); @@ -2109,7 +2109,7 @@ static void refresh_terminal(Terminal *term) refresh_screen(term, buf); int ml_added = buf->b_ml.ml_line_count - ml_before; - adjust_topline(term, buf, ml_added); + adjust_topline_cursor(term, buf, ml_added); // Copy pending events back to the main event queue multiqueue_move_events(main_loop.events, term->pending.events); @@ -2325,8 +2325,10 @@ static void refresh_screen(Terminal *term, buf_T *buf) term->invalid_end = -1; } -static void adjust_topline(Terminal *term, buf_T *buf, int added) +static void adjust_topline_cursor(Terminal *term, buf_T *buf, int added) { + linenr_T ml_end = buf->b_ml.ml_line_count; + FOR_ALL_TAB_WINDOWS(tp, wp) { if (wp->w_buffer == buf) { if (wp == curwin && is_focused(term)) { @@ -2335,9 +2337,7 @@ static void adjust_topline(Terminal *term, buf_T *buf, int added) continue; } - linenr_T ml_end = buf->b_ml.ml_line_count; bool following = ml_end == wp->w_cursor.lnum + added; // cursor at end? - if (following) { // "Follow" the terminal output wp->w_cursor.lnum = ml_end; @@ -2349,6 +2349,17 @@ static void adjust_topline(Terminal *term, buf_T *buf, int added) mb_check_adjust_col(wp); } } + + if (ml_end == buf->b_last_cursor.mark.lnum + added) { + buf->b_last_cursor.mark.lnum = ml_end; + } + + for (size_t i = 0; i < kv_size(buf->b_wininfo); i++) { + WinInfo *wip = kv_A(buf->b_wininfo, i); + if (ml_end == wip->wi_mark.mark.lnum + added) { + wip->wi_mark.mark.lnum = ml_end; + } + } } static int row_to_linenr(Terminal *term, int row) diff --git a/test/functional/autocmd/termxx_spec.lua b/test/functional/autocmd/termxx_spec.lua index c5bc5d928e..6841344aa4 100644 --- a/test/functional/autocmd/termxx_spec.lua +++ b/test/functional/autocmd/termxx_spec.lua @@ -254,7 +254,7 @@ describe('autocmd TextChangedT,WinResized', function() screen:expect([[ hello │ab^ | │ |*4 - {31:[Scratch] [-] }{32:[Scratch] [-] }| + {31:[Scratch] }{32:[Scratch] }| {1:-- TERMINAL --} | ]]) eq(2, exec_lua('return _G.t_triggered')) @@ -273,7 +273,7 @@ describe('autocmd TextChangedT,WinResized', function() screen:expect([[ hello │a^bc | │ |*4 - {31:[Scratch] [-] }{32:[Scratch] [-] }| + {31:[Scratch] }{32:[Scratch] }| | ]]) eq(1, exec_lua('return _G.n_triggered')) -- Happened in Normal mode. diff --git a/test/functional/terminal/scrollback_spec.lua b/test/functional/terminal/scrollback_spec.lua index 9532462e81..d7c7184096 100644 --- a/test/functional/terminal/scrollback_spec.lua +++ b/test/functional/terminal/scrollback_spec.lua @@ -3,7 +3,7 @@ local n = require('test.functional.testnvim')() local Screen = require('test.functional.ui.screen') local tt = require('test.functional.testterm') -local clear, eq = n.clear, t.eq +local clear, eq, neq = n.clear, t.eq, t.neq local feed, testprg = n.feed, n.testprg local fn = n.fn local eval = n.eval @@ -18,34 +18,74 @@ local assert_alive = n.assert_alive local skip = t.skip local is_os = t.is_os -describe(':terminal scrollback', function() - local screen +local function test_terminal_scrollback(hide_curbuf) + local screen --- @type test.functional.ui.screen + local buf --- @type integer + local chan --- @type integer + local otherbuf --- @type integer + local restore_terminal_mode --- @type boolean? + + local function may_hide_curbuf() + if hide_curbuf then + eq(nil, restore_terminal_mode) + restore_terminal_mode = vim.startswith(api.nvim_get_mode().mode, 't') + api.nvim_set_current_buf(otherbuf) + end + end + + local function may_restore_curbuf() + if hide_curbuf then + neq(nil, restore_terminal_mode) + eq(buf, fn.bufnr('#')) + feed('') -- "view" in 'jumpoptions' applies to this + if restore_terminal_mode then + feed('i') + else + -- Cursor position was restored from wi_mark, not b_last_cursor. + -- Check that b_last_cursor and wi_mark are the same. + local last_cursor = fn.getpos([['"]]) + local restored_cursor = fn.getpos('.') + if last_cursor[2] > 0 then + eq(restored_cursor, last_cursor) + else + eq({ 0, 0, 0, 0 }, last_cursor) + eq({ 0, 1, 1, 0 }, restored_cursor) + end + end + restore_terminal_mode = nil + end + end + + --- @param prefix string + --- @param start integer + --- @param stop integer + local function feed_lines(prefix, start, stop) + may_hide_curbuf() + local data = '' + for i = start, stop do + data = data .. prefix .. tostring(i) .. '\n' + end + api.nvim_chan_send(chan, data) + retry(nil, 1000, function() + eq({ prefix .. tostring(stop), '' }, api.nvim_buf_get_lines(buf, -3, -1, true)) + end) + may_restore_curbuf() + end before_each(function() clear() + command('set nostartofline jumpoptions+=view') screen = tt.setup_screen(nil, nil, 30) - end) - - local function feed_new_lines_and_wait(count) - local lines = {} - for i = 1, count do - table.insert(lines, 'new_line' .. tostring(i)) + buf = api.nvim_get_current_buf() + chan = api.nvim_get_option_value('channel', { buf = buf }) + if hide_curbuf then + otherbuf = api.nvim_create_buf(true, false) end - table.insert(lines, '') - feed_data(lines) - retry(nil, 1000, function() - eq({ 'new_line' .. tostring(count), '' }, api.nvim_buf_get_lines(0, -3, -1, true)) - end) - end + end) describe('when the limit is exceeded', function() before_each(function() - local lines = {} - for i = 1, 30 do - table.insert(lines, 'line' .. tostring(i)) - end - table.insert(lines, '') - feed_data(lines) + feed_lines('line', 1, 30) screen:expect([[ line26 | line27 | @@ -87,7 +127,7 @@ describe(':terminal scrollback', function() end) it("when outputting fewer than 'scrollback' lines", function() - feed_new_lines_and_wait(6) + feed_lines('new_line', 1, 6) screen:expect([[ line26 | line27 | @@ -102,7 +142,7 @@ describe(':terminal scrollback', function() end) it("when outputting more than 'scrollback' lines", function() - feed_new_lines_and_wait(11) + feed_lines('new_line', 1, 11) screen:expect([[ line27 | {8:line2^8} | @@ -117,7 +157,7 @@ describe(':terminal scrollback', function() end) it('when outputting more lines than whole buffer', function() - feed_new_lines_and_wait(20) + feed_lines('new_line', 1, 20) screen:expect([[ ^new_line6 | new_line7 | @@ -150,14 +190,14 @@ describe(':terminal scrollback', function() end) it("when outputting fewer than 'scrollback' lines", function() - feed_new_lines_and_wait(6) - screen:expect_unchanged() + feed_lines('new_line', 1, 6) + screen:expect_unchanged(hide_curbuf) eq({ 0, 4, 4, 0 }, fn.getpos("'m")) eq({ 0, 4, 6, 0 }, fn.getpos('.')) end) it("when outputting more than 'scrollback' lines", function() - feed_new_lines_and_wait(11) + feed_lines('new_line', 1, 11) screen:expect([[ ^line27 | line28 | @@ -175,7 +215,7 @@ describe(':terminal scrollback', function() describe('with cursor at last row', function() before_each(function() - feed_data({ 'line1', 'line2', 'line3', 'line4', '' }) + feed_lines('line', 1, 4) screen:expect([[ tty ready | line1 | @@ -201,7 +241,7 @@ describe(':terminal scrollback', function() it("when outputting more than 'scrollback' lines in Normal mode", function() feed([[]]) - feed_new_lines_and_wait(11) + feed_lines('new_line', 1, 11) screen:expect([[ new_line7 | new_line8 | @@ -222,11 +262,33 @@ describe(':terminal scrollback', function() | ]]) eq({ 0, 2, 4, 0 }, fn.getpos("'m")) + feed('G') + feed_lines('new_line', 12, 31) + screen:expect([[ + new_line27 | + new_line28 | + new_line29 | + new_line30 | + new_line31 | + ^ | + | + ]]) + feed('gg') + screen:expect([[ + ^new_line17 | + new_line18 | + new_line19 | + new_line20 | + new_line21 | + new_line22 | + | + ]]) + eq({ 0, 0, 0, 0 }, fn.getpos("'m")) end) describe('and 1 line is printed', function() before_each(function() - feed_data({ 'line5', '' }) + feed_lines('line', 5, 5) end) it('will hide the top line', function() @@ -245,7 +307,7 @@ describe(':terminal scrollback', function() describe('and then 3 more lines are printed', function() before_each(function() - feed_data({ 'line6', 'line7', 'line8', '' }) + feed_lines('line', 6, 8) end) it('will hide the top 4 lines', function() @@ -299,7 +361,9 @@ describe(':terminal scrollback', function() describe('and height decreased by 1', function() local function will_hide_top_line() feed([[]]) + may_hide_curbuf() screen:try_resize(screen._width - 2, screen._height - 1) + may_restore_curbuf() screen:expect([[ {8:line2} | line3 | @@ -316,7 +380,9 @@ describe(':terminal scrollback', function() describe('and then decreased by 2', function() before_each(function() will_hide_top_line() + may_hide_curbuf() screen:try_resize(screen._width - 2, screen._height - 2) + may_restore_curbuf() end) it('will hide the top 3 lines', function() @@ -357,7 +423,9 @@ describe(':terminal scrollback', function() describe('and the height is decreased by 2', function() before_each(function() + may_hide_curbuf() screen:try_resize(screen._width, screen._height - 2) + may_restore_curbuf() end) local function will_delete_last_two_lines() @@ -376,7 +444,9 @@ describe(':terminal scrollback', function() describe('and then decreased by 1', function() before_each(function() will_delete_last_two_lines() + may_hide_curbuf() screen:try_resize(screen._width, screen._height - 1) + may_restore_curbuf() end) it('will delete the last line and hide the first', function() @@ -408,7 +478,7 @@ describe(':terminal scrollback', function() describe('with 4 lines hidden in the scrollback', function() before_each(function() - feed_data({ 'line1', 'line2', 'line3', 'line4', '' }) + feed_lines('line', 1, 4) screen:expect([[ tty ready | line1 | @@ -430,7 +500,9 @@ describe(':terminal scrollback', function() ^ | {3:-- TERMINAL --} | ]]) + may_hide_curbuf() screen:try_resize(screen._width, screen._height - 3) + may_restore_curbuf() screen:expect([[ line4 | rows: 3, cols: 30 | @@ -448,7 +520,9 @@ describe(':terminal scrollback', function() return end local function pop_then_push() + may_hide_curbuf() screen:try_resize(screen._width, screen._height + 1) + may_restore_curbuf() screen:expect([[ line4 | rows: 3, cols: 30 | @@ -465,7 +539,9 @@ describe(':terminal scrollback', function() before_each(function() pop_then_push() eq(8, api.nvim_buf_line_count(0)) + may_hide_curbuf() screen:try_resize(screen._width, screen._height + 3) + may_restore_curbuf() end) local function pop3_then_push1() @@ -500,7 +576,9 @@ describe(':terminal scrollback', function() before_each(function() pop3_then_push1() feed('Gi') + may_hide_curbuf() screen:try_resize(screen._width, screen._height + 4) + may_restore_curbuf() end) it('will show all lines and leave a blank one at the end', function() @@ -527,6 +605,55 @@ describe(':terminal scrollback', function() end) end) end) + + it('reducing &scrollback deletes extra lines immediately', function() + feed_lines('line', 1, 30) + screen:expect([[ + line26 | + line27 | + line28 | + line29 | + line30 | + ^ | + {3:-- TERMINAL --} | + ]]) + local term_height = 6 -- Actual terminal screen height, not the scrollback + -- Initial + local scrollback = api.nvim_get_option_value('scrollback', { buf = buf }) + eq(scrollback + term_height, fn.line('$')) + eq(scrollback + term_height, fn.line('.')) + n.fn.setpos("'m", { 0, scrollback + 1, 4, 0 }) + local ns = api.nvim_create_namespace('test') + api.nvim_buf_set_extmark(0, ns, scrollback, 0, { end_col = 6, hl_group = 'ErrorMsg' }) + screen:expect([[ + {8:line26} | + line27 | + line28 | + line29 | + line30 | + ^ | + {3:-- TERMINAL --} | + ]]) + -- Reduction + scrollback = scrollback - 2 + may_hide_curbuf() + api.nvim_set_option_value('scrollback', scrollback, { buf = buf }) + may_restore_curbuf() + eq(scrollback + term_height, fn.line('$')) + eq(scrollback + term_height, fn.line('.')) + screen:expect_unchanged(hide_curbuf) + eq({ 0, scrollback + 1, 4, 0 }, n.fn.getpos("'m")) + end) +end + +describe(':terminal scrollback', function() + describe('in current buffer', function() + test_terminal_scrollback(false) + end) + + describe('in hidden buffer', function() + test_terminal_scrollback(true) + end) end) describe(':terminal prints more lines than the screen height and exits', function() @@ -658,48 +785,6 @@ describe("'scrollback' option", function() eq((is_os('win') and '27: line' or '26: line'), eval("getline(line('w0') - 10)->trim(' ', 2)")) end) - it('deletes extra lines immediately', function() - -- Scrollback is 10 on setup_screen - local screen = tt.setup_screen(nil, nil, 30) - local lines = {} - for i = 1, 30 do - table.insert(lines, 'line' .. tostring(i)) - end - table.insert(lines, '') - feed_data(lines) - screen:expect([[ - line26 | - line27 | - line28 | - line29 | - line30 | - ^ | - {3:-- TERMINAL --} | - ]]) - local ns = api.nvim_create_namespace('test') - local term_height = 6 -- Actual terminal screen height, not the scrollback - -- Initial - local scrollback = api.nvim_get_option_value('scrollback', {}) - eq(scrollback + term_height, fn.line('$')) - n.fn.setpos("'m", { 0, scrollback + 1, 4, 0 }) - api.nvim_buf_set_extmark(0, ns, scrollback, 0, { end_col = 6, hl_group = 'ErrorMsg' }) - screen:expect([[ - {8:line26} | - line27 | - line28 | - line29 | - line30 | - ^ | - {3:-- TERMINAL --} | - ]]) - -- Reduction - scrollback = scrollback - 2 - api.nvim_set_option_value('scrollback', scrollback, {}) - eq(scrollback + term_height, fn.line('$')) - screen:expect_unchanged() - eq({ 0, scrollback + 1, 4, 0 }, n.fn.getpos("'m")) - end) - it('defaults to 10000 in :terminal buffers', function() set_fake_shell() command('terminal')