Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

max-line-length with template literals #3905

Closed
artola opened this issue May 11, 2018 · 8 comments · Fixed by #4798
Closed

max-line-length with template literals #3905

artola opened this issue May 11, 2018 · 8 comments · Fixed by #4798

Comments

@artola
Copy link

artola commented May 11, 2018

Bug Report

  • TSLint version: 5.9.1
  • TypeScript version: 2.8.3
  • Running TSLint via: (pick one) CLI / VS Code

TypeScript code being linted

const str = `134567890 1234567890 134567890 1234567890 134567890 1234567890 134567890 1234567890 1234567890 1234567890
1234567890 1234567890 1234567890 1234567890 1234567890`;

with tslint.json configuration:

{
  "rules": {
    "max-line-length": [true, {"limit": 80}]
  }
}

Actual behavior

length: 118 const str = `134567890 1234567890 134567890 1234567890 134567890 1234567890 134567890 1234567890 1234567890 1234567890
1234567890 1234567890 1234567890 1234567890 1234567890`;

const str = `134567890 1234567890 134567890 1234567890 134567890 1234567890 134567890 1234567890 1234567890 1234567890

ERROR: /file.ts[1, 1]: Exceeds maximum line length of 80

Expected behavior

This rule should skip verification for "some" strings, including multiline template literal.

@joelday
Copy link

joelday commented Jun 24, 2018

@ashwinr @ajafff @adidahiya @jkillian How would you design the options schema for this while remaining backward compatible?

@joelday
Copy link

joelday commented Jul 2, 2018

@ashwinr @ajafff @adidahiya @jkillian Sorry to bump this again, but I'd like to get a PR in. Unless I'm missing something, I've been unable to find a case where the options can be either a number or an object.

@reduckted
Copy link
Contributor

@joelday The max-line-length rule already accepts either a number of an object. Take a look at the Config examples and Schema section of the rule's documentation:

"max-line-length": [true, 120]
"max-line-length": [true, {"limit": 120, "ignore-pattern": "^import |^export {(.*?)}"}]

So you should be able to add a new property to the object and it will remain backward compatible. Something like this:

{
  "type": "array",
  "items": {
    "oneOf": [
      {
        "type": "number"
      },
      {
        "type": "object",
        "properties": {
          "limit": {
            "type": "number"
          },
          "ignore-pattern": {
            "type": "string"
          },
          "ignore-multiline-template-literals": {
            "type": "boolean"
          }
        },
        "additionalProperties": false
      }
    ]
  },
  "minLength": 1,
  "maxLength": 2
}

@artola
Copy link
Author

artola commented Jul 16, 2018

@joelday @reduckted This rule has some problems, I found mainly 2: template literals and Regex. In both cases it is hard or not recommendable to cut these long strings.
It is not about ignore-multiline-template-literals ... even single line template literals are a problem as in the example above.

Then a property like ignore-template-literals would be great.

Could also be possible add support for regex?

@reduckted
Copy link
Contributor

even single line template literals are a problem as in the example above.

The option should probably ignore all strings, whether they're single-line, multi-line or template strings.

@alllx
Copy link

alllx commented Nov 16, 2018

For now, for a single line template literals, it is possible to use ignore-pattern:
if only space indention are used on the propject:
"max-line-length": [true, {"limit": 120, "ignore-pattern": "^\\s*(.+)$"}]
for both tabs ans spaces:
"max-line-length": [true, {"limit": 120, "ignore-pattern": "^[\t\\s]*(.+)$"}]
to alow single line template literal asignment:
"max-line-length": [true, {"limit": 120, "ignore-pattern": "(.+)"}]

@shabbyrobe
Copy link

If you intend to implement #3585, I would strongly recommend the sage advice of @reduckted is heeded, and that such an option should be enabled by default for all strings.

Strings could contain absolutely anything. A linter has no business expressing opinions on them, and a fixer certainly hasn't got any business whatsoever touching them because it can't possibly understand the rules that govern "the set of all possible strings".

@vmk1vmk
Copy link
Contributor

vmk1vmk commented Jul 13, 2019

Hi, I had the same issue in some projects I was participating in and here is the solution I came up with.
It only ignores strings and template strings but we could figure out what kind of options we would like to have, maybe even regex as @artola mentioned.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants