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

pattern-match and, and, or #584

Closed
igl opened this issue Oct 16, 2014 · 3 comments
Closed

pattern-match and, and, or #584

igl opened this issue Oct 16, 2014 · 3 comments

Comments

@igl
Copy link
Contributor

@igl igl commented Oct 16, 2014

Simple match test with expression in case:

match 1, 3, 3
| 1, 1, 2 or 3 => ok 0
| 1, 2 or 3, 3 => ok 1
| _            => ok 0

Compiles to:

// Generated by LiveScript 1.3.1
var ref$;
switch (ref$ = [1, 3, 3], false) {
case !(1 === ref$[0] && 1 === ref$[1] && 2 === ref$[2] || 3 === ref$[2]):
  ok(0);
  break;
case !(1 === ref$[0] && 2 === ref$[1] || 3 === ref$[1] && 3 === ref$[2]):
  ok(1);
  break;
default:
  ok(0);
}

The problem in words: Every case-expression is bundled in one long expression.
It should wrap braces around each column so the precedence for each case is preserved.

1 === ref$[1] && 2 === ref$[2] || 3 === ref$[2]
^---- 1 ----^ && ^------------ 2 -------------^ # yay
^------------ 1 -------------^ || ^---- 2 ----^ # nay

Correct output:

(1 === ref$[1]) && (2 === ref$[2] || 3 === ref$[2])
@igl
Copy link
Contributor Author

@igl igl commented Apr 9, 2015

Updated OP as it was a little.. plain.
I fiddled with ast.Case.compileCase but without much luck. I don't fully understand the code to approach this problem :/

@phanimahesh
Copy link

@phanimahesh phanimahesh commented Jun 17, 2015

I don't think LS should add parenthesis by default, but if it is the most commonly expected use case, it makes sense to do it.

@igl
Copy link
Contributor Author

@igl igl commented Jun 17, 2015

Well if we want to keep the meaning of , to end a expression they should be there.

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

Successfully merging a pull request may close this issue.

2 participants