-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Display source with exceptions #107
base: master
Are you sure you want to change the base?
Conversation
This is good in general, but required with the WIP Vader PR, which wraps the tests in a function. Ref: junegunn/vader.vim#107
It also helps with custom assert messages, where you often might wonder what gets compared:
|
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. |
Fair point, and thanks for explaining. I will keep working on this branch slowly during the next days/weeks. |
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. |
@junegunn |
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, and to confirm that a pull request doesn't break anything. |
This is good in general, but required with the WIP Vader PR, which wraps the tests in a function. Ref: junegunn/vader.vim#107
@junegunn |
00445b0
to
616cee1
Compare
Hmm, I've never used |
616cee1
to
fa9548b
Compare
fa9548b
to
8b9a8e2
Compare
cfb83a3
to
f1b4559
Compare
f1b4559
to
beec226
Compare
Oh, I've never replied to this really. 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 |
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? |
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. |
E.g. ( 1/13) [EXECUTE] (X) Vim(finish):E168: :finish used outside of a sourced file > /home/user/src/neomake/tests/utils.vader:11: finish " foo Also passes it through to `s:execute`.
This will use the actual file name for included lines. Fixes junegunn#88.
Wrapping the code in a function will cause them to not be available otherwise, or g:SyntaxAt() would have to be used.
While this means more overhead in general, it allows to access script-local variables in the tests. squash! FEAT: move s:prepare into vader#window#execute Use `vader#log` directly instead of `:Log`, since the latter might not be defined (yet).
This not only avoids multiple calls to `tempname()`, but makes Vim use the same script ID also when not run in Docker. For some reason it uses a new script ID (`<SID>`/`<SNR>`) when run normally, but uses the same ID when run in Docker. This means that the `!` with `function! s:vader_wrapper()` was not required in an environment that behaves like running it in Docker (with Neomake's tests). This also removes the `delete()`, which is handled by Vim itself when shutting down.
Vim does not generate profiling information for dict functions when the dict is local-scoped, since it apparently gets garbage collected automatically. Works around vim/vim#2350.
This is relevant for when `Include` is used, and is the same as `g:vader_file` otherwise.
`v:throwpoint` is empty in the first case.
This reverts commit 809e10c.
This gets handled by Vim automatically, and works around making Docker use the same inode or something similar (vim/vim#3353).
This is supported with Vim patch v8.1.0362.
This might be good in general for something to end up in the window in any case, but is mainly done to allow for testing it. Should/could maybe only be done with Vader's own tests, or some monkey-patching instead.
6453672
to
f7e0094
Compare
This improves display of error messages.
E.g.
with
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:
Caveats:
return
instead offinish
to return from a block. There is a hack to change this.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.