SublimeLinter / SublimeLinter-haml-lint

SublimeLinter 3 plugin for Haml, using haml-lint.
MIT License
4 stars 8 forks source link

Fix an issue with locating Rubocop config #3

Closed kamen-hursev closed 8 years ago

kamen-hursev commented 8 years ago

This PR fixes #2 in more or less the same way SublimeLinter-slim-lint is implemented

kamen-hursev commented 8 years ago

@jeroenj, if I fix this lint error D211: No blank lines allowed before class docstring, then I get another (that a blank line is required). Eventually I left this the way it was before.

Is there something not properly configured with the linter?

jeroenj commented 8 years ago

What happens if you add an extra newline above line 16 (class HamlLint(RubyLinter):)? In your PR you've removed one of the newlines. Maybe that's causing the linter to warn? SublimeLinter-contrib-haml-lint also has two newlines in there.

I can't test your changes but from what I can see in the code I'd expect them to work. :)

If adding the newline doesn't work I'll accept this PR as it is. But if I remember correctly SublimeLinter "requires" the linters to have no lint errors. (how meta :smile:)

kamen-hursev commented 8 years ago

Even if I add the new line I still get the same error. And the error is not (as far as I understand) about that new line, but about the line between class HamlLint(RubyLinter): and the docstring below it. But if the code is the way it is not I get the error you see in Travis. If I change the code like this:

class HamlLint(RubyLinter):
    """Provides an interface to haml-lint."""

I get:

projects/SublimeLinter-contrib-haml-lint$ pep257 . --ignore=D202
./linter.py:16 in public class `HamlLint`:
        D203: 1 blank line required before class docstring (found 0)

I'll add the new line that is missing, but the issue will still be there. These two D203 and D211 seem in conflict as described here

jeroenj commented 8 years ago

Funny. I'd say to ignore D203 too which seems advised. Could you add it to the Travis setup so that the build succeeds? I'll merge it afterwards.

Thanks a lot for the investigation! :+1:

kamen-hursev commented 8 years ago

OK, ignored D203, removed the blank line. The build is supposed to pass now (or at least it does locally :smile:)

jeroenj commented 8 years ago

Looks goot. Thanks! :smile:

LeEnno commented 8 years ago

Unfortunately this doesn't work for me.

Does this setting have to be a linter setting or a meta setting? In other words, which one is right?

{
    // A
    "linters": {
        "hamllint": {
            "rubocop-config": "/path/to/.rubocop.yml"
        }
    },
    // B
    "rubocop-config": "/path/to/.rubocop.yml"
}

I also see the printed path in the log (due to print(rubocop_config)), so I guess the transmission to the plugin works. The config path seems to get lost somewhere else. Maybe the environment variable can't be read by haml-lint in certain cases?

Any ideas?

kamen-hursev commented 8 years ago

You need to specify the configuration as you example A. However you need to specify just the filename (NOT the full path) and the config yml file needs to be somewhere in the parent directories of the linted file.

If all of the above doesn't help you might need to restart Sublime (unfortunately).

LeEnno commented 8 years ago

However you need to specify just the filename (NOT the full path) and the config yml file needs to be somewhere in the parent directories of the linted file.

I tried that, but unfortunately it didn't (and doesn't) work.

Current situation:

Style/HashSyntax:
  Enabled: false
%div{ :id => 'container' }

Any other ideas?

kamen-hursev commented 8 years ago

Unfortunately no. Even without the config value the linter looks for .rubocop.yml. Make sure you have up-to-date versions of rubocop, haml-lint and that running haml-lint from the console works fine.

If you enable debug mode for SublimeLinter user->debug. You should see something like: SublimeLinter: hamllint: file.html.haml ['/home/khursev/.rbenv/shims/ruby', '-S', 'haml-lint', '--config', '<path_to_haml_lint>/.haml-lint.yml'] this is the command SublimeLinter executes for file.html.haml

I don't have other ideas

LeEnno commented 8 years ago

Thanks for help.

This is the output I get:

SublimeLinter: hamllint version query: /Users/Enno/.rvm/bin/rvm-auto-ruby -S haml-lint --version 
SublimeLinter: hamllint version: 0.13.0 
SublimeLinter: hamllint: (>= 0.6.0) satisfied by 0.13.0 
SublimeLinter: hamllint activated: ['/Users/Enno/.rvm/bin/rvm-auto-ruby'] 
/Users/Enno/Sites/pictrs/master/pictrs/.rubocop.yml
SublimeLinter: hamllint: river.html.haml ['/Users/Enno/.rvm/bin/rvm-auto-ruby', '-S', 'haml-lint']

As you can see I use rvm instead of rbenv, but this shouldn't be the cause of the issue, I think. Also I'm missing the config path for .haml-lint.yml. Is this necessary for the .rubocop.yml to be recognized?

kamen-hursev commented 8 years ago

hamllint version: 0.13.0 the latest one is 0.16.0 is there a reason not to update?

Also try running this :

/Users/Enno/.rvm/bin/rvm-auto-ruby -S haml-lint /Users/Enno/Sites/pictrs/master/pictrs/app/views/adminbill/river.html.haml

to identify if the problem is in the linter or the Sublime plugin.

LeEnno commented 8 years ago

is there a reason not to update?

I think 0.13.0 is just the version the gemfile loaded with all of its dependency resolving magic.

Running your command from the terminal works fine and respect the .rubocop.yml.

kamen-hursev commented 8 years ago

haml-lint 0.13.0 doesn't seem to care about the ENV variable that in our case is needed to properly locate .rubocop.yml. They introduced the e

LeEnno commented 8 years ago

Ok, first off let me tell you that I love you very much right now ❤️

It turns out that there's a gem called haml-lint which is only maintained until version 0.13.0. The project which cares for further development and is at version 0.16.2 right now, is called haml_lint (underscore). This is quite confusing since the executable is still called haml-lint.

Once I installed the underscored version, it works totally fine. Thanks for pointing that out and bearing with me.

Nevertheless the documentation of this plugin could prevent others from running into this issue. Time for a p-p-pull request.