Skip to content

Loading…

Upgrade less to v1.5.0 and use source-maps. #6

Closed
ghost opened this Issue · 8 comments

2 participants

@ghost

The v1.5.0 release of less.js should make the implementation of this plugin a lot easier and less error prone due to support of source maps.

v1.5.0 is not released yet, but we could already try an implementation with the 1.5.0-b4 beta release and source-map.

What do you think?

@kevinsawicki
Collaborator

Yeah, definitely sounds good, the current approach is definitely not precise due to what less currently outputs as originating lines.

@kevinsawicki
Collaborator

Looked into this a bit, and ran into the following problem for some less like:

body {
  #test {
    color: blue;
  }
}

The css would be:

body #test {
  color: blue;
}

But csslint reports the Don't use IDs in selectors. error at line 1, column 1. And when reversing that back through the source map it will come out as line 1 instead of line 2 in the less file since the position reported by the error refers to the body part, not the #test part of the line of the css.

May need to open an issue/pr on csslint to make the error reporting more precise on the column so the source map lookup goes to the right line.

@ghost

You're right. Definitely open up an issue. csslint (sadly) has tons of open issues, but it seems like they started to merge pull requests again.

@ghost

By the way, how do you currently solve this problem?

As far as I can see findPropertyLineNumber solves this. We could keep this for the time being.

@kevinsawicki
Collaborator

Yeah, I was just hoping to switch to source maps 100% and remove the old hacky comment based line number mapping code.

@ghost

Yea.

@jgable
Owner

I took another look at this and also came to the same road block as @kevinsawicki. The ID results are not as accurate as they could be, but it's still semi relevant. I put a PR in to the csslint project to use the column of the first found ID in the selector, if they do not accept it I may have to try and put a hack in just for that rule.

I have an implementation of the reporting working with source maps and will put a PR in for it in a little bit.

@jgable jgable closed this in #26
@jgable
Owner

This is now published as 1.0.0. It will report the body { line for now, if csslint doesn't merge my PR I'll update with something to hack it in our project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.