diff --git a/src/nvim/terminal.c b/src/nvim/terminal.c index 0047a8c789..07c6d7bf4c 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -444,6 +444,34 @@ static void term_output_callback(const char *s, size_t len, void *user_data) terminal_send((Terminal *)user_data, s, len); } +/// Allocates a terminal's scrollback buffer if it hasn't been allocated yet. +/// Does nothing if it's already allocated, unlike adjust_scrollback(). +/// +/// @param term Terminal instance. +/// @param buf The terminal's buffer, or NULL to get it from buf_handle. +/// +/// @return whether the terminal now has a scrollback buffer. +static bool term_may_alloc_scrollback(Terminal *term, buf_T *buf) +{ + if (term->sb_buffer != NULL) { + return true; + } + if (buf == NULL) { + buf = handle_get_buffer(term->buf_handle); + if (buf == NULL) { // No need to allocate scrollback if buffer is deleted. + return false; + } + } + + if (buf->b_p_scbk < 1) { + buf->b_p_scbk = SB_MAX; + } + // Configure the scrollback buffer. + term->sb_size = (size_t)buf->b_p_scbk; + term->sb_buffer = xmalloc(sizeof(ScrollbackLine *) * term->sb_size); + return true; +} + // public API {{{ /// Initializes terminal properties, and triggers TermOpen. @@ -529,9 +557,11 @@ void terminal_open(Terminal **termpp, buf_T *buf, TerminalOptions opts) RESET_BINDING(curwin); // Reset cursor in current window. curwin->w_cursor = (pos_T){ .lnum = 1, .col = 0, .coladd = 0 }; - // Initialize to check if the scrollback buffer has been allocated in a TermOpen autocmd. - term->sb_buffer = NULL; - // Apply TermOpen autocmds _before_ configuring the scrollback buffer. + + // Apply TermOpen autocmds _before_ configuring the scrollback buffer, to avoid + // over-allocating in case TermOpen reduces 'scrollback'. + // In the rare case where TermOpen polls for events, the scrollback buffer will be + // allocated anyway if needed. apply_autocmds(EVENT_TERMOPEN, NULL, NULL, false, buf); aucmd_restbuf(&aco); @@ -540,14 +570,9 @@ void terminal_open(Terminal **termpp, buf_T *buf, TerminalOptions opts) return; // Terminal has already been destroyed. } - if (term->sb_buffer == NULL) { - // Local 'scrollback' _after_ autocmds. - if (buf->b_p_scbk < 1) { - buf->b_p_scbk = SB_MAX; - } - // Configure the scrollback buffer. - term->sb_size = (size_t)buf->b_p_scbk; - term->sb_buffer = xmalloc(sizeof(ScrollbackLine *) * term->sb_size); + // Local 'scrollback' _after_ autocmds. + if (!term_may_alloc_scrollback(term, buf)) { + abort(); } // Configure the color palette. Try to get the color from: @@ -1494,9 +1519,10 @@ static int term_sb_push(int cols, const VTermScreenCell *cells, void *data) { Terminal *term = data; - if (!term->sb_size) { + if (!term_may_alloc_scrollback(term, NULL)) { return 0; } + assert(term->sb_size > 0); // copy vterm cells into sb_buffer size_t c = (size_t)cols; diff --git a/test/functional/terminal/buffer_spec.lua b/test/functional/terminal/buffer_spec.lua index 854fd85ada..f9f896a507 100644 --- a/test/functional/terminal/buffer_spec.lua +++ b/test/functional/terminal/buffer_spec.lua @@ -772,10 +772,12 @@ describe(':terminal buffer', function() ]]) end) - it('does not drop data when job exits immediately after output #3030', function() + --- @param subcmd 'REP'|'REPFAST' + local function check_term_rep_20000(subcmd) local screen = Screen.new(50, 7) + api.nvim_set_option_value('scrollback', 30000, {}) api.nvim_create_autocmd('TermClose', { command = 'let g:did_termclose = 1' }) - fn.jobstart({ testprg('shell-test'), 'REPFAST', '20000', 'TEST' }, { term = true }) + fn.jobstart({ testprg('shell-test'), subcmd, '20000', 'TEST' }, { term = true }) retry(nil, nil, function() eq(1, api.nvim_get_var('did_termclose')) end) @@ -789,6 +791,23 @@ describe(':terminal buffer', function() [Process exited 0]^ | {5:-- TERMINAL --} | ]]) + local lines = api.nvim_buf_get_lines(0, 0, -1, true) + for i = 0, 19999 do + eq(('%d: TEST'):format(i), lines[i + 1]) + end + end + + it('does not drop data when job exits immediately after output #3030', function() + check_term_rep_20000('REPFAST') + end) + + -- it('does not drop data when autocommands poll for events #37559', function() + it('does not drop data when TermOpen polls for events', function() + -- api.nvim_create_autocmd('BufFilePre', { command = 'sleep 50m', nested = true }) + -- api.nvim_create_autocmd('BufFilePost', { command = 'sleep 50m', nested = true }) + api.nvim_create_autocmd('TermOpen', { command = 'sleep 50m', nested = true }) + -- REP pauses 1 ms every 100 lines, so each autocommand processes some output. + check_term_rep_20000('REP') end) it('handles unprintable chars', function() diff --git a/test/functional/terminal/channel_spec.lua b/test/functional/terminal/channel_spec.lua index 140390e41f..d18e8cc11e 100644 --- a/test/functional/terminal/channel_spec.lua +++ b/test/functional/terminal/channel_spec.lua @@ -140,17 +140,18 @@ end) describe('no crash when TermOpen autocommand', function() local screen + -- Use REPFAST for immediately output after start. + local term_args = { testprg('shell-test'), 'REPFAST', '50', 'TEST' } before_each(function() clear() - api.nvim_set_option_value('shell', testprg('shell-test'), {}) - command('set shellcmdflag=EXE shellredir= shellpipe= shellquote= shellxquote=') screen = Screen.new(60, 4) + command([[call setline(1, 'OLDBUF') | enew]]) end) - it('processes job exit event on jobstart(…,{term=true})', function() + it('processes job exit event when using jobstart(…,{term=true})', function() command([[autocmd TermOpen * call input('')]]) - async_meths.nvim_command('terminal foobar') + async_meths.nvim_call_function('jobstart', { term_args, { term = true } }) screen:expect([[ | {1:~ }|*2 @@ -158,14 +159,21 @@ describe('no crash when TermOpen autocommand', function() ]]) feed('') screen:expect([[ - ^ready $ foobar | - | - [Process exited 0] | + ^0: TEST | + 1: TEST | + 2: TEST | | ]]) - feed('i') + feed('i') screen:expect([[ - ^ | + 49: TEST | + | + [Process exited 0]^ | + {5:-- TERMINAL --} | + ]]) + feed('') + screen:expect([[ + ^OLDBUF | {1:~ }|*2 | ]]) @@ -174,7 +182,7 @@ describe('no crash when TermOpen autocommand', function() it('wipes buffer and processes events when using jobstart(…,{term=true})', function() command([[autocmd TermOpen * bwipe! | call input('')]]) - async_meths.nvim_command('terminal foobar') + async_meths.nvim_call_function('jobstart', { term_args, { term = true } }) screen:expect([[ | {1:~ }|*2 @@ -182,7 +190,7 @@ describe('no crash when TermOpen autocommand', function() ]]) feed('') screen:expect([[ - ^ | + ^OLDBUF | {1:~ }|*2 | ]]) @@ -199,7 +207,7 @@ describe('no crash when TermOpen autocommand', function() ]]) feed('') screen:expect([[ - ^ | + ^OLDBUF | {1:~ }|*2 | ]])