tpope / vim-bundler

bundler.vim: Lightweight support for Ruby's Bundler
https://www.vim.org/scripts/script.php?script_id=4280
403 stars 29 forks source link

Extract QuickFixCmdPreMake and QuickFixCmdPostMake and return early for non-Bundler projects #24

Closed johnsyweb closed 11 years ago

johnsyweb commented 11 years ago

In issue #21, @sigvei highlighted that s:project() is evaluated on "line 0" of the autocommand even when not in a Bundler project. This throws and manifests as 'E488: Trailing characters'.

This call was changed in ff2a956 consistent with @bpinto's comment.

Introducing and calling s:QuickFixCmdPostMake() prevents this evaluation.

autocommand call s:QuickFixCmdPostMake()
line 0: call s:QuickFixCmdPostMake()
calling function <SNR>68_QuickFixCmdPostMake()
line 1:   if &makeprg =~# '^bundle' && exists('b:bundler_root')
line 2:     call s:pop_command()
line 3:     call s:project().paths('refresh')
line 4:   endif
function <SNR>68_QuickFixCmdPostMake returning #0
bpinto commented 11 years ago

This fix didn't work for me. I believe it has only fixed for you because you are working on a non-ruby (without bundler) project.

With this change applied, the error message has changed (as expected) to:

Error detected while processing function <SNR>37_QuickFixCmdPostMake:
line    3:
E488: Trailing characters
johnsyweb commented 11 years ago

Hmm... I still get this error in MacVim (version 7.3), but not in a console Vim (version 7.3.884).

I tried to make the check for a Bundler project more robust, but even that fails:

Executing QuickFixCmdPost Auto commands for "lmake"
autocommand call s:QuickFixCmdPostMake()
line 0: call s:QuickFixCmdPostMake()
calling function <SNR>69_QuickFixCmdPostMake()
line 1:   if s:IsBundlerProject()
calling function <SNR>69_QuickFixCmdPostMake..<SNR>69_IsBundlerProject()
line 1:   try
line 2:     return &makeprg =~# '^bundle' && exists('b:bundler_root') && !empty(
s:project())
:return 0 made pending
line 3:   catch /bundler: /
line 4:     return 0
line 5:   endtry
:return 0 resumed
function <SNR>69_QuickFixCmdPostMake..<SNR>69_IsBundlerProject returning #0
continuing in function <SNR>69_QuickFixCmdPostMake
line 2:     call s:pop_command()
line 3:     call s:project().paths('refresh')
Error detected while processing function <SNR>69_QuickFixCmdPostMake:
line    3:
E488: Trailing characters
line 4:   endif
function <SNR>69_QuickFixCmdPostMake returning #0
continuing in QuickFixCmdPost Auto commands for "lmake"

Why Vim continues when IsBundlerProject() returns 0 is beyond my ken.

Back to the drawing board!

johnsyweb commented 11 years ago

@bpinto The latest commit in https://github.com/johnsyweb/vim-bundler/compare/syntastic-harmony seems to fix it for me in MacVim, with Bundler and non-Bundler projects.

I'm not sufficiently satisfied with this fix to reopen the pull-request, though.

If you get a chance to test it in your setup, please do let me know the result.

bpinto commented 11 years ago

@johnsyweb It did work for me now! :)

johnsyweb commented 11 years ago

Thanks @bpinto. it seems, then my change is merely a workaround for a bug in Vim itself, which has been fixed between version 7.3 and patch 884.

I'll take a look at this after work to isolate the patch.

@tpope: should I reopen this pull-request for those without a patched Vim v7.3?

johnsyweb commented 11 years ago

For what it's worth, I hg bisected Vim to find out when this was fixed:

The first good revision is:
changeset:   2678:faf4b09c396e
tag:         v7-3-097
user:        Bram Moolenaar <bram@vim.org>
date:        Tue Jan 04 19:03:27 2011 +0100
summary:     updated for version 7.3.097

See https://code.google.com/p/vim/source/detail?r=faf4b09c396e# for the change. It looks very much like our problem.

bpinto commented 11 years ago

Can't/should we use a workaround?

johnsyweb commented 11 years ago

Now that I can see why my change works, I'm happy to re-open this pull request, with a slightly re-written history. @tpope can decide whether to include the workaround or whether to include a check like:

if v:version < 703 || v:version == 703 && !has("patch97")
    echoerr "Upgrade your Vim!"
endif
tpope commented 11 years ago

Thanks for getting to the root of it, gang.

@johnsyweb does that second commit fix it for all versions? I don't really want to force people to upgrade over something so petty.

johnsyweb commented 11 years ago

@tpope: You're welcome. Thank you for these productivity-boosting plugins!

I agree that forcing an upgrade seems a little harsh for such a trivial change.

While I cannot guarantee "all" versions, I've successfully tested this pull-request against Vim version 7.3 with and without patch 97.

Thanks again.

bpinto commented 11 years ago

Confirmed it's working here also.