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.

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

Spaces Inside Parentheses #952

Closed
SomMeri opened this issue Sep 12, 2012 · 6 comments
Closed

Spaces Inside Parentheses #952

SomMeri opened this issue Sep 12, 2012 · 6 comments

Comments

@SomMeri
Copy link
Member

SomMeri commented Sep 12, 2012

When it comes to the space as a separator, it seems to be allowed inside parentheses too. This: something: (12 (13 + 10 -23)); compiles into something: 12 23 -23;.

Is this a feature or a bug? The comma is treated differently and is not allowed inside parentheses: something: (12,(13 + 10 -23)); leads to compile exception.

I'm asking because I wanted to port less.js into java (https://github.com/SomMeri/less4j project) and would like to know whether:

  • this is a feature and I should copy the behavior,
  • this is a bug and I should report it as such.

I'm using less.js-1.3.0 release as a reference version. I also hope you do not mind too much java port of the compiler.

@SomMeri
Copy link
Member Author

SomMeri commented Sep 12, 2012

I made some more experiments:

something: (12 + (13 + 10 -23)); //Compiles into 12
something: (12 \* (13 + 5 -23)); //Compiles into NaN
something: (12 (13 + 5 -23) + 5);  //Compile error: Object doesn't support this property or method

It seems like the left side decides what is going to happen with the right operand.

@lukeapage
Copy link
Member

There is already a java port..
https://github.com/cloudhead/less.js/wiki/Ports-of-LESS

you may want to consider helping that.

On this issue we are currently working to say that values in parenthesis are evalulated as maths and without parenthesis are evaluated as-is.

In your example

something: (12 + (13 + 10 -23)); //Compiles into 12
// looks right to me
something: (12 * (13 + 5 -23)); //Compiles into NaN
// looks like a bug
something: (12 (13 + 5 -23) + 5);  //Compile error: Object doesn't support this property or method
// looks like it should give a better error message and reject it
something: 12 ((13 + 10 - 23) + 5); // should compile to 12 5

@SomMeri
Copy link
Member Author

SomMeri commented Sep 12, 2012

Thank you for the answer. The intended behavior makes a lot of sense :).

Less for java project is a runner, it uses rhino interpreter to run less.js JavaScript implementation. As far as I know, the project works very well and gives 100% compatibility. Basically, they seems to be done.

Less4j project tries to re-implement the language in Java using ANTLR, so its aim is a bit different. If it turns out that re-implementation leads anywhere or is slower or no one wants to use it anyway, I hope it will still count as a great learning experience.

I also try to document as much as I can as I go, so even if the project would fail, I can contribute some documentation and maybe test cases (if there will be interest of course).

@dmcass
Copy link
Contributor

dmcass commented Sep 12, 2012

Right now we allow any expression inside of parentheses, where an expression is either math or any space delimited value. In your examples it looks like you're actually using a unary minus on a standalone value, resulting in the strange behavior.

something: (12 + (13 + 5 -23));    // Compiles into 12

Operation isn't performed and 12 is output...weird behavior. This should error because you're trying to perform:
operation('+', 12, expression(18, -23)). I'd actually almost expect this to output 12[object Object], but maybe we're stripping that somewhere.

Look what happens when you do the reverse:

something: ((13 + 5 -23) + 12);    // Type Error: Object #<Object> has no method operate

Not the best error message, but I think this is appropriate behavior for how the parser works right now.

something: (12 * (13 + 5 -23)); // NaN

Again, this is strange behavior and it's because you're multiplying a number by an object. 12 * {} == NaN

something: (12 (13 + 5 -23) + 5);  // Type Error: Object #<Object> has no method operate

Same deal, trying to perform an operation with a number and an object.


I think the questions this raises are:

  1. Should we be greedier when dealing with operations?
  2. Is there a good reason for why we allow expressions inside of parens instead of just math?

Relevant lines in the parser:
sub: 1299-1305
expression: 1377-1386

@SomMeri
Copy link
Member Author

SomMeri commented Sep 13, 2012

Thanks you, your answer clears up a lot. I think I understand now and know what is "as designed" and what is a bug.

I think that if less.js would interpret parentheses as a math only, it would be easier to use and read. Having to be careful about the difference between (12 (13 + 10 -23)) and (12 (13 + 10 - 23)) makes mistakes and misreads easy to happen.

There is one advantage of the current approach - consistency. Top level expressions and in parentheses expressions are evaluated the same way. So I can see the case for expressions in parentheses. However, the empty separator inside the parentheses and a comma should behave the same way. I do not see a relevant difference between them.

I still prefer parentheses as math only, but maybe other people have different opinion.

@lukeapage
Copy link
Member

removed, we now give errors for all above cases.

I have never come across a need for space seperated values inside parenthsis, but if the need comes up, we should handle it differently as mixing it with operations is prone to errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants