add pipe operator #152
add pipe operator #152
Conversation
|
It's a good start, but this misses support for the |
oxalica
left a comment
There was a problem hiding this comment.
Thanks, I just know that Nix got new experimental operators. It may worth referencing the source PR NixOS/nix#11131
CI failure is unrelated and is already fixed on main. A rebase should fix it.
| HlTag::Operator(HlOperator::Comparison) | ||
| } | ||
| T![+] | T![-] | T![*] | T![/] => HlTag::Operator(HlOperator::Arithmetic), | ||
| T![++] | T!["//"] => HlTag::Operator(HlOperator::Aggregation), |
There was a problem hiding this comment.
It's not a collection operator. It may need another HlOperator variant like Pipe.
| lhs_ty | ||
| } | ||
| BinaryOpKind::Concat => { | ||
| BinaryOpKind::Concat | BinaryOpKind::Pipe => { |
There was a problem hiding this comment.
We shouldn't unity arguments of pipe operators to be List.
They should follow the handling of Expr::Apply with two arguments flipped.
There was a problem hiding this comment.
i don't quite follow this, need an example... speaking of the other threads, i can make the changes and push them right away
that's quite dumb, but i have basically zero experience in parsers 💀
this PR (plus patches in tree-sitter-nix, rnix-parser and alejandra) emerged from my need of using pipe operator w/o errors from the language server and with working formatter. then i showed all of this to friends and they kinda expected me to make PRs to the base repos 💀 💀 💀
btw i also opened a PR in tree-sitter-nix, but considering the status quo of rnix-parser and it being a dependency of alejandra, i am totally unsure how to work this out 😭
sorry for this silly venting in this thread
| LT_EQ = [<=], | ||
| MINUS_GT = [->], | ||
| NOT_EQ = [!=], | ||
| PIPE = [|>], |
There was a problem hiding this comment.
| PIPE = [|>], | |
| OR_GT = [|>], |
Here we name symbols syntactically, not semantically.
| T![*] | | ||
| T![/] => (17, 18), | ||
| T![++] => (20, 19), | ||
| T![|>] => (21, 22), |
There was a problem hiding this comment.
This precedence is incorrect. According to Nix, it should have a lower precedence than logic-imply ->. So it should goes above to be (-1, 0), which underflows u8... Unfortunately, we need to shift all other BP values up by +10 (for simplicity and more future proof), so |> becomes (9, 10), -> becomes (12, 11), and etc.
|
I hate to be that one that writes "any progress on this?", but I need to be that one |
before
after