[vim/vim] :cdo does not abort on error (and the test for it seems wrong) (#5102)
D. Ben Knoble
Describe the bug
:cdo is expected to stop when it encounters an error. As far as I (and others) can tell, it does not.
Additionally, the test for this seems, well, broken:
" If the executed command fails, then the operation should be aborted
enew!
let subst_count = 0
exe "silent!" . Xdo . " s/Line/xLine/ | let subst_count += 1"
if subst_count != 1 || getline('.') != 'xLine1'
call add(v:errors, 'Abort command on error test failed')
endif
I cannot see how the given :cdo command is expected to fail performing the substitute; further, the test actually wants it to succeed, and only "fails" if the substitute is not done! This to me is not "abort on failure" but rather "succeeds at this particular substitute operation."
To Reproduce
Detailed steps to reproduce the behavior:
- Run
vim --clean(orgvim --clean, etc.) :help :cdo:vimgrep /cdo/ %:cdo norm! cwtest:messagesto see far more than just one error; if you check:cc, you'll see we are not stuck on the first location, but have traversed the entire list.
Expected behavior
:cdo aborts on a failure and stops traversing the list.
Environment (please complete the following information):
- Vim version (below)
- OS: macOS 10.14.5
- Terminal: Terminal.app combined with tmux
VIM - Vi IMproved 8.1 (2018 May 18, compilé Aug 16 2019 02:18:16)
Version macOS
Rustines incluses : 1-1850
Compilé par Homebrew
Énorme version sans interface graphique.
Fonctionnalités incluses (+) ou non (-) :
+acl -farsi -mouse_sysmouse -tag_any_white
+arabic +file_in_path +mouse_urxvt -tcl
+autocmd +find_in_path +mouse_xterm +termguicolors
+autochdir +float +multi_byte +terminal
-autoservername +folding +multi_lang +terminfo
-balloon_eval -footer -mzscheme +termresponse
+balloon_eval_term +fork() +netbeans_intg +textobjects
-browse +gettext +num64 +textprop
++builtin_terms -hangul_input +packages +timers
+byte_offset +iconv +path_extra +title
+channel +insert_expand +perl -toolbar
+cindent +job +persistent_undo +user_commands
-clientserver +jumplist +postscript +vartabs
+clipboard +keymap +printer +vertsplit
+cmdline_compl +lambda +profile +virtualedit
+cmdline_hist +langmap -python +visual
+cmdline_info +libcall +python3 +visualextra
+comments +linebreak +quickfix +viminfo
+conceal +lispindent +reltime +vreplace
+cryptv +listcmds +rightleft +wildignore
+cscope +localmap +ruby +wildmenu
+cursorbind +lua +scrollbind +windows
+cursorshape +menu +signs +writebackup
+dialog_con +mksession +smartindent -X11
+diff +modify_fname -sound -xfontset
+digraphs +mouse +spell -xim
-dnd -mouseshape +startuptime -xpm
-ebcdic +mouse_dec +statusline -xsmp
+emacs_tags -mouse_gpm -sun_workshop -xterm_clipboard
+eval -mouse_jsbterm +syntax -xterm_save
+ex_extra +mouse_netterm +tag_binary
+extra_search +mouse_sgr -tag_old_static
fichier vimrc système : "$VIM/vimrc"
fichier vimrc utilisateur : "$HOME/.vimrc"
2me fichier vimrc utilisateur : "~/.vim/vimrc"
fichier exrc utilisateur : "$HOME/.exrc"
fichier de valeurs par défaut : "$VIMRUNTIME/defaults.vim"
$VIM par défaut : "/usr/local/share/vim"
Compilation : clang -c -I. -Iproto -DHAVE_CONFIG_H -DMACOS_X -DMACOS_X_DARWIN -g -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
Édition de liens : clang -L. -fstack-protector-strong -L/usr/local/lib -L/usr/local/opt/libyaml/lib -L/usr/local/opt/openssl/lib -L/usr/local/opt/readline/lib -L/usr/local/lib -o vim -lncurses -liconv -lintl -framework AppKit -L/usr/local/opt/lua/lib -llua5.3 -mmacosx-version-min=10.14 -fstack-protector-strong -L/usr/local/lib -L/usr/local/Cellar/perl/5.30.0/lib/perl5/5.30.0/darwin-thread-multi-2level/CORE -lperl -lm -lutil -lc -L/usr/local/opt/python/Frameworks/Python.framework/Versions/3.7/lib/python3.7/config-3.7m-darwin -lpython3.7m -framework CoreFoundation -lruby.2.6
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Yegappan Lakshmanan
The :cdo command stops when there is an error in switching to the next buffer after making
changes to the current buffer. In this it behaves similar to the :bufdo, :windo, tabdo, argdo,
and :cfdo commands. The difference is that those commands run once for each buffer in the
list. But the :cdo command runs for each entry in the list (multiple times for the same buffer).
The above referenced test behaves as expected. The :cdo command fails because the buffer is
not saved after making changes to the buffer. So the command is not able to edit the
next buffer in the list.
In the case of help buffers, as they are readonly, the command is able to switch to the
next buffer, even though it is not able to make the changes. So the command runs
through all the entries.
D. Ben Knoble
@yegappan thanks for the comparison to the other do commands, for the explanation of the test, and for demonstrating that that definition of :cdo explains basically all the behavior.
It is still unexpected behavior, and here is why:
:help :cdo
< When the current file can't be |abandon|ed and the [!]
is not present, the command fails.
When an error is detected execution stops.
The last buffer (or where an error occurred) becomes
the current buffer.
I see where you might have a point re: abandoning buffers. But the help says: "When an error is detected execution stops." Does this mean an error in :cdo (like the aforementioned abandon failure)? Or does this mean an error in {cmd} given to :cdo? Most users (myself included) seem to believe it's the latter—are you clarifying it is the former? Either way, we must update the help to be unambiguous (and it may be more useful to use the latter as actual behavior, given that :cdo operates at each entry, like :global (which, in fact, stops at the first error)).
Yegappan Lakshmanan
@yegappan thanks for the comparison to the other
docommands, for the explanation of the test, and for demonstrating that that definition of:cdoexplains basically all the behavior.It is still unexpected behavior, and here is why:
:help :cdo < When the current file can't be |abandon|ed and the [!] is not present, the command fails. When an error is detected execution stops. The last buffer (or where an error occurred) becomes the current buffer.I see where you might have a point re: abandoning buffers. But the help says: "When an error is detected execution stops." Does this mean an error in
:cdo(like the aforementioned abandon failure)? Or does this mean an error in{cmd}given to:cdo? Most users (myself included) seem to believe it's the latter—are you clarifying it is the former? Either way, we must update the help to be unambiguous (and it may be more useful to use the latter as actual behavior, given that:cdooperates at each entry, like:global(which, in fact, stops at the first error)).
dedowsdi
@yegappan Thanks for the explanation.
IIUC, cdo stops execution if and only if last entry in a buffer that make changes cause an error.
In another word, if an error happens, the current buffer after cdo might not be the error buffer, the only way to locate the entry that cause an error after cdo is to go through the message ?
Ralf Schandl
> Hi,
>
> On Mon, Oct 21, 2019 at 12:12 PM D. Ben Knoble
> Hi,
>
> Can you try the attached patch?
>
> Thanks,
> Yegappan
>
thank you for providing a patch.
It seems the patch didn't make it to GitHub. I pinged Ben & Dedowsdi via
stackexchange. Hope they see it.
I tested your patch in three ways:
1) :cdo s/TEST/HELLO/
This will always fail, as there is no string "TEST" in any line. This
error was ignored and :cdo worked on all lines.
Is that intended?
2) Calling a function from :cdo
I build a qf list by :vimgrep for the word "dog". The function should
replace "dog" with "cat". On the third invocation it throws an exception.
2.1) :cdo call Test_cdo()
The first two entries were replaced and on the third entry a exception
was thrown. Processing stops immediately. All other "dog"s were unchanged.
2.2) :cdo silent call Test_cdo()
Acts like 2.1
2.3) :cdo silent! call Test_cdo()
Ignores the exception and works on all entries of the qf list.
3) :cdo echo x
The variable x is not defined. This was detected and the command was
executed for the first entry of the qf list.
With 'silent!' it processes all entries from the list.
I'm not sure if the behavior in the first test case is intended. In the
other two cases it worked like I expected.
Regards,
Ralf
Bram Moolenaar
> **Describe the bug**
>
> `:cdo` is expected to stop when it encounters an error. As far as I ([and others](https://vi.stackexchange.com/q/21590/10604)) can tell, it does not.
>
> Additionally, the test for this seems, well, broken:
>
> ```
> " If the executed command fails, then the operation should be aborted
> enew!
> let subst_count = 0
> exe "silent!" . Xdo . " s/Line/xLine/ | let subst_count += 1"
> if subst_count != 1 || getline('.') != 'xLine1'
> call add(v:errors, 'Abort command on error test failed')
> endif
> ```
>
> I cannot see how the given `:cdo` command is expected to fail performing the substitute; further, the test actually wants it to succeed, and only "fails" if the substitute is not done! This to me is not "abort on failure" but rather "succeeds at this particular substitute operation."
>
> Detailed steps to reproduce the behavior:
> 1. `:help :cdo`
> 1. `:vimgrep /cdo/ %`
> 1. `:cdo norm! cwtest`
> 1. `:messages` to see far more than just one error; if you check `:cc`, you'll see we are not stuck on the first location, but have traversed the entire list.
>
> **Expected behavior**
>
> `:cdo` aborts on a failure and stops traversing the list.
I would say this works as intended, since it's the same as with other
commands that loop over a list. But the documentation should say:
When going to the next entry fails execution stops.
This is so it doesn't get stuck in one position and loop forever.
If you use ":argdo s/xxx/yyy" you get as many failures as you have
arguments.
--
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 -- Br...@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 ///
D. Ben Knoble
@brammool I would appreciate a correction to the documentation.
That said, I understand why argdo, bufdo, cfdo, &c. stop on failures in a single buffer (file).
But cdo is far more like global—shouldn't it stop on the first entry that errors? (This probably all applies to ldo too, but I imagine they share some code in the implementation, so I'm ignoring that for now.)
D. Ben Knoble
@yegappan The patch posted to the mailing list gives global-like behavior (stopping at the first error).
Yegappan Lakshmanan
Thanks for testing the changes. See below.
On Tue, Oct 22, 2019 at 12:01 PM Ralf Schandl <ralf.s...@gmx.de> wrote:
>
> On 21.10.19 21:57, Yegappan Lakshmanan wrote:
> >
> > On Mon, Oct 21, 2019 at 12:12 PM D. Ben Knoble
> > Hi,
> >
> > Can you try the attached patch?
> >
> > Thanks,
> > Yegappan
> >
>
>
> thank you for providing a patch.
>
> It seems the patch didn't make it to GitHub. I pinged Ben & Dedowsdi via
> stackexchange. Hope they see it.
>
> I tested your patch in three ways:
>
> 1) :cdo s/TEST/HELLO/
>
> This will always fail, as there is no string "TEST" in any line. This
> error was ignored and :cdo worked on all lines.
>
> Is that intended?
>
to reproduce this behavior.
Thanks,
Yegappan