Added option to configure scopes to lint #374
Conversation
|
Sorry this didn't get any attention sooner, there currently isn't a maintainer for this provider and I apparently missed the notification! |
| @@ -33,11 +41,19 @@ module.exports = { | |||
| activate() { | |||
| require('atom-package-deps').install('linter-jshint'); | |||
|
|
|||
| this.scopes = ['source.js', 'source.js-semantic']; | |||
| this.scopes = []; | |||
There was a problem hiding this comment.
No need for this line (or the extra two blank lines), observing the value below will assign an initial value.
There was a problem hiding this comment.
Actually, now that the array reference is being properly kept, there is need of this line due to the ordering of how it is set.
lib/main.js
Outdated
| this.subscriptions = new CompositeDisposable(); | ||
|
|
||
| this.subscriptions.add(atom.config.observe('linter-jshint.scopes', (value) => { | ||
| this.scopes = value; |
There was a problem hiding this comment.
You must modify the array in place, you can't re-assign it. linter will be fed a reference from the initial call to provideLinter and will only ever check that first reference. Re-assigning the array will cause future changes to be ignored.
You can see a simple way of handling this here.
|
No response in 3 days and I'd like to get this out, so I'm going to try to address the comments in this PR instead of closing it and redoing the work so you can at least get some credit 😉. |
There will only ever be one reference to the scope array used, so it can't be simply replaced when updated. Change to clearing it's current contents and adding all the new contents while keeping the same array. Also ensures that the embedded scope is properly kept in the array if enabled.
|
Thanks for getting the ball rolling on this! Will be out in the next release. |
As discused at #372, added a option to define the scopes that JSHInt runs on.
Fixes #372.
Fixes #310.