-
Notifications
You must be signed in to change notification settings - Fork 216
feat: Add the pipeline operator |> for chained method calls. #1061
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
base: main
Are you sure you want to change the base?
Conversation
0d47614 to
2f5c842
Compare
2f5c842 to
3dd6cc5
Compare
| } | ||
|
|
||
| // Range iterator with step | ||
| #[derive(Clone, Hash, Eq, PartialEq)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove the derive attributes?
| #[cfg(not(feature = "no_module"))] | ||
| let (_index, name, namespace, _hash) = x; | ||
| #[cfg(feature = "no_module")] | ||
| let (index, name) = x; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI seems to complain about index being unused here...
|
|
||
| Expr::FnCall(fn_call.into(), pos) | ||
| } | ||
| _ => op_base.into_fn_call_expr(pos), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd need parsing errors if the stuff following the operator is not a proper function call, instead of allowing it to go through.
|
|
||
| Precedence::new(match self { | ||
| Or | XOr | Pipe => 30, | ||
| Or | XOr | Pipe | PipeArrow => 30, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a lower precedence for |>?
| } | ||
| ('|', '>') => { | ||
| stream.eat_next_and_advance(pos); | ||
| return (Token::Reserved(Box::new("|>".into())), start_pos); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to be careful here... some users may depend on the fact that |> is a reserved symbol and define it as a custom operator. This may break code, but I see that you're checking this after checking for custom operators...
| name: self.get_interned_string(&op), | ||
| hashes: FnCallHashes::from_native_only(hash), | ||
| args: IntoIterator::into_iter([root, rhs]).collect(), | ||
| args: IntoIterator::into_iter([root.clone(), rhs.clone()]).collect(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also is it necessary to clone the arguments?
Fixes #1062