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

Incorrect column reported by max-line-length #2286

Closed
m-allanson opened this issue Jan 24, 2017 · 1 comment · Fixed by singapore/lint-condo#230
Closed

Incorrect column reported by max-line-length #2286

m-allanson opened this issue Jan 24, 2017 · 1 comment · Fixed by singapore/lint-condo#230
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule

Comments

@m-allanson
Copy link
Member

m-allanson commented Jan 24, 2017

Describe the issue. Is it a bug or a feature request (new rule, new option, etc.)?

A bug where max-line-length will report an incorrect column number.

Which rule, if any, is this issue related to?

max-line-length

What CSS is needed to reproduce this issue?

a { background-image: url('some ignored url'); }

What stylelint configuration is needed to reproduce this issue?

{
  "rules": {
    "max-line-length": 20
  }
}

Which version of stylelint are you using?

7.7.1

(actually master@3183227, but I don't think that makes a difference)

How are you running stylelint: CLI, PostCSS plugin, Node API?

CLI

Does your issue relate to non-standard syntax (e.g. SCSS, nesting, etc.)?

No

What did you expect to happen?

A warning to be flagged, with the column indicating the end of the line:

test.css
1:48    Expected line length to be no more than 20 characters (max-line-length)

What actually happened (e.g. what warnings or errors you are getting)?

The following warning was flagged, listing an incorrect column number.

test.css
1:30    Expected line length to be no more than 20 characters (max-line-length)

Cause

The rule ignores long urls by replacing url('some ignored url') with url() before checking the line length.

If a line is still longer than the min length, the reported column number will be at the end of the line minus the url value:

a { background-image: url(); }

instead of the end of the line including the url value:

a { background-image: url('some ignored url'); }

Note that this bug is only triggered on lines that are still longer than max-line-length after the url value has been replaced.


Extra example

This issue can also cause errors to be reported on incorrect lines. The following CSS will report two errors on line 1, instead of an error on each line.

a { background-image: url('1234567890123456789012345678901234567890'); }
.foo .bar .baz .quux {}
 1:30  ✖  Expected line length to be no more than 20 characters   max-line-length
 1:54  ✖  Expected line length to be no more than 20 characters   max-line-length
@ntwb
Copy link
Member

ntwb commented Jan 24, 2017

Fantastic sleuthing @m-allanson 👍

Can you add a test or tests for that code (or similar) you have in the Extra example above, it'd be great to ensure that that issue is also sorted in the resolution of this issue

@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule labels Jan 25, 2017
davidtheclark pushed a commit that referenced this issue Jan 28, 2017
@davidtheclark davidtheclark added status: wip is being worked on by someone and removed status: ready to implement is ready to be worked on by someone labels Jan 28, 2017
sergesemashko pushed a commit to sergesemashko/stylelint that referenced this issue Mar 3, 2017
* Add a failing test for issue stylelint#2286

* Fix max-line-length column number for lines with urls

Closes stylelint#2286
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule
Development

Successfully merging a pull request may close this issue.

4 participants