fix(terminal): changing height may lead to wrong scrollback (#37824)

Problem:  Changing terminal height immediately after outputting lines
          may lead to wrong scrollback.
Solution: Insert pending scrollback lines before the old window height.

src/nvim/terminal.c

@@ -151,14 +151,16 @@ struct terminal {
   //  - receive data from libvterm as a result of key presses.
   char textbuf[TEXTBUF_SIZE];
 
-  ScrollbackLine **sb_buffer;       // Scrollback storage.
+  ScrollbackLine **sb_buffer;  ///< Scrollback storage.
-  size_t sb_current;                // Lines stored in sb_buffer.
+  size_t sb_current;           ///< Lines stored in sb_buffer.
-  size_t sb_size;                   // Capacity of sb_buffer.
+  size_t sb_size;              ///< Capacity of sb_buffer.
-  // "virtual index" that points to the first sb_buffer row that we need to
+  /// "virtual index" that points to the first sb_buffer row that we need to
-  // push to the terminal buffer when refreshing the scrollback.
+  /// push to the terminal buffer when refreshing the scrollback.
   int sb_pending;
-  size_t sb_deleted;                // Lines deleted from sb_buffer.
+  size_t sb_deleted;      ///< Lines deleted from sb_buffer.
-  size_t sb_deleted_last;           // Value of sb_deleted on last refresh_scrollback()
+  size_t old_sb_deleted;  ///< Value of sb_deleted on last refresh_scrollback().
+  /// Lines in the terminal buffer belonging to the screen instead of the scrollback.
+  int old_height;
 
   char *title;     // VTermStringFragment buffer
   size_t title_len;
@@ -561,6 +563,7 @@ Terminal *terminal_alloc(buf_T *buf, TerminalOptions opts)
     ml_delete_buf(buf, 1, false);
   }
   deleted_lines_buf(buf, 1, line_count);
+  term->old_height = 1;
 
   return term;
 }
@@ -1624,6 +1627,8 @@ static int term_sb_pop(int cols, VTermScreenCell *cells, void *data)
 
   if (term->sb_pending > 0) {
     term->sb_pending--;
+  } else {
+    term->old_height++;
   }
 
   ScrollbackLine *sbrow = term->sb_buffer[0];
@@ -2387,10 +2392,10 @@ static void adjust_scrollback(Terminal *term, buf_T *buf)
 // Refresh the scrollback of an invalidated terminal.
 static void refresh_scrollback(Terminal *term, buf_T *buf)
 {
-  linenr_T deleted = (linenr_T)(term->sb_deleted - term->sb_deleted_last);
+  linenr_T deleted = (linenr_T)(term->sb_deleted - term->old_sb_deleted);
   deleted = MIN(deleted, buf->b_ml.ml_line_count);
   mark_adjust_buf(buf, 1, deleted, MAXLNUM, -deleted, true, kMarkAdjustTerm, kExtmarkUndo);
-  term->sb_deleted_last = term->sb_deleted;
+  term->old_sb_deleted = term->sb_deleted;
 
   int width, height;
   vterm_get_size(term->vt, &height, &width);
@@ -2404,24 +2409,14 @@ static void refresh_scrollback(Terminal *term, buf_T *buf)
   }
   max_line_count += term->sb_pending;
 
-  // May still have pending scrollback after increase in terminal height if the
+  int old_height = MIN(term->old_height, buf->b_ml.ml_line_count);
-  // scrollback wasn't refreshed in time; append these to the top of the buffer.
-  int row_offset = term->sb_pending;
-  while (term->sb_pending > 0 && buf->b_ml.ml_line_count < height) {
-    fetch_row(term, term->sb_pending - row_offset - 1, width);
-    ml_append_buf(buf, 0, term->textbuf, 0, false);
-    appended_lines_buf(buf, 0, 1);
-    term->sb_pending--;
-  }
-
-  row_offset -= term->sb_pending;
   while (term->sb_pending > 0) {
     // This means that either the window height has decreased or the screen
     // became full and libvterm had to push all rows up. Convert the first
     // pending scrollback row into a string and append it just above the visible
     // section of the buffer.
-    fetch_row(term, -term->sb_pending - row_offset, width);
+    fetch_row(term, -term->sb_pending, width);
-    int buf_index = buf->b_ml.ml_line_count - height;
+    int buf_index = buf->b_ml.ml_line_count - old_height;
     ml_append_buf(buf, buf_index, term->textbuf, 0, false);
     appended_lines_buf(buf, buf_index, 1);
     term->sb_pending--;
@@ -2467,6 +2462,7 @@ static void refresh_screen(Terminal *term, buf_T *buf)
       added++;
     }
   }
+  term->old_height = height;
 
   int change_start = row_to_linenr(term, term->invalid_start);
   int change_end = change_start + changed;

test/functional/terminal/scrollback_spec.lua

@@ -1103,7 +1103,7 @@ describe('pending scrollback line handling', function()
   end)
 end)
 
-describe('nvim_open_term()', function()
+describe('scrollback is correct', function()
   local screen --- @type test.functional.ui.screen
   local buf --- @type integer
   local win --- @type integer
@@ -1141,6 +1141,8 @@ describe('nvim_open_term()', function()
     for i = start, stop do
       eq(('TEST %d'):format(i), lines[i - start + 1])
     end
+    eq('', lines[#lines])
+    eq(stop - start + 2, #lines)
   end
 
   local function check_common()
@@ -1156,7 +1158,7 @@ describe('nvim_open_term()', function()
     ]])
   end
 
-  it('on buffer with fewer lines than scrollback', function()
+  it('with nvim_open_term() on buffer with fewer lines than scrollback', function()
     exec_lua(function()
       vim.api.nvim_open_term(buf, {})
       vim.api.nvim_win_set_cursor(win, { 3, 0 })
@@ -1175,7 +1177,7 @@ describe('nvim_open_term()', function()
     check_common()
   end)
 
-  it('on buffer with more lines than scrollback', function()
+  it('with nvim_open_term() on buffer with more lines than scrollback', function()
     api.nvim_set_option_value('scrollback', 10, { buf = buf })
     exec_lua(function()
       vim.api.nvim_open_term(buf, {})
@@ -1194,4 +1196,144 @@ describe('nvim_open_term()', function()
     check_buffer_lines(86, 99)
     check_common()
   end)
+
+  describe('when window height', function()
+    before_each(function()
+      feed('<C-W>lGV4kdgg')
+      screen:try_resize(30, 20)
+      command('botright 9new | wincmd p')
+      exec_lua(function()
+        vim.g.chan = vim.api.nvim_open_term(buf, {})
+        vim.cmd('$')
+      end)
+      screen:expect([[
+                  │{100:TEST} 88            |
+        {1:~         }│{100:TEST} 89            |
+        {1:~         }│{100:TEST} 90            |
+        {1:~         }│{100:TEST} 91            |
+        {1:~         }│{100:TEST} 92            |
+        {1:~         }│{100:TEST} 93            |
+        {1:~         }│{100:TEST} 94            |
+        {1:~         }│^                   |
+        {2:[No Name]  }{102:[Scratch] [-]      }|
+                                      |
+        {1:~                             }|*8
+        {2:[No Name]                     }|
+                                      |
+      ]])
+      check_buffer_lines(0, 94)
+    end)
+
+    local send_cmd = 'call chansend(g:chan, @")'
+
+    describe('increases in the same refresh cycle as outputting lines', function()
+      --- @type string[][]
+      local perms = t.concat_tables(
+        t.permutations({ 'resize +2', send_cmd }),
+        t.permutations({ 'resize +4', 'resize -2', send_cmd }),
+        t.permutations({ 'resize +6', 'resize -4', send_cmd })
+      )
+      assert(#perms == 2 + 6 + 6)
+      local screen_final = [[
+                  │{100:TEST} 91            |
+        {1:~         }│{100:TEST} 92            |
+        {1:~         }│{100:TEST} 93            |
+        {1:~         }│{100:TEST} 94            |
+        {1:~         }│{100:TEST} 95            |
+        {1:~         }│{100:TEST} 96            |
+        {1:~         }│{100:TEST} 97            |
+        {1:~         }│{100:TEST} 98            |
+        {1:~         }│{100:TEST} 99            |
+        {1:~         }│^                   |
+        {2:[No Name]  }{102:[Scratch] [-]      }|
+                                      |
+        {1:~                             }|*6
+        {2:[No Name]                     }|
+                                      |
+      ]]
+
+      for i, perm in ipairs(perms) do
+        it(('permutation %d'):format(i), function()
+          exec_lua(function()
+            for _, cmd in ipairs(perm) do
+              vim.cmd(cmd)
+            end
+          end)
+          screen:expect(screen_final)
+          check_buffer_lines(0, 99)
+        end)
+      end
+    end)
+
+    describe('decreases in the same refresh cycle as outputting lines', function()
+      --- @type string[][]
+      local perms = t.concat_tables(
+        t.permutations({ 'resize -2', send_cmd }),
+        t.permutations({ 'resize -4', 'resize +2', send_cmd }),
+        t.permutations({ 'resize -6', 'resize +4', send_cmd })
+      )
+      assert(#perms == 2 + 6 + 6)
+      local screen_final = [[
+                  │{100:TEST} 95            |
+        {1:~         }│{100:TEST} 96            |
+        {1:~         }│{100:TEST} 97            |
+        {1:~         }│{100:TEST} 98            |
+        {1:~         }│{100:TEST} 99            |
+        {1:~         }│^                   |
+        {2:[No Name]  }{102:[Scratch] [-]      }|
+                                      |
+        {1:~                             }|*10
+        {2:[No Name]                     }|
+                                      |
+      ]]
+
+      for i, perm in ipairs(perms) do
+        it(('permutation %d'):format(i), function()
+          exec_lua(function()
+            for _, cmd in ipairs(perm) do
+              vim.cmd(cmd)
+            end
+          end)
+          screen:expect(screen_final)
+          check_buffer_lines(0, 99)
+        end)
+      end
+    end)
+
+    describe("decreases by more than 'scrollback'", function()
+      before_each(function()
+        api.nvim_set_option_value('scrollback', 4, { buf = buf })
+        check_buffer_lines(84, 94)
+      end)
+
+      local perms = {
+        { send_cmd, 'resize -6' },
+        { 'resize -6', send_cmd },
+        { send_cmd, 'resize +6', 'resize -12' },
+        { 'resize +6', send_cmd, 'resize -12' },
+        { 'resize +6', 'resize -12', send_cmd },
+      }
+      local screen_final = [[
+                  │{100:TEST} 99            |
+        {1:~         }│^                   |
+        {2:[No Name]  }{102:[Scratch] [-]      }|
+                                      |
+        {1:~                             }|*14
+        {2:[No Name]                     }|
+                                      |
+      ]]
+
+      for i, perm in ipairs(perms) do
+        it(('permutation %d'):format(i), function()
+          exec_lua(function()
+            for _, cmd in ipairs(perm) do
+              vim.cmd(cmd)
+            end
+          end)
+          screen:expect(screen_final)
+          check_buffer_lines(95, 99)
+        end)
+      end
+    end)
+  end)
 end)

test/testutil.lua

@@ -646,6 +646,29 @@ function M.concat_tables(...)
   return ret
 end
 
+--- Get all permutations of an array.
+---
+--- @param arr any[]
+--- @return any[][]
+function M.permutations(arr)
+  local res = {} --- @type any[][]
+  --- @param a any[]
+  --- @param n integer
+  local function gen(a, n)
+    if n == 0 then
+      res[#res + 1] = M.shallowcopy(a)
+      return
+    end
+    for i = 1, n do
+      a[n], a[i] = a[i], a[n]
+      gen(a, n - 1)
+      a[n], a[i] = a[i], a[n]
+    end
+  end
+  gen(M.shallowcopy(arr), #arr)
+  return res
+end
+
 --- @param str string
 --- @param leave_indent? integer
 --- @return string