Display source with exceptions

[blueyed]

This improves display of error messages.

E.g.

    ( 4/13) [EXECUTE] (X) Assertion failure
      > in <SNR>49_Bar[2]
      >   2: Assert a
      > in <SNR>49_Foo[1]
      >   1: call s:Bar()
      > 8: call s:Foo()

with

Execute (Foo):
  function! s:Bar()
    let a = 0
    Assert a
  endfunction
  function! s:Foo()
    call s:Bar()
  endfunction
  call s:Foo()

It would be nice to get the actual source file and location, e.g. tests/utils.vader:13, but that would involve more refactoring to pass through the file and location of blocks.

Or:

    ( 5/13) [EXECUTE] (X) Vim(return):E121: Undefined variable: g:maker
      > in GetSetting, line 1
      >   1: return neomake#utils#GetSetting('args', g:maker, 'default', ['myft'], bufnr('%'))
      > 16: AssertEqual GetSetting(), 'string'

Caveats:

• it wraps the block in a function to get the location of exception that Vader throws itself. Therefore you have to use return instead of finish to return from a block. There is a hack to change this.
• you need to use g: prefixes now to access variables/functions defined in Before etc. This was necessary before in some cases already, but it really required now.

What do you think?

TODO:

• get script ID handling straight inside and outside of Docker (Same SID used for different script files in Docker vim/vim#3353)

• tests are broken because s:console_buffered is not filled anymore.

[blueyed]

It also helps with custom assert messages, where you often might wonder what gets compared:

    (1/1) [EXECUTE] (X) a
      > /home/user/foo/tests/processing.vader:19: AssertEqual len(g:neomake_test_countschanged), 1, "a"

[junegunn]

Hi, thanks for your continuous effort on improving Vader. To be honest, I'm not highly motivated to improve Vader any further. I'm pretty much settled with my current Vim configuration and I'm not actively developing a new Vim plugin as of now. So I don't really have much reason to actively seek for improvements in Vader. I can review and merge small changes, but if a patch requires non-trivial time and effort to review, I may not respond in timely fashion as my time is limited, and I may turn out to be very conservative in making changes. Please understand that, and also consider maintaining and using your own branch of changes, if you find yourself wanting new features and improvements. I won't be making any changes so we won't diverge too much.

[blueyed]

Fair point, and thanks for explaining.

I will keep working on this branch slowly during the next days/weeks.
While it would be OK to keep it in my fork, I certainly think that these improvements really help all users of Vader.

[blueyed]

What about creating an org for it, or add me as a contributor?

I've created https://github.com/Vimjas a while ago, which could be for Vim what https://github.com/jazzband/ / https://github.com/jazzband/ is for Vim: letting Vim jinjas maintain plugins of (partial) common interest.
(There is also https://github.com/vimmer-net created by @mhinz, but I like my name better.. ;))

[blueyed]

@junegunn
I guess your use case is more running Vader from Vim while developing, right?

[junegunn]

What about creating an org for it, or add me as a contributor?

I'm fine with either way. I'm okay with minor enhancements and usability improvements, but I'm not looking for more features as I'd like to keep Vader simple and small, or radical changes that might break backward compatibility. If you agree with my direction, I'll add you as a contributor to the repository. But if you don't, and would like to actively experiment with new ideas and stuff, I'd suggest that it should be done on your fork. Whether the fork is under your username, or some organization, the decision is up to you. That's totally fine for me. I probably won't be involved in the development of the fork.

I guess your use case is more running Vader from Vim while developing, right?

I guess, and to confirm that a pull request doesn't break anything.

[blueyed]

@junegunn
Any idea/input about the finish vs. return issue?
Currently you have to use finish in your tests, but when wrapped into a function (as with this PR), it needs to be return.
Not sure how many tests are using this, but can it be made compatible in some way?

[junegunn]

Hmm, I've never used finish in my test cases. Not sure how many users are relying on the behavior, but it was not documented anyway.

[blueyed]

@junegunn

I'm okay with minor enhancements and usability improvements, but I'm not looking for more features as I'd like to keep Vader simple and small, or radical changes that might break backward compatibility. If you agree with my direction, I'll add you as a contributor to the repository.

Oh, I've never replied to this really.
I would be happy to become a contributor, and agree with your direction.
I wouldn't merge anything without your ACK though anyway, so it's not really necessary to have write access, of course.

Regarding this PR I consider it to be a huge usability improvement.

The other big thing is #124, but that more like covering Vader's own tests, and maybe refactoring it in the future to make this (tests in Docker / using tmux) easily available for Vader users, e.g. by having contrib scripts.

[junegunn]

It sure seems to be a nice improvement, no argue with that. It's just that the patch is not trivial, and I haven't had the mental bandwidth to review the change, probably more so because I'm not actively writing new Vim plugins that I don't need more than what is currently available.

I'll try to look into it when I get some time. Hmm, or maybe I should just add you as a contributor and leave this all up to you, given that you won't break any backward compatibility?

[blueyed]

Hmm, or maybe I should just add you as a contributor and leave this all up to you, given that you won't break any backward compatibility?

Sure. I can understand if you have no time to review it, and I could merge (and maintain it afterwards!) myself.

I am using it myself since a while, and will now go ahead and use it with Vader by default already.
We can wait some more before merging this.