From a47be0b2d90b26905866faf5b7cc82d9c17be9bb Mon Sep 17 00:00:00 2001 From: Sean Dewar Date: Mon, 24 Jul 2023 11:56:26 +0100 Subject: [PATCH] fix(window): prevent win_size_restore from changing cmdheight Currently it only skips if `Rows` changed, but it's possible for the height of the usable area for windows to change (e.g: via `&ch`, `&stal` or `&ls`), which can cause the value of `&cmdheight` to change when the sizes are restored. This is a Vim bug, so I've submitted a PR there too. No telling when it'll be merged though, given the current lack of activity there. `ROWS_AVAIL` is convenient here, but also subtracts the `global_stl_height()`. Not ideal, as we also care about the height of the last statusline for other values of `&ls`. Meh. Introduce `last_stl_height` for getting the height of the last statusline and use it in `win_size_save/restore` and `last_status` (means `last_status_rec`'s `statusline` argument will now be true if `&ls` is 3, but that does not change the behaviour). Also corrects the logic in `comp_col` to not assume there's a last statusline if `&ls` is 1 and the last window is floating. --- src/nvim/drawscreen.c | 2 +- src/nvim/window.c | 21 +++++++++++++----- test/functional/ui/cmdline_spec.lua | 34 ++++++++++++++++++++++++++++- 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/nvim/drawscreen.c b/src/nvim/drawscreen.c index 1554a9304d..73dd584fb1 100644 --- a/src/nvim/drawscreen.c +++ b/src/nvim/drawscreen.c @@ -1137,7 +1137,7 @@ static void recording_mode(int attr) /// of 'ru_col'. void comp_col(void) { - int last_has_status = (p_ls > 1 || (p_ls == 1 && !ONE_WINDOW)); + bool last_has_status = last_stl_height(false) > 0; sc_col = 0; ru_col = 0; diff --git a/src/nvim/window.c b/src/nvim/window.c index d55bda18c8..d6d677de3f 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -5712,8 +5712,9 @@ void win_size_save(garray_T *gap) { ga_init(gap, (int)sizeof(int), 1); ga_grow(gap, win_count() * 2 + 1); - // first entry is value of 'lines' - ((int *)gap->ga_data)[gap->ga_len++] = Rows; + // first entry is the total lines available for windows + ((int *)gap->ga_data)[gap->ga_len++] = + (int)ROWS_AVAIL + global_stl_height() - last_stl_height(false); FOR_ALL_WINDOWS_IN_TAB(wp, curtab) { ((int *)gap->ga_data)[gap->ga_len++] = @@ -5723,13 +5724,14 @@ void win_size_save(garray_T *gap) } // Restore window sizes, but only if the number of windows is still the same -// and 'lines' didn't change. +// and total lines available for windows didn't change. // Does not free the growarray. void win_size_restore(garray_T *gap) FUNC_ATTR_NONNULL_ALL { if (win_count() * 2 + 1 == gap->ga_len - && ((int *)gap->ga_data)[0] == Rows) { + && ((int *)gap->ga_data)[0] == + (int)ROWS_AVAIL + global_stl_height() - last_stl_height(false)) { // The order matters, because frames contain other frames, but it's // difficult to get right. The easy way out is to do it twice. for (int j = 0; j < 2; j++) { @@ -6993,8 +6995,7 @@ char *file_name_in_line(char *line, int col, int options, int count, char *rel_f void last_status(bool morewin) { // Don't make a difference between horizontal or vertical split. - last_status_rec(topframe, (p_ls == 2 || (p_ls == 1 && (morewin || !one_nonfloat()))), - global_stl_height() > 0); + last_status_rec(topframe, last_stl_height(morewin) > 0, global_stl_height() > 0); } // Remove status line from window, replacing it with a horizontal separator if needed. @@ -7193,6 +7194,14 @@ int global_stl_height(void) return (p_ls == 3) ? STATUS_HEIGHT : 0; } +/// Return the height of the last window's statusline, or the global statusline if set. +/// +/// @param morewin pretend there are two or more windows if true. +int last_stl_height(bool morewin) +{ + return (p_ls > 1 || (p_ls == 1 && (!one_nonfloat() || morewin))) ? STATUS_HEIGHT : 0; +} + /// Return the minimal number of rows that is needed on the screen to display /// the current number of windows. int min_rows(void) diff --git a/test/functional/ui/cmdline_spec.lua b/test/functional/ui/cmdline_spec.lua index dc29b765bd..a8cc8e00f0 100644 --- a/test/functional/ui/cmdline_spec.lua +++ b/test/functional/ui/cmdline_spec.lua @@ -984,14 +984,46 @@ it('tabline is not redrawn in Ex mode #24122', function() end) describe("cmdline height", function() + before_each(clear) + it("does not crash resized screen #14263", function() - clear() local screen = Screen.new(25, 10) screen:attach() command('set cmdheight=9999') screen:try_resize(25, 5) assert_alive() end) + + it('unchanged when trying to restore window sizes', function() + command('set showtabline=0 cmdheight=2 laststatus=0') + feed('q:') -- Closing cmdwin tries to restore sizes + command('set cmdheight=1 | quit') + eq(1, eval('&cmdheight')) + + command('set showtabline=2 cmdheight=3') + feed('q:') + command('set showtabline=0 | quit') + eq(3, eval('&cmdheight')) + + command('set cmdheight=1 laststatus=2') + feed('q:') + command('set laststatus=0 | quit') + eq(1, eval('&cmdheight')) + + command('set laststatus=2') + feed('q:') + command('set laststatus=1 | quit') + eq(1, eval('&cmdheight')) + + command('set laststatus=2 | belowright vsplit | wincmd _') + local restcmds = eval('winrestcmd()') + feed('q:') + command('set laststatus=1 | quit') + -- As we have 2 windows, &ls = 1 should still have a statusline on the last + -- window. As such, the number of available rows hasn't changed and the window + -- sizes should be restored. + eq(restcmds, eval('winrestcmd()')) + end) end) describe('cmdheight=0', function()