Mercurial > vim

changeset 34194:a522c6c0127b v9.1.0047

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
patch 9.1.0047: issues with temp curwin/buf while cmdwin is open Commit: https://github.com/vim/vim/commit/988f74311c26ea9917e84fbae608de226dba7e5f Author: Sean Dewar <seandewar@users.noreply.github.com> Date: Wed Aug 16 14:17:36 2023 +0100 patch 9.1.0047: issues with temp curwin/buf while cmdwin is open Problem: Things that temporarily change/restore curwin/buf (e.g: win_execute, some autocmds) may break assumptions that curwin/buf is the cmdwin when "cmdwin_type != 0", causing issues. Solution: Expose the cmdwin's real win/buf and check that instead. Also try to ensure these variables are NULL if "cmdwin_type == 0", allowing them to be used directly in most cases without checking cmdwin_type. (Sean Dewar) Alternatively, we could ban win_execute in the cmdwin and audit all places that temporarily change/restore curwin/buf, but I didn't notice any problems arising from allowing this (standard cmdwin restrictions still apply, so things that may actually break the cmdwin are still forbidden). closes: #12819 Signed-off-by: Sean Dewar <seandewar@users.noreply.github.com> Signed-off-by: Christian Brabandt <cb@256bit.org>
author Christian Brabandt <cb@256bit.org>
date Tue, 23 Jan 2024 23:00:04 +0100
parents 00b0bd66410b
children 5f4e62326459
files src/drawline.c src/drawscreen.c src/evalwindow.c src/ex_cmds.c src/ex_getln.c src/globals.h src/gui.c src/mouse.c src/move.c src/testdir/test_cmdwin.vim src/textformat.c src/version.c src/window.c
diffstat 13 files changed, 67 insertions(+), 13 deletions(-) [+]
line wrap: on
line diff
--- a/src/drawline.c
+++ b/src/drawline.c
@@ -1901,7 +1901,7 @@ win_line(
 	    if (wlv.draw_state == WL_CMDLINE - 1 && wlv.n_extra == 0)
 	    {
 		wlv.draw_state = WL_CMDLINE;
-		if (cmdwin_type != 0 && wp == curwin)
+		if (wp == cmdwin_win)
 		{
 		    // Draw the cmdline character.
 		    wlv.n_extra = 1;
--- a/src/drawscreen.c
+++ b/src/drawscreen.c
@@ -1087,7 +1087,7 @@ fold_line(
 
     // 1. Add the cmdwin_type for the command-line window
     // Ignores 'rightleft', this window is never right-left.
-    if (cmdwin_type != 0 && wp == curwin)
+    if (wp == cmdwin_win)
     {
 	ScreenLines[off] = cmdwin_type;
 	ScreenAttrs[off] = HL_ATTR(HLF_AT);
--- a/src/evalwindow.c
+++ b/src/evalwindow.c
@@ -1076,7 +1076,7 @@ f_win_gettype(typval_T *argvars, typval_
     else if (WIN_IS_POPUP(wp))
 	rettv->vval.v_string = vim_strsave((char_u *)"popup");
 #endif
-    else if (wp == curwin && cmdwin_type != 0)
+    else if (wp == cmdwin_win)
 	rettv->vval.v_string = vim_strsave((char_u *)"command");
 #ifdef FEAT_QUICKFIX
     else if (bt_quickfix(wp->w_buffer))
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -2782,9 +2782,16 @@ do_ecmd(
 	{
 	    bufref_T	save_au_new_curbuf;
 	    int		save_cmdwin_type = cmdwin_type;
+	    win_T	*save_cmdwin_win = cmdwin_win;
+
+	    // Should only be possible to get here if the cmdwin is closed, or
+	    // if it's opening and its buffer hasn't been set yet (the new
+	    // buffer is for it).
+	    assert(cmdwin_buf == NULL);
 
 	    // BufLeave applies to the old buffer.
 	    cmdwin_type = 0;
+	    cmdwin_win = NULL;
 
 	    /*
 	     * Be careful: The autocommands may delete any buffer and change
@@ -2801,7 +2808,10 @@ do_ecmd(
 	    save_au_new_curbuf = au_new_curbuf;
 	    set_bufref(&au_new_curbuf, buf);
 	    apply_autocmds(EVENT_BUFLEAVE, NULL, NULL, FALSE, curbuf);
+
 	    cmdwin_type = save_cmdwin_type;
+	    cmdwin_win = save_cmdwin_win;
+
 	    if (!bufref_valid(&au_new_curbuf))
 	    {
 		// new buffer has been deleted
--- a/src/ex_getln.c
+++ b/src/ex_getln.c
@@ -4494,8 +4494,9 @@ open_cmdwin(void)
     // Don't let quitting the More prompt make this fail.
     got_int = FALSE;
 
-    // Set "cmdwin_type" before any autocommands may mess things up.
+    // Set "cmdwin_..." variables before any autocommands may mess things up.
     cmdwin_type = get_cmdline_type();
+    cmdwin_win = curwin;
 
     // Create the command-line buffer empty.
     if (do_ecmd(0, NULL, NULL, NULL, ECMD_ONE, ECMD_HIDE, NULL) == FAIL)
@@ -4504,8 +4505,10 @@ open_cmdwin(void)
 	win_close(curwin, TRUE);
 	ga_clear(&winsizes);
 	cmdwin_type = 0;
+	cmdwin_win = NULL;
 	return Ctrl_C;
     }
+    cmdwin_buf = curbuf;
 
     apply_autocmds(EVENT_BUFFILEPRE, NULL, NULL, FALSE, curbuf);
     (void)setfname(curbuf, (char_u *)_("[Command Line]"), NULL, TRUE);
@@ -4615,6 +4618,8 @@ open_cmdwin(void)
 # endif
 
     cmdwin_type = 0;
+    cmdwin_buf = NULL;
+    cmdwin_win = NULL;
     exmode_active = save_exmode;
 
     // Safety check: The old window or buffer was deleted: It's a bug when
--- a/src/globals.h
+++ b/src/globals.h
@@ -1686,6 +1686,8 @@ EXTERN int	km_startsel INIT(= FALSE);
 
 EXTERN int	cmdwin_type INIT(= 0);	// type of cmdline window or 0
 EXTERN int	cmdwin_result INIT(= 0); // result of cmdline window or 0
+EXTERN buf_T	*cmdwin_buf INIT(= NULL); // buffer of cmdline window or NULL
+EXTERN win_T	*cmdwin_win INIT(= NULL); // window of cmdline window or NULL
 
 EXTERN char_u no_lines_msg[]	INIT(= N_("--No lines in buffer--"));
 
--- a/src/gui.c
+++ b/src/gui.c
@@ -3980,7 +3980,7 @@ gui_drag_scrollbar(scrollbar_T *sb, long
     if (hold_gui_events)
 	return;
 
-    if (cmdwin_type != 0 && sb->wp != curwin)
+    if (cmdwin_type != 0 && sb->wp != cmdwin_win)
 	return;
 
     if (still_dragging)
--- a/src/mouse.c
+++ b/src/mouse.c
@@ -1696,7 +1696,7 @@ retnomove:
 	}
 #if defined(FEAT_CLIPBOARD)
 	// Continue a modeless selection in another window.
-	if (cmdwin_type != 0 && row < curwin->w_winrow)
+	if (cmdwin_type != 0 && row < cmdwin_win->w_winrow)
 	    return IN_OTHER_WIN;
 #endif
 #ifdef FEAT_PROP_POPUP
@@ -1824,7 +1824,7 @@ retnomove:
 # ifdef FEAT_RIGHTLEFT
 			    wp->w_p_rl ? col < wp->w_width - wp->w_p_fdc :
 # endif
-			    col >= wp->w_p_fdc + (cmdwin_type == 0 && wp == curwin ? 0 : 1)
+			    col >= wp->w_p_fdc + (wp != cmdwin_win ? 0 : 1)
 			    )
 #endif
 			&& (flags & MOUSE_MAY_STOP_VIS))))
@@ -1832,7 +1832,7 @@ retnomove:
 	    end_visual_mode_keep_button();
 	    redraw_curbuf_later(UPD_INVERTED);	// delete the inversion
 	}
-	if (cmdwin_type != 0 && wp != curwin)
+	if (cmdwin_type != 0 && wp != cmdwin_win)
 	{
 	    // A click outside the command-line window: Use modeless
 	    // selection if possible.  Allow dragging the status lines.
@@ -1844,7 +1844,7 @@ retnomove:
 #else
 	    row = 0;
 	    col += wp->w_wincol;
-	    wp = curwin;
+	    wp = cmdwin_win;
 #endif
 	}
 #if defined(FEAT_PROP_POPUP) && defined(FEAT_TERMINAL)
@@ -1937,7 +1937,7 @@ retnomove:
 
 #if defined(FEAT_CLIPBOARD)
 	// Continue a modeless selection in another window.
-	if (cmdwin_type != 0 && row < curwin->w_winrow)
+	if (cmdwin_type != 0 && row < cmdwin_win->w_winrow)
 	    return IN_OTHER_WIN;
 #endif
 #ifdef FEAT_PROP_POPUP
@@ -2075,7 +2075,7 @@ retnomove:
 # ifdef FEAT_RIGHTLEFT
 	    curwin->w_p_rl ? col < curwin->w_width - curwin->w_p_fdc :
 # endif
-	    col >= curwin->w_p_fdc + (cmdwin_type == 0 ? 0 : 1)
+	    col >= curwin->w_p_fdc + (cmdwin_win != curwin ? 0 : 1)
        )
 	mouse_char = ' ';
 #endif
--- a/src/move.c
+++ b/src/move.c
@@ -1067,7 +1067,7 @@ validate_cursor_col(void)
 win_col_off(win_T *wp)
 {
     return (((wp->w_p_nu || wp->w_p_rnu) ? number_width(wp) + 1 : 0)
-	    + (cmdwin_type == 0 || wp != curwin ? 0 : 1)
+	    + (wp != cmdwin_win ? 0 : 1)
 #ifdef FEAT_FOLDING
 	    + wp->w_p_fdc
 #endif
--- a/src/testdir/test_cmdwin.vim
+++ b/src/testdir/test_cmdwin.vim
@@ -470,4 +470,37 @@ func Test_cmdwin_restore_heights()
   set cmdheight& showtabline& laststatus&
 endfunc
 
+func Test_cmdwin_temp_curwin()
+  func CheckWraps(expect_wrap)
+    setlocal textwidth=0 wrapmargin=1
+
+    call deletebufline('', 1, '$')
+    let as = repeat('a', winwidth(0) - 2 - &wrapmargin)
+    call setline(1, as .. ' b')
+    normal! gww
+
+    setlocal textwidth& wrapmargin&
+    call assert_equal(a:expect_wrap ? [as, 'b'] : [as .. ' b'], getline(1, '$'))
+  endfunc
+
+  func CheckCmdWin()
+    call assert_equal('command', win_gettype())
+    " textoff and &wrapmargin formatting considers the cmdwin_type char.
+    call assert_equal(1, getwininfo(win_getid())[0].textoff)
+    call CheckWraps(1)
+  endfunc
+
+  func CheckOtherWin()
+    call assert_equal('', win_gettype())
+    call assert_equal(0, getwininfo(win_getid())[0].textoff)
+    call CheckWraps(0)
+  endfunc
+
+  call feedkeys("q::call CheckCmdWin()\<CR>:call win_execute(win_getid(winnr('#')), 'call CheckOtherWin()')\<CR>:q<CR>", 'ntx')
+
+  delfunc CheckWraps
+  delfunc CheckCmdWin
+  delfunc CheckOtherWin
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab
--- a/src/textformat.c
+++ b/src/textformat.c
@@ -795,7 +795,7 @@ comp_textwidth(
 	// The width is the window width minus 'wrapmargin' minus all the
 	// things that add to the margin.
 	textwidth = curwin->w_width - curbuf->b_p_wm;
-	if (cmdwin_type != 0)
+	if (curbuf == cmdwin_buf)
 	    textwidth -= 1;
 #ifdef FEAT_FOLDING
 	textwidth -= curwin->w_p_fdc;
--- 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 */
 /**/
+    47,
+/**/
     46,
 /**/
     45,
--- a/src/window.c
+++ b/src/window.c
@@ -3375,6 +3375,8 @@ win_free_all(void)
 
     // avoid an error for switching tabpage with the cmdline window open
     cmdwin_type = 0;
+    cmdwin_buf = NULL;
+    cmdwin_win = NULL;
 
     while (first_tabpage->tp_next != NULL)
 	tabpage_close(TRUE);