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

Fixed sourceMapBasepath bug as the option had no affect on the sourceMapURL value. #2946

Merged
merged 3 commits into from
Aug 5, 2016

Conversation

barnabycolby
Copy link
Contributor

As discussed in #1781, the sourceMapBasepath option has no affect on the sourceMapURL value output in the css file. This pull request fixes the issue.

There do not currnetly appear to be any tests for the sourceMapURL CSS appendage.

@matthew-dean
Copy link
Member

@barnabycolby The tests for source maps are currently pretty slim in Less. Can you articulate and demonstrate the original issue more clearly, especially since #1781 was closed? What output was expected and what Less was generating? And then how this PR affects that (in the form of a test)?

…correctness of the sourceMappingURL CSS appendage.
@barnabycolby
Copy link
Contributor Author

@matthew-dean The --source-map-basepath option is supposed to remove the given basepath from the start of the source map output path, and more precisely, from the sourceMappingURL value found at the bottom of the compiled CSS. For example, running the following:

node bin/lessc --source-map=foo/bar.css.map bar.less bar.css

results in the following CSS appendage in bar.css

/*# sourceMappingURL=foo/bar.css.map */

Adding the --source-map-basepath option should allow us to remove the foo/ prefix. So running

node bin/lessc --source-map=foo/bar.css.map --source-map-basepath=foo bar.less bar.css

should result in the following CSS appendage in bar.css

/*# sourceMappingURL=bar.css.map */

However, the --source-map-basepath option did not cause the basepath to be removed, and appeared to have no effect at all.

@barnabycolby
Copy link
Contributor Author

That last commit broke older versions of node, will investigate!

…, preventing a problem when testSourceMap is run on older versions of node.
@barnabycolby
Copy link
Contributor Author

The problem was that I used String.prototype.endsWith, which does not exist in older versions of node. In those cases, I fixed the problem by defining endsWith myself. @matthew-dean Over to you! :)

@matthew-dean matthew-dean merged commit 24523c6 into less:3.x Aug 5, 2016
@matthew-dean
Copy link
Member

matthew-dean commented Aug 5, 2016

Looks legit. 👍 I'll pull this into 3.x. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants