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

[dmcass]

Quite a few changes here so please review carefully. If you'd prefer me to squash this into one commit so that it's more straight-forward, please let me know and I'll close this and open a new PR. Pinging @cloudhead, @agatronic, @MatthewDL, @Synchro

A few notes on how division will work with this change:

• If there is a slash outside of parens and inside a value, it will compile to a literal slash
• If there is a slash inside a function call, it will not require extra parentheses to initiate an operation
• The exceptions to the above rules are media queries, mixin calls, and mixin definitions, where I intentionally did not count parens outside of the value because of rules like aspect-ratio and elliptical border-radius.

Let me know if the way any of this works is not as intuitive as you have imagined. Mixin calls, for example, is one thing that I debated about allowing operations in without extra parens.

Example
Input

@var: 2px;
.mixin(@a: 2px/5px) {
  border-radius: @a;
}

@media only screen and (aspect-ratio: 16/9) {
  body {
    margin: (@var * 10 / 5);
    padding: round(@var * 10 / 3);
    background: url(img.jpg) center / 100px;
    .mixin()
    .mixin(5px/10px);
  }
}

Output

@media only screen and (aspect-ratio: 16/9) {
  body {
    margin: 4px;
    padding: 7px;
    background: url(img.jpg) center/100px;
    border-radius: 2px/5px;
    border-radius: 10px/5px;
  }
}

[dmcass]

P.S. I recommend you look at the diff with whitespace ignored. My editor did some whitespace clean-up.

[dmcass]

Linking with #146 & #122

[matthew-dean]

Actually, I think the same would apply within mixins. Because, of course.....

/* Elliptical radius set with a default value */
.mymixin(@radius: 10px / 2px) {
  border-radius: @radius;
}

Should compile to:

  border-radius: 10px / 2px;

Required syntax:

/* Elliptical radius set with mathed value */
.mymixin(@radius: (10px / 2px)) {
  border-radius: @radius;
}

Expected output:

  border-radius: 5px;

The rule must apply universally or else we'll have problems. If you're already "within" parentheses, then a second set should be required to trigger math division.

I want us to think carefully about mixins, because there is also a proposal on the table to solve the comma problem using parentheses (a suggestion I brought in, but echoes SASS implementation). See issue #35.

However, I don't want us to be over-parenthesized. So, just want to make sure that's considered as well.

[dmcass]

You make a very good point, and I actually did omit mixin definitions, so now I'm not sure why I included mixin calls. I'll revert the change that includes mixin calls and update the information in the OP accordingly. It's a fairly small change.

[matthew-dean]

Mixin calls are the same situation:

.someclass {
  .mymixin(10px / 2px);
}

Expected output:

.someclass {
  border-radius: 10px / 2px;
}

Force division:

.someclass {
  .mymixin((10px / 2px));
}

Output:

.someclass {
  border-radius: 5px;
}

[lukeapage]

@MatthewDL - think dmcass beat you to it - he had already altered the pull.

I don't like the anonymous delim that is added as an entity - because to me, entities are space seperated values - and this forces you to put a hack to remove the spaces. You don't have it in less.js, but in dotless (a port), which we try to keep as similar as possible, we allow extensions to modify the abstract syntax tree and its nice if the syntax tree is kept tidy with as little anonymous things in as possible.

My first thought was to keep ratio in, expand it to allow variables on either side and then use save/restore back tracking - but it seems quite wasteful. Second thought was to modify tree.Operation as to whether it should "operate" or output, with ratios being output in non-operate mode - I like that better but I don't like that a ratio is an operation, but at least a single entity is a single entity.
My third thought was to keep it closer to how it is at the moment but have an intermediary function before multiplication, which calls multiplication, then checks for an operator - if not returning the result, and if so, creating a new WhateverYouWantToCallItRatioShortHandPrintedOperationImNoGoodAtNaming( where the left hand is the multiplication, the delimiter we just got and the right hand side is gained by calling itself again (in order to get nested values.)

I'm sure there are other alternatives.. anything that keeps an "3px/4em" as a single entity would do for me.

Another test case for you (and an argument in support of @MatthewDL

width: calc(100% - 5em + 10px);

or is that the stage after this?

[lukeapage]

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

[dmcass]

Yep, you're absolutely right. Silly mistake.

[dmcass]

Actually, I attempted a couple things to keep it one entity before arriving at this solution. That's why I managed to forget to remove tree.Shorthand, because I was attempting to use it since it does exactly what I want. I ran into some parse errors with more complex operations, such as 5 * (3 + 2) / 6 - (3 * 1), where the expected output would be 25/3. Perhaps I went about it the wrong way though. I'm going to take your advice on the intermediary parser, as I like that solution the most, and see if I can repurpose the shorthand parser for exactly that.

Regarding the calc function, that should definitely be left up to the browser, but I think that's part of stage 2 where all operations are only allowed in parens. Also, I know Stylus isn't doing this correctly right now either. I'm not sure if SASS is. We're not terribly behind the curve on that one.

[dmcass]

Also, worth noting, it's not actually necessary that I remove the spaces...that was purely cosmetic and to stay in line with the way it outputs now. All CSS engines currently interpret 16 / 9 and 16/9 the same way, as a ratio. In addition, there's times where it's not necessarily a ratio, but a delimiter between two similar rules in a shorthand property. Again, all engines interpret the difference the same way. 100px 30px / 30px 100px is the same as 100px 30px/30px 100px, and all the slash is doing is separating the two pairs of values from each other.

[matthew-dean]

Parentheses are also meaningful in CSS, and also have additional usage in LESS, so if we're covering all those, great, just want to make sure we don't shoot ourselves in the foot with a breaking change that we have to make a breaking change to later. Ideally, our test cases would demonstrate every example of standalone and nested parentheses.

[dmcass]

I've been a bit too busy in the past 2 weeks to get anything done on this, but I'll try to make some progress soon. I apologize for the delay.

[matthew-dean]

No reason for apologies. This is all free labor for an open source project. The moment any of us get paid for helping out, THEN we can expect apologies. ^_^

Thanks for all your contributions already!

[lukeapage]

@dmcass I think we are ready to start building a 1.4.0 branch so once you've finished on this I'l pull it there...

[lukeapage]

@MatthewDL your comment "Parentheses are also meaningful in CSS, and..."

what would that mean ? Are we suggesting that (3 + 3) + 3 is unchanged and that only a un-necessary set of brackets turns it into an operation, e.g. ((3 + 3) + 3) becomes 9 or ((3 + 3)) + 3 becomes 6 + 3.. or is that just over the top?

Or were you just saying that this needs to work?