Skip to content

Commit

Permalink
Fix ordering of @import and @charset rules less#1954 less#2013
Browse files Browse the repository at this point in the history
The genCss method of ruleset.js splits child nodes into two groups:
* rules,
* rulesets.

Rules are always printed first and have special handling for last rule.
Rulesets are always printed second. Wrong ordering was caused by the
condition that determined what is rule and what is ruleset.

Issue less#2013: The condition made no difference between @charset and @page,
because both are compiled into tree.Directive nodes. I added isRulesetLike
method to the tree.Directive to differentiate between them.

Issue less#1954: The condition treated all tree.Anonymous types as rules and
caused them to float up. That is incorrect, because `@import (inline)` is
compiled into tree.Anonymous too, but should be treated as ruleset and
stay where it is.
  • Loading branch information
jurcovicovam committed Jul 22, 2014
1 parent 546bedd commit 864c63d
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 5 deletions.
8 changes: 6 additions & 2 deletions lib/less/tree/anonymous.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
(function (tree) {

tree.Anonymous = function (value, index, currentFileInfo, mapLines) {
tree.Anonymous = function (value, index, currentFileInfo, mapLines, rulesetLike) {
this.value = value;
this.index = index;
this.mapLines = mapLines;
this.currentFileInfo = currentFileInfo;
this.rulesetLike = (typeof rulesetLike === 'undefined')? false : rulesetLike;
};
tree.Anonymous.prototype = {
type: "Anonymous",
eval: function () {
return new tree.Anonymous(this.value, this.index, this.currentFileInfo, this.mapLines);
return new tree.Anonymous(this.value, this.index, this.currentFileInfo, this.mapLines, this.rulesetLike);
},
compare: function (x) {
if (!x.toCSS) {
Expand All @@ -25,6 +26,9 @@ tree.Anonymous.prototype = {

return left < right ? -1 : 1;
},
isRulesetLike: function() {
return this.rulesetLike;
},
genCSS: function (env, output) {
output.add(this.value, this.currentFileInfo, this.index, this.mapLines);
},
Expand Down
3 changes: 3 additions & 0 deletions lib/less/tree/directive.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ tree.Directive.prototype = {
value = visitor.visit(value);
}
},
isRulesetLike: function() {
return "@charset" !== this.name;
},
genCSS: function (env, output) {
var value = this.value, rules = this.rules;
output.add(this.name, this.currentFileInfo, this.index);
Expand Down
2 changes: 1 addition & 1 deletion lib/less/tree/import.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ tree.Import.prototype = {

if (this.options.inline) {
//todo needs to reference css file not import
var contents = new(tree.Anonymous)(this.root, 0, {filename: this.importedFilename}, true);
var contents = new(tree.Anonymous)(this.root, 0, {filename: this.importedFilename}, true, true);
return this.features ? new(tree.Media)([contents], this.features.value) : [contents];
} else if (this.css) {
var newImport = new(tree.Import)(this.evalPath(env), features, this.options, this.index);
Expand Down
22 changes: 21 additions & 1 deletion lib/less/tree/ruleset.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,29 @@ tree.Ruleset.prototype = {
tabSetStr = env.compress ? '' : Array(env.tabLevel).join(" "),
sep;

function isRulesetLikeNode(rule, root) {
// if it has nested rules, then it should be treated like a ruleset
if (rule.rules)
return true;

// medias and comments do not have nested rules, but should be treated like rulesets anyway
if ( (rule instanceof tree.Media) || (root && rule instanceof tree.Comment))
return true;

// some directives and anonumoust nodes are ruleset like, others are not
if ((rule instanceof tree.Directive) || (rule instanceof tree.Anonymous)) {
return rule.isRulesetLike();
}

//anything else is assumed to be a rule
return false;
}

for (i = 0; i < this.rules.length; i++) {
rule = this.rules[i];
if (rule.rules || (rule instanceof tree.Media) || rule instanceof tree.Directive || (this.root && rule instanceof tree.Comment)) {
// console.log(rule.type);
// if (rule.rules || (rule instanceof tree.Media) || rule instanceof tree.Directive || (this.root && rule instanceof tree.Comment)) {
if (isRulesetLikeNode(rule, this.root)) {
rulesetNodes.push(rule);
} else {
ruleNodes.push(rule);
Expand Down
5 changes: 4 additions & 1 deletion test/css/import-inline.css
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
this isn't very valid CSS.
#import {
color: #ff0000;
}
@media (min-width: 600px) {
#css { color: yellow; }

}
this isn't very valid CSS.
1 change: 1 addition & 0 deletions test/css/import.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@charset "UTF-8";
@import url(http://fonts.googleapis.com/css?family=Open+Sans);
@import url(/absolute/something.css) screen and (color) and (max-width: 600px);
@import url("//ha.com/file.css") (min-width: 100px);
Expand Down
1 change: 1 addition & 0 deletions test/less/import-inline.less
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
@import url("import/import-test-c.less");// import inline should not float above this #1954
@import (inline) url("import/import-test-d.css") (min-width:600px);
@import (inline, css) url("import/invalid-css.less");
1 change: 1 addition & 0 deletions test/less/import.less
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@charset "UTF-8"; // stay on top #2013
@import url(http://fonts.googleapis.com/css?family=Open+Sans);

@import url(/absolute/something.css) screen and (color) and (max-width: 600px);
Expand Down

0 comments on commit 864c63d

Please sign in to comment.