tomtom / quickfixsigns_vim

Mark quickfix & location list items with signs
http://www.vim.org/scripts/script.php?script_id=2584
GNU General Public License v3.0
131 stars 13 forks source link

Use 'silent!' with 'redir' to handle nesting #62

Closed blueyed closed 8 years ago

blueyed commented 8 years ago

Nested 'redir' is not supported, and quickfixsigns would cause errors when using other plugins like ArgsAndMore or EnhancedJumps, which are using 'redir' themselves.

This is the suggested method by Ingo Karkat.

Ref: https://github.com/chrisbra/Recover.vim/blob/master/autoload/recover.vim#L14

Maybe this patch should / can be enhanced to try the function again on e.g. CursorHold, in case it failed e.g. during BufEnter etc?!

tomtom commented 8 years ago

I'm against using silent on redir since it might cause all sort of trailing errors.

Does it help to prefix the commands within the redir with :noautocmd?

blueyed commented 8 years ago

I'm against using silent on redir since it might cause all sort of trailing errors.

I agree in general, but this appears to be the only workaround in this case.

Does it help to prefix the commands within the redir with :noautocmd?

I've tried the noautocmd within ArgsAndMore before, but it caused the syntax highlighting to be broken.

Do you mean to use it from quickfixsign's side?

tomtom commented 8 years ago

Do you mean to use it from quickfixsign's side? Well yes, that's what I meant.

It would be be good to know which autocommands these plugins use to determine when these conflicts actually happen.

blueyed commented 8 years ago

Does it help to prefix the commands within the redir

The problem appears to be that quickfixsign's :redir is the inner one already.

tomtom commented 8 years ago

The problem appears to be that quickfixsign's |:redir| is the inner one already.

I got that. But when exactly does the problem occur? It could be possible to execute the :redir earlier or later -- assuming it actually is qfs's fault.

blueyed commented 8 years ago

The problem is that ArgsAndMore switches the buffer (and sets up a redir to get any messages). The buffer switching then triggers quickfixsigns.

From my email to @inkarkat:

with ArgsAndMore (http://www.vim.org/scripts/script.php?script_id=4152 - via the Github mirror at https://github.com/vim-scripts/ArgsAndMore) I am seeing the following strange error with

:CDoEntry s/foo/bar/gc

when it reaches the end of the first entry:

E121: Undefined variable: l:nextLocationOutput Error detected while processing function ArgsAndMore#Iteration#QuickfixDo: line 127: E121: Undefined variable: rv

I was only seeing the last one before (about "rv", which is not used in the plugin itself). After updating to the latest ingo-library (1.024) and the latest plugin, the error changed to the one above.

It seems to be a conflict with https://github.com/tomtom/quickfixsigns_vim, which also uses :redir. According to the help, redir commands cannot be nested, and that's triggering this.

A workaround might be to use 'noautocmd' in ArgsAndMore#Iteration#QuickfixDo, but that will break syntax highlighting with the 'cnext' command:

redir => l:nextLocationOutput silent execute 'keepalt' 'noautocmd' l:iterationCommand redir END

quickfixsigns' redirection function is this one (from https://github.com/tomtom/quickfixsigns_vim/blob/edaeed13118c3b0803112d800a0866f93f5d56a8/plugin/quickfixsigns.vim#L226-L238):

function! s:Redir(cmd) "{{{3 let verbose = &verbose let &verbose = 0 try let rv = '' redir => rv exec 'silent' a:cmd redir END return exists('rv')? rv : '' finally let &verbose = verbose endtry endf

The flow of commands is this:

execute keepalt cnext sign place buffer=3 E121: Undefined variable: l:nextLocationOutput Error detected while processing function ArgsAndMore#Iteration#QuickfixDo: line 129: E121: Undefined variable: rv

I've came across your message on vim-dev, where you've said you had a fix for another plugin already, which might be related: https://groups.google.com/d/msg/vim_dev/H3Z3ChSUh_4/RSXhKLTfh5gJ

Can you think of a good fix for this?

blueyed commented 8 years ago

It could be possible to execute the :redir earlier or later

Yeah, that would be the solution: try it (but with silent!) and do it later (e.g. on CursorHold) in case of error.

tomtom commented 8 years ago
E121: Undefined variable: l:nextLocationOutput
Error detected while processing function
ArgsAndMore#Iteration#QuickfixDo:
line 127:
E121: Undefined variable: rv

The error occurs in ArgsAndMore, doesn't it? It seems qfs cannot detect whether a :redir is active.

Example:

unlet! foo bar
redir => foo
     redir => bar
     silent echo 1
     redir END " bar
     silent echo 'bar '. bar
redir END " foo
echo 'bar '. string(bar)
echo 'foo '. string(foo)

At least with vim 7.4-960, foo is "" while bar is set. I.e. the redir itself doesn't fail but the variable isn't set properly. I don't see a way for qfs to determine whether there is an :redir pending. IMHO it's up to the plugins you mentioned to deal with these errors -- or maybe set a flag to tell other plugins that they shouldn't use :redir.

blueyed commented 8 years ago

Thanks for your insights, I look forward to @inkarkat to chime it.

FWIW, I consider the plugin I am actively using to be more important in this regard, and could live with e.g. signs not being up to date in this case, although it would be nice to have them updated later / as soon as possible.

tomtom commented 8 years ago

Since commit 7951d55 there is a variable g:quickfixsigns_events_default = ['BufEnter'], which you could set to something else in order to avoid this particular problem.

The better solution would be though if the ArgsAndMore plugin added BufEnter to eventsignore and if it called :doautoall BufEnter after the redir. But this would be just an ad hoc solution for this particular problem. Eventually :redir should be fixed or there should be a capture() function in VIM -- or maybe wait for neovim to take over.

inkarkat commented 8 years ago

The better solution would be though if the ArgsAndMore plugin added BufEnter to eventsignore and if it called :doautoall BufEnter after the redir.

Well, my plugin needs to capture any messages emitted while iterating through the buffers (to provide a summary). Normally, this works well, it's only when another plugin also uses :redir during this.

But this would be just an ad hoc solution for this particular problem. Eventually :redir should be fixed or there should be a capture() function in VIM -- or maybe wait for neovim to take over.

I fully agree; :redir is fundamentally broken. Right now, one of the conflicting plugins has to give up using :redir. Using :silent! redir (as suggested here) quite gracefully handles the conflict, assuming the plugin can live with not getting the information right now. (Which would work for quickfixsigns; initially, the signs wouldn't be updated, but later (on CursorHold or another buffer switch) it would correct itself.

Alternatively, I would have to add a configuration flag to turn off capturing; then, users of both plugins would keep up-to-date signs information, but would lose all reporting of ArgsAndMore (even if no signs are currently active). That looks like the worse deal to me...

blueyed commented 8 years ago

Thanks for fixing this, and keeping the habit of not using silent!.. :)