-
Notifications
You must be signed in to change notification settings - Fork 141
Conversation
Move the generation of the debug string to a helper function as we will be using to also generate debug information when creating an issue.
Instead of throwing errors, trap the message and create a Linter message telling the user that something went wrong. Include a link to report the problem including all the debug information.
Move processing of messages to a helper function so the code isn't as complex in the main program.
The generation of the trace messages doesn't need to be in the main message processing.
The worker wasn't being passed into the call to `getDebugInfo()` properly.
Make the rule name in the generated issue show as code instead of raw text.
Looks like the changes to the debugging information command broke some of the specs, investigating. |
Correct the specs for the updated debug text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works well, and I love the re-organization you've done here. I had one question about the catch block, and some suggestions to add comments for future understandability.
Great work, and I think this lays some good groundwork for using this same pattern for other types of noisy errors as well.
// Here we always want the column to be a number | ||
msgCol = Math.max(0, column - 1) | ||
msgEndLine = endLine - 1 | ||
msgEndCol = endColumn - 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be good to include a comment about why we are subtracting 1 from a lot of these values, as it isn't immediately obvious.
} | ||
} | ||
|
||
export async function processMessages(response, textEditor, showRule, worker) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a pretty generic function name, it might be worthwhile to add a short description of what it does in a comment. Or better yet in a doc block, although we haven't been good about adding them up until now.
ret.fix = linterFix | ||
} | ||
} catch (err) { | ||
ret = await generateInvalidTrace( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you pretty sure that the only kinds of errors which will be caught will be from invalid ranges? If not, it would be good to check the err
and re-throw if it is not due to an invalid point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reminding me, I'd thought about that during development but forgot to actually implement it in the middle of getting this all working 😛.
Provide a note in the code as to why the points are all having 1 subtracted from them.
If the error doesn't look like one from rangeFromLineNumber, then just re-throw it.
@IanVS updated 😉. |
Gracefully handle invalid points reported by ESLint, transforming them into regular Linter messages. These messages include a link to allow easy reporting of an issue to this repository.
Example of messages in the in-development Linter UI:
And the generated issue:
Fixes #735.