Preparation for interpolate/output refactor #899
Conversation
And token is stored on parser state
| @@ -1326,10 +1331,10 @@ namespace Sass { | |||
| // "flat" strings. | |||
| //////////////////////////////////////////////////////////////////////// | |||
| class String : public Expression { | |||
| ADD_PROPERTY(bool, needs_unquoting); | |||
| ADD_PROPERTY(bool, sass_fix_1291); | |||
There was a problem hiding this comment.
I don't know about this variable name
There was a problem hiding this comment.
The needs_unquoting flag will not be needed with the full refactoring. I my WIP this flag is needed to have the same behavior as ruby sass (sass/sass#1291) which will be deprecated. Which means the sass_fix_1291 can probably be removed once sass has it deprecated. But you're right, currently it doesn't make sense as needs_unquoting is still in the code. Please see comment below about String_Constant vs. String_Quoted. I would favor to ignore it for now.
|
Please squash these commits before merging :) |
|
Can you provide a list of specs that now pass due to this change? You may need to rebase master. |
|
Thanks for the review! I'm making progress with the fallow up PR: |
|
I've nearly finished the merge of my WIP branch. Although it currently only supports nested output and it probably is off by quite a bit. But everything is prepared to implement different output styles inside the Emitter class. Next I hope I can refactor my last part of my working WIP branch, becuase that was actually the most dirty one. But I know it is doable via Emitter class! |
|
I think there's room to reduce the number of string types in the near 👍
|
|
Just checked which tests pass with the output-preparation branch:
I guess some also pass on current master! Maybe you have time to move them? |
|
I have squashed the whole refactoring into two commits (the first one beeing exactly this PR) at: |
|
Some of these pass on master, however a few appear to be interpolation related and pass because of this PR 👍 I'll enable the passing tests on master. They're due to my recent normalization PR. |
|
Enabled the specs that pass on master sass/sass-spec#263. |
|
Got a first implementation for nested output rendering at: mgreter@3a983e1 That branch passes the following additional tests:
It also passes all but 31 spec test exactly to ruby sass output (I only normalize multiple linefeed to one linefeed). Next is to refactor the whitespace/linefeed preserving for selectors and lists. |
|
Mixed in the line-feed preserving from my WIP branch at: mgreter@1210dfa With this it additionaly seems to pass one more spec test (only fails 2 nested spec tests):
|
|
I have pretty much a finished implementation in another WIP branch:
Some of these seem to be bugs in ruby sass. |
|
@mgreter feel free to just do what you need to do, and merge at will. I'd prefer you just merge you changes incrementally (as long as specs are passing). Frankly there is no much going on here that it can't really be reviewed as is. At this point the best way forward is to merge as you feel comfortable and we can address an incremental work, or refactors as they come up. |
|
@xzyfer 👍 Btw: you can think if |
👍 I'm keen to get this work over the line. Long lasting branches make me uneasy :) |
|
Closing in favor of #910 |
This is in preparation for #839
@xzyfer Maybe you could give it a review? It's already a pretty big PR and the rest of the refactoring will be much more new code. This PR should contain most refactoring I needed in the code base. Some code does not do much yet (like String_Quoted type), but it should make it easier this way to review.
I tried to make usefull commits, but I can merge them to one commit if wanted!?