fix(terminal): losing scrollback when TermOpen polls for events (#37573)

Problem:  When TermOpen polls for enough events to use the scrollback
          buffer, scrollback is lost until the next terminal refresh.
Solution: Allocate the scrollback buffer when it's needed.

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, to avoid
-  // Apply TermOpen autocmds _before_ configuring the scrollback buffer.
+  // 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.
-    // Local 'scrollback' _after_ autocmds.
+  if (!term_may_alloc_scrollback(term, buf)) {
-    if (buf->b_p_scbk < 1) {
+    abort();
-      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);
   }
 
   // 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;

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()

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('<CR>')
     screen:expect([[
-      ^ready $ foobar                                              |
+      ^0: TEST                                                     |
-                                                                  |
+      1: TEST                                                     |
-      [Process exited 0]                                          |
+      2: TEST                                                     |
                                                                   |
     ]])
-    feed('i<CR>')
+    feed('i')
     screen:expect([[
-      ^                                                            |
+      49: TEST                                                    |
+                                                                  |
+      [Process exited 0]^                                          |
+      {5:-- TERMINAL --}                                              |
+    ]])
+    feed('<CR>')
+    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('<CR>')
     screen:expect([[
-      ^                                                            |
+      ^OLDBUF                                                      |
       {1:~                                                           }|*2
                                                                   |
     ]])
@@ -199,7 +207,7 @@ describe('no crash when TermOpen autocommand', function()
     ]])
     feed('<CR>')
     screen:expect([[
-      ^                                                            |
+      ^OLDBUF                                                      |
       {1:~                                                           }|*2
                                                                   |
     ]])