vim / vim

The official Vim repository
https://www.vim.org
Vim License
36.12k stars 5.4k forks source link

Sporadic "E523 Not allowed here wundo" errors #5507

Closed myitcv closed 4 years ago

myitcv commented 4 years ago

Describe the bug

Very occasionally we see E523: Not allowed here: wundo errors in our govim tests. However we're struggling to understand how this corresponds to the docs for E523:

                                                'secure' 'nosecure' E523
'secure'                boolean (default off)
                        global
        When on, ":autocmd", shell and write commands are not allowed in
        ".vimrc" and ".exrc" in the current directory and map commands are
        displayed.  Switch it off only if you know that you will not run into
        problems, or when the 'exrc' option is off.  On Unix this option is
        only used if the ".vimrc" or ".exrc" is not owned by you.  This can be
        dangerous if the systems allows users to do a "chown".  You better set
        'secure' at the end of your ~/.vimrc then.
        This option cannot be set from a modeline or in the sandbox, for
        security reasons.

Please can someone explain how wundo is connected to E523?

We see this error in the context of wundo being executed by govim which is a remote plugin if that is significant. The full error message looks like this:

Vim(wundo):E523: Not allowed here: wundo! /tmp/1579529963907093695532890104'' in function GOVIM_internal_BalloonExpr[3]..<SNR>22_callbackFunction[2]..<SNR>22_ch_evalexpr[15]..<SNR>22_drainScheduleBacklog[14]..<SNR>22_ch_evalexpr[10]..<SNR>22_define[40]..<SNR>22_callbackCommand[3]..<SNR>22_ch_evalexpr[10]..<SNR>22_define, line 40

To Reproduce

We don't have repro steps; this is a very random and somewhat rare occurrence.

Expected behavior

Unclear; not sure why we are seeing the error.

Screenshots

n/a

Environment (please complete the following information):

VIM - Vi IMproved 8.2 (2019 Dec 12, compiled Jan  7 2020 12:54:48)
Included patches: 1-96
Compiled by myitcv@myitcv
Huge version with GTK2 GUI.  Features included (+) or not (-):
+acl               -farsi             -mouse_sysmouse    -tag_old_static
+arabic            +file_in_path      +mouse_urxvt       -tag_any_white
+autocmd           +find_in_path      +mouse_xterm       -tcl
+autochdir         +float             +multi_byte        +termguicolors
-autoservername    +folding           +multi_lang        +terminal
+balloon_eval      -footer            -mzscheme          +terminfo
+balloon_eval_term +fork()            -netbeans_intg     +termresponse
+browse            +gettext           +num64             +textobjects
++builtin_terms    -hangul_input      +packages          +textprop
+byte_offset       +iconv             +path_extra        +timers
+channel           +insert_expand     -perl              +title
+cindent           +job               +persistent_undo   +toolbar
+clientserver      +jumplist          +popupwin          +user_commands
+clipboard         +keymap            +postscript        +vartabs
+cmdline_compl     +lambda            +printer           +vertsplit
+cmdline_hist      +langmap           +profile           +virtualedit
+cmdline_info      +libcall           +python/dyn        +visual
+comments          +linebreak         +python3/dyn       +visualextra
+conceal           +lispindent        +quickfix          +viminfo
+cryptv            +listcmds          +reltime           +vreplace
+cscope            +localmap          +rightleft         +wildignore
+cursorbind        +lua               -ruby              +wildmenu
+cursorshape       +menu              +scrollbind        +windows
+dialog_con_gui    +mksession         +signs             +writebackup
+diff              +modify_fname      +smartindent       +X11
+digraphs          +mouse             -sound             -xfontset
+dnd               +mouseshape        +spell             +xim
-ebcdic            +mouse_dec         +startuptime       +xpm
+emacs_tags        -mouse_gpm         +statusline        +xsmp_interact
+eval              -mouse_jsbterm     -sun_workshop      +xterm_clipboard
+ex_extra          +mouse_netterm     +syntax            -xterm_save
+extra_search      +mouse_sgr         +tag_binary        
   system vimrc file: "$VIM/vimrc"
     user vimrc file: "$HOME/.vimrc"
 2nd user vimrc file: "~/.vim/vimrc"
      user exrc file: "$HOME/.exrc"
  system gvimrc file: "$VIM/gvimrc"
    user gvimrc file: "$HOME/.gvimrc"
2nd user gvimrc file: "~/.vim/gvimrc"
       defaults file: "$VIMRUNTIME/defaults.vim"
    system menu file: "$VIMRUNTIME/menu.vim"
  fall-back for $VIM: "/home/myitcv/usr/vim/share/vim"
Compilation: gcc -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_GTK  -pthread -I/usr/include/gtk-2.0 -I/usr/lib/x86_64-linux-gnu/gtk-2.0/include -I/usr/include/pango-1.0 -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/pango-1.0 -I/usr/include/harfbuzz -I/usr/include/pango-1.0 -I/usr/include/fribidi -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16   -g -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1       
Linking: gcc   -L/usr/local/lib -Wl,--as-needed -o vim   -lgtk-x11-2.0 -lgdk-x11-2.0 -lpangocairo-1.0 -latk-1.0 -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lpangoft2-1.0 -lpango-1.0 -lgobject-2.0 -lglib-2.0 -lfontconfig -lfreetype -lSM -lICE -lXpm -lXt -lX11 -lXdmcp -lSM -lICE  -lm -ltinfo -lnsl    -ldl  -L/usr/lib/x86_64-linux-gnu -lluajit-5.1         

Additional context Add any other context about the problem here.

lacygoill commented 4 years ago

I think E523 is sometimes raised when you try to do something which is forbidden right now.

As a – contrived – example, consider this shell command:

vim -Nu NONE +'nno <expr> cd execute("wundo /tmp/undo")' /tmp/file

It installs an <expr> mapping – triggered by the keysequence cd – which executes :wundo /tmp/undo. But if you press cd, E523 is raised, because this action is forbidden while textlock is active (see :h textlock).

Here, the solution is to refactor the mapping so that it's not defined with the <expr> argument:

vim -Nu NONE +'nno cd :wundo /tmp/undo<cr>' /tmp/file

Now pressing cd doesn't raise E523 anymore.


In the past, I had this error when one of my plugin was trying to execute :checktime while the buffer was modified and I was on the command-line. I can't reproduce anymore, so I don't know whether Vim's behavior has been changed by a patch, or the issue needs some other condition to be satisfied to be reproduced.

At the time, the solution that I found was to slightly delay the execution of :checktime via a timer.


I don't know whether it will help, but as a workaround, maybe you could try to delay the execution of :wundo, either via a timer:

call timer_start(0, {-> execute('wundo ...')})
                 ^
                 increase the waiting time if needed

Or via a one-shot autocmd listening to SafeState:

au SafeState * ++once wundo ...

In both cases, you'll probably want a guard to make sure the active buffer is still the one you expect.

call timer_start(0, {-> expand('%:p') ==# 'my expected buffer' && execute('wundo ...')})
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

au SafeState * ++once if expand('%:p') ==# 'my expected buffer' | wundo ... | endif
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The idea being that the state in which Vim is, and which currently forbids :wundo, will be over after a short time.

lacygoill commented 4 years ago

Just to illustrate how a timer can fix E523, the same problematic <expr> mapping does not raise the error anymore once :wundo is slightly delayed:

vim -Nu NONE +'nno <expr> cd timer_start(0, {-> execute("wundo /tmp/undo")})[-1]' /tmp/file
brammool commented 4 years ago

Describe the bug

Very occasionally we see E523: Not allowed here: wundo errors in our govim tests. However we're struggling to understand how this corresponds to the docs for E523:

                                                'secure' 'nosecure' E523
'secure'                boolean (default off)
                        global
        When on, ":autocmd", shell and write commands are not allowed in
        ".vimrc" and ".exrc" in the current directory and map commands are
        displayed.  Switch it off only if you know that you will not run into
        problems, or when the 'exrc' option is off.  On Unix this option is
        only used if the ".vimrc" or ".exrc" is not owned by you.  This can be
        dangerous if the systems allows users to do a "chown".  You better set
        'secure' at the end of your ~/.vimrc then.
        This option cannot be set from a modeline or in the sandbox, for
        security reasons.

Please can someone explain how wundo is connected to E523?

We see this error in the context of wundo being executed by govim which is a remote plugin if that is significant. The full error message looks like this:

Vim(wundo):E523: Not allowed here: wundo! /tmp/1579529963907093695532890104'' in function GOVIM_internal_BalloonExpr[3]..<SNR>22_callbackFunction[2]..<SNR>22_ch_evalexpr[15]..<SNR>22_drainScheduleBacklog[14]..<SNR>22_ch_evalexpr[10]..<SNR>22_define[40]..<SNR>22_callbackCommand[3]..<SNR>22_ch_evalexpr[10]..<SNR>22_define, line 40

To Reproduce

We don't have repro steps; this is a very random and somewhat rare occurrence.

Expected behavior

Unclear; not sure why we are seeing the error.

I guess it comes from do_one_cmd():

    if (text_locked() && !(ea.argt & EX_CMDWIN)
    && !IS_USER_CMDIDX(ea.cmdidx))
{
    // Command not allowed when editing the command line.
    errormsg = _(get_text_locked_msg());
    goto doend;
}

"textlock" is set in general_beval_cb(), perhaps that's where it gets triggered from? It evaluates 'balloonexpr' there.

Why do you use ":wundo" somewhere deep in the plugin? You do something and the reload the undo info to restore it? Well, writing undo info only works when not changing the text.

-- ARTHUR: Now stand aside worthy adversary. BLACK KNIGHT: (Glancing at his shoulder) 'Tis but a scratch. ARTHUR: A scratch? Your arm's off. "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

/// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\ \\ an exciting new programming language -- http://www.Zimbu.org /// \\ help me help AIDS victims -- http://ICCF-Holland.org ///

myitcv commented 4 years ago

Whilst I'm still perplexed by how we end up in that stack state, let me step back a second to make sure our use of wundo (and rundo) is correct here.

We use wundo just before we apply a set of changes from gopls, and match that with rundo afterwards.

This can happen when:

I suppose I should first ask, is there a better way to achieve what we're doing here?

brammool commented 4 years ago

Paul Jolly wrote:

Whilst I'm still perplexed by how we end up in that stack state, let me step back a second to make sure our use of wundo (and rundo) is correct here.

We use wundo just before we apply a set of changes from gopls, and match that with rundo afterwards.

This can happen when:

  • the user completes completion and there are changes required because of the candidate selected
  • formatting the current buffer
  • fixing imports in the current buffer

I suppose I should first ask, is there a better way to achieve what we're doing here?

What happens if you drop this "wundo"/"rundo" completely?

Perhaps using ":undojoin" would work better?

-- ARTHUR: What are you going to do. bleed on me? "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

/// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\ \\ an exciting new programming language -- http://www.Zimbu.org /// \\ help me help AIDS victims -- http://ICCF-Holland.org ///

myitcv commented 4 years ago

What happens if you drop this "wundo"/"rundo" completely?

We're using wundo and rundo because I couldn't see a way to make :undojoin play nicely with repeated function calls from govim. My understanding is that with :undojoin you have to prefix every call with that command, e.g. :undojoin call setbufline(...). Which means we can't use the call channel command. Unless I'm missing something?

That said, I've actually done a bit more digging on our side.

We're only seeing this fail against an older version of GVim, v8.1.1711. But I think the more interesting thing is understanding why we trigger balloonexpr at all.

All of these tests run in a headless environment, with gvim run via:

xvfb-run -a gvim -f

It seems that after we open a file, balloonexpr is triggered with the mouse at screen position {"col":40,"row":11,"endcol":40,"curscol":40}. Calling test_setmouse(1,1) before this doesn't appear to have any effect on the position at which balloonexpr is triggered, and passing -nocursor to xvfb-run causes GVim to exit hard (thought I'd try this for completeness).

Based on this I think we need to understand:

Bram, any thoughts?

brammool commented 4 years ago

What happens if you drop this "wundo"/"rundo" completely?

We're using wundo and rundo because I couldn't see a way to make :undojoin play nicely with repeated function calls from govim. My understanding is that with :undojoin you have to prefix every call with that command, e.g. :undojoin call setbufline(...). Which means we can't use the call channel command. Unless I'm missing something?

That said, I've actually done a bit more digging on our side.

We're only seeing this fail against an older version of GVim, v8.1.1711. But I think the more interesting thing is understanding why we trigger balloonexpr at all.

All of these tests run in a headless environment, with gvim run via:

xvfb-run -a gvim -f

It seems that after we open a file, balloonexpr is triggered with the mouse at screen position {"col":40,"row":11,"endcol":40,"curscol":40}. Calling test_setmouse(1,1) before this doesn't appear to have any effect on the position at which balloonexpr is triggered, and passing -nocursor to xvfb-run causes GVim to exit hard (thought I'd try this for completeness).

Based on this I think we need to understand:

  • why the balloonexpr is being automatically triggered

This is in the GUI? gui_beval.c has the logic to trigger this, which is when the mouse pointer has been in the same position for a while. Perhaps you can avoid it by setting 'balloondelay' to a large value. Or not enabling 'ballooneval' most of the time.

But I have no idea what event xvfb-run sends to Vim.

  • why test_setmouse doesn't appear to have any effect in GVim

I guess that is because in get_beval_info() it uses the coordinates of the BalloonEval struct, instead of mouse_row/mouse_col.

-- "Beware of bugs in the above code; I have only proved it correct, not tried it." -- Donald Knuth

/// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\ \\ an exciting new programming language -- http://www.Zimbu.org /// \\ help me help AIDS victims -- http://ICCF-Holland.org ///

myitcv commented 4 years ago

gui_beval.c has the logic to trigger this, which is when the mouse pointer has been in the same position for a while

This could well explain what we're seeing, thank you.

  • why test_setmouse doesn't appear to have any effect in GVim

I guess that is because in get_beval_info() it uses the coordinates of the BalloonEval struct, instead of mouse_row/mouse_col.

Ok. Actually test_setmouse must be working because we're using it in "hover" tests to ensure that we get the correct popup.

For now we have skipped this test with the older version of GVim. Not ideal but it doesn't seem worth too much effort right now on anyone's behalf

Thank you very much for filling in the gaps above, Bram. I'm going to close this issue because it's not something we are actively concerned about right now. We can always re-open is it rears its head again.