# HG changeset patch # User Christian Brabandt # Date 1710358203 -3600 # Node ID db67c09ccd53046e086f115eb48cd40c700e360b # Parent 5ddbf662a64a56a8b7b47500e847001490ceb9a9 patch 9.1.0175: wrong window positions with 'winfix{width,height}' Commit: https://github.com/vim/vim/commit/5866bc3a0f54115d5982fdc09bdbe4c45069265a Author: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Wed Mar 13 20:17:24 2024 +0100 patch 9.1.0175: wrong window positions with 'winfix{width,height}' Problem: winframe functions incorrectly recompute window positions if the altframe wasn't adjacent to the closed frame, which is possible if adjacent windows had 'winfix{width,height}' set. Solution: recompute for windows within the parent of the altframe and closed frame. Skip this (as before) if the altframe was top/left, but only if adjacent to the closed frame, as positions won't change in that case. Also correct the return value documentation for win_screenpos. (Sean Dewar) The issue revealed itself after removing the win_comp_pos call below winframe_restore in win_splitmove. Similarly, wrong positions could result from windows closed in other tabpages, as win_free_mem uses winframe_remove (at least until it is entered later, where enter_tabpage calls win_comp_pos). NOTE: As win_comp_pos handles only curtab, it's possible via other means for positions in non-current tabpages to be wrong (e.g: after changing 'laststatus', 'showtabline', etc.). Given enter_tabpage recomputes it, maybe it's intentional as an optimization? Should probably be documented in win_screenpos then, but I won't address that here. closes: #14191 Signed-off-by: Sean Dewar <6256228+seandewar@users.noreply.github.com> Signed-off-by: Christian Brabandt diff --git a/runtime/doc/builtin.txt b/runtime/doc/builtin.txt --- a/runtime/doc/builtin.txt +++ b/runtime/doc/builtin.txt @@ -1,4 +1,4 @@ -*builtin.txt* For Vim version 9.1. Last change: 2024 Mar 12 +*builtin.txt* For Vim version 9.1. Last change: 2024 Mar 13 VIM REFERENCE MANUAL by Bram Moolenaar @@ -10829,8 +10829,7 @@ win_screenpos({nr}) *win_screenpos() [1, 1], unless there is a tabline, then it is [2, 1]. {nr} can be the window number or the |window-ID|. Use zero for the current window. - Returns [0, 0] if the window cannot be found in the current - tabpage. + Returns [0, 0] if the window cannot be found. Can also be used as a |method|: > GetWinid()->win_screenpos() diff --git a/src/testdir/test_window_cmd.vim b/src/testdir/test_window_cmd.vim --- a/src/testdir/test_window_cmd.vim +++ b/src/testdir/test_window_cmd.vim @@ -218,11 +218,11 @@ func Test_window_split_edit_bufnr() %bw! endfunc -func s:win_layout_info() abort +func s:win_layout_info(tp = tabpagenr()) abort return #{ - \ layout: winlayout(), - \ pos_sizes: range(1, winnr('$')) - \ ->map({_, nr -> win_getid(nr)->getwininfo()[0]}) + \ layout: winlayout(a:tp), + \ pos_sizes: range(1, tabpagewinnr(a:tp, '$')) + \ ->map({_, nr -> win_getid(nr, a:tp)->getwininfo()[0]}) \ ->map({_, wininfo -> #{id: wininfo.winid, \ row: wininfo.winrow, \ col: wininfo.wincol, @@ -2210,4 +2210,69 @@ func Test_win_gotoid_splitmove_textlock_ \ .. ":call assert_equal('', win_gettype(winnr('#')))\", 'ntx') endfunc +func Test_winfixsize_positions() + " Check positions are correct when closing a window in a non-current tabpage + " causes non-adjacent window to fill the space due to 'winfix{width,height}'. + tabnew + vsplit + wincmd | + split + set winfixheight + split foo + tabfirst + + bwipe! foo + " Save actual values before entering the tabpage. + let info = s:win_layout_info(2) + tabnext + " Compare it with the expected value (after win_comp_pos) from entering. + call assert_equal(s:win_layout_info(), info) + + $tabnew + split + split + wincmd k + belowright vsplit + set winfixwidth + belowright vsplit foo + tabprevious + + bwipe! foo + " Save actual values before entering the tabpage. + let info = s:win_layout_info(3) + tabnext + " Compare it with the expected value (after win_comp_pos) from entering. + call assert_equal(s:win_layout_info(), info) + + " Check positions unchanged when failing to move a window, if 'winfix{width, + " height}' would otherwise cause a non-adjacent window to fill the space. + %bwipe + call assert_fails('execute "split|"->repeat(&lines)', 'E36:') + wincmd p + vsplit + set winfixwidth + vsplit + set winfixwidth + vsplit + vsplit + set winfixwidth + wincmd p + + let info = s:win_layout_info() + call assert_fails('wincmd J', 'E36:') + call assert_equal(info, s:win_layout_info()) + + only + call assert_fails('execute "vsplit|"->repeat(&columns)', 'E36:') + belowright split + set winfixheight + belowright split + + let info = s:win_layout_info() + call assert_fails('wincmd H', 'E36:') + call assert_equal(info, s:win_layout_info()) + + %bwipe +endfunc + " vim: shiftwidth=2 sts=2 expandtab diff --git a/src/version.c b/src/version.c --- a/src/version.c +++ b/src/version.c @@ -705,6 +705,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 175, +/**/ 174, /**/ 173, diff --git a/src/window.c b/src/window.c --- a/src/window.c +++ b/src/window.c @@ -3493,6 +3493,7 @@ winframe_remove( frame_T *frp, *frp2, *frp3; frame_T *frp_close = win->w_frame; win_T *wp; + int row, col; /* * If there is only one window there is nothing to remove. @@ -3500,6 +3501,12 @@ winframe_remove( if (tp == NULL ? ONE_WINDOW : tp->tp_firstwin == tp->tp_lastwin) return NULL; + // Save the position of the containing frame (which will also contain the + // altframe) before we remove anything, to recompute window positions later. + wp = frame2win(frp_close->fr_parent); + row = wp->w_winrow; + col = wp->w_wincol; + /* * Remove the window from its frame. */ @@ -3584,15 +3591,10 @@ winframe_remove( *dirp = 'h'; } - // If rows/columns go to a window below/right its positions need to be - // updated. Can only be done after the sizes have been updated. - if (frp2 == frp_close->fr_next) - { - int row = win->w_winrow; - int col = win->w_wincol; - - frame_comp_pos(frp2, &row, &col); - } + // If the altframe wasn't adjacent and left/above, resizing it will have + // changed window positions within the parent frame. Recompute them. + if (frp2 != frp_close->fr_prev) + frame_comp_pos(frp_close->fr_parent, &row, &col); if (unflat_altfr == NULL) frame_flatten(frp2); @@ -3666,8 +3668,6 @@ frame_flatten(frame_T *frp) winframe_restore(win_T *wp, int dir, frame_T *unflat_altfr) { frame_T *frp = wp->w_frame; - int row = wp->w_winrow; - int col = wp->w_wincol; // Put "wp"'s frame back where it was. if (frp->fr_prev != NULL) @@ -3691,19 +3691,23 @@ winframe_restore(win_T *wp, int dir, fra { frame_new_height(unflat_altfr, unflat_altfr->fr_height - frp->fr_height, unflat_altfr == frp->fr_next, FALSE); - row += frp->fr_height; } else if (dir == 'h') { frame_new_width(unflat_altfr, unflat_altfr->fr_width - frp->fr_width, unflat_altfr == frp->fr_next, FALSE); - col += frp->fr_width; - } - - // If rows/columns went to a window below/right, its positions need to be - // restored. Can only be done after the sizes have been updated. - if (unflat_altfr == frp->fr_next) - frame_comp_pos(unflat_altfr, &row, &col); + } + + // Recompute window positions within the parent frame to restore them. + // Positions were unchanged if the altframe was adjacent and left/above. + if (unflat_altfr != frp->fr_prev) + { + win_T *topleft = frame2win(frp->fr_parent); + int row = topleft->w_winrow; + int col = topleft->w_wincol; + + frame_comp_pos(frp->fr_parent, &row, &col); + } } /*