Skip to content

Commit b11a557

Browse files
committed
Consolidate the Declaration and Propset AST nodes
In order to fix #1585 we need to introduce the `Trace` node from Ruby Sass. This node interacts with propsets in an interesting way. Have two node to represent the single Ruby `Prop` node caused a lot of trouble with correct implementing check nesting logic for `Trace`. The trouble with implementing `Trace` were exaggerated by us incorrectly eval propsets into declarations in the expand step. This PR moves the de-nesting of prop nodes the cssize step to [better match Ruby Sass][1]. Later we should rename this node `Prop` but it's out of scope of this PR. This should allow my work on porting the `Trace` node over from Ruby Sass to fix #1585. [1]: https://github.com/sass/sass/blob/63586f1e51c0aec47a7afcc8cd17aa03f32f0a29/lib/sass/tree/visitors/cssize.rb#L149-L166
1 parent 6e4409e commit b11a557

File tree

10 files changed

+74
-17
lines changed

10 files changed

+74
-17
lines changed

src/ast.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -580,15 +580,15 @@ namespace Sass {
580580
////////////////////////////////////////////////////////////////////////
581581
// Declarations -- style rules consisting of a property name and values.
582582
////////////////////////////////////////////////////////////////////////
583-
class Declaration : public Statement {
583+
class Declaration : public Has_Block {
584584
ADD_PROPERTY(String*, property)
585585
ADD_PROPERTY(Expression*, value)
586586
ADD_PROPERTY(bool, is_important)
587587
ADD_PROPERTY(bool, is_indented)
588588
public:
589589
Declaration(ParserState pstate,
590-
String* prop, Expression* val, bool i = false)
591-
: Statement(pstate), property_(prop), value_(val), is_important_(i), is_indented_(false)
590+
String* prop, Expression* val, bool i = false, Block* b = 0)
591+
: Has_Block(pstate, b), property_(prop), value_(val), is_important_(i), is_indented_(false)
592592
{ statement_type(DECLARATION); }
593593
ATTACH_OPERATIONS()
594594
};

src/ast_factory.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace Sass {
1919
At_Root_Block* new_At_Root_Block(std::string p, size_t l, Selector* sel, Block* b);
2020
Directive* new_At_Rule(std::string p, size_t l, std::string kwd, Selector* sel, Block* b);
2121
Keyframe_Rule* new_Keyframe_Rule(std::string p, size_t l, Block* b);
22-
Declaration* new_Declaration(std::string p, size_t l, String* prop, List* vals);
22+
Declaration* new_Declaration(std::string p, size_t l, String* prop, List* vals, Block* b);
2323
Assignment* new_Assignment(std::string p, size_t l, std::string var, Expression* val, bool guarded = false);
2424
Import<Function_Call*>* new_CSS_Import(std::string p, size_t l, Function_Call* loc);
2525
Import<String*>* new_SASS_Import(std::string p, size_t l, String* loc);

src/check_nesting.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,15 @@ namespace Sass {
4242
return static_cast<Statement*>(n);
4343
}
4444

45-
bool CheckNesting::is_valid_prop_parent(AST_Node* p)
45+
bool CheckNesting::is_valid_prop_parent(AST_Node* p)
4646
{
4747
if (Definition* def = dynamic_cast<Definition*>(p)) {
4848
return def->type() == Definition::MIXIN;
4949
}
5050

5151
return dynamic_cast<Ruleset*>(p) ||
5252
dynamic_cast<Keyframe_Rule*>(p) ||
53-
dynamic_cast<Propset*>(p) ||
53+
dynamic_cast<Declaration*>(p) ||
5454
dynamic_cast<Directive*>(p) ||
5555
dynamic_cast<Mixin_Call*>(p);
5656
}

src/cssize.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,45 @@ namespace Sass {
4444
return t->block()->perform(this);
4545
}
4646

47+
Statement* Cssize::operator()(Declaration* d)
48+
{
49+
String* property = dynamic_cast<String*>(d->property());
50+
51+
if (Declaration* dd = dynamic_cast<Declaration*>(parent())) {
52+
String* parent_property = dynamic_cast<String*>(dd->property());
53+
property = SASS_MEMORY_NEW(ctx.mem, String_Constant,
54+
d->property()->pstate(),
55+
parent_property->to_string() + "-" + property->to_string());
56+
if (!dd->value()) {
57+
d->tabs(dd->tabs() + 1);
58+
}
59+
}
60+
61+
Declaration* dd = SASS_MEMORY_NEW(ctx.mem, Declaration,
62+
d->pstate(),
63+
property,
64+
d->value(),
65+
d->is_important());
66+
dd->is_indented(d->is_indented());
67+
dd->tabs(d->tabs());
68+
69+
p_stack.push_back(dd);
70+
Block* bb = d->block() ? d->block()->perform(this)->block() : 0;
71+
p_stack.pop_back();
72+
73+
if (bb && bb->length()) {
74+
if (dd->value() && !dd->value()->is_invisible()) {
75+
bb->unshift(dd);
76+
}
77+
return bb;
78+
}
79+
else if (dd->value() && !dd->value()->is_invisible()) {
80+
return dd;
81+
}
82+
83+
return 0;
84+
}
85+
4786
Statement* Cssize::operator()(Directive* r)
4887
{
4988
if (!r->block() || !r->block()->length()) return r;

src/cssize.hpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ namespace Sass {
1313

1414
class Cssize : public Operation_CRTP<Statement*, Cssize> {
1515

16-
Context& ctx;
17-
std::vector<Block*> block_stack;
18-
std::vector<Statement*> p_stack;
16+
Context& ctx;
17+
std::vector<Block*> block_stack;
18+
std::vector<Statement*> p_stack;
1919
std::vector<Selector_List*> s_stack;
20-
Backtrace* backtrace;
20+
Backtrace* backtrace;
2121

2222
Statement* fallback_impl(AST_Node* n);
2323

@@ -37,7 +37,7 @@ namespace Sass {
3737
Statement* operator()(Directive*);
3838
Statement* operator()(Keyframe_Rule*);
3939
Statement* operator()(Trace*);
40-
// Statement* operator()(Declaration*);
40+
Statement* operator()(Declaration*);
4141
// Statement* operator()(Assignment*);
4242
// Statement* operator()(Import*);
4343
// Statement* operator()(Import_Stub*);

src/debugger.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ inline void debug_ast(AST_Node* node, std::string ind, Env* env)
155155
Propset* selector = dynamic_cast<Propset*>(node);
156156
std::cerr << ind << "Propset " << selector;
157157
std::cerr << " (" << pstate_source_position(node) << ")";
158+
std::cerr << " <" << dynamic_cast<String_Constant*>(selector->property_fragment())->value() << ">";
158159
std::cerr << " " << selector->tabs() << std::endl;
159160
if (selector->block()) for(auto i : selector->block()->elements()) { debug_ast(i, ind + " ", env); }
160161
} else if (dynamic_cast<Wrapped_Selector*>(node)) {
@@ -389,6 +390,7 @@ inline void debug_ast(AST_Node* node, std::string ind, Env* env)
389390
std::cerr << " " << block->tabs() << std::endl;
390391
debug_ast(block->property(), ind + " prop: ", env);
391392
debug_ast(block->value(), ind + " value: ", env);
393+
debug_ast(block->block(), ind + " ", env);
392394
} else if (dynamic_cast<Keyframe_Rule*>(node)) {
393395
Keyframe_Rule* has_block = dynamic_cast<Keyframe_Rule*>(node);
394396
std::cerr << ind << "Keyframe_Rule " << has_block;

src/expand.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,15 @@ namespace Sass {
157157
// mixes string_schema and statement
158158
Statement* Expand::operator()(Propset* p)
159159
{
160+
String* fragment = dynamic_cast<String*>(p->property_fragment()->perform(&eval));
161+
Propset* prop = SASS_MEMORY_NEW(ctx.mem, Propset,
162+
p->pstate(),
163+
fragment,
164+
p->block()->perform(this)->block());
165+
prop->tabs(p->tabs());
166+
return prop;
167+
168+
160169
property_stack.push_back(p->property_fragment());
161170
Block* expanded_block = p->block()->perform(this)->block();
162171
for (size_t i = 0, L = expanded_block->length(); i < L; ++i) {
@@ -258,15 +267,20 @@ namespace Sass {
258267

259268
Statement* Expand::operator()(Declaration* d)
260269
{
270+
Block* ab = d->block();
261271
String* old_p = d->property();
262272
String* new_p = static_cast<String*>(old_p->perform(&eval));
263273
Expression* value = d->value()->perform(&eval);
264-
if (!value || (value->is_invisible() && !d->is_important())) return 0;
274+
Block* bb = ab ? ab->perform(this)->block() : 0;
275+
if (!bb) {
276+
if (!value || (value->is_invisible() && !d->is_important())) return 0;
277+
}
265278
Declaration* decl = SASS_MEMORY_NEW(ctx.mem, Declaration,
266279
d->pstate(),
267280
new_p,
268281
value,
269-
d->is_important());
282+
d->is_important(),
283+
bb);
270284
decl->tabs(d->tabs());
271285
return decl;
272286
}

src/output.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,9 @@ namespace Sass {
118118
for (size_t i = 0, L = b->length(); i < L; ++i) {
119119
Statement* stm = (*b)[i];
120120
if (dynamic_cast<Has_Block*>(stm)) {
121-
stm->perform(this);
121+
if (typeid(*stm) != typeid(Declaration)) {
122+
stm->perform(this);
123+
}
122124
}
123125
}
124126
return;

src/parser.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ namespace Sass {
284284
if (decl->is_indented()) ++ indentation;
285285
// parse a propset that rides on the declaration's property
286286
stack.push_back(Scope::Properties);
287-
(*block) << SASS_MEMORY_NEW(ctx.mem, Propset, pstate, decl->property(), parse_block());
287+
decl->block(parse_block());
288288
stack.pop_back();
289289
if (decl->is_indented()) -- indentation;
290290
}

src/util.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,8 @@ namespace Sass {
470470
Statement* stm = (*b)[i];
471471
if (dynamic_cast<Directive*>(stm)) {
472472
return true;
473+
} else if (Declaration* d = dynamic_cast<Declaration*>(stm)) {
474+
return isPrintable(d, style);
473475
} else if (dynamic_cast<Has_Block*>(stm)) {
474476
Block* pChildBlock = ((Has_Block*)stm)->block();
475477
if (isPrintable(pChildBlock, style)) {
@@ -484,8 +486,6 @@ namespace Sass {
484486
if (c->is_important()) {
485487
hasDeclarations = c->is_important();
486488
}
487-
} else if (Declaration* d = dynamic_cast<Declaration*>(stm)) {
488-
return isPrintable(d, style);
489489
} else {
490490
hasDeclarations = true;
491491
}

0 commit comments

Comments
 (0)