-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Resolves #2688 - Incorrect JSLINT status bar count #2690
Resolves #2688 - Incorrect JSLINT status bar count #2690
Conversation
Array.prototype.filter should be fine. According to MDN, that's supported by IE9+ (the same browsers that support Array.forEach, which we use all over the place). I don't think we'll support less than IE9 anyhow, but beyond that Array.prototype.filter can be easily added via a shim. |
Yeah good point, I'm just always suspicious of those non-backwards compatible functions. I think this way is significantly more readable and semantically rich though. |
Feel free to ignore this, but I think an even cooler option would be to change the status bar to say "3+ errors" or "> 3 errors" or something like that -- since the null seems to indicate JSLint giving up prematurely without scanning the entire file. |
Waiting for another fix based on comment from @peterflynn. See issue #2688 for a discussion about this. |
@redmunds please review. Particularly: should it be a '+' suffix or a '>' prefix, and should it be in the case brought up in the issue where there is one error and one stop notice '2+' errors, or '1+' errors. |
The stop notice should not be counted, so when there is a null entry, you should subtract 1 from the total. Also, when JSLint stops scanning, there could be more errors, but maybe not. A prefix of ">=" would be more accurate, but I'm not sure it really adds anything. You could also conditionally add "(or more)", but that would require an additional localizable string. I'm OK with the "+" suffix. |
If there is a stop notice then JSLINT.errors looks like: [..., an error, an error, stop notice, null]. This means there are JSLint.errors less nulls subtract one many actual errors to count.
Yeah I thought as much, but I did note that the original issue considered displaying '3' as an off by one error which would imply that walesmd thinks a stop notice should count as an error as well. I'm away from my dev laptop so I made the change through the new github editor and linted it with jslint.com, so I haven't been able to test but it's literally subtracting one from a number so hopefully I can manage that : ) |
Looks good. Merging. |
Resolves #2688 - Incorrect JSLINT status bar count
Resolves #2688 - Currently jslint will push a
null
value ontoJSLINT.errors
at line 6597. This means an accurate status bar needs to only count non-null jslint warnings.This would be more elegant by using
Array.prototype.filter
, but as this is only implemented in JS versions 1.6 onwards I'll prefer the uglierfor
loop version for higher compatibility.