feat($parse): add support for ternary operators to parser #2482
Conversation
Add '?' token to lexer, add ternary rule to parser at (hopefully) proper precedence and associativity (based on https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Operators/Operator_Precedence). Since (exp1 && exp2 || exp3) is supported by the parser, and (exp1 ? exp2 : exp3) works the same way, it seems reasonable to add this minor form of control to templates (see angular#719).
|
One thing I wasn't totally sure about was the assignments in It seemed to me that this shouldn't be replaced with Obviously let me know if there's problems or I should format this differently, etc; I haven't submitted to or worked on Angular before. Cheers! |
|
|
|
||
| // Precedence with respect to logical operators. | ||
| expect(scope.$eval('0&&1?0:1')).toEqual(0&&1?0:1); | ||
| expect(scope.$eval('0&&1?0:1')).toEqual((0&&1)?0:1); |
There was a problem hiding this comment.
what does this line add over the previous one?
There was a problem hiding this comment.
shouldn't you also be including expectations with logical operators in the second or third clause?
e.g. 0?0||1:2 or 1?1&&0:1
There was a problem hiding this comment.
The with+without parentheses was more for me to be sure I was actually understanding the precedence and associativity correctly.
As for more expectations, will do!
|
Regarding JSON and https://github.com/zachsnow/angular.js/blob/16af3b855198dc377083251cad5c7bead3c0d875/src/ng/parse.js#L293. It appears that as soon as any invalid JSON is found, Since JSON does not allow ternary or binary expressions of any kind, then I think that actually this is a bug in the JSON parser here and in fact this line should be: And of course we would need tests to prove it. But this should go into a new PR if we decided to go that way. |
|
Once you have dealt with my comments, this looks good to merge. |
…mprove clarity of tests; remove redundant tests.
|
Updated. Also, I have signed the CLA (Zach Snow). |
|
Merged as 6798fec. Thanks!! Great PR. Can you do more of these? |
|
@petebacondarwin no problem; as for more, I'd be happy to :) Also it seems like the last commit of the request didn't get merged (f7da538)? |
|
Oops! Committed the changed tests here: 2a7043f. No need to have that extra |
Add '?' token to lexer, add ternary rule to parser at
(hopefully) proper precedence and associativity (based
on https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Operators/Operator_Precedence).
Since (exp1 && exp2 || exp3) is supported by the parser,
and (exp1 ? exp2 : exp3) works the same way, it seems
reasonable to add this minor form of control to templates
(see #719).