Skip to content
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

Contribute an indent test file upstream #383

Open
dkearns opened this issue Dec 11, 2018 · 7 comments
Open

Contribute an indent test file upstream #383

dkearns opened this issue Dec 11, 2018 · 7 comments
Labels

Comments

@dkearns
Copy link
Member

dkearns commented Dec 11, 2018

A test harness for indent files was recently added upstream. It might be worth contributing some tests.

https://github.com/vim/vim/blob/master/runtime/indent/testdir/README.txt

@dkearns dkearns added the indent label Dec 11, 2018
@tpope
Copy link
Member

tpope commented Dec 19, 2018

I like the idea of moving to a standard rather than our ad hoc RSpec setup (but not enough to do the work myself). Having the tests duplicated upstream doesn't seem super high value, but if we can get it for free why not?

@AndrewRadev
Copy link
Member

AndrewRadev commented Dec 19, 2018

I like the idea of moving to a standard, myself, but I wish this wasn't the standard. From what I can see from the instructions, all the examples need to be in one big file, and one for the output. Adding a new example is going to mean placing it in the right location in both places. Reading examples to figure out if something's already covered would mean reading the entire file from start to finish. There's no grouping of any sort, although I suppose we can add comments. Maybe I could figure something out with do blocks, or even just use rspec "syntax", given it wouldn't break indentation... I'll have to experiment.

The running of tests also leaves something to be desired. "Run make test in the root of the vim source tree" is not gonna work if vim-ruby is outside the vim source tree. (Not to mention I'd rather not run all vim tests every time.) We'd have to replicate the testing setup (as in, copy the relevant bits from the Makefile), and hope it doesn't change too much in the future. At least that's what I can think of right now. I'll need to look into the process in more detail.

There's also the output being a .fail file with just the whole thing indented. I'm not sure how "isolated" indenting is going to be, but if one thing fails, everything after it might be indented incorrectly as well. Maybe if I wrap every example in a do or specify, and the test runner only indents the area, it would be fine, because the indent script isn't going to parse above the end line. Another thing I'll need to check.

Still, it is supposed to be a standard, and given we actually have tests, we might as well. I don't exactly work on vim-ruby particularly often, so the pains of the setup might not be a huge deal. I plan to allocate some time to work on this over the winter holidays. I'm thinking of writing a bunch of regexes and vimscript to pull out the examples from the rspec files and compile this .in file. Depending on how reliable it turns out to be, I'm wondering if I could generate these files from the rspec and have a script that does it when @dkearns releases? I don't know, need to try it.

@tpope
Copy link
Member

tpope commented Dec 19, 2018

Given the use of INDENT_EXE, I think every example would have to be isolated.

I'd be inclined to take the opposite approach. Add a ruby.in and ruby.ok as master files, but stick with a Ruby runner that parses it into RSpec assertions. It's a very simple format so I would expect rolling a parser to be pretty trivial. If you have a build process that only runs at distribution time, each release is going to have a "debug what went wrong with the build" phase.

@AndrewRadev
Copy link
Member

# START_INDENT
foo.bar().
# END_INDENT

# START_INDENT
baz()
# END_INDENT

If the tests run = only on the bracketed lines, group by group, the second one would still be indented one shiftwidth extra. This is a very deliberately broken example, but what I mean is, a broken indent somewhere at the top could still mess up something down below, because looking for a "parent line" ignores comments (this might be avoidable if each example is bracketed in do-end, but hard to say for sure).

It could work if each individual example is pulled into its own buffer, indented and put back, but I doubt it's implemented that way. I would be happy to be wrong on this.

Given the use of INDENT_EXE, I think every example would have to be isolated.

Well, I don't know about "have to". Note the warning: "Note that the command is not undone, you may need to reverse the effect for the next block of lines." This makes sense, because it's a global variable. The only way to isolate the example in the sense of reverting these would be to spawn a new vim instance. Would have been useful to see an example for a buffer-local variable, and how that works out -- would have given a hint as to whether they use a fresh buffer. Well, either way, I'll read the code. But my bet would be on "search for every start/end, apply indent-exe lines, apply = on the range, keep going".

Generating rspec from the text files... it's an option, but it removes a lot of the benefits of writing tests in ruby. No organisation in separate files (might be something could be done with extra magic comments), debugging issues might lead to changing tests, which would be one indirection away. Contributors would have to be made aware that the rspec tests are auto-generated, which I'd personally find confusing.

There's also other stuff that can be tested with rspec that you couldn't with this. Vim-elixir use the same toolchain for syntax and folding tests as well. Not that I'd have the energy to write them, but my point is, ruby tests would always be strictly more powerful.

Anyway, my point is, adding generated rspec files doesn't feel like it would add much. At this moment, I'm leaning more towards going full-ruby.in, rather than generating other tests from those. Your "convert things at build time" point does make sense -- it'd be an annoying process to debug.

As a side thought, it might have been nice to have a "ruby.in" directory, rather than file, similar to what you could do with ftplugins. Might be something to propose to the core team, but I'd have to work out if it's feasible first.

@tpope
Copy link
Member

tpope commented Dec 22, 2018

If the tests run = only on the bracketed lines, group by group, the second one would still be indented one shiftwidth extra.

Oh yeah, didn't think this part through.

The only way to isolate the example in the sense of reverting these would be to spawn a new vim instance.

I was only trying to say that it wasn't doing gg=G, which I guess doesn't mean much.

I'm now inclined to think this isn't worth bothering with, but don't let that stop you.

@dkearns
Copy link
Member Author

dkearns commented Jan 20, 2019

Well, either way, I'll read the code. But my bet would be on "search for every start/end, apply indent-exe lines, apply = on the range, keep going".

I had a quick look and that appears to be exactly how it works.

@AndrewRadev
Copy link
Member

An update on my part: my bold plan to work on this failed. Ended up working on other stuff and generally procrastinating. I'm also semi-vacationing in warmer climates during the entirety of January, so it'll take me a while to get around to it again. I also think my conclusion is likely to match Tim's "not worth bothering with", but I do want to at least do a pass over it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants