Regression in 'inline source comments' and 'source map mappings'

[am11]

With the latest libsass: 702ac5a, two of the node-sass tests are failing
https://travis-ci.org/am11/node-sass/jobs/78170861#L1888-L1937.

The first one is about source comment and is referring to this input and the corresponding expected output. The line number is off by 2 in the actual output.

The second failure is related to source-map mappings, here is the diff of decoding: https://www.diffnow.com/?report=bohl1 (left: old, right: new). Though, I have not verified yet whether old behavior was correct in this test case or the new one.

[mgreter]

#1510

[am11]

Apparently it is happening with those changes inclusive; as my libsass submodule is pointing to 702ac5a:
https://github.com/am11/node-sass/tree/request-ci-artifacts/src.

[am11]

@mgreter, also tested with stable release v3.2.5 of libsass and sassc:

• Extracted https://github.com/sass/libsass/archive/3.2.5.zip

• Then extracted: https://github.com/sass/sassc/archive/3.2.5.zip inside libsass.

• Built libsass/sassc/win/sassc.sln.

• Saved a file \temp\test.scss with the following code:

    .a {
        color: #000;
    }
  
    .b {
        color: #001;
    }
  
    .c {
        color: #002;
    }

  (tested with both line endings \r and \r\n, there is no difference in the output)

• Ran libsas/sassc/bin/sassc /temp/test.scss, on stdout I got:

  /* line 1, /temp/test.scss */
  .a {
    color: #000; }
  
  /* line 5, /temp/test.scss */
  .b {
    color: #001; }
  
  /* line 9, /temp/test.scss */
  .c {
    color: #002; }

With current master, it is producing:

/* line 1, C:/temp/test.scss */
.a {
  color: #000; }

/* line 3, C:/temp/test.scss */
.b {
  color: #001; }

/* line 7, C:/temp/test.scss */
.c {
  color: #002; }

The drive letter is present in latest, but line numbers are incorrect.

[mgreter]

Can you please try #1515

[am11]

Thanks @mgreter, thanks! I can confirm that #1515 fixed the line number (source comment) issue. 👍

The last failing test is about source-mappings difference: https://travis-ci.org/am11/node-sass/jobs/78265337#L1898-L1916. Could it be due to the improvement in source map since the last release?

[mgreter]

Yes, source-mapping was slightly updated and improved ...

[am11]

Thanks @mgreter. We will probably need to update the expected output in node-sass test fixture.

In any event, I wanted to run the difference by you before we lock it down. :)

Input (node-sass/test/fixtures/source-map/index.scss):

#navbar {
  width: 80%;
  height: 23px;
}

#navbar ul {
  list-style-type: none;
}

#navbar li {
  float: left;

  a {
    font-weight: bold;
  }
}

Compiled with:

cd node-sass
node bin/node-sass test/fixtures/source-map/index.scss --source-map=true /test/fixtures/source-map/index.css

Produces:

#navbar {
  width: 80%;
  height: 23px; }

#navbar ul {
  list-style-type: none; }

#navbar li {
  float: left; }
  #navbar li a {
    font-weight: bold; }

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

node-sass 3.3.2 (=> libsass 3.2.5) decompiled mappings on left and that produced with your branch bugfix/ruleset-line-reporting on the right, this diff is produced: http://www.mergely.com/57gV3nMs/

[mgreter]

Have you tried my source-map inspector? With it I was able to confirm the validity in 1 minute 🚑

[am11]

Wow, The inspector is super helpful! 💯

Confirm that both the outputs are accurate and we will only need to update the node-sass test after libsass v3.3 is shipped.

[mgreter]

Wow, The inspector is super helpful! 💯

Yeah, I needed something that works, otherwise source-maps are nearly undebugable ...
As it turns out it's also helpfull debugging the different options on the C-API (added this moment) ...
Can you give a short confirmation in #1515 so it can be merged? Thanks!

[saper]

@mgreter Would that be possible to be able to paste the suspected "incorrect" source map into the inspector third window and browse that? I can paste a different map but when I click "Inspect" it switches to the generated map automatically.

Another wish item from me would be to save some horizontal screen space - I think the leftmost column can go away complete - I tried to have two browser windows side-by-side with two times source map tool to compare the "bad" map and the "good" map. Would be cool to save space a bit :)

[mgreter]

@saper have you tried "[Analyze]". The whole UI and functionality is just hacked together, so from this point I would need to refactor it quite a bit to add more features. Also the UI is very much static and not that easy to change from this point on (it's really just a POC), but I may be able to hack some more specific js together to close the left panel ...

[saper]

What does [analyze] do?

[mgreter]

It should analyze the output, so you should be able to paste your own results into the output textarea.
=> http://libsass.ocbnet.ch/srcmap/about.html

Paste some "Output" yourself (make sure it has sources embedded)!
Hit the "Analyze" link and everything should be setup to start the inspection!

[saper]

@am11 saper/node-sass@5aafff7 includes the patched test, so now node-sass tests should pass. You only need to manually update libsass to bd8b50a and then you can try sass/node-sass#1040

[saper]

Thanks @mgreter will try that one!