New: max-len: ignoreStrings+ignoreTemplateLiterals (fixes #5805)
#7049
Conversation
|
@ljharb, thanks for your PR! By analyzing the annotation information on this pull request, we identified @vitorbal, @nzakas and @kaicataldo to be potential reviewers |
|
LGTM |
| @@ -185,6 +185,26 @@ SourceCode.prototype = { | |||
| }, | |||
|
|
|||
| /** | |||
| * Retrieves an array containing all strings (" or ') in the source code. | |||
There was a problem hiding this comment.
This object is part of the public API, and I'd rather not expose these methods. Can you move these into local functions in the rule?
There was a problem hiding this comment.
Sure, I'd figured it was cleaner to expose it here, but I'll move them in.
ljharb
force-pushed
the
ljharb/max_len_ignore_strings
branch
from
b09d7a5
to
5b0b368
Compare
|
LGTM |
| @@ -23,6 +23,8 @@ This rule has a number or object option: | |||
| * `"ignoreComments": true` ignores all trailing comments and comments on their own line | |||
| * `"ignoreTrailingComments": true` ignores only trailing comments | |||
| * `"ignoreUrls": true` ignores lines that contain a URL | |||
| * `"ignoreStrings": true` ignores lines that contain a string (" or ') | |||
There was a problem hiding this comment.
Maybe "contain a double-quoted or single-quoted string"?
There was a problem hiding this comment.
sure, sounds good
|
Raised a question about the use of |
ljharb
force-pushed
the
ljharb/max_len_ignore_strings
branch
from
5b0b368
to
6f65ba8
Compare
|
LGTM |
|
Thanks! Just one more thing (and I only thought of this just now, sorry): Can we test with a line continuation somehow? Since the code in each test case is itself a string, I think that would look something like this: "var str = \"a long string\\\nwith continuation\"";
// -----------------------^ Line continuation backslash
// -------------------------^ Literal newline within the stringThanks a lot! |
|
@platinumazure done |
ljharb
force-pushed
the
ljharb/max_len_ignore_strings
branch
from
6f65ba8
to
91df35f
Compare
|
LGTM |
|
Thanks, LGTM but would like another set of eyes on this. |
ljharb
force-pushed
the
ljharb/max_len_ignore_strings
branch
from
91df35f
to
26aaa2e
Compare
|
LGTM |
|
LGTM, but waiting another day for others to look |
|
@ilyavolodin What do you think? Is this ready to go? |
eslint-deprecated
bot
added
the
archived due to age
What issue does this pull request address?
#5805
What changes did you make? (Give an overview)
getAllStringsandgetAllTemplateLiteralsto source-codemax-len, and ignored lines that contained strings/template literals, matching the optionsIs there anything you'd like reviewers to focus on?
The overall approach.