From aff0802d4b22e3d6f1e97c503aa8c5f3dfedd53b Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Fri, 14 Dec 2012 14:13:22 +0200 Subject: [PATCH] Use separate scope for variables within a mixin (#10453) Also contains another fix related to how variable values are replaced in mixins. Change-Id: I7a53e0a62041da6557eea10e124e64a55c7823f4 --- .../vaadin/sass/internal/ScssStylesheet.java | 36 ++++++++++++++++--- .../vaadin/sass/internal/tree/MixinNode.java | 23 ++++++++++-- .../automatic/css/chained_mixins.css | 3 ++ .../automatic/css/mixin_variables.css | 15 ++++++++ .../automatic/scss/chained_mixins.scss | 15 ++++++++ .../automatic/scss/mixin_variables.scss | 10 ++++++ 6 files changed, 94 insertions(+), 8 deletions(-) create mode 100644 theme-compiler/tests/resources/automatic/css/chained_mixins.css create mode 100644 theme-compiler/tests/resources/automatic/css/mixin_variables.css create mode 100644 theme-compiler/tests/resources/automatic/scss/chained_mixins.scss create mode 100644 theme-compiler/tests/resources/automatic/scss/mixin_variables.scss diff --git a/theme-compiler/src/com/vaadin/sass/internal/ScssStylesheet.java b/theme-compiler/src/com/vaadin/sass/internal/ScssStylesheet.java index 2a5d617a6b..6c759167c7 100644 --- a/theme-compiler/src/com/vaadin/sass/internal/ScssStylesheet.java +++ b/theme-compiler/src/com/vaadin/sass/internal/ScssStylesheet.java @@ -20,6 +20,7 @@ import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -217,9 +218,7 @@ public class ScssStylesheet extends Node { node.traverse(); - @SuppressWarnings("unchecked") - HashMap variableScope = (HashMap) variables - .clone(); + Map variableScope = openVariableScope(); // the size of the child list may change on each iteration: current node // may get deleted and possibly other nodes have been inserted where it @@ -232,8 +231,7 @@ public class ScssStylesheet extends Node { } } - variables.clear(); - variables.putAll(variableScope); + closeVariableScope(variableScope); // clean up insert point so that processing of the next block will // insert after that block @@ -248,6 +246,34 @@ public class ScssStylesheet extends Node { } } + /** + * Start a new scope for variables. Any variables set or modified after + * opening a new scope are only valid until the scope is closed, at which + * time they are replaced with their old values. + * + * @return old scope to give to a paired {@link #closeVariableScope(Map)} + * call at the end of the scope (unmodifiable map). + */ + public static Map openVariableScope() { + @SuppressWarnings("unchecked") + HashMap variableScope = (HashMap) variables + .clone(); + return Collections.unmodifiableMap(variableScope); + } + + /** + * End a scope for variables, replacing all active variables with those from + * the original scope (obtained from {@link #openVariableScope()}). + * + * @param originalScope + * original scope + */ + public static void closeVariableScope( + Map originalScope) { + variables.clear(); + variables.putAll(originalScope); + } + public void removeEmptyBlocks(Node node) { // depth first for avoiding re-checking parents of removed nodes for (Node child : new ArrayList(node.getChildren())) { diff --git a/theme-compiler/src/com/vaadin/sass/internal/tree/MixinNode.java b/theme-compiler/src/com/vaadin/sass/internal/tree/MixinNode.java index d92e3ebe9d..9fa7e369c0 100644 --- a/theme-compiler/src/com/vaadin/sass/internal/tree/MixinNode.java +++ b/theme-compiler/src/com/vaadin/sass/internal/tree/MixinNode.java @@ -18,6 +18,7 @@ package com.vaadin.sass.internal.tree; import java.util.ArrayList; import java.util.Collection; +import java.util.Map; import com.vaadin.sass.internal.ScssStylesheet; import com.vaadin.sass.internal.parser.LexicalUnitImpl; @@ -59,14 +60,23 @@ public class MixinNode extends Node implements IVariableNode { this.arglist = arglist; } + /** + * Replace variable references with their values in the mixin argument list. + */ @Override public void replaceVariables(ArrayList variables) { for (final VariableNode var : variables) { for (final LexicalUnitImpl arg : new ArrayList( arglist)) { - if (arg.getLexicalUnitType() == LexicalUnitImpl.SCSS_VARIABLE - && arg.getStringValue().equals(var.getName())) { - arg.replaceValue(var.getExpr()); + LexicalUnitImpl unit = arg; + // only perform replace in the value if separate argument name + // and value + if (unit.getNextLexicalUnit() != null) { + unit = unit.getNextLexicalUnit(); + } + if (unit.getLexicalUnitType() == LexicalUnitImpl.SCSS_VARIABLE + && unit.getStringValue().equals(var.getName())) { + unit.replaceValue(var.getExpr()); } } @@ -86,8 +96,15 @@ public class MixinNode extends Node implements IVariableNode { @Override public void traverse() { try { + // limit variable scope to the mixin + Map variableScope = ScssStylesheet + .openVariableScope(); + replaceVariables(ScssStylesheet.getVariables()); MixinNodeHandler.traverse(this); + + ScssStylesheet.closeVariableScope(variableScope); + } catch (Exception e) { e.printStackTrace(); } diff --git a/theme-compiler/tests/resources/automatic/css/chained_mixins.css b/theme-compiler/tests/resources/automatic/css/chained_mixins.css new file mode 100644 index 0000000000..73a270a1ab --- /dev/null +++ b/theme-compiler/tests/resources/automatic/css/chained_mixins.css @@ -0,0 +1,3 @@ +.bar-link { + a: b; +} diff --git a/theme-compiler/tests/resources/automatic/css/mixin_variables.css b/theme-compiler/tests/resources/automatic/css/mixin_variables.css new file mode 100644 index 0000000000..d879a080b7 --- /dev/null +++ b/theme-compiler/tests/resources/automatic/css/mixin_variables.css @@ -0,0 +1,15 @@ +.foo { + color: purple; +} + +.baz { + color: red; +} + +.foobar { + color: green; +} + +.foobaz { + color: red; +} diff --git a/theme-compiler/tests/resources/automatic/scss/chained_mixins.scss b/theme-compiler/tests/resources/automatic/scss/chained_mixins.scss new file mode 100644 index 0000000000..007d611149 --- /dev/null +++ b/theme-compiler/tests/resources/automatic/scss/chained_mixins.scss @@ -0,0 +1,15 @@ +/* based on reindeer nativebutton */ + +@mixin foo-link($styleName : bar) { + +.#{$styleName}-link { + a: b; + } + +} + +@mixin foo($styleName : bar) { + @include foo-link($styleName); +} + +@include foo; diff --git a/theme-compiler/tests/resources/automatic/scss/mixin_variables.scss b/theme-compiler/tests/resources/automatic/scss/mixin_variables.scss new file mode 100644 index 0000000000..28cde661dd --- /dev/null +++ b/theme-compiler/tests/resources/automatic/scss/mixin_variables.scss @@ -0,0 +1,10 @@ +@mixin color($color : red, $tag : bar) { + .#{$tag} { + color: $color; + } +} +$color : green; +@include color($color: purple, $tag : foo); +@include color($tag : baz); +@include color($color: $color, $tag : foobar); +@include color($tag : foobaz); -- 2.39.5