Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Fix #6441. #6442

Merged
merged 3 commits into from
Jan 15, 2014
Merged

Fix #6441. #6442

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0566d3b
Fix #6441.
busykai Jan 9, 2014
7421b59
Check for valid codeSnippet.
busykai Jan 9, 2014
0f0ee7f
Do not set display line number for invalid lines
busykai Jan 15, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/language/CodeInspection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,13 +305,14 @@ define(function (require, exports, module) {

if (inspectionResult.result) {
inspectionResult.result.errors.forEach(function (error) {
// some inspectors don't always provide a line number
if (!isNaN(error.pos.line)) {
// some inspectors don't always provide a line number or report a negative line number
if (!isNaN(error.pos.line) &&
(error.pos.line + 1) > 0 &&
(error.codeSnippet = currentDoc.getLine(error.pos.line)) !== undefined) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@busykai @ingorichter I had a concern that this silently hides bugs in extensions. I'd prefer it if we don't accept NaN and -1 as line numbers. Can we file a bug on whatever extension has this problem, and then remove this code once that extension is fixed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterflynn it's understandable, so what do you think the strategy should be for linters to report their internal issues? just log it to console? see this issue for example: cfjedimaster/brackets-jshint#24 with this change, the way the output looks likes is the following:
problem
which is much nicer than seeing "nothing to lint".

Now, I understand your point of view. I do think, however, that having the checks for valid input in the code does not actually contradict to the requirement to the extension to report things better. For example, this message "bad option" could have been reported as meta instead of error (on the other hand, having only meta would not cause problem panel to pop up).

So with this background what your suggestion would be for a long-term proper solution? Note that I also filed #6460 to as an idea to address this.

error.friendlyLine = error.pos.line + 1;
error.codeSnippet = currentDoc.getLine(error.pos.line);
error.codeSnippet = error.codeSnippet.substr(0, Math.min(175, error.codeSnippet.length)); // limit snippet width
}

}
if (error.type !== Type.META) {
numProblems++;
}
Expand Down