CommentRegex Doesn't Find Comments on Same Line as Code

[tybruffy]

I'm not sure if the source map spec says source map comments must be on new lines or not. If that's the case this is a bug in Less and I can post it there.

I'm using Less with the inline source map and compression options and my code and an inline source map end up on one line at the end of the file.

The commentRegex is set to only look for comments on their own line, and there are 4 tests (4, 5, 10, 11 in comment-regex.js) that fail if you remove that piece of the regex. Those tests seem specifically designed to fail if the comment isn't on it's own line so that makes sense. The tests fail but the captured parts remain the same throughout, so it seems like it would still be safe.

Thoughts on this? Is this a bug in Less's compression for allowing source maps to be stuck on the same line as code?

[thlorenz]

Could you provide an example snippet please? AFAIK the spec requires the source map comment to be on it's own line. It's all described in this proposal. Nothing that hints at supporting the Less case.

Also this post points at it needing to be on it's own line.

Are you sure you are dealing with source maps and not source urls? I remember less had those.

Again a snippet would be very helpful to clarify.

[tybruffy]

I'm not entirely clear on the difference between Source Map and Source Urls, so no, I'm not sure what I'm dealing with :)

Here's a link to bootstrap compiled with the options I mentioned above: http://pastebin.com/6Rb5tb40
lessc --source-map-map-inline -x bootstrap.less test.css

That's not the exact code I was using when I ran into this, but it's close.

[thlorenz]

The sourcemap is broken up in a weird manner:

[....] inline-block{display:inline-block !important}}@media print{.hidden-print{display:none !important}}/*# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJm [...] == */

I mean its all on one line! The source map needs to be on a separate line.

Looks like minification broke it. I don't think that abides by the spec and would expect lots of tools to fail in parsing this out correctly.

Please file a bug with the authors of the tools that generated this.
Closing this for now as this is not something that we can fix here.

[tybruffy]

I'm not actually seeing anywhere in the Source Map 3 Proposal that specifies that a source map needs to be on its own line. Am I being thick?

The source map is read fine by chrome/firefox, so if it's not explicitly said to need to be on its own line in the spec, isn't it better to allow it?

[tybruffy]

@thlorenz Can you point me to where it says source maps need to be on their own line? I don't see anything that conclusively states source maps are required to be on their own line in a file in the proposal but I fully acknowledge I could be missing something.

Seems to me that if source maps don't explicitly need to be on their own line, then the error is with the CommentRegex.

[thlorenz]

You may be right, but we need to confirm that other tools even handle source maps like that.

Pulling out a source map with regex becomes near impossible to get right without false positives (i.e. inside a string) once you don't assume them to be on a separate line.

As a test you could generate a browserify bundle and put all code on one line (including source map).
Then see if chrome can even still parse it. If that's the case I'm willing to consider to introduce a flag that makes convert-source-map look for source maps on same line.

[tybruffy]

I can confirm that Chrome and Firefox will correctly parse a source all on one line in that manner. It works for both Javsacript as well as CSS maps. (http://pastebin.com/6Rb5tb40 is successfully parsed back to it's less files.)

I'm happy to put together a PR for you to add a flag for this if you want. If you're set on keeping the original regex as an option you could just add an additional regex called CommentRegexGreedy or something and export another getter for it?

[thlorenz]

Actually if you remove ?:^[ \t]* in the two places it occurs in this regex it should work for your case.

Since we still have the $ to ensure it has to be at the end of the line we'd rule out in-string occurrences.
Therefore we may not even have to use a flag, but change the default behavior instead.

It'd be awesome if you could PR with that change along with some tests for your particular case and some that ensure that it won't match false positives, i.e. try to pull a source map from within a string.

Thanks.

[tybruffy]

Sure thing, I'll try to put some tests together this afternoon. Changing the default behavior definitely seems more friendly.

[thlorenz]

Actually this regex doesn't have the $ so we should add that.

[tybruffy]

I can confirm that this pull request works for the pastebin we've been using in this thread.