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

Vint linting does not work on some machines #81

Closed
w0rp opened this issue Oct 10, 2016 · 30 comments
Closed

Vint linting does not work on some machines #81

w0rp opened this issue Oct 10, 2016 · 30 comments
Labels

Comments

@w0rp
Copy link
Member

w0rp commented Oct 10, 2016

On some machines, Vint linting does not work.

@w0rp w0rp added the bug label Oct 10, 2016
@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

@neersighted Could you paste the output of Vint on a file with some problems from your machine? I can check the lines against the regex, and see if there's some kind of issue there.

@neersighted
Copy link
Member

Sure... Something funky is going on, as some quick print-debugging is showing that HandleGCCFormat is not getting called. If it matters, my shell is fish.

Here's some vint output on my system:

plugin/unimpaired.vim:6:11: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:16:28: Use robust operators `==#` or `==?` instead of `==` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:16:44: Use robust operators `==#` or `==?` instead of `==` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:41:31: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:42:32: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:43:44: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:43:47: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:63:13: Use robust operators `==#` or `==?` instead of `==` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:298:10: Use robust operators `=~#` or `=~?` instead of `=~` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:413:13: Use robust operators `=~#` or `=~?` instead of `=~` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:415:17: Use robust operators `=~#` or `=~?` instead of `=~` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:416:16: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:416:40: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:417:17: Use robust operators `==#` or `==?` instead of `==` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:419:17: Use robust operators `==#` or `==?` instead of `==` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:422:16: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:433:13: Use robust operators `=~#` or `=~?` instead of `=~` (see Google VimScript Style Guide (Matching))

@neersighted
Copy link
Member

neersighted commented Oct 10, 2016

Here's the same thing with ale's formatting string:

plugin/unimpaired.vim:6:11: warning: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:16:28: warning: Use robust operators `==#` or `==?` instead of `==` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:16:44: warning: Use robust operators `==#` or `==?` instead of `==` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:41:31: warning: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:42:32: warning: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:43:44: warning: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:43:47: warning: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:63:13: warning: Use robust operators `==#` or `==?` instead of `==` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:298:10: warning: Use robust operators `=~#` or `=~?` instead of `=~` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:413:13: warning: Use robust operators `=~#` or `=~?` instead of `=~` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:415:17: warning: Use robust operators `=~#` or `=~?` instead of `=~` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:416:16: warning: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:416:40: warning: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:417:17: warning: Use robust operators `==#` or `==?` instead of `==` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:419:17: warning: Use robust operators `==#` or `==?` instead of `==` (see Google VimScript Style Guide (Matching))
plugin/unimpaired.vim:422:16: warning: Prefer single quoted strings (see Google VimScript Style Guide (Strings))
plugin/unimpaired.vim:433:13: warning: Use robust operators `=~#` or `=~?` instead of `=~` (see Google VimScript Style Guide (Matching))

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

Hmm, do you have bash installed on your machine? That might be the issue.

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

If the answer to that is "no," do you have a good old Bourne Shell (sh) installed?

@neersighted
Copy link
Member

That is indeed the issue, when I override SHELL to bash, it works.

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

Maybe the shebang line in the Bash wrapper script is to blame. What's the output of which bash?

@neersighted
Copy link
Member

/usr/bin/bash, however, this is arch, so /usr/bin is a symlink and the real bash is /bin/bash.

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

If you edit stdin-wrapper and change the shebang line to #!/usr/bin/env bash, does that work?

@neersighted
Copy link
Member

It does nothing, and it looks like stdin-wrapper never runs. I do believe the issue is in Vim, probably when it tries to execute stdin-wrapper. By the way, mktemp has a --suffix option, no need for gymnastics.

@neersighted
Copy link
Member

I'm guessing something with the generated command is going wrong.

This is what I get on my system:

/home/neersighted/.config/nvim/bundles/ale//stdin-wrapper .vim vint -w --no-color -f "{file_path}:{line_number}:{column_number}: {severity}: {description} (see {reference})

I'm wondering, why is there only one quote?

@neersighted
Copy link
Member

...the missing quote fixed it.

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

That quote is suspicious. @KabbAmine added that, and it was conspicuously only running if there was a single quote there. Which was... very odd.

@neersighted
Copy link
Member

It works for me in bash and in fish with both quotes. I can send a PR with the quote fix, shebang change and mktemp change if you'd like.

w0rp added a commit that referenced this issue Oct 10, 2016
@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

I pushed a commit now with just that fix for the quote.

@KabbAmine Do you want to test Vint linting again on your machine? You had the issue originally with the quote.

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

Ah, I recall now. Somehow... for some reason, the double quote appears in the echoed message if you add it. I'll see what I can do about that.

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

For now, it's dinner and X-Files time.

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

Check out the branch fix-vint-issues, with the commit referenced above. I don't know what's going on with the double quote issue, but I can avoid it, and it seems to work. Try it out and let me know how it goes.

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

I also noticed separately that vint has a -s flag which I could add an option for, and which could be applied in Travis for this repository. I'll make it happen after this bug has been fixed.

@neersighted
Copy link
Member

As a matter of fact, master works fine and fix-vint-issues does not.

@neersighted
Copy link
Member

I don't see quotes with fish, zsh, or bash on master.
fix-vint-issues only works on bash for me.

@neersighted
Copy link
Member

Why don't we just use escaped singlequotes for the whole thing? And I'd prefer to keep the vint parsing based on GCC, if possible. The fewer lines of code, the better.

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

The trouble is that somehow quotes can appear in the text. I get some lines like so: Unexpected EOL (see ynkdir/vim-vimlparser)"

I don't quite understand how that's happening.

@neersighted
Copy link
Member

Try logging everything to a file in your stdin-wrapper so you can look over the raw text? The layers of abstraction might be munging things.

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

I think I have found the issue. NeoVim does not show the issue on my machine. I looked at the documentation for jobstart() in NeoVim, and it said that it executes command strings as if you had passed the commands like split(&shell) + split(&shellcmdflag) + [command]. So I did exactly that for Vim 8. That seems to fix the issue on my machine.

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

Curiously, the job_start() documentation for Vim 8 says the command works better as a String on Windows and a List on Unix, so... I'll keep using a string on Windows.

@neersighted
Copy link
Member

Sweet! I can confirm master functions for me in fish and bash.

@neersighted
Copy link
Member

Lists are really the proper way to execute commands anyway, shell parsing makes things screwy when you use strings.

@w0rp
Copy link
Member Author

w0rp commented Oct 10, 2016

Yeah. I don't know why Vim's documentation says Windows is different, but I'll just accept that. I'll consider this issue closed.

@w0rp w0rp closed this as completed Oct 10, 2016
@KabbAmine
Copy link
Contributor

Good job guys 👍

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