Skip to content
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

max-len overestimates the length of lines that contain tabs #4661

Closed
nre opened this Issue Dec 10, 2015 · 6 comments

Comments

Projects
None yet
4 participants
@nre
Copy link
Contributor

nre commented Dec 10, 2015

  • Version 1.10.3.
  • Input:
/* eslint max-len: [1, 16, 4, {ignoreComments: true}] */
var one     = 1;
var three   = 3;
/*
16-char line:
1234567890123456

The lines contain tabs:
var one\t\t= 1;
var three\t= 3;
*/
  • Output:
test.js
  2:1  warning  Line 2 exceeds the maximum line length of 16  max-len
  3:1  warning  Line 3 exceeds the maximum line length of 16  max-len

? 2 problems (0 errors, 2 warnings)
  • The lines do not exceed 16 characters. The problem is that the max-len rule assigns a fixed width to each tab character, instead of to each tab stop.

I have a PR ready to correct this.

@eslintbot

This comment has been minimized.

Copy link

eslintbot commented Dec 10, 2015

@nre Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage label Dec 10, 2015

@ilyavolodin

This comment has been minimized.

Copy link
Member

ilyavolodin commented Dec 10, 2015

Could you explain a bit more about tab stop vs tab character?

@ilyavolodin ilyavolodin added bug rule evaluating and removed triage labels Dec 10, 2015

@nre

This comment has been minimized.

Copy link
Contributor Author

nre commented Dec 10, 2015

If the tab width is 4 spaces:

Tab stops are always 4 spaces wide:
|   |   |   |   |
1234123412341234

Each tab character is 1, 2, 3 or 4 spaces wide:
    <-- tab character is 4 spaces wide.
1   <-- tab character is 3 spaces wide.
12  <-- tab character is 2 spaces wide.
123 <-- tab character is 1 space wide.

But max-len will always replace each tab character with 4 spaces (never 3, 2 or 1), which is why it overestimates the length of some lines.

@ilyavolodin

This comment has been minimized.

Copy link
Member

ilyavolodin commented Dec 10, 2015

Ahh.. got it, thanks. In that case, I think it's a reasonable fix.

@ilyavolodin ilyavolodin added accepted and removed evaluating labels Dec 10, 2015

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Dec 10, 2015

@nre do you want to submit a pull request for this?

@nre

This comment has been minimized.

Copy link
Contributor Author

nre commented Dec 11, 2015

Working on this.

@nzakas nzakas closed this in 4204832 Dec 11, 2015

nzakas added a commit that referenced this issue Dec 11, 2015

Merge pull request #4670 from nre/issue4661
Fix: max-len rule overestimates the width of some tabs (fixes #4661)

@eslint eslint bot locked and limited conversation to collaborators Feb 6, 2018

@eslint eslint bot added the archived due to age label Feb 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.