Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not allow division when outside of parens (Fixes #146; indirectly fixes #122) #915

Closed
wants to merge 7 commits into from
Closed

Do not allow division when outside of parens (Fixes #146; indirectly fixes #122) #915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
29e8251
Division only when inside parentheses
dmcass Aug 17, 2012
3a45c4b
Updating parens and operations tests
dmcass Aug 17, 2012
f559fca
Merge branch 'master' into division
dmcass Aug 21, 2012
805cb82
Removed Shorthand and Ratio parsers; Fixed some issues with paren dep…
dmcass Aug 22, 2012
c6e454e
Remove spaces around slashes in final CSS output and update tests to …
dmcass Aug 22, 2012
846f70e
Arithmetic within mixin calls must be in nested parentheses; Added te…
dmcass Aug 22, 2012
c482515
Fix CSS output for compression
dmcass Aug 22, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/less/index.js
View file
Open in desktop
Expand Up @@ -88,7 +88,7 @@ var less = {
'call', 'url', 'alpha', 'import',
'mixin', 'comment', 'anonymous', 'value',
'javascript', 'assignment', 'condition', 'paren',
'media', 'ratio'
'media'
].forEach(function (n) {
require('./tree/' + n);
});
Expand Down
113 changes: 41 additions & 72 deletions lib/less/parser.js
View file
Open in desktop
Expand Up @@ -61,6 +61,7 @@ less.Parser = function Parser(env) {
furthest, // furthest index the parser has gone to
chunks, // chunkified input
current, // index of current chunk, in `input`
parens = 0, // current depth of nested parentheses
parser;

var that = this;
Expand Down Expand Up @@ -397,6 +398,7 @@ less.Parser = function Parser(env) {
else throw new(LessError)(importError, env);
}

css = css.replace(/\s+(\/)\s+/g, "$1");
if (options.yuicompress && less.mode === 'node') {
return require('./cssmin').compressor.cssmin(css);
} else if (options.compress) {
Expand Down Expand Up @@ -577,7 +579,7 @@ less.Parser = function Parser(env) {
nameLC = name.toLowerCase();

if (nameLC === 'url') { return null }
else { i += name.length }
else { i += name.length }

if (nameLC === 'alpha') {
alpha_ret = $(this.alpha);
Expand All @@ -587,10 +589,15 @@ less.Parser = function Parser(env) {
}

$('('); // Parse the '(' and consume whitespace.
parens++;

args = $(this.entities.arguments);

if (! $(')')) return;
if (! $(')')) {
parens--;
return;
}
parens--;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you just put parens-- once on the line before and not alter the if-return block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you're absolutely right. Silly mistake.

if (name) { return new(tree.Call)(name, args, index, env.filename) }
},
Expand All @@ -604,8 +611,7 @@ less.Parser = function Parser(env) {
return args;
},
literal: function () {
return $(this.entities.ratio) ||
$(this.entities.dimension) ||
return $(this.entities.dimension) ||
$(this.entities.color) ||
$(this.entities.quoted);
},
Expand Down Expand Up @@ -702,20 +708,6 @@ less.Parser = function Parser(env) {
}
},

//
// A Ratio
//
// 16/9
//
ratio: function () {
var value, c = input.charCodeAt(i);
if (c > 57 || c < 48) return;

if (value = $(/^(\d+\/\d+)/)) {
return new(tree.Ratio)(value[1]);
}
},

//
// JavaScript code to be evaluated
//
Expand Down Expand Up @@ -746,27 +738,6 @@ less.Parser = function Parser(env) {
if (input.charAt(i) === '@' && (name = $(/^(@[\w-]+)\s*:/))) { return name[1] }
},

//
// A font size/line-height shorthand
//
// small/12px
//
// We need to peek first, or we'll match on keywords and dimensions
//
shorthand: function () {
var a, b;

if (! peek(/^[@\w.%-]+\/[@\w.-]+/)) return;

save();

if ((a = $(this.entity)) && $('/') && (b = $(this.entity))) {
return new(tree.Shorthand)(a, b);
}

restore();
},

//
// Mixins
//
Expand All @@ -786,7 +757,7 @@ less.Parser = function Parser(env) {
var elements = [], e, c, args = [], arg, index = i, s = input.charAt(i), name, value, important = false;

if (s !== '.' && s !== '#') { return }

save(); // stop us absorbing part of an invalid selector

while (e = $(/^[#.](?:[\w-]|\\(?:[A-Fa-f0-9]{1,6} ?|[^A-Fa-f0-9]))+/)) {
Expand Down Expand Up @@ -826,7 +797,7 @@ less.Parser = function Parser(env) {
if (elements.length > 0 && ($(';') || peek('}'))) {
return new(tree.mixin.Call)(elements, args, index, env.filename, important);
}

restore();
},

Expand Down Expand Up @@ -885,7 +856,7 @@ less.Parser = function Parser(env) {
}
} while ($(','))

// .mixincall("@{a}");
// .mixincall("@{a}");
// looks a bit like a mixin definition.. so we have to be nice and restore
if (!$(')')) {
furthest = i;
Expand Down Expand Up @@ -1014,7 +985,7 @@ less.Parser = function Parser(env) {
// :nth-child(@a)
// vs
// .a:hover(@a)

e = $('&');
$('(');
if (e) {
Expand Down Expand Up @@ -1098,8 +1069,6 @@ less.Parser = function Parser(env) {
if ((name.charAt(0) != '@') && (match = /^([^@+\/'"*`(;{}-]*);/.exec(chunks[j]))) {
i += match[0].length - 1;
value = new(tree.Anonymous)(match[1]);
} else if (name === "font") {
value = $(this.font);
} else {
value = $(this.value);
}
Expand All @@ -1126,9 +1095,9 @@ less.Parser = function Parser(env) {
//
"import": function () {
var path, features, index = i;

save();

var dir = $(/^@import(?:-(once))?\s+/);

if (dir && (path = $(this.entities.quoted) || $(this.entities.url))) {
Expand All @@ -1137,7 +1106,7 @@ less.Parser = function Parser(env) {
return new(tree.Import)(path, imports, features, (dir[1] === 'once'), index);
}
}

restore();
},

Expand All @@ -1149,7 +1118,7 @@ less.Parser = function Parser(env) {
nodes.push(e);
} else if ($('(')) {
p = $(this.property);
e = $(this.entity);
e = $(this.value);
if ($(')')) {
if (p && e) {
nodes.push(new(tree.Paren)(new(tree.Rule)(p, e, null, i, true)));
Expand Down Expand Up @@ -1209,7 +1178,7 @@ less.Parser = function Parser(env) {
if (value = $(this['import']) || $(this.media)) {
return value;
}

save();

name = $(/^@[a-z-]+/);
Expand Down Expand Up @@ -1265,24 +1234,8 @@ less.Parser = function Parser(env) {
return new(tree.Directive)(name, value);
}
}

restore();
},
font: function () {
var value = [], expression = [], weight, shorthand, font, e;

while (e = $(this.shorthand) || $(this.entity)) {
expression.push(e);
}
value.push(new(tree.Expression)(expression));

if ($(',')) {
while (e = $(this.expression)) {
value.push(e);
if (! $(',')) { break }
}
}
return new(tree.Value)(value);
restore();
},

//
Expand Down Expand Up @@ -1313,15 +1266,28 @@ less.Parser = function Parser(env) {
sub: function () {
var e;

if ($('(') && (e = $(this.expression)) && $(')')) {
return e;
if ($('(')) {
parens++;
if (e = $(this.expression)) {
expect(')');
parens--;
return e;
}
}
},
multiplication: function () {
var m, a, op, operation;
var m, a, op, operation, expression = [];
if (m = $(this.operand)) {
while (!peek(/^\/\*/) && (op = ($('/') || $('*'))) && (a = $(this.operand))) {
operation = new(tree.Operation)(op, [operation || m, a]);
while (!peek(/^\/\*/) && (op = ($('/') || $('*')))) {
if (op === '/' && !parens) {
expression.push(operation || m);
expression.push(new(tree.Anonymous)('/'));
operation = new(tree.Expression)(expression);
} else if (a = $(this.operand)) {
operation = new(tree.Operation)(op, [operation || m, a]);
} else {
break;
}
}
return operation || m;
}
Expand Down Expand Up @@ -1393,6 +1359,9 @@ less.Parser = function Parser(env) {

while (e = $(this.addition) || $(this.entity)) {
entities.push(e);
if (!peek(/^\/\*/) && (delim = $('/'))) {
entities.push(new(tree.Anonymous)(delim));
}
}
if (entities.length > 0) {
return new(tree.Expression)(entities);
Expand Down
13 changes: 0 additions & 13 deletions lib/less/tree/ratio.js
View file
Open in desktop

This file was deleted.

3 changes: 3 additions & 0 deletions test/css/css.css
View file
Open in desktop
Expand Up @@ -62,6 +62,9 @@ p + h1 {
padding: 1px 0 2px 0;
font: normal small/20px 'Trebuchet MS', Verdana, sans-serif;
font: 0/0 a;
border-radius: 5px/10px;
background: url("img.jpg") center/100px;
background: #ffffff url(image.png) center/1px 100px repeat-x scroll content-box padding-box;
}
.misc {
-moz-border-radius: 2px;
Expand Down
5 changes: 5 additions & 0 deletions test/css/mixins-args.css
View file
Open in desktop
Expand Up @@ -72,3 +72,8 @@ body {
border: "{";
width: "{";
}
.slash-vs-math {
border-radius: 2px/5px;
border-radius: 5px/10px;
border-radius: 6px;
}
1 change: 1 addition & 0 deletions test/css/parens.css
View file
Open in desktop
Expand Up @@ -9,6 +9,7 @@
width: 96;
height: 113;
margin: 12;
border-radius: 8px/7px;
}
.nested-parens {
width: 71;
Expand Down
5 changes: 4 additions & 1 deletion test/less/css.less
View file
Open in desktop
Expand Up @@ -67,8 +67,11 @@ p + h1 {
#more-shorthands {
margin: 0;
padding: 1px 0 2px 0;
font: normal small/20px 'Trebuchet MS', Verdana, sans-serif;
font: normal small/20px 'Trebuchet MS', Verdana, sans-serif;
font: 0/0 a;
border-radius: 5px / 10px;
background: url("img.jpg") center / 100px;
background: #fff url(image.png) center / 1px 100px repeat-x scroll content-box padding-box;
}

.misc {
Expand Down
4 changes: 2 additions & 2 deletions test/less/media.less
View file
Open in desktop
Expand Up @@ -20,7 +20,7 @@
body { max-width: @base * 60; }
}

@media all and (device-aspect-ratio: 16/9) {
@media all and (device-aspect-ratio: 16 / 9) {
body { max-width: 800px; }
}

Expand Down Expand Up @@ -172,7 +172,7 @@ body {
}
}

@media (-webkit-min-device-pixel-ratio: 2), (min--moz-device-pixel-ratio: 2), (-o-min-device-pixel-ratio: 2/1), (min-resolution: 2dppx), (min-resolution: 128dpcm) {
@media (-webkit-min-device-pixel-ratio: 2), (min--moz-device-pixel-ratio: 2), (-o-min-device-pixel-ratio: 2 / 1), (min-resolution: 2dppx), (min-resolution: 128dpcm) {
.b {
background: red;
}
Expand Down
10 changes: 10 additions & 0 deletions test/less/mixins-args.less
View file
Open in desktop
Expand Up @@ -128,3 +128,13 @@ body {
.edge-case {
.mixin-arguments("{");
}

// Division vs. Literal Slash
.border-radius(@r: 2px/5px) {
border-radius: @r;
}
.slash-vs-math {
.border-radius();
.border-radius(5px/10px);
.border-radius((3px * 2px));
}
12 changes: 6 additions & 6 deletions test/less/operations.less
View file
Open in desktop
@@ -1,13 +1,13 @@
#operations {
color: #110000 + #000011 + #001100; // #111111
height: 10px / 2px + 6px - 1px * 2; // 9px
height: (10px / 2px + 6px - 1px * 2); // 9px
width: 2 * 4 - 5em; // 3em
.spacing {
height: 10px / 2px+6px-1px*2;
height: (10px / 2px+6px-1px*2);
width: 2 * 4-5em;
}
substraction: 20 - 10 - 5 - 5; // 0
division: 20 / 5 / 4; // 1
division: (20 / 5 / 4); // 1
}

@x: 4;
Expand All @@ -20,7 +20,7 @@
}

.with-functions {
color: rgb(200, 200, 200) / 2;
color: (rgb(200, 200, 200) / 2);
color: 2 * hsl(0, 50%, 50%);
color: rgb(10, 10, 10) + hsl(0, 50%, 50%);
}
Expand All @@ -37,7 +37,7 @@
}

.rem-dimensions {
font-size: 20rem / 5 + 1.5rem; // 5.5rem
font-size: (20rem / 5 + 1.5rem); // 5.5rem
}

.colors {
Expand All @@ -46,7 +46,7 @@
background-color: #222222 - #fff; // #000000
.other {
color: 2 * #111; // #222222
border-color: #333333 / 3 + #111; // #222222
border-color: (#333333 / 3 + #111); // #222222
}
}

Expand Down