MiguelCastillo / Brackets-InteractiveLinter

Interactive linting for Brackets
Other
119 stars 26 forks source link

Specifying rules in .eslintrc stops interactive linter working #129

Closed JakeGinnivan closed 9 years ago

JakeGinnivan commented 9 years ago

In .brackets.json we added "interactive-linter.javascript": ["eslint"]

Then our .eslintrc looks like this

{
  "env": {
    "browser": true,
    "es6":true,
    "mocha":true
  },
  "ecmaFeatures": {
    "modules": true,
    "jsx": true
  },
  "rules": {
    "semi": 0,
    "global-strict": 0,
    "quotes": [2, "single"],
    "key-spacing": 0,
    "no-trailing-spaces": 0,
    "eol-last": 0
  }
}

And we get nothing. If we remove the rules section things start working like we would expect. Any ideas why having custom rules breaks everything?

JakeGinnivan commented 9 years ago

Ok looking into the issue it seems eslint expects all options to have a value, and util.mixin is not recursive. As soon as I specify rules, the defaults are not mixed in.

MiguelCastillo commented 9 years ago

@JakeGinnivan thanks for logging this issue. So I tried using your .eslintrc and it worked just fine. I took this project https://github.com/MiguelCastillo/Brackets-Harmony. I added a .eslintrc with your settings and jsx and es6 worked as expected. I changed the rule about quotes and changed it to just verify that things were working, and I had no issues...

Do you think you can clonse the Brackets-Harmony project and test your .eslintrc file with it?

MiguelCastillo commented 9 years ago

@JakeGinnivan I did find an issue and fixed it here https://github.com/MiguelCastillo/Brackets-InteractiveLinter/pull/131. The issue is that warning were reported and errors and errors as warning... Had my logic mixed up.

JakeGinnivan commented 9 years ago

Strange. Basically when I debugged I saw https://github.com/MiguelCastillo/Brackets-InteractiveLinter/blob/master/plugins/default/eslint/main.js#L18

options.rules contain my small subset, then options.defaultRules contained all of the defaults. After that line options.rules still contained by subset.

I will have to wait until Tuesday to be back at work to see the exact setup and plugins.

JakeGinnivan commented 9 years ago

Oh, and results was null, so it looked like when you did not pass all options to eslint.verify(source, options) it returned null...

Because it failed on https://github.com/MiguelCastillo/Brackets-InteractiveLinter/blob/master/plugins/default/eslint/main.js#L28 with .length not existing on undefined

MiguelCastillo commented 9 years ago

@JakeGinnivan yeah that's totally an oversight! I have fixed it here https://github.com/MiguelCastillo/Brackets-InteractiveLinter/pull/133

As far as merging rules go... If you specify an entry in your local .eslintrc, then those will take precedence over what's defined in default.json. So if you specify rules in you .eslintrc, then whatever is specified in default.json will not be used.

Is this not working for you?

Also, if you don't mind using what's on github master, that would be awesome. I have added a few fixes and I will be releasing it soon. Hopefully you won't run into issues with what's on master.

JakeGinnivan commented 9 years ago

Will give it a go tomorrow. Thanks

MiguelCastillo commented 9 years ago

@JakeGinnivan awesome! thank you

raybooysen commented 9 years ago

@MiguelCastillo, I'm working with @JakeGinnivan and master is working much better, using just the subset of rules Jake references above.

MiguelCastillo commented 9 years ago

@raybooysen @JakeGinnivan Awesome, glad to hear. I will keep this issue open for a couple more days while you guys use it a bit more... We can close this issue then. Thanks for helping me test this.

MiguelCastillo commented 9 years ago

Hey guys, I am going to go ahead close this issue as resolved. We can reopen it, or log news ones accordingly. Thanks for helping debug this.

JakeGinnivan commented 9 years ago

Thanks for the fixes!