-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
new rule option request: max-len ignores strings #5805
Comments
Sounds good to me, but let's see what the rest of the team thinks. |
I too would like this, and would be happy to take a crack at a PR. |
So to clarify, should this apply to every node of type |
@ilyavolodin I was only thinking string and template literals - however, I'd be equally fine with the option being called |
@eslint/eslint-team does anyone want to champion this? |
What is the proposed behaviour here? Ignore the whole line if it contains a string, like |
@alberto i'd be fine with either. discounting the length of the string seems most useful to me, however. |
I would like to single out template literal strings specifically for this kind of rule, rather than all strings in general. Usually, template strings will contain HTML which sometimes can be quite lengthy on one line. |
I'm fine with two options - |
Broken and concatenated long strings are painful to work with and produce less readable and searchable code. I think we should reverse this rule. Unfortunately, the max-len rule currently does not allow for this, but there is currently a proposal to add an option to ESLint's max-len rule that would allow for strings to be ignored. eslint/eslint#5805 There have also been discussions around performance of string concatenation (#40), but I don't think that is very relevant here so I removed the links to them.
Broken and concatenated long strings are painful to work with and produce less readable and searchable code. I think we should reverse this rule. Unfortunately, the max-len rule currently does not allow for this, but there is currently a proposal to add an option to ESLint's max-len rule that would allow for strings to be ignored. eslint/eslint#5805 There have also been discussions around performance of string concatenation (#40), but I don't think that is very relevant here so I removed the links to them.
Broken and concatenated long strings are painful to work with and produce less readable and searchable code. I think we should reverse this rule. Unfortunately, the max-len rule currently does not allow for this, but there is currently a proposal to add an option to ESLint's max-len rule that would allow for strings to be ignored. eslint/eslint#5805 There have also been discussions around performance of string concatenation (#40), but I don't think that is very relevant here so I removed the links to them.
The Airbnb JavaScript style guide now recommends not breaking and concatenating long strings. https://github.com/airbnb/javascript#strings--line-length |
Hi @ljharb, thanks for the issue. It looks like there's not enough information for us to know how to help you. Requesting a new rule? Please see Proposing a New Rule for instructions. |
Can someone please put together a concrete proposal using the "Proposing a Rule Change" instructions above, which answers all of the open questions? If someone can get that put together, I'm willing to champion. Open questions:
If we can get consensus around these questions, I think we can move forward with this. |
I'll write what you're requesting now. |
What version of ESLint are you using? What rule do you want to change? What code should be flagged as incorrect with this change? // these warn
// eslint max-len: [2, 10, { "ignoreStringLiterals": false, "ignoreTemplateLiterals": false }]
a = 'too long';
a = `too long`;
// this is correct
// eslint max-len: [2, 10, { "ignoreStringLiterals": true }]
a = 'too long';
// eslint max-len: [2, 10, { "ignoreTemplateLiterals": true }]
a = `too long`; What happens when the rule is applied to this code now? Note: just like "ignoreURLs", both of these options should cause the entire line on which the literals appear to be ignored for the |
I will champion @ljharb's proposal. |
👍 from me. |
👍 |
This would help a lot. 👍 |
👍 |
@ljharb This is now accepted. Were you thinking of writing a PR? |
If nobody gets to it first, definitely! Just need to find the time :-) |
nice job @ljharb , can't wait to see this merged. |
Broken and concatenated long strings are painful to work with and produce less readable and searchable code. I think we should reverse this rule. Unfortunately, the max-len rule currently does not allow for this, but there is currently a proposal to add an option to ESLint's max-len rule that would allow for strings to be ignored. eslint/eslint#5805 There have also been discussions around performance of string concatenation (airbnb#40), but I don't think that is very relevant here so I removed the links to them.
Broken and concatenated long strings are painful to work with and produce less readable and searchable code. I think we should reverse this rule. Unfortunately, the max-len rule currently does not allow for this, but there is currently a proposal to add an option to ESLint's max-len rule that would allow for strings to be ignored. eslint/eslint#5805 There have also been discussions around performance of string concatenation (airbnb#40), but I don't think that is very relevant here so I removed the links to them.
Broken and concatenated long strings are painful to work with and produce less readable and searchable code. I think we should reverse this rule. Unfortunately, the max-len rule currently does not allow for this, but there is currently a proposal to add an option to ESLint's max-len rule that would allow for strings to be ignored. eslint/eslint#5805 There have also been discussions around performance of string concatenation (airbnb/javascript#40), but I don't think that is very relevant here so I removed the links to them.
Currently,
max-len
supportsignoreComments
,ignoreUrls
, etc. I'd like to request an additional option,ignoreStrings
.My rationale is the following:
This affects both inline strings as well as "require" paths and "import from" paths (and should apply to template literals as well as single/double-quoted strings)
The text was updated successfully, but these errors were encountered: