feat(parser): add support for parsing return pipelines as subexpressions#17786
feat(parser): add support for parsing return pipelines as subexpressions#17786fdncred wants to merge 3 commits intonushell:mainfrom
Conversation
| TokenContents::Pipe => { | ||
| pipeline.push(&mut command); | ||
| command.pipe = Some(token.span); | ||
| // If this command begins with `return` and doesn't have its own redirection, we want to treat the rest of the pipeline as the |
There was a problem hiding this comment.
Is the special treatment of redirection the reason not to put this check in parse_builtin_commands?
Actually I don't understand why we allow redirection s in keyword return.
There was a problem hiding this comment.
Ok, I think I get the difference between let and return here, there's an assignment mode when the lite parser sees the = sign.
There was a problem hiding this comment.
TBH I'm afraid this is still too hacky. If there's not yet enough utilities to make it a straightforward implementation, I think we should discuss whether this is a sensible choice to make.
There was a problem hiding this comment.
ok, this is my last attempt to make this thing work. Let me know what you think.
There was a problem hiding this comment.
ok, this is my last attempt to make this thing work. Let me know what you think.
I feel it would be better if we introduce a new parsing mode for the first element of a pipeline.
At the end of the day, typing a pair of parentheses for return isn't that annoying IMHO. Let's wait for other people's opinions, shall we?
This PR allows the
returncommand to have a pipeline. Previously the parser would interpretreturn x | yas(return x) | y. Now it interprets it asreturn (x | y)closes #13648
Release notes summary - What our users need to know
Allow the
returncommand to have a pipeline. Previously the parser would interpretreturn x | yas(return x) | y. Now it interprets it asreturn (x | y).Example from issue after this PR
Before (incorrect rounding)
After (correct rounding)
Tasks after submitting
N/A