Transition package to new linter provider API #58
Conversation
|
Nice! Thanks @proman21 |
|
OK just found the reason I was getting exit code 127 for my env. I'm using rvm, and since Atom launched from icon doesn't inherit env, it would fail. I've tried with a system install and it seems to be good. However, I've now run into the problem that scss-lint return with exit code 66 and outputs For any file I throw at it. I'm not skilled enough to introspect the spawned process to make sure the argument is being passed properly, but I cannot get passed the error. |
| filePath = editor.getPath() | ||
| resultJson = [] | ||
| config = findFile path.dirname(filePath), '.scss-lint.yml' | ||
| process = new BufferedProcess |
There was a problem hiding this comment.
How about you use the helper modules that we've packaged as atom-linter in npm.
It has a very nice execution API that also supports executing node bins using atom as a node binary.
|
Also, make sure to remove the old outdated files |
|
Thanks steelbrain. I'm not really familiar with the linter and Atom API. Could you post some links to some stuff to help? Thanks if you can. |
|
This is the Migration Guide and this is the full API guide. and here's a list of linters transitioned to the new API, their source code will trigger new ideas for you. Or if you want a simple one see linter-php and linter-jshint for a node version. |
|
Having just written AtomLinter/linter-rubocop#54, it appears that |
|
@caseywebdev we have worked around the writing to file by writing to stdin of the linters, You can see that linter-jshint. It's more efficient and faster. |
|
That makes sense when the linter accepts stdin as input, but AFAIK |
|
Just applied some of the refactoring suggested by @steelbrain, also got config and additional params to work, even if empty. |
| filePath: filePath, | ||
| range: [[line, col], [line, col + (msg.length || 0)]] | ||
| else return [] | ||
| .catch (error) => |
There was a problem hiding this comment.
You don't really need to catch it, linter does it itself IIRC
|
Nice work so far 👏 |
|
Also, congratulations on your first PR 🎉 |
|
Thanks for the help @steelbrain. should be all done now |
|
Looks good to me, will merge after confirming that it works. |
|
Might be a good idea to include some of this help in the linter API wiki. Wiki doesn't explain things like the promise rejection notifications or the existence of the helper methods. |
| text: (msg.reason || 'Unknown Error') + | ||
| (if msg.linter then " (#{msg.linter})" else ''), | ||
| filePath: filePath, | ||
| range: [[line, col], [line, col + (msg.length || 0)]] |
There was a problem hiding this comment.
You can just do +1 instead of doing msg.length.
As the length of the error message has nothing to do with the buffer itself.
There was a problem hiding this comment.
The length field isn't the length of the lint message, its the length of the warning/error in the file.
|
I agree with you, that's why we've put the documentation in a wiki instead of a repo, anyone can edit/correct/expand it :) |
|
I just tested it and it works, now here's some last recommendations
|
Transition package to new linter provider API
|
If I'm reading this correctly |
|
@caseywebdev Disabling |
|
Alright |
Hi
I've done some work converting this package to the new Linter API, just using the examples as reference and trying to import as much functionality from the old version.
One problem I'm having is that the process always exits with status code 127 for me, which is the sh default for 'executable not found', even though the executable exists in the path I provide in config.
Any ideas would be appreciated.
This change is