You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Sep 6, 2021. It is now read-only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Resolves#2688 - Currently jslint will push a null value onto JSLINT.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 uglier for loop version for higher compatibility.
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.
@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 : )
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
None yet
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.