Skip to content

Pipeline Chain Operators (&& / ||) #192

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

Merged
merged 16 commits into from
Oct 14, 2019

Conversation

rjmholt
Copy link
Contributor

@rjmholt rjmholt commented Jun 12, 2019

No description provided.

@kilasuit
Copy link

My honest opinion is that the usecases shown in the RFC do not give this enough justice for me to feel that this is a useful addition to the language, however that may be because I am a long term Windows PowerShell User & for some of the usecases I would have already coded around the supposed benefits this may bring in a much more subjectively readable manner

I have one other Key problem with the RFC and that this is pre marked as Planned to Implement : Yes
Technically this reads to me like you've filled in the RFC and regardless of the commments here, this will happen. Which I hope is not the case and that comments here will be properly taken on board (like it should for other RFCs too)

I think this should be renamed to Desire to Implement : (Low|Medium|High) though that's an aside for the RFC template

Would be interested in the thoughts of @markekraus @Jaykul @KirkMunro @rkeithhill @vexx32 @bergmeister to name a few on this one

@kilasuit
Copy link

I will add I can see where the benefit may be coming from Linux but is this actually going to be hugely beneficial to Windows users too?

@SeeminglyScience
Copy link

SeeminglyScience commented Jun 12, 2019

I have one other Key problem with the RFC and that this is pre marked as Planned to Implement : Yes
Technically this reads to me like you've filled in the RFC and regardless of the commments here, this will happen. Which I hope is not the case and that comments here will be properly taken on board (like it should for other RFCs too)

@kilasuit pretty sure that just means "if the RFC is accepted I will personally implement it". You can write an RFC for something you want but leave the actual implementation to the PowerShell team/other community members.

@SeeminglyScience
Copy link

Also FWIW I'd definitely use this interactively. I'd probably request changes if I saw it being used in a PR, but I don't think that's a good enough reason not to include it in the language.

@rjmholt
Copy link
Contributor Author

rjmholt commented Jun 12, 2019

if the RFC is accepted I will personally implement it

That's my intended meaning there.

Worth noting that the original issue in PowerShell/PowerShell has over 100 👍s and is currently the most 👍'd open issue in the repo.

@KirkMunro
Copy link
Contributor

KirkMunro commented Jun 12, 2019

@kilasuit Take note of the RFC process documentation. In particular, the last bullet in this section. Plan to Implement is an indication that the RFC author plans to implement the RFC if it is approved. RFCs with this set to Yes get a little higher priority during the PowerShell Committee review process than other RFCs, simply because they have a lot to review and there is a better chance that someone will do the work if the RFC is approved, so why not focus on those RFCs first?

As for usefulness, I think this part of your question doesn't really matter anymore:

...but is this actually going to be hugely beneficial to Windows users too?

There are already plenty of things in PowerShell that are beneficial to Windows users. Cross platform PowerShell is where we're at today though, so it's not surprising to see operators like this coming into the language for bash users. If PowerShell 7 is easier for bash users to adopt because it contains operators that they're already familiar with and use in bash, that's a win IMHO.

That said, while I have used bash/Linux a decent amount, most of my professional experience has been in Windows, and I can absolutely see myself using these operators. I was reading the PR on this before the RFC came out today and personally I welcome these additions once the kinks have been worked out through the RFC process.

@oising
Copy link

oising commented Jun 12, 2019

I will add I can see where the benefit may be coming from Linux but is this actually going to be hugely beneficial to Windows users too?

The operator && is already supported in cmd.exe so in the early days of powershell/monad, there was demand for it and subsequent grumpiness when we learned it wasn't coming anytime soon. In the meantime I think people have probably figured out it's not all that necessary in powershell as we're pretty spoiled [compared to cmd].

TL;DR I think the demand for chain operators is probably inversely proportional to the time one has used powershell [as compared to cmd.]

@oising
Copy link

oising commented Jun 12, 2019

That said, I'm not poo-pooing the effort that's gone into this. There was a time where we had to beg for language features :) If it makes it easier for cmd users to migrate, all the better.

@rjmholt
Copy link
Contributor Author

rjmholt commented Jun 12, 2019

There was a time where we had to beg for language features

With the default interface implementation feature in .NET, the difficulty here is greatly reduced. Previously, new AST elements (i.e. new syntaxes) would cause a significant problem with any C# library trying to link against the AST APIs (ICustomAstVisitor).

One example.

@rjmholt
Copy link
Contributor Author

rjmholt commented Jun 13, 2019

/cc @BrucePay @lzybkr

This RFC proposes adding AND-OR lists to PowerShell, using the same `&&` and `||`.
Because "AND-OR list" is neither intuitive nor common as a terminology,
instead the term **pipeline chains** is proposed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So && and || would be called "pipeline chain operators"? What would the specific operators be called? "Successful pipeline operator (&&)" and "Failed pipeline operator (||)"? I like the "pipeline chains" name, and am just seeking clarification on what the operators will be called in the documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

POSIX calls them "control operators", but that doesn't identify them very well to my mind. I like your proposal.


> As a PowerShell user, I can chain commands with `&&` and `||`
> so that I have an ergonomic syntax to conditionally invoke side-effectful commands.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more literal motivation for this might be:

As a bash user,
I can chain pipelines in PowerShell with && and || just like I can in bash,
so that PowerShell is easier for me to adopt and use.

😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha!

Well they're also present in cmd, but really I'm a PowerShell user and I just want these in PowerShell so I can do stuff like Import-Module ./build.psm1 && Start-PSBuild && Start-PSPester and sudo apt update && sudo apt upgrade.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But my thinking on this feature is not to make PowerShell more bash-like. The operators are nice, but they must be PowerShell's own. Taking an operator doesn't mean we should take all the semantics.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KirkMunro Is it so wrong to make it easier for bash users to adopt PowerShell? It seems that you are saying that with a derisive attitude rather than a positive one. I think it's great to make it as easy as possible for people to adapt to powershell, and they days of Microsoft's exclusive attitude are gone, they "love linux".

In addition, it's not just a cosmetic issue, there are very real and valuable benefits to the && and || operators that are a pain to work around, somewhat less intuitive, and harder/more to remember.

For example, I'm currently building a command line tool, and I want it to compile, and if the compilation works, then run the command, like so:

go install && mycmd run

Pretty simple right? Easy/quick to type, and it's very clear and easy to read.

With the current implementation of powershell though, I would have to do something like this:

$(command -arg1 -arg2 | Out-Host;$?) -and $(command2 -arg1 | Out-Host;$?)

That's a lot to type, when you just want to run a quick command, and there's really no reason for it to be that way. I'm not saying you couldn't develop a workaround, but I just don't see why anyone on God's green earth would be against making this simpler.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zwhitchcox I wasn't being derisive, and I fully embrace making PowerShell more familiar for bash users, C# programmers, etc.

The comment I made was meant to suggest that we could just literally state the motivation behind this RFC. Nothing more was behind it than that.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KirkMunro Sorry, I think I misinterpreted your meaning, after I read the rest of this forum, it was clear you weren't trying to exclude bash users

Command failed
Backup
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For users not familiar with these types of operators, it would be better if the examples in the UX section had comments before the commands explaining how they will work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean explaining something like "cmd1 succeeds, && then continues to cmd2"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, something simple for each one to describe the use case it is demonstrating.

| pipeline_chain "&&" [newlines] pipeline
| pipeline_chain "||" [newlines] pipeline
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This grammar should be amended to allow for automatic continuance when lines start with && and || as well, like so:

pipeline_chain:
    | pipeline
    | pipeline_chain [newline] "&&" [newlines] pipeline
    | pipeline_chain [newline] "||" [newlines] pipeline

Note that when you are continuing a command with a pipeline on a subsequent line, only a single newline is allowed. This is consistent with how commands continue with a pipe symbol on a newline (a recent addition to PowerShell 7). Also note as an implementation detail that comment lines are allowed in the middle, but we don't specify those in the grammar. Additional test cases will need to be added for lines starting with these operators.

Further, while I did not change the multiple newlines after the operator to a single newline in this updated grammar, I believe we should consider that change. While PowerShell allows multiple newlines after a pipe or an operator, aside from supporting lines with comments, is there any real benefit in doing so?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't aware that was in PowerShell -- will change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we decided to limit that type of lookahead multi-line continuance to just the pipeline operator, @PowerShell/powershell-committee agrees we should remove the [newline] from each of these. [newlines] after the operator are fine.

cmd2 ||
cmd3
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider adding:

Additionally, pipeline chain operators can be placed at the beginning of a line. Newlines before the pipeline chain operators will be skipped in anticipation of the following pipeline.

For example, the following would be a single pipeline chain:

cmd1
    && cmd2
    || cmd3

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As said above, we in the @PowerShell/powershell-committee explicitly don't want this for perf and interactiving/scripting consistency reasons.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joeyaiello From an authoring and readability perspective I personally would like this in and be happy with the performance hit that may come with it.

As I believe many others would as well


Semantically, this means that `cmd1 && cmd2` would be evaluated first,
and its result used to govern the evaluation of `|| cmd3`.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think an additional text is needed here for clarity, as follows:

If cmd1 is successful, the results of cmd2 will be tested for failure, in which case cmd3 will be invoked.

If cmd1 is unsuccessful, cmd3 will be invoked.

The consequence of this will be that an entire pipeline chain
can be sent to a background job for evaluation,
rather than individual pipelines within it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is worth calling out right here that this is the opposite of bash. I also question whether or not this is the right thing to do for that reason. I know you have additional comments below, but if this is really the way we want to move forward here, attention needs to be drawn to this fact, coupled with reasoning why this is better. In the past though, the Linux community has not responded favorably to differing behaviours with commands, and I worry they will respond similarly if we change operator precedence for operators. Does it really buy us enough to change the precedence, or should we just follow bash?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this proposal makes more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it really buy us enough to change the precedence, or should we just follow bash?

Actually we already changed the precedence. I think I mention it under Background operator changes (line 446 currently).

In PowerShell, the background operator applies to a pipeline rather than a statement.

So currently in a return statement, the background operator binds to the pipeline subordinate to the statement. This function returns a job:

function Test
{
   return Write-Output "something" &
}

Similarly with throw:

function Bad
{
    throw "Message" &
}

The function Bad does not succeed, it throws a Job. If the background operator applied to the statement as a whole (wherein the statement is backgrounded) the throw itself would be backgrounded and the function would succeed.

Also consider:

foreach ($x in 1..100) { break & }

Copy link
Contributor Author

@rjmholt rjmholt Jun 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally think that elevating backgrounding to statement level invites serious inconsistencies and complexities into the grammar and semantics of PowerShell, but I thought it was worth writing up since I discussed it with @daxian-dbw.

$x = 'a','b','c' | Write-Output && Write-Output 'd'
$x # 'a','b','c','d'
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's an important question:

When multiple objects are returned from a command, you can capture the output from that command in different variables, like this:

$x, $y = 1..10

If you run that, $x will have a value of 1, and $y will have values 2 to 10.

In pipeline chaining, if you assign the results of a pipeline chain with two pipelines to two variables like this, will the first variable receive the first object returned from the first pipeline chain that returns something, and the second variable will receive everything else? Or will the first variable receive the results from the first pipeline in the chain, and the second variable will receive the results of all other pipelines in the chain (an array of arrays, or just a single array if there is just one additional pipeline)?

For scripting purposes if it would be more useful if concatenation of pipeline data were done as an array of arrays so that you could assign a single pipeline chain to multiple variables, with each variable storing the results of one of the pipelines that were being chained. Chaining is often used for completely different commands, so we really wouldn't want to just group their output all together as a single collection of data.

If you want to skip the output of one of the pipelines in the chain, just assign that to $null.

For example:

$p1,$null,$p3 = 'a','b','c' | Write-Output && Write-Output 'd' && Write-Output 'e'
$p1 # 'a','b','c'
# 'd' gets discarded since it is the result of the second pipeline in the chain and that is assigned to $null
$p3 # 'e'

You would need additional tests specifically for this scenario as well.

Also, in the examples above and below this you should point out that the value of $x will be @('a','b','c'),@('d').

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, PowerShell permits assignment from any statement. So this will work with the same semantics:

$x, $y = if ($true) { 1; if ($true) { 2 } }
$x # 1
$y # 2

It will just assign each element in the pipeline to variables on the LHS until the last, which gets the tail.

But I'll mention it in the RFC for good measure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That wasn't my point. The point I was trying to make is that if I'm chaining pipelines, it would be of very questionable value for users to mix the results of the commands in that chain, which would often be completely different objects. At the same time, it is likely for users to want to capture the results of individual pipelines in the chain, in a more simple syntax. With this proposal in its current state we're chaining pipelines, not statements, so I can't do this:

$r1 = p1Command && p2Command && $r3 = p3Command

Instead, with pipeline chaining, my point was that this:

$r1,$null,$r3 = p1Command && p2Command && p3Command

would result in $r1 containing all of the results of p1Command, the results of $r2 would be thrown away, and $r3 would contain all of the results of p3Command. That behaviour is new/different, but based on the proposal if we're chaining pipelines instead of statements, I really think it's the right thing to do. Otherwise, when you have no idea how many results come back from different commands, how would you capture them in different variables in pipeline chaining?

Make sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do this, though, right?

($r1 = p1Command) && p2Command && ($r3 = p3Command)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. But with that syntax I would go back to my argument that these should work more like bash, and allow that without having to use brackets.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And even if that syntax is a possibility, I still think it makes more sense for results from individual segments in the chain to be grouped together, such that the entire result is easier to segment (array of arrays/values).

or constructions like `$x = cmd1 && $y = cmd2 && $x + $y`.

Also see the [alternate proposals section](#alternate-proposals-and-considerations).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following some of the earlier comments on how to store results of different pipelines in different variables (which I believe will be an important scenario to support), and the comments about our precedence differing from bash, I'm leaning towards feeling this would be better as a statement chain than as a pipeline chain.

an entire pipeline chain; `cmd1 & && cmd2` would be equivalent to
`cmd1 &; cmd2` since a backgrounded pipeline
will never fail in the invoking context.

Copy link
Contributor

@KirkMunro KirkMunro Jun 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to background an entire pipeline chain, wrap it in a script block invoked with the call operator and use the background operator with that script block (note: there is a bug/design flaw with this when using variables for the script blocks that are being run in the background).

Speaking of script blocks, this RFC doesn't talk about them, but bash supports commands like this:

command1 && {command2; command3;} || command4

We should consider doing the same.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so command1 && {command2; command3;} || command4 would actually be equivalent to command1 && $(& { command2; command3 }) || command4? I'm not sure I like that implied invocation. What if I want that scriptblock to be sent to output or captured as an RVALUE?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use &&& for that! I'm kidding -- I think command1 && $(command2; command3) || command4 is sufficient and not worth the syntactic sugar of curly braces (nor the added ambiguity.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah cmd1 && $(cmd2; cmd3) && cmd4 is a valid syntax under the proposal ($(...) is a valid pipeline), but unfortunately always sets $? to true.

I think the answer to that is to fix $? though.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it so? :)

Copy link
Contributor

@KirkMunro KirkMunro Jun 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subexpressions really don't feel like the right syntax to recommend here.

My argument for simply dot-sourcing a script block was because that is what users would want the vast majority of the time from that syntax, and for the other times when they do indeed want a script block they can still get one. That said, I recognize that it diverges from the current syntax.

I feel subexpressions are really the wrong syntax to promote for invocation of multiple commands in a staged execution statement like this though, and I worry that's likely to lead to confusion and users trying to use them in other locations that they cannot or in situations where script blocks are a better answer.

@rjmholt: The following should work, correct?

pipeline1 && . {command1; command2} || pipeline3

As long as that works, I don't have much issue with this. But given the fact that you can only use an expression to start a pipeline, it would be better to promote script block invocations over subexpressions in a pipeline chaining model IMHO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following should work, correct?

Yeah that will work perfectly.

Without the ., it will also execute successfully but you'll be passing a scriptblock expression into the pipeline (i.e. it won't do what's intended)

Copy link
Contributor

@vexx32 vexx32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking rather nice. Couple questions and comments. Appreciate you taking the time to write this up! 💖


This RFC proposes adding AND-OR lists to PowerShell, using the same `&&` and `||`.
Because "AND-OR list" is neither intuitive nor common as a terminology,
instead the term **pipeline chains** is proposed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why pipeline chains? These operators have nothing to do with the pipeline itself, do they? Wouldn't "command chains" be a bit more appropriate since pipelines are not really part of this operation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically because the proposed implementation is for the operators to be interspersed between pipelines. They are syntactically related to the PipelineAst:

1,2,3 | % { Write-Output $_ } && Write-Host "Succeeded"

Command chain operator doesn't illustrate the fact that you can slide a whole pipeline between them.

## Motivation

> As a PowerShell user, I can chain commands with `&&` and `||`
> so that I have an ergonomic syntax to conditionally invoke side-effectful commands.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Side-effectful" doesn't carry any significance that I can see here, and seems to confuse the statement a bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a wording to capture the fact that native commands can succeed or fail orthogonally to their output; their output is the effect, but we're interested in the side-effect, the operation of the command and the exit code (or success boolean) they set.

#### Example 1

```powershell
Write-Output "Hello" && Write-Output "Hello again"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will this interact when different streams are present? For example, currently:

Write-Output "Output"
Write-Host "Host"

Currently this sequence of commands will pretty much always result in Host being displayed first. If we chain these in the same sequence:

Write-Output "Output" && Write-Host "Host"

Will the output reach the host before the second command is invoked?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as:

Write-Output "Output"; if ($?) { Write-Host "Host" }

I can write in an example.


The consequence of this will be that an entire pipeline chain
can be sent to a background job for evaluation,
rather than individual pipelines within it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will this work with parentheses? The background job operator currently returns a job object. So, how would this operate?

(cmd1 &) && cmd2

I'm sure the more natural usage would be the reverse, but that's probably less problematic?

cmd1 && (cmd2 &)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parentheses always set $? to true, so it will always succeed.

That might seem unhelpful, but I believe the solution there is to ensure $? is set properly.

Also, the & operator is translated into Start-Job -ScriptBlock { ... }. So <expr> & will succeed as long as Start-Job succeeds.

The use of this is possibly less than the use of being able to background
an entire pipeline chain; `cmd1 & && cmd2` would be equivalent to
`cmd1 &; cmd2` since a backgrounded pipeline
will never fail in the invoking context.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could still background an entire chain either way, just one method would mandate the use of parentheses to do so. But this calls back to what I mentioned before, too. Since the job operator returns job objects, it would only ever fail if the job creation itself failed... which I guess it possible, so it might still have some occasional utility for very specific cases.

I think I agree that the default should lean towards allowing simply backgrounding entire chains though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that's why my default proposal is to background the entire chain. Evaluating the success of cmd1 & is not helpful and possibly confusing.

Being able to do cmd1 && cmd2 & seems much more useful to me.

Copy link
Contributor

@KirkMunro KirkMunro Jun 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bear with me while I'm think something out loud here. I've had asynchronous execution of PowerShell on my mind lately, and I'm exploring those thoughts a bit.

What if you changed the way cmd1 & is evaluated for success? Sure, a job gets created, which is always successful unless there's some odd bug in Start-Job, so that's pretty useless. How would you really want to measure the success of a job? Wouldn't it be better to measure whether that job completes without error or not?

PowerShell could benefit from a lot better asynchronous support, so I find myself wondering if this syntax:

cmd1 & && cmd2 & && cmd3 & || cmd4

Could be interpreted as a way to launch cmd1 in the background, and if that job succeeds, launch cmd2 in the background, and if that job succeeds launch cmd3 in the background, and if any of them fail invoke cmd4 (i.e. what if when the & background operator is used in a pipeline chain, PowerShell automatically waits for that job to complete, and uses the result of the job to check for success or failure).

This is like having await support in PowerShell, which is exactly what I would expect from the & background operator followed by a pipeline chain operator, because simply deciding success or failure on the basis of job creation is pretty much useless.

Further, if the & background operator gets $! support for the current job that is running (see some discussion in PowerShell Issue 9873 and RFC #193), then cmd4 could use that to identify which job failed and perform error handling for that job.

The flipside of this is that if & has lower precedence than && and || as is currently proposed in this RFC, to accomplish the same thing would be quite complicated. You would have to write something like this:

# But how would PowerShell recognize that this should wait for the
# job to complete since the background operators are embedded
# in script blocks? It would seem that this would result in success
# being measured simply by looking at $?, so that doesn't seem to
# work.
.{cmd1 &} && .{cmd2 &} && .{cmd3 &} || cmd4

Instead we would need to be much more explicit with something like this:

# This is explicit, but much more cumbersome syntactically
.{cmd1 & | Receive-Job -Keep *> $null; $?}
   && .{cmd2 & | Receive-Job -Keep *> $null; $?}
   && .{cmd3 & | Receive-Job -Keep *> $null; $?}
   || cmd4

So it may still be doable, but with a bunch of extra enclosures to change the precedence and extra glue to get the right result.

Also, consider the value of being able to do this in PowerShell:

cmd1 & cmd2 & && cmd3 & || cmd4

This could be interpreted as a way to easily fan out multiple jobs (cmd1, cmd2), wait for them to complete and then run cmd3 in a job only if the first two completed without failures, and then wait for that job to complete, with cmd4 running if any of the previous 3 jobs failed (the actual work in the jobs, not the creation of the jobs themselves).

I think these scenarios are really worth thinking about. They add a lot of value to PowerShell, and I really like the notion that a pipeline chain with a background operator in the last command in any one of the segments would result in the chain waiting on the job(s) automatically, with the measurement of success for the segment that is specifically backgrounded being whether or not the job(s) had errors (again, that's pretty much the only thing that makes sense).

One thing this doesn't give up front is easy background of an entire chain with a single & operator. Scripters can easily get that behaviour with parentheses. That is very low effort. If chain operators have a higher precedence than & background operators, then there is no await, and trying to enable scenarios like those demonstrated above becomes much more cumbersome.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I think about my previous comment, the more I really like it. Other thoughts/comments would be appreciated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move this interesting proposal in another RFC to keep current RFC more simple?

Copy link
Contributor

@iSazonov iSazonov Jun 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not exclude the possibility of interpreting it differently in this job context. If of course it is possible to identify this context.

There is a Paul's RFC for ForEach-Parallel where I vote for "parallel pipeline" - it intersects with what I'm trying to comprehend here.

Copy link
Contributor

@KirkMunro KirkMunro Jun 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think an additional context for a single | couldn't possibly work. In your cmd1 & cmd2 & cmd3 | cmd4 example, I read that as: run cmd1 and cmd2 in the background and cmd3 | cmd4 in the current runspace.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're off in the weeds:

What is the value of cmd1 & && cmd2 vs cmd1 && cmd2?
Nothing.

UNLIKE other programming languages, you get no benefit from await because your thread doesn't have anything better to do than wait around for cmd1 to finish.

Copy link
Contributor

@KirkMunro KirkMunro Jun 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jaykul I'm working with jobs, and looking at this as a way to wait for them and then proceed based on the results. That's what I meant by "like having await".

What is the value of cmd1 & cmd2 & cmd3 & || cmd4 & cmd5 &? Launch cmd1, cmd2, and cmd3 asynchronously in the background, and if any of them fails, launch cmd4 and cmd5 asynchronously in the background. You get benefit from waiting for those jobs to complete, and only launching cmd4 and cmd5 if one or more of the jobs comes back with an error.

Also, the argument I'm making is that I disagree with the proposed operator precedence, which suggests that & should have a higher precedence than && and ||. This is the opposite of the precedence in bash, where we're borrowing these operators from (that alone raises a red flag). By having && and || have a higher precedence than &, and properly checking to see if a job succeeded or failed, which means waiting on the job(s) (where I was finding similarity with await) to check the results instead of just checking to see if the job launched successfully or not (which is useless), we get some additional fan-out support in PowerShell.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jaykul Also you're thinking code-only, not ad-hoc/shell. The value of cmd1 & && cmd2 & in a shell, in particular if some of RFC #193 gets accepted/implemented such that jobs can be launched and still show visible progress without blocking the current shell, is very high when compared to cmd1 && cmd2.

For example:

```powershell
cmd1 || throw "cmd1 failed"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually really like this particular idea. This would make the chains significantly more useful, I feel, serving a purpose to simplify conditional control flow statements.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's in my actual implementation in the PR (seems useful to me). But was controversial, so I instead made the more conservative approach my main proposal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this as well, despite the corner cases that could cause confusion.


- The compromise nature of this approach means the PowerShell Language
becomes more complicated and arguably less consistent,
both conceptually and in terms of maintenance.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to have subsequent pipeline chains require the use of a subexpression or something, rather than implicitly allowed here. This does muddy the waters a bit, but if it is deliberately prevented here, it would be more or less OK, I feel.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you expand a bit more on the subexpression requirement, maybe an example of what you're thinking?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So rather than just allowing pipeline chains to follow something like throw or return or whatever...

Attempting to do so without enclosing it should be a parse error, imo. But you should still be able to do it by properly enclosing it:

cmd1 || throw ("Bad" && cmd3)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean cmd1 || throw "Bad" && cmd_cleanup ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I'm saying I don't think should be permitted.

It looks like the && should never trigger there, because of the throw. But in fact it will because after the throw it's a separate pipeline chain.

I feel this should be required to be enclosed, because otherwise it's very unclear what will happen in these situations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vexx32: That's a nice way to allow flow control with pipeline chains while dodging semantic confusion. I like it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to write something about this being complicated in terms of the grammar because of needing to support simple pipelines without parens, but it might not be as complicated as I first thought.

Essentially throw <expr> currently allows a pipeline statement where <expr> is:

  • throw "Bad"
  • throw 1,2,3 | % { "Bad $_" }

The original proposal in the RFC is to just allow pipeline chains wherever a pipeline is, but we can modify that so that throw still only allows a pipeline (no chain). A pipeline can optionally start with a pipeline expression, like (cmd1 && cmd2), or a subexpression, like $(if (...) { ... }) (that's always been the case), so that works out.

I think I can deal with that, even though it makes the grammar more complicated. (The reason I want to avoid that is because it's a common trap that's afflicted everything from PL/I to Python -- keeping a grammar consistent enough that you can reason about it in your head without being reminded of things usually means you aren't engineering awful pitfalls into your language that you'll never be able to get rid of.)

Other possibilities are:

- AND-OR lists
- Command chain operators
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd vote for this, with short-circuit operators being a close second for me, I think.


## Alternate Proposals and Considerations

### `&&` and `||` as a statement separator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We’re talking about making Bash users easier to adapt, but the limitations mentioned in this section worry me a lot. Most likely this will be misleading Bash users and we will receive a lot of negative feedback and requests to implement it.
Bash users will see these operators, but will find that they do "not work as expected". This is the worst thing that can happen. It is like implementing operator + which "does not always work as expected" (0+1=1 but 1+0=0).

Although I like this proposal in general, I would not want to accept it in this form and create a headache for Bash users.

I see two ways - either try to get as close as possible to bash or act like with the name: we have already chosen another name - pipeline chains, which means we can choose another implementation - maybe use &&& and ||| that should indicate that it works almost like && and || but a little different.

@rjmholt Great work!

@mklement0 Please review the RFC.

Copy link
Contributor

@KirkMunro KirkMunro Jun 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iSazonov Glad to see that I'm not the only one with those concerns. I dislike &&& and |||, but I agree about respecting bash syntax.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please no &&& and |||

- `$LASTEXITCODE`
- Whether errors have been written
- A new custom semantics

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jobs could/should be handled differently. See my brain dump above about how that might work.

and may occur anywhere a `PipelineBaseAst` does in a PowerShell AST.

`ICustomAstVisitor2` and `AstVisitor2` would be extended to deal with this AST,
and .NET Core 3's new default interface implementation feature would be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the default interface implementation should throw an exception or do nothing?
I think doing nothing would be consistent with the old way of introducing a new interface when changing the Ast, but it also hides the fact that implementors might be missing something they should be implementing.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lzybkr to that point I think it would be a good time to introduce a DefaultVisit, that way in the future the implementer can choose how to react to a new AST type.

That doesn't really help for this round of changes though. Personally I would prefer that it do nothing. Most of my ICustomAstVisitor2's will continue to work for the most part. And at the very least PowerShellEditorServices would have some hoops to jump through to get a significantly amount of analysis visitors working again otherwise.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DefaultVisit is a great idea.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any of you mind elaborating the DefaultVisit idea? 😄

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daxian-dbw basically this:

interface ICustomAstVisitor
{
    object DefaultVisit(Ast ast) => null;

    object VisitCommandAst(CommandAst commandAst) => DefaultVisit(commandAst);

    object VisitNewAst(NewAst newAst) => DefaultVisit(newAst);
}

That way if you haven't implemented VisitNewAst, but you have implemented DefaultVisit, you can choose how to react to new AST types. roslyn does this with it's syntax visitor.

#### Error semantics

Errors will have the same semantics as the equivalent
`cmd1; if ($?) { cmd2 }` syntax.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expressions typically set $? to $true, so there will be surprising behavior:

$($true || 1) -eq $($false || 1)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed -- perhaps I didn't address that directly enough. But my feeling is that any other logic that differs from that will confuse people and will be much more complex to maintain. My feeling is that improving the proposed experience in this respect is a matter of improving the way $? works.

I've also suggested expression-based &&/|| equivalents (the way JavaScript does it) elsewhere, but it seems that command success is the desired feature.

In your particular example, I don't know if I'd characterise that as surprising specifically, but I think (Invoke-Failure) || "Hello" will be surprising, since the pipeline expression will set $? to true in the first instance.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Invoke-Failure) || "Hello" will be surprising

Indeed! How do we explain that behavior to folks? Recommend that folks not use grouping/sub-expressions with this feature? Add a PSSA rule for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think:

  • This chaining stuff depends on the $? concept
  • Educate about (...) having funny effects on $? and by extension &&/||
  • Write a PSSA rule perhaps (easy to do)
  • Have a serious discussion about $?'s behaviour in PowerShell in an RFC (although I wouldn't be that disappointed if we keep existing behaviour -- I don't like breaking things)

I don't want to unhitch from $? because then we're off in the weeds with yet another thing in PowerShell we have to explain to people and why it's not consistent with the other stuff. Plus we'd have to start discussing commands where a non-zero exit code doesn't mean failure...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, we could make it so that (…) behaves the same way as  when it comes to setting $?.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah -- that's what I mean in point 4. I think it's bigger than this syntactic feature and that change (which would be large and breaking) should be discussed separately

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a bit misleading that simply adding parentheses changes whether PS considers a command successful or not... We should probably revisit that. 🙂

Copy link
Contributor

@mklement0 mklement0 Sep 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vexx32: Agreed - please see PowerShell/PowerShell#3359

@stereokai
Copy link

Is there any plan to merge this back to PowerShell 5?

@vexx32
Copy link
Contributor

vexx32 commented Jun 20, 2019

@stereokai nope. PS 5 has not been getting updates for quite a while now. That's not going to change. PS 7 will be a more effective full replacement for PS5 though. The PS7 preview currently released is very promising. :)

@rkeithhill
Copy link

Any chance this will land in preview.2 as an experimental feature?

@stereokai
Copy link

@vexx32 Thanks for clarifying :) I was just curious, as I've read that PS5 is integrated much more deeply with Windows than PS Core. What did you mean by PS7 being very promising?

@iSazonov
Copy link
Contributor

Any chance this will land in preview.2 as an experimental feature?

We still haven't got consensus on some questions. It seems @rjmholt has a public branch with prototype - you can fork, compile and try in any time.

@vexx32
Copy link
Contributor

vexx32 commented Jul 2, 2019

@stereokai Lots of fun new features, we're getting back a lot of what was "lost" in the transition to .NET Core in 6.0, and a whole bunch of bugfixes have happened since. :)

@rjmholt rjmholt changed the title Pipeline Chain Operators Pipeline Chain Operators (&& / ||) Jul 18, 2019
Copy link
Contributor

@joeyaiello joeyaiello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wanted to get some of these comments through, but I've made it down to "Higher precedence than * and ;". Will finish up later.

+ FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,FailInProcess

4
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, the inconsistency between this example not succeeding and the previous example succeeding is the crux of a discussion within @PowerShell/powershell-committee.

The real question is whether we want to fix $? before or after relying upon it in these chain operators.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d rather we fix $? before relying on it here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PowerShell/powershell-committee agrees we shouldn't block this functionality on fixing $?. It's useful today, and this new functionality will be useful in spite of the quirks.

In my opinion, it's also going to help drag issues with $? out into the light.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joeyaiello should we file an issue around the problematic nature of $? to track that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd be awesome, though I suspect we have a very old one (2017 or earlier) on error handling in general where a lot of this is already captured.

Copy link
Contributor

@mklement0 mklement0 Oct 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have PowerShell/PowerShell#3359, though it comes with a lot of discussion - if we now agree that it should be fixed, perhaps opening a new, focused issue is called for.

| pipeline_chain "&&" [newlines] pipeline
| pipeline_chain "||" [newlines] pipeline
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we decided to limit that type of lookahead multi-line continuance to just the pipeline operator, @PowerShell/powershell-committee agrees we should remove the [newline] from each of these. [newlines] after the operator are fine.

cmd2 ||
cmd3
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As said above, we in the @PowerShell/powershell-committee explicitly don't want this for perf and interactiving/scripting consistency reasons.

@kilasuit
Copy link

Any chance this will land in preview.2 as an experimental feature?

Same question but for preview.5

@rjmholt rjmholt requested a review from joeyaiello September 30, 2019 18:19
@rjmholt
Copy link
Contributor Author

rjmholt commented Oct 1, 2019

I've just updated this after feedback from the @PowerShell/powershell-committee, simplifying it to take out the control flow statements aspects.

The implementation PR in PowerShell/PowerShell#9849 also reflects these changes.

Tagging the @PowerShell/powershell-committee for review.

@joeyaiello
Copy link
Contributor

@PowerShell/powershell-committee is approving this one with @JamesWTruher, @daxian-dbw, @SteveL-MSFT and myself in the room.

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

Successfully merging this pull request may close these issues.