search

SublimeLinter / SublimeLinter-rubocop

SublimeLinter 3 plugin for Ruby, using rubocop.
MIT License
159 stars 40 forks source link

Use the rubocop bin directly #65

Closed jeffbyrnes closed 5 years ago

jeffbyrnes commented 5 years ago

This avoids invoking ruby to then call rubocop, when the binary should be called directly instead.

kaste commented 5 years ago

Is this a replacement of my PR just shorter 😁 ?

Can you show via the debug output which command SL now actually tries. On Windows, it should just take whatever which rubocop returns, but on Mac/Linux probably not? Of course initial 😨 of a breaking change as well.

jeffbyrnes commented 5 years ago

@kaste hah! Not quite, though it does do a little bit of what your PR does.

On my Mac (macOS 10.14.5), here’s what it attempts to run:

cat $file_name | /usr/local/var/rbenv/shims/ruby /usr/local/var/rbenv/shims/rubocop --format emacs --force-exclusion --stdin $file

So it’s still hooking into rbenv, in my case, likely thanks to the RubyLinter class. Seems unnecessary, frankly, so perhaps this should go further…

jeffbyrnes commented 5 years ago

Modified the commit, and now rubocop is called like this:

cat $file_name | /usr/local/var/rbenv/shims/rubocop --format emacs --force-exclusion --stdin $file

which is decidedly better. Leaves rbenv to do its thing in my shell environment, and doesn’t try to be “smart” about running rubocop.

kaste commented 5 years ago

Yeah, cool. So in the end it is now a fixed version of my PR. In my PR I also don't inherit from the core RubyLinter but do the $file -> $file_path thing wrong.

jeffbyrnes commented 5 years ago

@kaste yeah, I think this has the same net effect as #55, but without messing about with RubyLinter.

I used this approach in SublimeLinter-contrib-cookstyle, and it works well!

kaste commented 5 years ago

I would like to take this one, and close #55. @braver?

braver commented 5 years ago

I don’t know what the Ruby linter ecosystem looks like, but if a linter can be run like this that’s definitely preferable. So this one is 👍🏻👍🏻👍🏻. If we still need 55 or even need a RubyLinter at all I can’t say, but I don’t disagree with your proposal.

kaste commented 5 years ago

I merge this now although per comment https://github.com/SublimeLinter/SublimeLinter-rubocop/pull/55#issuecomment-475775803 in my PR

  1. --stdin now properly works by specifying the folder instead of file.

You, @jeffbyrnes, said it should be $file, the other user thinks it should be $file_path Basically I 🤷‍♂ don't know, the rubocop users must figure this.

jeffbyrnes commented 5 years ago

@kaste left this comment over on #55, which hopefully clarifies what I determined in my own testing of rubocop --stdin.