vim-patch:9.1.0993: New 'cmdheight' behavior may be surprising #31892

Problem:  Although patch 9.1.0990 fixed a real problem/inconsistency,
          it also introduced new behavior that may break BWC and/or be
          unexpected. Before 9.1.0990, window commands could make the
          topframe smaller (without changing 'cmdheight'; quirk that is
          now fixed), but did not allow extending the topframe beyond
          the 'cmdheight' set by the user. After 9.1.0990, the user can
          reduce the 'cmdheight' below the value they set explicitly,
          through window commands, which may lead to confusion.
          (aftere v9.1.0990)
Solution: Store the value explicitly set by the user and clamp the
          'cmdheight' when resizing the topframe. This also applies to
          dragging laststatus, which in contrast to window commands
          _did_ allow reducing the 'cmdheight' to values below the one
          set by the user. So with this patch there is still new
          behavior, but I think in a way that is less surprising.
          While at it, also fix a Coverity warning, introduced in
          v9.1.0990 (Luuk van Baal)

https://github.com/vim/vim/commit/c97e8695353565d6b20adffa48aad47f6e09967f

5 files changed, 29 insertions, 18 deletions

diff --git a/src/nvim/option.c b/src/nvim/option.c
index 824c9988f4..551ea0be20 100644
--- a/src/nvim/option.c
+++ b/src/nvim/option.c
@@ -2872,8 +2872,6 @@ static const char *validate_num_option(OptIndex opt_idx, OptInt *newval, char *e
   case kOptCmdheight:
     if (value < 0) {
       return e_positive;
-    } else {
-      p_ch_was_zero = value == 0;
     }
     break;
   case kOptHistory:
diff --git a/src/nvim/window.c b/src/nvim/window.c
index d459a046ca..8c8df72590 100644
--- a/src/nvim/window.c
+++ b/src/nvim/window.c
@@ -3500,6 +3500,10 @@ static bool is_bottom_win(win_T *wp)
   return true;
 }
 
+// 'cmdheight' value explicitly set by the user: window commands are allowed to
+// resize the topframe to values higher than this minimum, but not lower.
+static OptInt min_set_ch = 1;
+
 /// Set a new height for a frame.  Recursively sets the height for contained
 /// frames and windows.  Caller must take care of positions.
 ///
@@ -3512,11 +3516,11 @@ void frame_new_height(frame_T *topfrp, int height, bool topfirst, bool wfh, bool
 {
   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);
+    OptInt new_ch = MAX(min_set_ch, p_ch + topfrp->fr_height - height);
     if (new_ch != p_ch) {
-      const bool was_zero = p_ch_was_zero;
+      const OptInt save_ch = min_set_ch;
       set_option_value(kOptCmdheight, NUMBER_OPTVAL(new_ch), 0);
-      p_ch_was_zero = was_zero;
+      min_set_ch = save_ch;
     }
     height = (int)MIN(ROWS_AVAIL, height);
   }
@@ -6247,7 +6251,7 @@ void win_drag_status_line(win_T *dragwin, int offset)
     room = Rows - cmdline_row;
     if (curfr->fr_next != NULL) {
       room -= (int)p_ch + global_stl_height();
-    } else if (!p_ch_was_zero) {
+    } else if (min_set_ch > 0) {
       room--;
     }
     room = MAX(room, 0);
@@ -6750,7 +6754,7 @@ void command_height(void)
     old_p_ch += h;
     frp = frp->fr_prev;
   }
-  if (p_ch < old_p_ch && command_frame_height) {
+  if (p_ch < old_p_ch && command_frame_height && frp != NULL) {
     frame_add_height(frp, (int)(old_p_ch - p_ch));
   }
 
@@ -6774,6 +6778,7 @@ void command_height(void)
   // 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;
+  min_set_ch = p_ch;
 }
 
 // Resize frame "frp" to be "n" lines higher (negative for less high).
diff --git a/src/nvim/window.h b/src/nvim/window.h
index 9618ff1c2a..b5808d3451 100644
--- a/src/nvim/window.h
+++ b/src/nvim/window.h
@@ -35,9 +35,6 @@ enum {
 
 EXTERN int tabpage_move_disallowed INIT( = 0);  ///< moving tabpages around disallowed
 
-/// Set to true if 'cmdheight' was explicitly set to 0.
-EXTERN bool p_ch_was_zero INIT( = false);
-
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "window.h.generated.h"
 #endif
diff --git a/test/functional/legacy/cmdline_spec.lua b/test/functional/legacy/cmdline_spec.lua
index 22700990c1..819b40323a 100644
--- a/test/functional/legacy/cmdline_spec.lua
+++ b/test/functional/legacy/cmdline_spec.lua
@@ -175,19 +175,17 @@ describe('cmdline', function()
       {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
+                                                                  |*6
     ]])
 
     -- reducing cmdheight moves status line down
-    feed(':set cmdheight-=2<CR>')
+    feed(':set cmdheight-=3<CR>')
     screen:expect([[
       ^                                                            |
       {1:~                                                           }|*3
@@ -231,6 +229,16 @@ describe('cmdline', function()
       bar                                                         |
       {6:Press ENTER or type command to continue}^                     |
     ]])
+
+    -- window commands do not reduce 'cmdheight' to value lower than :set by user
+    feed('<CR>:wincmd _<CR>')
+    screen:expect([[
+      ^                                                            |
+      {1:~                                                           }|*4
+      {3:[No Name]                                                   }|
+      :wincmd _                                                   |
+                                                                  |
+    ]])
   end)
 
   -- oldtest: Test_cmdheight_tabline()
diff --git a/test/old/testdir/test_cmdline.vim b/test/old/testdir/test_cmdline.vim
index 3fce086040..d4ad63d43e 100644
--- a/test/old/testdir/test_cmdline.vim
+++ b/test/old/testdir/test_cmdline.vim
@@ -294,17 +294,16 @@ func Test_changing_cmdheight()
   " :resize now also changes 'cmdheight' accordingly
   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>")
   call VerifyScreenDump(buf, 'Test_changing_cmdheight_3', {})
 
   " reducing cmdheight moves status line down
-  call term_sendkeys(buf, ":set cmdheight-=2\<CR>")
+  call term_sendkeys(buf, ":set cmdheight-=3\<CR>")
   call VerifyScreenDump(buf, 'Test_changing_cmdheight_4', {})
 
-  " reducing window size and then setting cmdheight 
+  " reducing window size and then setting cmdheight
   call term_sendkeys(buf, ":resize -1\<CR>")
   call term_sendkeys(buf, ":set cmdheight=1\<CR>")
   call VerifyScreenDump(buf, 'Test_changing_cmdheight_5', {})
@@ -313,10 +312,14 @@ func Test_changing_cmdheight()
   call term_sendkeys(buf, ":call EchoTwo()\<CR>")
   call VerifyScreenDump(buf, 'Test_changing_cmdheight_6', {})
 
-  " decreasing 'cmdheight' doesn't clear the messages that need hit-enter
+  " increasing 'cmdheight' doesn't clear the messages that need hit-enter
   call term_sendkeys(buf, ":call EchoOne()\<CR>")
   call VerifyScreenDump(buf, 'Test_changing_cmdheight_7', {})
 
+  " window commands do not reduce 'cmdheight' to value lower than :set by user
+  call term_sendkeys(buf, "\<CR>:wincmd _\<CR>")
+  call VerifyScreenDump(buf, 'Test_changing_cmdheight_8', {})
+
   " clean up
   call StopVimInTerminal(buf)
 endfunc