vim-patch:9.1.0990: Inconsistent behavior when changing cmdheight (#31830)

Problem:  Inconsistent behavior when changing cmdheight by resizing the
          topframe through wincmds and dragging laststatus. Changing
          cmdheight by resizing the topframe does not trigger OptionSet.
Solution: Consolidate logic for changing the cmdheight, set the option
          value to handle side-effects (Luuk van Baal)

e15cbc1af4

vim-patch:9.0.0187: command line height changes when maximizing window height

Problem:  Command line height changes when maximizing window height.
Solution: Do not change the command line height. (closes vim/vim#10885)

96bde99bf8

src/nvim/option.c

@@ -2120,14 +2120,14 @@ static const char *did_set_laststatus(optset_T *args)
   // When switching to global statusline, decrease topframe height
   // Also clear the cmdline to remove the ruler if there is one
   if (value == 3 && old_value != 3) {
-    frame_new_height(topframe, topframe->fr_height - STATUS_HEIGHT, false, false);
+    frame_new_height(topframe, topframe->fr_height - STATUS_HEIGHT, false, false, false);
     win_comp_pos();
     clear_cmdline = true;
   }
   // When switching from global statusline, increase height of topframe by STATUS_HEIGHT
   // in order to to re-add the space that was previously taken by the global statusline
   if (old_value == 3 && value != 3) {
-    frame_new_height(topframe, topframe->fr_height + STATUS_HEIGHT, false, false);
+    frame_new_height(topframe, topframe->fr_height + STATUS_HEIGHT, false, false, false);
     win_comp_pos();
   }
 

src/nvim/window.c

@@ -1462,7 +1462,7 @@ win_T *win_split_ins(int size, int flags, win_T *new_wp, int dir, frame_T *to_fl
           frame_add_statusline(curfrp);
         }
       }
-      frame_new_height(curfrp, new_fr_height, flags & WSP_TOP, false);
+      frame_new_height(curfrp, new_fr_height, flags & WSP_TOP, false, false);
     } else {
       win_new_height(oldwin, oldwin_height - (new_size + STATUS_HEIGHT));
     }
@@ -2136,7 +2136,7 @@ static void win_equal_rec(win_T *next_curwin, bool current, frame_T *topfr, int
         || topfr->fr_width != width
         || topfr->fr_win->w_wincol != col) {
       topfr->fr_win->w_winrow = row;
-      frame_new_height(topfr, height, false, false);
+      frame_new_height(topfr, height, false, false, false);
       topfr->fr_win->w_wincol = col;
       frame_new_width(topfr, width, false, false);
       redraw_all_later(UPD_NOT_VALID);
@@ -3154,7 +3154,7 @@ win_T *winframe_remove(win_T *win, int *dirp, tabpage_T *tp, frame_T **unflat_al
 
   if (*dirp == 'v') {
     frame_new_height(altfr, altfr->fr_height + frp_close->fr_height,
-                     altfr == frp_close->fr_next, false);
+                     altfr == frp_close->fr_next, false, false);
   } else {
     assert(*dirp == 'h');
     frame_new_width(altfr, altfr->fr_width + frp_close->fr_width,
@@ -3356,7 +3356,7 @@ void winframe_restore(win_T *wp, int dir, frame_T *unflat_altfr)
   // adjusts window sizes to fit restored statuslines/separators, if needed.
   if (dir == 'v') {
     frame_new_height(unflat_altfr, unflat_altfr->fr_height - frp->fr_height,
-                     unflat_altfr == frp->fr_next, false);
+                     unflat_altfr == frp->fr_next, false, false);
   } else if (dir == 'h') {
     frame_new_width(unflat_altfr, unflat_altfr->fr_width - frp->fr_width,
                     unflat_altfr == frp->fr_next, false);
@@ -3499,15 +3499,27 @@ static bool is_bottom_win(win_T *wp)
   }
   return true;
 }
+
 /// Set a new height for a frame.  Recursively sets the height for contained
 /// frames and windows.  Caller must take care of positions.
 ///
 /// @param topfirst  resize topmost contained frame first.
 /// @param wfh       obey 'winfixheight' when there is a choice;
 ///                  may cause the height not to be set.
-void frame_new_height(frame_T *topfrp, int height, bool topfirst, bool wfh)
+/// @param set_ch    set 'cmdheight' to resize topframe.
+void frame_new_height(frame_T *topfrp, int height, bool topfirst, bool wfh, bool set_ch)
   FUNC_ATTR_NONNULL_ALL
 {
+  if (topfrp->fr_parent == NULL && set_ch) {
+    // topframe: update the command line height, with side effects.
+    OptInt new_ch = MAX(!p_ch_was_zero, p_ch + topfrp->fr_height - height);
+    if (new_ch != p_ch) {
+      const bool was_zero = p_ch_was_zero;
+      set_option_value(kOptCmdheight, NUMBER_OPTVAL(new_ch), 0);
+      p_ch_was_zero = was_zero;
+    }
+    height = (int)MIN(ROWS_AVAIL, height);
+  }
   if (topfrp->fr_win != NULL) {
     // Simple case: just one window.
     win_T *wp = topfrp->fr_win;
@@ -3520,7 +3532,7 @@ void frame_new_height(frame_T *topfrp, int height, bool topfirst, bool wfh)
     do {
       // All frames in this row get the same new height.
       FOR_ALL_FRAMES(frp, topfrp->fr_child) {
-        frame_new_height(frp, height, topfirst, wfh);
+        frame_new_height(frp, height, topfirst, wfh, set_ch);
         if (frp->fr_height > height) {
           // Could not fit the windows, make the whole row higher.
           height = frp->fr_height;
@@ -3562,10 +3574,9 @@ void frame_new_height(frame_T *topfrp, int height, bool topfirst, bool wfh)
         int h = frame_minheight(frp, NULL);
         if (frp->fr_height + extra_lines < h) {
           extra_lines += frp->fr_height - h;
-          frame_new_height(frp, h, topfirst, wfh);
+          frame_new_height(frp, h, topfirst, wfh, set_ch);
         } else {
-          frame_new_height(frp, frp->fr_height + extra_lines,
+          frame_new_height(frp, frp->fr_height + extra_lines, topfirst, wfh, set_ch);
-                           topfirst, wfh);
           break;
         }
         if (topfirst) {
@@ -3584,7 +3595,7 @@ void frame_new_height(frame_T *topfrp, int height, bool topfirst, bool wfh)
       }
     } else if (extra_lines > 0) {
       // increase height of bottom or top frame
-      frame_new_height(frp, frp->fr_height + extra_lines, topfirst, wfh);
+      frame_new_height(frp, frp->fr_height + extra_lines, topfirst, wfh, set_ch);
     }
   }
   topfrp->fr_height = height;
@@ -4001,6 +4012,10 @@ void unuse_tabpage(tabpage_T *tp)
   tp->tp_curwin = curwin;
 }
 
+// When switching tabpage, handle other side-effects in command_height(), but
+// avoid setting frame sizes which are still correct.
+static bool command_frame_height = true;
+
 /// Set the relevant pointers to use tab page "tp".  May want to call
 /// unuse_tabpage() first.
 void use_tabpage(tabpage_T *tp)
@@ -4402,6 +4417,16 @@ static void enter_tabpage(tabpage_T *tp, buf_T *old_curbuf, bool trigger_enter_a
 
   use_tabpage(tp);
 
+  if (p_ch != curtab->tp_ch_used) {
+    // Use the stored value of p_ch, so that it can be different for each tab page.
+    // Handle other side-effects but avoid setting frame sizes, which are still correct.
+    OptInt new_ch = curtab->tp_ch_used;
+    curtab->tp_ch_used = p_ch;
+    command_frame_height = false;
+    set_option_value(kOptCmdheight, NUMBER_OPTVAL(new_ch), 0);
+    command_frame_height = true;
+  }
+
   if (old_curtab != curtab) {
     tabpage_check_windows(old_curtab);
   }
@@ -4415,27 +4440,9 @@ static void enter_tabpage(tabpage_T *tp, buf_T *old_curbuf, bool trigger_enter_a
   prevwin = next_prevwin;
 
   last_status(false);  // status line may appear or disappear
-  const int row = win_comp_pos();  // recompute w_winrow for all windows
+  win_comp_pos();      // recompute w_winrow for all windows
   diff_need_scrollbind = true;
 
-  // Use the stored value of p_ch, so that it can be different for each tab page.
-  if (p_ch != curtab->tp_ch_used) {
-    clear_cmdline = true;
-    if (msg_grid.chars && p_ch < curtab->tp_ch_used) {
-      // TODO(bfredl): a bit expensive, should be enough to invalidate the
-      // region between the old and the new p_ch.
-      grid_invalidate(&msg_grid);
-    }
-  }
-  p_ch = curtab->tp_ch_used;
-
-  // When cmdheight is changed in a tab page with '<C-w>-', cmdline_row is
-  // changed but p_ch and tp_ch_used are not changed. Thus we also need to
-  // check cmdline_row.
-  if (row < cmdline_row && cmdline_row <= Rows - p_ch) {
-    clear_cmdline = true;
-  }
-
   // If there was a click in a window, it won't be usable for a following
   // drag.
   reset_dragwin();
@@ -5407,9 +5414,9 @@ void win_new_screen_rows(void)
 
   // First try setting the heights of windows with 'winfixheight'.  If
   // that doesn't result in the right height, forget about that option.
-  frame_new_height(topframe, h, false, true);
+  frame_new_height(topframe, h, false, true, false);
   if (!frame_check_height(topframe, h)) {
-    frame_new_height(topframe, h, false, false);
+    frame_new_height(topframe, h, false, false, false);
   }
 
   win_comp_pos();  // recompute w_winrow and w_wincol
@@ -5844,22 +5851,7 @@ void win_setheight_win(int height, win_T *win)
     frame_setheight(win->w_frame, height + win->w_hsep_height + win->w_status_height);
 
     // recompute the window positions
-    int row = win_comp_pos();
+    win_comp_pos();
-
-    // If there is extra space created between the last window and the command
-    // line, clear it.
-    if (full_screen && msg_scrolled == 0 && row < cmdline_row) {
-      grid_clear(&default_grid, row, cmdline_row, 0, Columns, 0);
-      if (msg_grid.chars) {
-        clear_cmdline = true;
-      }
-    }
-    cmdline_row = row;
-    p_ch = MAX(Rows - cmdline_row, 0);
-    curtab->tp_ch_used = p_ch;
-    msg_row = row;
-    msg_col = 0;
-
     win_fix_scroll(true);
 
     redraw_all_later(UPD_NOT_VALID);
@@ -5887,14 +5879,8 @@ static void frame_setheight(frame_T *curfrp, int height)
 
   if (curfrp->fr_parent == NULL) {
     // topframe: can only change the command line height
-    if (height > ROWS_AVAIL) {
-      // If height is greater than the available space, try to create space for
-      // the frame by reducing 'cmdheight' if possible, while making sure
-      // `cmdheight` doesn't go below 1 if it wasn't set to 0 explicitly.
-      height = (int)MIN(ROWS_AVAIL + p_ch - !p_ch_was_zero, height);
-    }
     if (height > 0) {
-      frame_new_height(curfrp, height, false, false);
+      frame_new_height(curfrp, height, false, false, true);
     }
   } else if (curfrp->fr_parent->fr_layout == FR_ROW) {
     // Row of frames: Also need to resize frames left and right of this
@@ -5973,7 +5959,7 @@ static void frame_setheight(frame_T *curfrp, int height)
     }
 
     // set the current frame to the new height
-    frame_new_height(curfrp, height, false, false);
+    frame_new_height(curfrp, height, false, false, true);
 
     // First take lines from the frames after the current frame.  If
     // that is not enough, takes lines from frames above the current
@@ -5995,15 +5981,15 @@ static void frame_setheight(frame_T *curfrp, int height)
               room_reserved = frp->fr_height - take;
             }
             take -= frp->fr_height - room_reserved;
-            frame_new_height(frp, room_reserved, false, false);
+            frame_new_height(frp, room_reserved, false, false, true);
             room_reserved = 0;
           }
         } else {
           if (frp->fr_height - take < h) {
             take -= frp->fr_height - h;
-            frame_new_height(frp, h, false, false);
+            frame_new_height(frp, h, false, false, true);
           } else {
-            frame_new_height(frp, frp->fr_height - take, false, false);
+            frame_new_height(frp, frp->fr_height - take, false, false, true);
             take = 0;
           }
         }
@@ -6281,7 +6267,7 @@ void win_drag_status_line(win_T *dragwin, int offset)
   // Grow frame fr by "offset" lines.
   // Doesn't happen when dragging the last status line up.
   if (fr != NULL) {
-    frame_new_height(fr, fr->fr_height + offset, up, false);
+    frame_new_height(fr, fr->fr_height + offset, up, false, true);
   }
 
   if (up) {
@@ -6294,9 +6280,9 @@ void win_drag_status_line(win_T *dragwin, int offset)
     int n = frame_minheight(fr, NULL);
     if (fr->fr_height - offset <= n) {
       offset -= fr->fr_height - n;
-      frame_new_height(fr, n, !up, false);
+      frame_new_height(fr, n, !up, false, true);
     } else {
-      frame_new_height(fr, fr->fr_height - offset, !up, false);
+      frame_new_height(fr, fr->fr_height - offset, !up, false, true);
       break;
     }
     if (up) {
@@ -6305,15 +6291,7 @@ void win_drag_status_line(win_T *dragwin, int offset)
       fr = fr->fr_next;
     }
   }
-  int row = win_comp_pos();
+  win_comp_pos();
-  grid_clear(&default_grid, row, cmdline_row, 0, Columns, 0);
-  if (msg_grid.chars) {
-    clear_cmdline = true;
-  }
-  cmdline_row = row;
-  p_ch = MAX(Rows - cmdline_row, p_ch_was_zero ? 0 : 1);
-  curtab->tp_ch_used = p_ch;
-
   win_fix_scroll(true);
 
   redraw_all_later(UPD_SOME_VALID);
@@ -6750,21 +6728,6 @@ void command_height(void)
 {
   int old_p_ch = (int)curtab->tp_ch_used;
 
-  // Use the value of p_ch that we remembered.  This is needed for when the
-  // GUI starts up, we can't be sure in what order things happen.  And when
-  // p_ch was changed in another tab page.
-  curtab->tp_ch_used = p_ch;
-
-  // Update cmdline_row to what it should be: just below the last window.
-  cmdline_row = topframe->fr_height + tabline_height() + global_stl_height();
-
-  // If cmdline_row is smaller than what it is supposed to be for 'cmdheight'
-  // then set old_p_ch to what it would be, so that the windows get resized
-  // properly for the new value.
-  if (cmdline_row < Rows - p_ch) {
-    old_p_ch = Rows - cmdline_row;
-  }
-
   // Find bottom frame with width of screen.
   frame_T *frp = lastwin_nofloating()->w_frame;
   while (frp->fr_width != Columns && frp->fr_parent != NULL) {
@@ -6772,60 +6735,52 @@ void command_height(void)
   }
 
   // Avoid changing the height of a window with 'winfixheight' set.
-  while (frp->fr_prev != NULL && frp->fr_layout == FR_LEAF
+  while (frp->fr_prev != NULL && frp->fr_layout == FR_LEAF && frp->fr_win->w_p_wfh) {
-         && frp->fr_win->w_p_wfh) {
     frp = frp->fr_prev;
   }
 
-  if (starting != NO_SCREEN) {
+  while (p_ch > old_p_ch && command_frame_height) {
-    cmdline_row = Rows - (int)p_ch;
-
-    if (p_ch > old_p_ch) {                  // p_ch got bigger
-      while (p_ch > old_p_ch) {
     if (frp == NULL) {
       emsg(_(e_noroom));
       p_ch = old_p_ch;
-          curtab->tp_ch_used = p_ch;
-          cmdline_row = Rows - (int)p_ch;
       break;
     }
-        int h = frp->fr_height - frame_minheight(frp, NULL);
+    int h = MIN((int)(p_ch - old_p_ch), frp->fr_height - frame_minheight(frp, NULL));
-        h = MIN(h, (int)p_ch - old_p_ch);
-        old_p_ch += h;
     frame_add_height(frp, -h);
+    old_p_ch += h;
     frp = frp->fr_prev;
   }
+  if (p_ch < old_p_ch && command_frame_height) {
+    frame_add_height(frp, (int)(old_p_ch - p_ch));
+  }
 
   // Recompute window positions.
   win_comp_pos();
+  cmdline_row = Rows - (int)p_ch;
+  redraw_cmdline = true;
 
-      if (!need_wait_return) {
+  // Clear the cmdheight area.
-        // clear the lines added to cmdline
+  if (msg_scrolled == 0 && full_screen) {
-        if (full_screen) {
+    ScreenGrid *grid = &default_grid;
-          grid_clear(&default_grid, cmdline_row, Rows, 0, Columns, 0);
+    if (!ui_has(kUIMessages)) {
+      msg_grid_validate();
+      grid = &msg_grid_adj;
     }
+    grid_clear(grid, cmdline_row, Rows, 0, Columns, 0);
     msg_row = cmdline_row;
   }
-      redraw_cmdline = true;
-      return;
-    }
 
-    msg_row = MAX(msg_row, cmdline_row);
+  // Use the value of p_ch that we remembered.  This is needed for when the
-    redraw_cmdline = true;
+  // GUI starts up, we can't be sure in what order things happen.  And when
-  }
+  // p_ch was changed in another tab page.
-  frame_add_height(frp, (int)(old_p_ch - p_ch));
+  curtab->tp_ch_used = p_ch;
-
-  // Recompute window positions.
-  if (frp != lastwin->w_frame) {
-    win_comp_pos();
-  }
 }
 
 // Resize frame "frp" to be "n" lines higher (negative for less high).
 // Also resize the frames it is contained in.
 static void frame_add_height(frame_T *frp, int n)
 {
-  frame_new_height(frp, frp->fr_height + n, false, false);
+  frame_new_height(frp, frp->fr_height + n, false, false, false);
   while (true) {
     frp = frp->fr_parent;
     if (frp == NULL) {
@@ -6895,7 +6850,7 @@ static bool resize_frame_for_status(frame_T *fr)
     emsg(_(e_noroom));
     return false;
   } else if (fp != fr) {
-    frame_new_height(fp, fp->fr_height - 1, false, false);
+    frame_new_height(fp, fp->fr_height - 1, false, false, false);
     frame_fix_height(wp);
     win_comp_pos();
   } else {
@@ -6916,7 +6871,7 @@ static bool resize_frame_for_winbar(frame_T *fr)
     emsg(_(e_noroom));
     return false;
   }
-  frame_new_height(fp, fp->fr_height - 1, false, false);
+  frame_new_height(fp, fp->fr_height - 1, false, false, false);
   win_new_height(wp, wp->w_height + 1);
   frame_fix_height(wp);
   win_comp_pos();
@@ -7305,7 +7260,7 @@ static win_T *restore_snapshot_rec(frame_T *sn, frame_T *fr)
   fr->fr_height = sn->fr_height;
   fr->fr_width = sn->fr_width;
   if (fr->fr_layout == FR_LEAF) {
-    frame_new_height(fr, fr->fr_height, false, false);
+    frame_new_height(fr, fr->fr_height, false, false, false);
     frame_new_width(fr, fr->fr_width, false, false);
     wp = sn->fr_win;
   }

test/functional/legacy/cmdline_spec.lua

@@ -159,6 +159,52 @@ describe('cmdline', function()
       endfunc
     ]])
 
+    feed(':resize -3<CR>')
+    screen:expect([[
+      ^                                                            |
+      {1:~                                                           }|*2
+      {3:[No Name]                                                   }|
+                                                                  |*4
+    ]])
+
+    -- :resize now also changes 'cmdheight' accordingly
+    feed(':set cmdheight+=1<CR>')
+    screen:expect([[
+      ^                                                            |
+      {1:~                                                           }|
+      {3:[No Name]                                                   }|
+                                                                  |*5
+    ]])
+    feed(':set cmdheight-=1<CR>')
+
+    -- using more space moves the status line up
+    feed(':set cmdheight+=1<CR>')
+    screen:expect([[
+      ^                                                            |
+      {1:~                                                           }|
+      {3:[No Name]                                                   }|
+                                                                  |*5
+    ]])
+
+    -- reducing cmdheight moves status line down
+    feed(':set cmdheight-=2<CR>')
+    screen:expect([[
+      ^                                                            |
+      {1:~                                                           }|*3
+      {3:[No Name]                                                   }|
+                                                                  |*3
+    ]])
+
+    -- reducing window size and then setting cmdheight
+    feed(':resize -1<CR>')
+    feed(':set cmdheight=1<CR>')
+    screen:expect([[
+      ^                                                            |
+      {1:~                                                           }|*5
+      {3:[No Name]                                                   }|
+                                                                  |
+    ]])
+
     -- setting 'cmdheight' works after outputting two messages
     feed(':call EchoTwo()')
     screen:expect([[

test/functional/legacy/messages_spec.lua

@@ -82,7 +82,7 @@ describe('messages', function()
       NoSuchFil^e                                                  |
       three                                                       |
       {1:~                                                           }|*5
-      from DebugSilent visual                                     |
+                                                                  |
       {9:E447: Can't find file "NoSuchFile" in path}                  |
     ]])
   end)

test/functional/legacy/window_cmd_spec.lua

@@ -299,7 +299,7 @@ describe('splitkeep', function()
       c                                                    |
       {1:~                                                    }|
       {3:[No Name]                                            }|
-                                                           |
+      :call win_move_statusline(win, 1)                    |
     ]])
   end)
 

test/old/testdir/test_autocmd.vim

@@ -4181,4 +4181,28 @@ func Test_autocmd_BufWinLeave_with_vsp()
   exe "bw! " .. dummy
 endfunc
 
+func Test_OptionSet_cmdheight()
+  set mouse=a laststatus=2
+  au OptionSet cmdheight :let &l:ch = v:option_new
+
+  resize -1
+  call assert_equal(2, &l:ch)
+  resize +1
+  call assert_equal(1, &l:ch)
+
+  call Ntest_setmouse(&lines - 1, 1)
+  call feedkeys("\<LeftMouse>", 'xt')
+  call Ntest_setmouse(&lines - 2, 1)
+  call feedkeys("\<LeftDrag>", 'xt')
+  call assert_equal(2, &l:ch)
+
+  tabnew | resize +1
+  call assert_equal(1, &l:ch)
+  tabfirst
+  call assert_equal(2, &l:ch)
+
+  tabonly
+  set cmdheight& mouse& laststatus&
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

test/old/testdir/test_cmdline.vim

@@ -291,9 +291,10 @@ func Test_changing_cmdheight()
   call term_sendkeys(buf, ":resize -3\<CR>")
   call VerifyScreenDump(buf, 'Test_changing_cmdheight_1', {})
 
-  " using the space available doesn't change the status line
+  " :resize now also changes 'cmdheight' accordingly
-  call term_sendkeys(buf, ":set cmdheight+=3\<CR>")
+  call term_sendkeys(buf, ":set cmdheight+=1\<CR>")
   call VerifyScreenDump(buf, 'Test_changing_cmdheight_2', {})
+  call term_sendkeys(buf, ":set cmdheight-=1\<CR>")
 
   " using more space moves the status line up
   call term_sendkeys(buf, ":set cmdheight+=1\<CR>")
@@ -312,7 +313,7 @@ func Test_changing_cmdheight()
   call term_sendkeys(buf, ":call EchoTwo()\<CR>")
   call VerifyScreenDump(buf, 'Test_changing_cmdheight_6', {})
 
-  " increasing 'cmdheight' doesn't clear the messages that need hit-enter
+  " decreasing 'cmdheight' doesn't clear the messages that need hit-enter
   call term_sendkeys(buf, ":call EchoOne()\<CR>")
   call VerifyScreenDump(buf, 'Test_changing_cmdheight_7', {})
 

test/old/testdir/test_window_cmd.vim

@@ -56,7 +56,6 @@ func Test_window_cmd_cmdwin_with_vsp()
 endfunc
 
 func Test_cmdheight_not_changed()
-  throw 'Skipped: N/A'
   set cmdheight=2
   set winminheight=0
   augroup Maximize