From f7414a1072094240c0b464ddf8f20d67e7cd88de Mon Sep 17 00:00:00 2001 From: Luke Page Date: Sun, 9 Feb 2014 22:20:08 +0000 Subject: [PATCH 01/14] detached rulesets --- build/build.yml | 1 + lib/less/index.js | 1 + lib/less/parser.js | 59 ++++++++++++++++++++++++-------- lib/less/tree/ruleset-call.js | 15 ++++++++ lib/less/tree/ruleset.js | 13 +++++++ test/css/detached-rulesets.css | 7 ++++ test/less/detached-rulesets.less | 16 +++++++++ 7 files changed, 97 insertions(+), 15 deletions(-) create mode 100644 lib/less/tree/ruleset-call.js create mode 100644 test/css/detached-rulesets.css create mode 100644 test/less/detached-rulesets.less diff --git a/build/build.yml b/build/build.yml index dd4236b99..68fa26075 100644 --- a/build/build.yml +++ b/build/build.yml @@ -143,6 +143,7 @@ tree: - <%= build.lib %>/tree/quoted.js - <%= build.lib %>/tree/rule.js - <%= build.lib %>/tree/ruleset.js + - <%= build.lib %>/tree/ruleset-call.js - <%= build.lib %>/tree/selector.js - <%= build.lib %>/tree/unicode-descriptor.js - <%= build.lib %>/tree/url.js diff --git a/lib/less/index.js b/lib/less/index.js index 7aee17c89..b85a84f94 100644 --- a/lib/less/index.js +++ b/lib/less/index.js @@ -120,6 +120,7 @@ require('./tree/media'); require('./tree/unicode-descriptor'); require('./tree/negative'); require('./tree/extend'); +require('./tree/ruleset-call'); var isUrlRe = /^(?:https?:)?\/\//i; diff --git a/lib/less/parser.js b/lib/less/parser.js index 0f19d487b..04330f7e1 100644 --- a/lib/less/parser.js +++ b/lib/less/parser.js @@ -729,7 +729,7 @@ less.Parser = function Parser(env) { while (current) { node = this.extendRule() || mixin.definition() || this.rule() || this.ruleset() || - mixin.call() || this.comment() || this.directive(); + mixin.call() || this.comment() || this.rulesetCall() || this.directive(); if (node) { root.push(node); } else { @@ -1027,6 +1027,19 @@ less.Parser = function Parser(env) { if (input.charAt(i) === '@' && (name = $re(/^(@[\w-]+)\s*:/))) { return name[1]; } }, + // + // The variable part of a variable definition. Used in the `rule` parser + // + // @fink(); + // + rulesetCall: function () { + var name; + + if (input.charAt(i) === '@' && (name = $re(/^(@[\w-]+)\s*\(\s*\)\s*;/))) { + return new tree.RulesetCall(name[1]); + } + }, + // // extend syntax - used to extend selectors // @@ -1126,7 +1139,7 @@ less.Parser = function Parser(env) { while (true) { if (isCall) { - arg = parsers.expression(); + arg = parsers.blockRuleset() || parsers.expression(); } else { parsers.comments(); if (input.charAt(i) === '.' && $re(/^\.{3}/)) { @@ -1154,7 +1167,7 @@ less.Parser = function Parser(env) { if (isCall) { // Variable - if (arg.value.length == 1) { + if (arg.value && arg.value.length == 1) { val = arg.value[0]; } } else { @@ -1442,6 +1455,14 @@ less.Parser = function Parser(env) { } }, + blockRuleset: function() { + var block = this.block(); + if (block) { + block = new tree.Ruleset(null, block); + } + return block; + }, + // // div, .class, body > p {...} // @@ -1484,25 +1505,33 @@ less.Parser = function Parser(env) { } }, rule: function (tryAnonymous) { - var name, value, c = input.charAt(i), important, merge; + var name, value, c = input.charAt(i), important, merge, isVariable; save(); if (c === '.' || c === '#' || c === '&') { return; } name = this.variable() || this.ruleProperty(); if (name) { - // prefer to try to parse first if its a variable or we are compressing - // but always fallback on the other one - value = !tryAnonymous && (env.compress || (name.charAt && (name.charAt(0) === '@'))) ? - (this.value() || this.anonymousValue()) : - (this.anonymousValue() || this.value()); - - important = this.important(); + isVariable = typeof name === "string"; - // a name returned by this.ruleProperty() is always an array of the form: - // [string-1, ..., string-n, ""] or [string-1, ..., string-n, "+"] - // where each item is a tree.Keyword or tree.Variable - merge = name.pop && name.pop().value; + if (isVariable) { + value = this.blockRuleset(); + } + + if (!value) { + // prefer to try to parse first if its a variable or we are compressing + // but always fallback on the other one + value = !tryAnonymous && (env.compress || isVariable) ? + (this.value() || this.anonymousValue()) : + (this.anonymousValue() || this.value()); + + important = this.important(); + + // a name returned by this.ruleProperty() is always an array of the form: + // [string-1, ..., string-n, ""] or [string-1, ..., string-n, "+"] + // where each item is a tree.Keyword or tree.Variable + merge = !isVariable && name.pop().value; + } if (value && this.end()) { return new (tree.Rule)(name, value, important, merge, memo, env.currentFileInfo); diff --git a/lib/less/tree/ruleset-call.js b/lib/less/tree/ruleset-call.js new file mode 100644 index 000000000..ec9e98d2c --- /dev/null +++ b/lib/less/tree/ruleset-call.js @@ -0,0 +1,15 @@ +(function (tree) { + +tree.RulesetCall = function (variable) { + this.variable = variable; +}; +tree.RulesetCall.prototype = { + type: "RulesetCall", + accept: function (visitor) { + }, + eval: function (env) { + return new(tree.Variable)(this.variable).eval(env); + } +}; + +})(require('../tree')); diff --git a/lib/less/tree/ruleset.js b/lib/less/tree/ruleset.js index e7895760d..15cc80d9b 100644 --- a/lib/less/tree/ruleset.js +++ b/lib/less/tree/ruleset.js @@ -90,6 +90,19 @@ tree.Ruleset.prototype = { rsRuleCnt += rules.length - 1; i += rules.length-1; ruleset.resetCache(); + } else if (rsRules[i] instanceof tree.RulesetCall) { + /*jshint loopfunc:true */ + rules = rsRules[i].eval(env).rules.filter(function(r) { + if ((r instanceof tree.Rule) && r.variable) { + // do not pollute the scope at all + return false; + } + return true; + }); + rsRules.splice.apply(rsRules, [i, 1].concat(rules)); + rsRuleCnt += rules.length - 1; + i += rules.length-1; + ruleset.resetCache(); } } diff --git a/test/css/detached-rulesets.css b/test/css/detached-rulesets.css new file mode 100644 index 000000000..08e82bc5d --- /dev/null +++ b/test/css/detached-rulesets.css @@ -0,0 +1,7 @@ +.wrap-selector { + color: black; +} +.wrap-selector { + color: black; + background: white; +} diff --git a/test/less/detached-rulesets.less b/test/less/detached-rulesets.less new file mode 100644 index 000000000..493607634 --- /dev/null +++ b/test/less/detached-rulesets.less @@ -0,0 +1,16 @@ +@ruleset: { + color: black; + background: white; + }; + +.wrap-mixin(@ruleset) { + .wrap-selector { + @ruleset(); + } +}; + +.wrap-mixin({ + color: black; +}); + +.wrap-mixin(@ruleset); \ No newline at end of file From 55033c77ed7942d5223b07b90e5df119fc5ea3dd Mon Sep 17 00:00:00 2001 From: Luke Page Date: Tue, 11 Feb 2014 22:01:26 +0000 Subject: [PATCH 02/14] more tests and name arguments for caller --- lib/less/parser.js | 18 ++++++++++++++++-- test/css/detached-rulesets.css | 14 ++++++++++++++ test/less/detached-rulesets.less | 19 ++++++++++++++++++- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/lib/less/parser.js b/lib/less/parser.js index 4fa31f586..76f312e5c 100644 --- a/lib/less/parser.js +++ b/lib/less/parser.js @@ -1182,7 +1182,17 @@ less.Parser = function Parser(env) { } expressionContainsNamed = true; } - value = expect(parsers.expression); + // we do not support setting a ruleset as a default variable - it doesn't make sense + // and to implement it we need backtracking with multiple saves + value = (isCall && parsers.blockRuleset()) || parsers.expression(); + if (!value) { + if (isCall) { + error("could not understand value for named argument"); + } else { + returner.args = []; + return returner; + } + } nameLoop = (name = val.name); } else if (!isCall && $re(/^\.{3}/)) { returner.variadic = true; @@ -1267,10 +1277,14 @@ less.Parser = function Parser(env) { variadic = argInfo.variadic; // .mixincall("@{a}"); - // looks a bit like a mixin definition.. so we have to be nice and restore + // looks a bit like a mixin definition.. + // also + // .mixincall(@a: {rule: set;}); + // so we have to be nice and restore if (!$char(')')) { furthest = i; restore(); + return; } parsers.comments(); diff --git a/test/css/detached-rulesets.css b/test/css/detached-rulesets.css index 08e82bc5d..dc706a626 100644 --- a/test/css/detached-rulesets.css +++ b/test/css/detached-rulesets.css @@ -1,7 +1,21 @@ .wrap-selector { color: black; } +.wrap-selector { + color: red; +} .wrap-selector { color: black; background: white; } +header { + background: blue; +} +@media screen and (min-width: 1200) { + header { + background: red; + } +} +html.lt-ie9 header { + background: red; +} diff --git a/test/less/detached-rulesets.less b/test/less/detached-rulesets.less index 493607634..01abdd7e0 100644 --- a/test/less/detached-rulesets.less +++ b/test/less/detached-rulesets.less @@ -13,4 +13,21 @@ color: black; }); -.wrap-mixin(@ruleset); \ No newline at end of file +.wrap-mixin(@ruleset: { + color: red; +}); + +.wrap-mixin(@ruleset); + +.desktop-and-old-ie(@rules) { + @media screen and (min-width: 1200) { @rules(); } + html.lt-ie9 & { @rules(); } +} + +header { + background: blue; + + .desktop-and-old-ie({ + background: red; + }); +} From 7f26515630dddc31cf95c3cc198f51acccdff230 Mon Sep 17 00:00:00 2001 From: Luke Page Date: Tue, 11 Feb 2014 22:02:48 +0000 Subject: [PATCH 03/14] small simplification --- lib/less/parser.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/less/parser.js b/lib/less/parser.js index 76f312e5c..064e5f27b 100644 --- a/lib/less/parser.js +++ b/lib/less/parser.js @@ -1785,10 +1785,7 @@ less.Parser = function Parser(env) { } if (hasBlock) { - rules = this.block(); - if (rules) { - rules = new(tree.Ruleset)(null, rules); - } + rules = this.blockRuleset(); } if (rules || (!hasBlock && value && $char(';'))) { From ed0e13a0ecd2f2bb31d2874c61d351c54803245d Mon Sep 17 00:00:00 2001 From: Luke Page Date: Wed, 12 Feb 2014 22:34:58 +0000 Subject: [PATCH 04/14] do not chunk inside parens so that we can predict mixin calls containing detached rulesets. Also now save the current chunk (may not be required). --- lib/less/parser.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/less/parser.js b/lib/less/parser.js index 064e5f27b..fc3c1fe98 100644 --- a/lib/less/parser.js +++ b/lib/less/parser.js @@ -45,6 +45,7 @@ less.Parser = function Parser(env) { j, // current chunk temp, // temporarily holds a chunk's state, for backtracking memo, // temporarily holds `i`, when backtracking + memoChunk, // `j` furthest, // furthest index the parser has gone to chunks, // chunkified input current, // current chunk @@ -111,8 +112,8 @@ less.Parser = function Parser(env) { } }; - function save() { temp = current; memo = currentPos = i; } - function restore() { current = temp; currentPos = i = memo; } + function save() { temp = current; memo = currentPos = i; memoChunk = j; } + function restore() { current = temp; currentPos = i = memo; j = memoChunk; } function sync() { if (i > currentPos) { @@ -424,7 +425,7 @@ less.Parser = function Parser(env) { if (--level < 0) { return fail("missing opening `{`"); } - if (!level) { emitChunk(); } + if (!level && !parenLevel) { emitChunk(); } continue; case 92: // \ if (parserCurrentIndex < len - 1) { parserCurrentIndex++; continue; } From e3168e343460539556a86023833d1a033e013cba Mon Sep 17 00:00:00 2001 From: Luke Page Date: Wed, 12 Feb 2014 22:40:20 +0000 Subject: [PATCH 05/14] add some more failing tests --- test/less/detached-rulesets.less | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/less/detached-rulesets.less b/test/less/detached-rulesets.less index 01abdd7e0..665fdfbad 100644 --- a/test/less/detached-rulesets.less +++ b/test/less/detached-rulesets.less @@ -31,3 +31,18 @@ header { background: red; }); } + +.wrap-mixin-calls-wrap(@ruleset) { + .wrap-mixin(@ruleset); +}; + +.wrap-mixin({ + test: extra-wrap; + .wrap-mixin-calls-wrap({ + test: wrapped-twice; + }); +}); + +.wrap-mixin({ + test-func: unit(90px); +}); From e3576b9c01799ee7e033b7413c8ce1b9bdfbe805 Mon Sep 17 00:00:00 2001 From: Luke Page Date: Wed, 12 Feb 2014 23:10:52 +0000 Subject: [PATCH 06/14] implement n level back-tracking and then don't absorb a parenthesis, fixing both issues with 2 new test cases --- lib/less/parser.js | 26 +++++++++++++++++++------- test/css/detached-rulesets.css | 10 ++++++++++ test/less/detached-rulesets.less | 1 + 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/lib/less/parser.js b/lib/less/parser.js index fc3c1fe98..8231bf838 100644 --- a/lib/less/parser.js +++ b/lib/less/parser.js @@ -43,9 +43,7 @@ less.Parser = function Parser(env) { var input, // LeSS input string i, // current index in `input` j, // current chunk - temp, // temporarily holds a chunk's state, for backtracking - memo, // temporarily holds `i`, when backtracking - memoChunk, // `j` + saveStack = [], // holds state for backtracking furthest, // furthest index the parser has gone to chunks, // chunkified input current, // current chunk @@ -112,8 +110,9 @@ less.Parser = function Parser(env) { } }; - function save() { temp = current; memo = currentPos = i; memoChunk = j; } - function restore() { current = temp; currentPos = i = memo; j = memoChunk; } + function save() { currentPos = i; saveStack.push( { current: current, i: i, j: j }); } + function restore() { var state = saveStack.pop(); current = state.current; currentPos = i = state.i; j = state.j; } + function forget() { saveStack.pop(); } function sync() { if (i > currentPos) { @@ -1126,6 +1125,7 @@ less.Parser = function Parser(env) { } if (parsers.end()) { + forget(); return new(tree.mixin.Call)(elements, args, index, env.currentFileInfo, important); } } @@ -1297,6 +1297,7 @@ less.Parser = function Parser(env) { ruleset = parsers.block(); if (ruleset) { + forget(); return new(tree.mixin.Definition)(name, params, ruleset, cond, variadic); } else { restore(); @@ -1364,10 +1365,16 @@ less.Parser = function Parser(env) { this.entities.variableCurly(); if (! e) { + save(); if ($char('(')) { if ((v = this.selector()) && $char(')')) { e = new(tree.Paren)(v); + forget(); + } else { + restore(); } + } else { + forget(); } } @@ -1508,6 +1515,7 @@ less.Parser = function Parser(env) { } if (selectors && (rules = this.block())) { + forget(); var ruleset = new(tree.Ruleset)(selectors, rules, env.strictImports); if (env.dumpLineNumbers) { ruleset.debugInfo = debugInfo; @@ -1520,7 +1528,7 @@ less.Parser = function Parser(env) { } }, rule: function (tryAnonymous) { - var name, value, c = input.charAt(i), important, merge, isVariable; + var name, value, startOfRule = i, c = input.charAt(startOfRule), important, merge, isVariable; save(); if (c === '.' || c === '#' || c === '&') { return; } @@ -1549,7 +1557,7 @@ less.Parser = function Parser(env) { } if (value && this.end()) { - return new (tree.Rule)(name, value, important, merge, memo, env.currentFileInfo); + return new (tree.Rule)(name, value, important, merge, startOfRule, env.currentFileInfo); } else { furthest = i; restore(); @@ -1557,6 +1565,8 @@ less.Parser = function Parser(env) { return this.rule(true); } } + } else { + forget(); } }, anonymousValue: function () { @@ -1590,6 +1600,7 @@ less.Parser = function Parser(env) { if (dir && (path = this.entities.quoted() || this.entities.url())) { features = this.mediaFeatures(); if ($char(';')) { + forget(); features = features && new(tree.Value)(features); return new(tree.Import)(path, features, options, index, env.currentFileInfo); } @@ -1790,6 +1801,7 @@ less.Parser = function Parser(env) { } if (rules || (!hasBlock && value && $char(';'))) { + forget(); return new(tree.Directive)(name, value, rules, index, env.currentFileInfo, env.dumpLineNumbers ? getDebugInfo(index, input, env) : null); } diff --git a/test/css/detached-rulesets.css b/test/css/detached-rulesets.css index dc706a626..e6b646a45 100644 --- a/test/css/detached-rulesets.css +++ b/test/css/detached-rulesets.css @@ -19,3 +19,13 @@ header { html.lt-ie9 header { background: red; } +.wrap-selector { + test: extra-wrap; +} +.wrap-selector .wrap-selector { + test: wrapped-twice; +} +.wrap-selector { + test-func: 90; + test-arithmetic: 18px; +} diff --git a/test/less/detached-rulesets.less b/test/less/detached-rulesets.less index 665fdfbad..3829f681d 100644 --- a/test/less/detached-rulesets.less +++ b/test/less/detached-rulesets.less @@ -45,4 +45,5 @@ header { .wrap-mixin({ test-func: unit(90px); + test-arithmetic: unit((9+9), px); }); From ef3c63fb9ae920f1e969b707ba296ac6d930e579 Mon Sep 17 00:00:00 2001 From: Luke Page Date: Wed, 12 Feb 2014 23:34:14 +0000 Subject: [PATCH 07/14] Test out some theoretical back tracking and fix bugs --- lib/less/parser.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/less/parser.js b/lib/less/parser.js index 8231bf838..46c74cfc4 100644 --- a/lib/less/parser.js +++ b/lib/less/parser.js @@ -1138,6 +1138,8 @@ less.Parser = function Parser(env) { expressions = [], argsSemiColon = [], argsComma = [], isSemiColonSeperated, expressionContainsNamed, name, nameLoop, value, arg; + save(); + while (true) { if (isCall) { arg = parsers.blockRuleset() || parsers.expression(); @@ -1183,13 +1185,17 @@ less.Parser = function Parser(env) { } expressionContainsNamed = true; } + // we do not support setting a ruleset as a default variable - it doesn't make sense - // and to implement it we need backtracking with multiple saves + // However if we do want to add it, there is nothing blocking it, just don't error + // and remove isCall dependency below value = (isCall && parsers.blockRuleset()) || parsers.expression(); + if (!value) { if (isCall) { error("could not understand value for named argument"); } else { + restore(); returner.args = []; return returner; } @@ -1238,6 +1244,7 @@ less.Parser = function Parser(env) { } } + forget(); returner.args = isSemiColonSeperated ? argsSemiColon : argsComma; return returner; }, @@ -1302,6 +1309,8 @@ less.Parser = function Parser(env) { } else { restore(); } + } else { + forget(); } } }, @@ -1529,10 +1538,11 @@ less.Parser = function Parser(env) { }, rule: function (tryAnonymous) { var name, value, startOfRule = i, c = input.charAt(startOfRule), important, merge, isVariable; - save(); if (c === '.' || c === '#' || c === '&') { return; } + save(); + name = this.variable() || this.ruleProperty(); if (name) { isVariable = typeof name === "string"; @@ -1557,6 +1567,7 @@ less.Parser = function Parser(env) { } if (value && this.end()) { + forget(); return new (tree.Rule)(name, value, important, merge, startOfRule, env.currentFileInfo); } else { furthest = i; From 15174c0860010be3969fed3008b2c79ce2cae8ad Mon Sep 17 00:00:00 2001 From: Luke Page Date: Wed, 12 Feb 2014 23:38:35 +0000 Subject: [PATCH 08/14] small approx 1% speed improvement --- lib/less/parser.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/less/parser.js b/lib/less/parser.js index 46c74cfc4..7e0c4a224 100644 --- a/lib/less/parser.js +++ b/lib/less/parser.js @@ -737,6 +737,9 @@ less.Parser = function Parser(env) { break; } } + if (peekChar('}')) { + break; + } } return root; From e0692fa199fc6828f0136da05948cb2c0194b611 Mon Sep 17 00:00:00 2001 From: Luke Page Date: Thu, 13 Feb 2014 20:36:34 +0000 Subject: [PATCH 09/14] add scope tests to the detached ruleset test-set --- test/css/detached-rulesets.css | 13 +++++++++++++ test/less/detached-rulesets.less | 9 +++++++++ 2 files changed, 22 insertions(+) diff --git a/test/css/detached-rulesets.css b/test/css/detached-rulesets.css index e6b646a45..b98bf60b0 100644 --- a/test/css/detached-rulesets.css +++ b/test/css/detached-rulesets.css @@ -1,12 +1,19 @@ .wrap-selector { color: black; + one: 1px; + visible-one: visible; + visible-two: visible; } .wrap-selector { color: red; + visible-one: visible; + visible-two: visible; } .wrap-selector { color: black; background: white; + visible-one: visible; + visible-two: visible; } header { background: blue; @@ -21,11 +28,17 @@ html.lt-ie9 header { } .wrap-selector { test: extra-wrap; + visible-one: visible; + visible-two: visible; } .wrap-selector .wrap-selector { test: wrapped-twice; + visible-one: visible; + visible-two: visible; } .wrap-selector { test-func: 90; test-arithmetic: 18px; + visible-one: visible; + visible-two: visible; } diff --git a/test/less/detached-rulesets.less b/test/less/detached-rulesets.less index 3829f681d..57c2cb437 100644 --- a/test/less/detached-rulesets.less +++ b/test/less/detached-rulesets.less @@ -3,14 +3,23 @@ background: white; }; +@a: 1px; .wrap-mixin(@ruleset) { + @a: hidden and if you see this in the output its a bug; + @b: visible; .wrap-selector { + @c: visible; @ruleset(); + visible-one: @b; + visible-two: @c; } }; .wrap-mixin({ color: black; + one: @a; + @b: hidden and if you see this in the output its a bug; + @c: hidden and if you see this in the output its a bug; }); .wrap-mixin(@ruleset: { From b46ca11286e3c4fb8eb351ff01725cc9c4223add Mon Sep 17 00:00:00 2001 From: Luke Page Date: Thu, 13 Feb 2014 21:42:32 +0000 Subject: [PATCH 10/14] error tests and test detached rulesets without a mixin call --- lib/less/tree/rule.js | 17 ++++++++++++++--- test/css/detached-rulesets.css | 3 +++ test/less/detached-rulesets.less | 7 +++++++ test/less/errors/detached-ruleset-1.less | 6 ++++++ test/less/errors/detached-ruleset-1.txt | 4 ++++ test/less/errors/detached-ruleset-2.less | 6 ++++++ test/less/errors/detached-ruleset-2.txt | 4 ++++ test/less/errors/detached-ruleset-3.less | 4 ++++ test/less/errors/detached-ruleset-3.txt | 4 ++++ test/less/errors/detached-ruleset-4.less | 5 +++++ test/less/errors/detached-ruleset-4.txt | 3 +++ test/less/errors/detached-ruleset-5.less | 4 ++++ test/less/errors/detached-ruleset-5.txt | 3 +++ test/less/errors/detached-ruleset-6.less | 5 +++++ test/less/errors/detached-ruleset-6.txt | 4 ++++ 15 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 test/less/errors/detached-ruleset-1.less create mode 100644 test/less/errors/detached-ruleset-1.txt create mode 100644 test/less/errors/detached-ruleset-2.less create mode 100644 test/less/errors/detached-ruleset-2.txt create mode 100644 test/less/errors/detached-ruleset-3.less create mode 100644 test/less/errors/detached-ruleset-3.txt create mode 100644 test/less/errors/detached-ruleset-4.less create mode 100644 test/less/errors/detached-ruleset-4.txt create mode 100644 test/less/errors/detached-ruleset-5.less create mode 100644 test/less/errors/detached-ruleset-5.txt create mode 100644 test/less/errors/detached-ruleset-6.less create mode 100644 test/less/errors/detached-ruleset-6.txt diff --git a/lib/less/tree/rule.js b/lib/less/tree/rule.js index a2d14f2b6..ac90049c6 100644 --- a/lib/less/tree/rule.js +++ b/lib/less/tree/rule.js @@ -30,7 +30,7 @@ tree.Rule.prototype = { }, toCSS: tree.toCSS, eval: function (env) { - var strictMathBypass = false, name = this.name; + var strictMathBypass = false, name = this.name, evaldValue; if (typeof name !== "string") { // expand 'primitive' name directly to get // things faster (~10% for benchmark.less): @@ -43,14 +43,25 @@ tree.Rule.prototype = { env.strictMath = true; } try { + evaldValue = this.value.eval(env); + + if (!this.variable && evaldValue.type === "Ruleset") { + console.log(this.index); + throw { message: "Rulesets cannot be evaluated on a property.", + index: this.index, filename: this.currentFileInfo.filename }; + } + return new(tree.Rule)(name, - this.value.eval(env), + evaldValue, this.important, this.merge, this.index, this.currentFileInfo, this.inline); } catch(e) { - e.index = e.index || this.index; + if (typeof e.index !== 'number') { + e.index = this.index; + e.filename = this.currentFileInfo.filename; + } throw e; } finally { diff --git a/test/css/detached-rulesets.css b/test/css/detached-rulesets.css index b98bf60b0..d723a0c4b 100644 --- a/test/css/detached-rulesets.css +++ b/test/css/detached-rulesets.css @@ -42,3 +42,6 @@ html.lt-ie9 header { visible-one: visible; visible-two: visible; } +.without-mixins { + b: 1; +} diff --git a/test/less/detached-rulesets.less b/test/less/detached-rulesets.less index 57c2cb437..787cc19bc 100644 --- a/test/less/detached-rulesets.less +++ b/test/less/detached-rulesets.less @@ -56,3 +56,10 @@ header { test-func: unit(90px); test-arithmetic: unit((9+9), px); }); +// without mixins +@ruleset-2: { + b: 1; +}; +.without-mixins { + @ruleset-2(); +} diff --git a/test/less/errors/detached-ruleset-1.less b/test/less/errors/detached-ruleset-1.less new file mode 100644 index 000000000..ac5b8db03 --- /dev/null +++ b/test/less/errors/detached-ruleset-1.less @@ -0,0 +1,6 @@ +@a: { + b: 1; +}; +.a { + a: @a; +} \ No newline at end of file diff --git a/test/less/errors/detached-ruleset-1.txt b/test/less/errors/detached-ruleset-1.txt new file mode 100644 index 000000000..7407741c8 --- /dev/null +++ b/test/less/errors/detached-ruleset-1.txt @@ -0,0 +1,4 @@ +SyntaxError: Rulesets cannot be evaluated on a property. in {path}detached-ruleset-1.less on line 5, column 3: +4 .a { +5 a: @a; +6 } diff --git a/test/less/errors/detached-ruleset-2.less b/test/less/errors/detached-ruleset-2.less new file mode 100644 index 000000000..51a7af6be --- /dev/null +++ b/test/less/errors/detached-ruleset-2.less @@ -0,0 +1,6 @@ +@a: { + b: 1; +}; +.a { + a: @a(); +} \ No newline at end of file diff --git a/test/less/errors/detached-ruleset-2.txt b/test/less/errors/detached-ruleset-2.txt new file mode 100644 index 000000000..f18e09353 --- /dev/null +++ b/test/less/errors/detached-ruleset-2.txt @@ -0,0 +1,4 @@ +ParseError: Unrecognised input in {path}detached-ruleset-2.less on line 5, column 3: +4 .a { +5 a: @a(); +6 } diff --git a/test/less/errors/detached-ruleset-3.less b/test/less/errors/detached-ruleset-3.less new file mode 100644 index 000000000..c50119d99 --- /dev/null +++ b/test/less/errors/detached-ruleset-3.less @@ -0,0 +1,4 @@ +@a: { + b: 1; +}; +@a(); \ No newline at end of file diff --git a/test/less/errors/detached-ruleset-3.txt b/test/less/errors/detached-ruleset-3.txt new file mode 100644 index 000000000..15d281fa3 --- /dev/null +++ b/test/less/errors/detached-ruleset-3.txt @@ -0,0 +1,4 @@ +SyntaxError: properties must be inside selector blocks, they cannot be in the root. in {path}detached-ruleset-3.less on line 2, column 3: +1 @a: { +2 b: 1; +3 }; diff --git a/test/less/errors/detached-ruleset-4.less b/test/less/errors/detached-ruleset-4.less new file mode 100644 index 000000000..14ac314bb --- /dev/null +++ b/test/less/errors/detached-ruleset-4.less @@ -0,0 +1,5 @@ +.mixin-definition(@a: { + b: 1; +}) { + @a(); +} \ No newline at end of file diff --git a/test/less/errors/detached-ruleset-4.txt b/test/less/errors/detached-ruleset-4.txt new file mode 100644 index 000000000..d6d6526d4 --- /dev/null +++ b/test/less/errors/detached-ruleset-4.txt @@ -0,0 +1,3 @@ +ParseError: Unrecognised input in {path}detached-ruleset-4.less on line 1, column 18: +1 .mixin-definition(@a: { +2 b: 1; diff --git a/test/less/errors/detached-ruleset-5.less b/test/less/errors/detached-ruleset-5.less new file mode 100644 index 000000000..174ebf354 --- /dev/null +++ b/test/less/errors/detached-ruleset-5.less @@ -0,0 +1,4 @@ +.mixin-definition(@b) { + @a(); +} +.mixin-definition({color: red;}); \ No newline at end of file diff --git a/test/less/errors/detached-ruleset-5.txt b/test/less/errors/detached-ruleset-5.txt new file mode 100644 index 000000000..561897950 --- /dev/null +++ b/test/less/errors/detached-ruleset-5.txt @@ -0,0 +1,3 @@ +SyntaxError: variable @a is undefined in {path}detached-ruleset-5.less on line 4, column 1: +3 } +4 .mixin-definition({color: red;}); diff --git a/test/less/errors/detached-ruleset-6.less b/test/less/errors/detached-ruleset-6.less new file mode 100644 index 000000000..121099f75 --- /dev/null +++ b/test/less/errors/detached-ruleset-6.less @@ -0,0 +1,5 @@ +.a { + b: { + color: red; + }; +} \ No newline at end of file diff --git a/test/less/errors/detached-ruleset-6.txt b/test/less/errors/detached-ruleset-6.txt new file mode 100644 index 000000000..07840445e --- /dev/null +++ b/test/less/errors/detached-ruleset-6.txt @@ -0,0 +1,4 @@ +ParseError: Unrecognised input in {path}detached-ruleset-6.less on line 2, column 3: +1 .a { +2 b: { +3 color: red; From c730829d1d609f250b522775fa1745537f6b77a4 Mon Sep 17 00:00:00 2001 From: Luke Page Date: Sun, 16 Feb 2014 17:50:51 +0000 Subject: [PATCH 11/14] Fix one issue with media queries and detached rulesets, one to go --- lib/less/tree/media.js | 2 ++ lib/less/tree/rule.js | 3 +-- test/css/detached-rulesets.css | 17 +++++++++++++ test/less/detached-rulesets.less | 41 ++++++++++++++++++++++++++++++-- 4 files changed, 59 insertions(+), 4 deletions(-) diff --git a/lib/less/tree/media.js b/lib/less/tree/media.js index c3b85107e..4531cf4cd 100644 --- a/lib/less/tree/media.js +++ b/lib/less/tree/media.js @@ -147,6 +147,8 @@ tree.Media.prototype = { } }, bubbleSelectors: function (selectors) { + if (!selectors) + return; this.rules = [new(tree.Ruleset)(selectors.slice(0), [this.rules[0]])]; } }; diff --git a/lib/less/tree/rule.js b/lib/less/tree/rule.js index ac90049c6..5567b0764 100644 --- a/lib/less/tree/rule.js +++ b/lib/less/tree/rule.js @@ -2,7 +2,7 @@ tree.Rule = function (name, value, important, merge, index, currentFileInfo, inline) { this.name = name; - this.value = (value instanceof tree.Value) ? value : new(tree.Value)([value]); + this.value = (value instanceof tree.Value || value instanceof tree.Ruleset) ? value : new(tree.Value)([value]); this.important = important ? ' ' + important.trim() : ''; this.merge = merge; this.index = index; @@ -46,7 +46,6 @@ tree.Rule.prototype = { evaldValue = this.value.eval(env); if (!this.variable && evaldValue.type === "Ruleset") { - console.log(this.index); throw { message: "Rulesets cannot be evaluated on a property.", index: this.index, filename: this.currentFileInfo.filename }; } diff --git a/test/css/detached-rulesets.css b/test/css/detached-rulesets.css index d723a0c4b..b9642d4eb 100644 --- a/test/css/detached-rulesets.css +++ b/test/css/detached-rulesets.css @@ -45,3 +45,20 @@ html.lt-ie9 header { .without-mixins { b: 1; } +@media (orientation: portrait) { + /* .wrap-media-mixin({ + @media tv { + .triple-wrapped-mq { + triple: true; + } + } + });*/ +} +@media (orientation: portrait) and tv { + .my-selector { + background-color: black; + } +} +.a { + test: test; +} diff --git a/test/less/detached-rulesets.less b/test/less/detached-rulesets.less index 787cc19bc..5af2a745e 100644 --- a/test/less/detached-rulesets.less +++ b/test/less/detached-rulesets.less @@ -10,8 +10,8 @@ .wrap-selector { @c: visible; @ruleset(); - visible-one: @b; - visible-two: @c; + visible-one: @b; + visible-two: @c; } }; @@ -63,3 +63,40 @@ header { .without-mixins { @ruleset-2(); } +@my-ruleset: { + .my-selector { + @media tv { + background-color: black; + } + } + }; +@media (orientation:portrait) { + @my-ruleset(); + // doesn't work yet +/* .wrap-media-mixin({ + @media tv { + .triple-wrapped-mq { + triple: true; + } + } + });*/ +} +.wrap-media-mixin(@ruleset) { + @media widescreen { + @media print { + @ruleset(); + } + @ruleset(); + } + @ruleset(); +} +// unlocking mixins +@my-mixins: { + .mixin() { + test: test; + } +}; +@my-mixins(); +.a { + .mixin(); +} \ No newline at end of file From baba33ea6a1a4b9a42fa07870c14581a78e05559 Mon Sep 17 00:00:00 2001 From: Luke Page Date: Mon, 17 Feb 2014 19:15:47 +0000 Subject: [PATCH 12/14] Fix some bugs with detached rulesets and media queries --- build/build.yml | 1 + lib/less/index.js | 1 + lib/less/parser.js | 14 +++++++++++--- lib/less/tree/detached-ruleset.js | 21 +++++++++++++++++++++ lib/less/tree/rule.js | 2 +- lib/less/tree/ruleset-call.js | 3 ++- lib/less/tree/ruleset.js | 3 +++ test/css/detached-rulesets.css | 25 ++++++++++++++++--------- test/less/detached-rulesets.less | 10 ++++++---- 9 files changed, 62 insertions(+), 18 deletions(-) create mode 100644 lib/less/tree/detached-ruleset.js diff --git a/build/build.yml b/build/build.yml index 68fa26075..b5a2234a8 100644 --- a/build/build.yml +++ b/build/build.yml @@ -127,6 +127,7 @@ tree: - <%= build.lib %>/tree/color.js - <%= build.lib %>/tree/comment.js - <%= build.lib %>/tree/condition.js + - <%= build.lib %>/tree/detached-ruleset.js - <%= build.lib %>/tree/dimension.js - <%= build.lib %>/tree/directive.js - <%= build.lib %>/tree/element.js diff --git a/lib/less/index.js b/lib/less/index.js index b85a84f94..a18b81790 100644 --- a/lib/less/index.js +++ b/lib/less/index.js @@ -94,6 +94,7 @@ var less = { require('./tree/color'); require('./tree/directive'); +require('./tree/detached-ruleset'); require('./tree/operation'); require('./tree/dimension'); require('./tree/keyword'); diff --git a/lib/less/parser.js b/lib/less/parser.js index 7e0c4a224..032381dc2 100644 --- a/lib/less/parser.js +++ b/lib/less/parser.js @@ -1145,7 +1145,7 @@ less.Parser = function Parser(env) { while (true) { if (isCall) { - arg = parsers.blockRuleset() || parsers.expression(); + arg = parsers.detachedRuleset() || parsers.expression(); } else { parsers.comments(); if (input.charAt(i) === '.' && $re(/^\.{3}/)) { @@ -1192,7 +1192,7 @@ less.Parser = function Parser(env) { // we do not support setting a ruleset as a default variable - it doesn't make sense // However if we do want to add it, there is nothing blocking it, just don't error // and remove isCall dependency below - value = (isCall && parsers.blockRuleset()) || parsers.expression(); + value = (isCall && parsers.detachedRuleset()) || parsers.expression(); if (!value) { if (isCall) { @@ -1491,11 +1491,19 @@ less.Parser = function Parser(env) { blockRuleset: function() { var block = this.block(); + if (block) { block = new tree.Ruleset(null, block); } return block; }, + + detachedRuleset: function() { + var blockRuleset = this.blockRuleset(); + if (blockRuleset) { + return new tree.DetachedRuleset(blockRuleset); + } + }, // // div, .class, body > p {...} @@ -1551,7 +1559,7 @@ less.Parser = function Parser(env) { isVariable = typeof name === "string"; if (isVariable) { - value = this.blockRuleset(); + value = this.detachedRuleset(); } if (!value) { diff --git a/lib/less/tree/detached-ruleset.js b/lib/less/tree/detached-ruleset.js new file mode 100644 index 000000000..87e76f26c --- /dev/null +++ b/lib/less/tree/detached-ruleset.js @@ -0,0 +1,21 @@ +(function (tree) { + +tree.DetachedRuleset = function (ruleset, frames) { + this.ruleset = ruleset; + this.frames = frames; +}; +tree.DetachedRuleset.prototype = { + type: "DetachedRuleset", + accept: function (visitor) { + this.ruleset = visitor.visit(this.ruleset); + }, + eval: function (env) { + // TODO - handle mixin definition like this + var frames = this.frames || env.frames.slice(0); + return new tree.DetachedRuleset(this.ruleset, frames); + }, + callEval: function (env) { + return this.ruleset.eval(new(tree.evalEnv)(env, this.frames.concat(env.frames))); + } +}; +})(require('../tree')); diff --git a/lib/less/tree/rule.js b/lib/less/tree/rule.js index 5567b0764..4e1ccde69 100644 --- a/lib/less/tree/rule.js +++ b/lib/less/tree/rule.js @@ -45,7 +45,7 @@ tree.Rule.prototype = { try { evaldValue = this.value.eval(env); - if (!this.variable && evaldValue.type === "Ruleset") { + if (!this.variable && evaldValue.type === "DetachedRuleset") { throw { message: "Rulesets cannot be evaluated on a property.", index: this.index, filename: this.currentFileInfo.filename }; } diff --git a/lib/less/tree/ruleset-call.js b/lib/less/tree/ruleset-call.js index ec9e98d2c..a543c55e5 100644 --- a/lib/less/tree/ruleset-call.js +++ b/lib/less/tree/ruleset-call.js @@ -8,7 +8,8 @@ tree.RulesetCall.prototype = { accept: function (visitor) { }, eval: function (env) { - return new(tree.Variable)(this.variable).eval(env); + var detachedRuleset = new(tree.Variable)(this.variable).eval(env); + return detachedRuleset.callEval(env); } }; diff --git a/lib/less/tree/ruleset.js b/lib/less/tree/ruleset.js index 15cc80d9b..3ec0706d2 100644 --- a/lib/less/tree/ruleset.js +++ b/lib/less/tree/ruleset.js @@ -358,6 +358,9 @@ tree.Ruleset.prototype = { toCSS: tree.toCSS, markReferenced: function () { + if (!this.selectors) { + return; + } for (var s = 0; s < this.selectors.length; s++) { this.selectors[s].markReferenced(); } diff --git a/test/css/detached-rulesets.css b/test/css/detached-rulesets.css index b9642d4eb..300c08d09 100644 --- a/test/css/detached-rulesets.css +++ b/test/css/detached-rulesets.css @@ -1,6 +1,7 @@ .wrap-selector { color: black; one: 1px; + four: magic-frame; visible-one: visible; visible-two: visible; } @@ -45,20 +46,26 @@ html.lt-ie9 header { .without-mixins { b: 1; } -@media (orientation: portrait) { - /* .wrap-media-mixin({ - @media tv { - .triple-wrapped-mq { - triple: true; - } - } - });*/ -} @media (orientation: portrait) and tv { .my-selector { background-color: black; } } +@media (orientation: portrait) and widescreen and print and tv { + .triple-wrapped-mq { + triple: true; + } +} +@media (orientation: portrait) and widescreen and tv { + .triple-wrapped-mq { + triple: true; + } +} +@media (orientation: portrait) and tv { + .triple-wrapped-mq { + triple: true; + } +} .a { test: test; } diff --git a/test/less/detached-rulesets.less b/test/less/detached-rulesets.less index 5af2a745e..7e1241171 100644 --- a/test/less/detached-rulesets.less +++ b/test/less/detached-rulesets.less @@ -7,6 +7,7 @@ .wrap-mixin(@ruleset) { @a: hidden and if you see this in the output its a bug; @b: visible; + @d: magic-frame; // same behaviour as mixin calls - falls back to this frame .wrap-selector { @c: visible; @ruleset(); @@ -20,6 +21,7 @@ one: @a; @b: hidden and if you see this in the output its a bug; @c: hidden and if you see this in the output its a bug; + four: @d; }); .wrap-mixin(@ruleset: { @@ -72,14 +74,13 @@ header { }; @media (orientation:portrait) { @my-ruleset(); - // doesn't work yet -/* .wrap-media-mixin({ + .wrap-media-mixin({ @media tv { .triple-wrapped-mq { triple: true; } } - });*/ + }); } .wrap-media-mixin(@ruleset) { @media widescreen { @@ -99,4 +100,5 @@ header { @my-mixins(); .a { .mixin(); -} \ No newline at end of file +} +// Same fallback frame behaviour as mixin calls \ No newline at end of file From 88b44dfc43d9dd9e11a8993be8635ed85d359715 Mon Sep 17 00:00:00 2001 From: Luke Page Date: Mon, 17 Feb 2014 19:50:43 +0000 Subject: [PATCH 13/14] make mixin definitions have similar coding style to detached rulesets for grabbing frames --- lib/less/tree/detached-ruleset.js | 3 +-- lib/less/tree/mixin.js | 13 ++++++++----- lib/less/tree/ruleset.js | 6 +++--- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/less/tree/detached-ruleset.js b/lib/less/tree/detached-ruleset.js index 87e76f26c..0a5512aae 100644 --- a/lib/less/tree/detached-ruleset.js +++ b/lib/less/tree/detached-ruleset.js @@ -10,12 +10,11 @@ tree.DetachedRuleset.prototype = { this.ruleset = visitor.visit(this.ruleset); }, eval: function (env) { - // TODO - handle mixin definition like this var frames = this.frames || env.frames.slice(0); return new tree.DetachedRuleset(this.ruleset, frames); }, callEval: function (env) { - return this.ruleset.eval(new(tree.evalEnv)(env, this.frames.concat(env.frames))); + return this.ruleset.eval(this.frames ? new(tree.evalEnv)(env, this.frames.concat(env.frames)) : env); } }; })(require('../tree')); diff --git a/lib/less/tree/mixin.js b/lib/less/tree/mixin.js index 1d903e871..506561bac 100644 --- a/lib/less/tree/mixin.js +++ b/lib/less/tree/mixin.js @@ -103,7 +103,7 @@ tree.mixin.Call.prototype = { mixin.originalRuleset = mixins[m].originalRuleset || mixins[m]; } Array.prototype.push.apply( - rules, mixin.eval(env, args, this.important).rules); + rules, mixin.evalCall(env, args, this.important).rules); } catch (e) { throw { message: e.message, index: this.index, filename: this.currentFileInfo.filename, stack: e.stack }; } @@ -150,7 +150,7 @@ tree.mixin.Call.prototype = { } }; -tree.mixin.Definition = function (name, params, rules, condition, variadic) { +tree.mixin.Definition = function (name, params, rules, condition, variadic, frames) { this.name = name; this.selectors = [new(tree.Selector)([new(tree.Element)(null, name, this.index, this.currentFileInfo)])]; this.params = params; @@ -164,7 +164,7 @@ tree.mixin.Definition = function (name, params, rules, condition, variadic) { else { return count; } }, 0); this.parent = tree.Ruleset.prototype; - this.frames = []; + this.frames = frames; }; tree.mixin.Definition.prototype = { type: "MixinDefinition", @@ -258,9 +258,12 @@ tree.mixin.Definition.prototype = { return frame; }, - eval: function (env, args, important) { + eval: function (env) { + return new tree.mixin.Definition(this.name, this.params, this.rules, this.condition, this.variadic, this.frames || env.frames.slice(0)); + }, + evalCall: function (env, args, important) { var _arguments = [], - mixinFrames = this.frames.concat(env.frames), + mixinFrames = this.frames ? this.frames.concat(env.frames) : env.frames, frame = this.evalParams(env, new(tree.evalEnv)(env, mixinFrames), args, _arguments), rules, ruleset; diff --git a/lib/less/tree/ruleset.js b/lib/less/tree/ruleset.js index 3ec0706d2..7571b0e73 100644 --- a/lib/less/tree/ruleset.js +++ b/lib/less/tree/ruleset.js @@ -66,8 +66,8 @@ tree.Ruleset.prototype = { // so they can be evaluated like closures when the time comes. var rsRules = ruleset.rules, rsRuleCnt = rsRules ? rsRules.length : 0; for (i = 0; i < rsRuleCnt; i++) { - if (rsRules[i] instanceof tree.mixin.Definition) { - rsRules[i].frames = envFrames.slice(0); + if (rsRules[i] instanceof tree.mixin.Definition || rsRules[i] instanceof tree.DetachedRuleset) { + rsRules[i] = rsRules[i].eval(env); } } @@ -109,7 +109,7 @@ tree.Ruleset.prototype = { // Evaluate everything else for (i = 0; i < rsRules.length; i++) { rule = rsRules[i]; - if (! (rule instanceof tree.mixin.Definition)) { + if (! (rule instanceof tree.mixin.Definition || rule instanceof tree.DetachedRuleset)) { rsRules[i] = rule = rule.eval ? rule.eval(env) : rule; // for rulesets, check if it is a css guard and can be removed if (rule instanceof tree.Ruleset && rule.selectors && rule.selectors.length === 1) { From 42aff6f35c8890039c8dee6d65dfd04c94336904 Mon Sep 17 00:00:00 2001 From: Luke Page Date: Mon, 17 Feb 2014 19:53:35 +0000 Subject: [PATCH 14/14] remove bad comment --- test/less/detached-rulesets.less | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/less/detached-rulesets.less b/test/less/detached-rulesets.less index 7e1241171..6a98d890a 100644 --- a/test/less/detached-rulesets.less +++ b/test/less/detached-rulesets.less @@ -100,5 +100,4 @@ header { @my-mixins(); .a { .mixin(); -} -// Same fallback frame behaviour as mixin calls \ No newline at end of file +} \ No newline at end of file