Rewrite #11
Rewrite #11
Conversation
| fse = require 'fs-extra' | ||
| path = require 'path' | ||
| temp = require 'temp' | ||
| XRegExp = require('xregexp').XRegExp |
There was a problem hiding this comment.
Please use the helper module for parsing instead of using xregexp directly
There was a problem hiding this comment.
Wow, you're fast. :)
My biggest reason for not using the helper module for parsing is that is not able to take into account indenting and ends up underlining all the leading whitespace. My parsing uses the textEditor object to get the indentation level so it can be taken into account (a trick I picked up from linter-coffeelint). Is there some way to have the best of both worlds?
There was a problem hiding this comment.
We also have a rangeFromLineNumber method here https://github.com/AtomLinter/atom-linter/blob/master/lib/helpers.coffee#L46
There was a problem hiding this comment.
Looks like that was added after I wrote this (and will require a version bump in the dependencies). I will work on refactoring this to use the helper module.
|
I see that you have implemented your own method of temp and copy, I would suggest you wait a bit until we add this to the helper module, see steelbrain/atom-linter#30 |
|
Given that Linter has removed the legacy API and this package is currently broken, I feel a bit of haste to get a working version published. I have subscribed to that issue and can incorporate it in a future version if there is not a pressing need for that to block release. |
|
My vote would be to go with a working version ASAP to get people able to use this again, with a better/cleaner version pushed out when that is ready. |
|
I agree. Something imperfect but working is far better than something that is broken. I think the best course of action is to publish this as-is and create new issues for the proposed improvements. |
|
You guys have my vote! 👍 |
Closes #2.
Closes #7.
Closes #8.
Closes #9.