[WIP] Refactor Output Emitter (White-Space and Modes)

[mgreter]

My next massive WIP branch to unify output emitting!

Currently a lot of implementation is duplicated for output rendering. This makes the source-map topic more complicated than it has to. The rendering is mostly done via Inspect and Output_Nested/Output_Compressed. This is IMHO not organized very optimal and I tried to refactor the object inheritance to make the code more re-usable. I have added new Classes and Structs and re-organized the inheritance:

OutputBuffer:

This simply holds a string buffer and a sourcemap. You need the instantiate this on your own and pass it to Emitter. You can use the same OutputBuffer with several Emitters and you can create it in the stack, so you know exactly the lifetime of the object.

Emitter:

The Emitter is responsible to append tokens to the buffer and to manage the source-map. To do this the Emitter provides various methods, like:

• append_optional_linefeed
• append_optional_space
• append_mandatory_space
• append_colon_separator
• append_open_bracket
• ...

Before this Patch every Output would at some point create an Inspect object to do the further rendering (and copying the buffer to the existing one). This effectively means that at this point we loose the Output Style, since everything will be rendered in Inspect format. With this Patch, every Output inherits from Inspect, and Inspect inherits from Emitter (and Emitter holds a "shared" buffer). Currently there is only one Output class that contains switches for the different Output Styles, but could also be a derived Class as before. The Emitter should be powerfull enough to generate all (formal) output styles automatically. We only need to overload AST_Node visitors in Output if the rendering should be different from Inspect (ie. Comments in Compressed).

Output -> Inspect -> Emitter and Operation_CRTP

Code Example:

OutputBuffer buffer;
Emitter emitter(buffer, &ctx, NESTED);
Inspect insp(emitter);
AST_Node->perform(&insp);

I guess the hard part is already done. This branch currently fails 94 out of 651 spec tests if all whitespace is considered. If I normalize multiple linefeeds to one, it passes 636 out of 651 tests. Next task is to re-implement compressed style. I already have a prepared spec-suite in perl-libsass to do that 😄 But this still needs a lot of details to work on, but should pretty soon be on better than current libsass output. Adding the other missing output styles should also be pretty easy with this setup!

Btw. I created a gist with a debug_ast function which might be usefull for others (incomplete!): https://gist.github.com/mgreter/4ae93a9bf2d34a6e9044

[QuLogic]

This is fairly unsafe. Use #if 0 if you absolutely must comment out a block (but removal is better).

[mgreter]

This is just temporary. Will have to do the compressed mode first and than add back source-maps. At least I got some unit test for this part!

[mgreter]

Just a heads up. I currently have a (very dirty) WIP branch that is pretty much 99.5% on par with ruby sass for extended, expanded and compressed output style ☀️ I currently only need to normalize multiple linefeeds to one (/\n+/\n/), since I did not yet get the logic how ruby sass does it.

There are also some issues with whitespace preserving on extended selectors (actually it is more a line-feed preserving). After manually analyzing the input vs. output, I came to the conclusion that this is a very small ruby sass bug (I will eventually open a bug report to discuss that more later).

Anyway, I'm currently trying to extract some already useable parts, since I IMO fixed quite a few other bugs. One such PR is #847, which improves handling for interpolates and some static value parsing. Ruby sass actually mostly just preserves whitespace for static values (which make absolute sense, even if it could be considered a bug in compressed output).

[mgreter]

So let's go with another heads up. I have been hacking through the code like wild to get the test suite working in case of how strings and interpolates etc. are represented in the output (single/double quotes or not quoted at all). Was really not that much fun ... but it seemed easier to fix it once and for all(?) than implementing even more hacks around the real issue.

Now to the interesting parts, the latest WIP branch:
https://github.com/mgreter/libsass/tree/wip/ugly-but-working

I also published the test-suite I used to run against with perl-libsass (the test runner part is not published on github). To make it clear, the WIP branch bellow disables some Improvements I have in another branch for white space preserving. This here is really about interpolation and the output. It now even accepts some (IMO valid) interpolations, which ruby sass chokes on 😄
https://github.com/mgreter/sass-specs/tree/wip/ugly-but-working/suite/parser
https://github.com/mgreter/sass-specs/blob/wip/ugly-but-working/suite/parser/escaping/interpolate/input.scss

Unfortunately I had to debug the code quite intensively, which made the resulting code very dirty. So next step would be to extract the important parts from the latest WIP branch, which will probably take quite some time. I already extracted some improvements in PR #847. IMO this is a pretty significant change and improvement, so I thought I'd share the latest status.

P.s. the whole excercise at some point felt like this here 😄

[xzyfer]

This all sounds great. I'll take a closer look over the weekend. It'd be great to start merging bits and pieces of this work before it get stale.

[mgreter]

I finally got my WIP branch to pass the auto-generated tests for interpolants:
https://github.com/mgreter/libsass/tree/wip/ugly-but-working
https://github.com/mgreter/sass-specs/tree/wip/ugly-but-working/suite/parser/interpolate

[mgreter]

Next update with another branch that passes the current spec suite plus a lot more interpolations:
https://github.com/mgreter/libsass/tree/267cfd1907734057ac76d56c1464cef146d0abec

This branch currently passes these todo test (some probably passing due to other people' fixes):

• libsass-todo-issues/issue_615 Interpolation inside an interpolation isn't handled correctly #615
• libsass-todo-issues/issue_694 Interpolation of quoted strings differs from Ruby SASS #694
• libsass-todo-tests/12_pseudo_classes_and_elements
• libsass-todo-tests/env
• libsass-todo-tests/selector_interpolation_in_string/input.scss
• libsass-todo-tests/scss-tests/109_test_selector_interpolation_in_string/input.scss

It also passes my complete parser suite I already mentioned in this thread:
https://github.com/mgreter/sass-specs/tree/wip/ugly-but-working/suite/parser/interpolate

I feel like I currently have it covered pretty well. There are now a few basic things I need to do:
[ ] Extrapolate the minimal set of functions for quote, unquote and escape
[ ] Check which "preserve" flags are really needed for correct rendering (Strings)
[ ] Check if it really makes sense to factor out Strings into different types (Constant, Quoted)
[ ] Refactor pretty much everything in output, emitter and somewhat inspect
[ ] Re-implement source maps directly into output and emitter
[ ] ...

All in all a step in the right direction; if I'm able to port this all over to master!

[xzyfer]

Great work here @mgreter. I recommend rebase off master before this diverges much more. It'll also give a clearer indication as to which specs this fixes.

[mgreter]

Closing in favor of #910