Support ternary operator in PowerShell language #10367
Conversation
There was a problem hiding this comment.
Just some minor comments and questions.
src/System.Management.Automation/engine/parser/VariableAnalysis.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Why cannot ? and : just be added to CharTraits.ForceStartNewTokenAfterNumber? This trait is only used while the tokenizer is in expression mode.
There was a problem hiding this comment.
I might answer my own question … because it needs to be hidden behind an experimental feature flag at this time?
There was a problem hiding this comment.
I don't think the PR context explained clearly enough what this is for:
(if the following are entire command lines)
1?2:3 is a command
1-lt2?3:4 is a ternary expression.
It will be even harder for EditorSyntax (PowerShell/EditorSyntax#184) to correctly syntax scope this via TextMate. This will require an extra set of numericConstant rules to handle the 'first possible operand of an expression' vs 'a numeric value clearly in expression mode' vs 'a numeric constant in command mode'. However, it is an interesting challenge to see if I can simplify.
There was a problem hiding this comment.
Comments were added for the parameter forceEndNumberOnTernaryOperatorChars.
/// <param name="forceEndNumberOnTernaryOperatorChars">
/// In some cases, we want '?' and ':' to end a number token too, so they can be
/// treated as the ternary operator tokens.
Also, comments were added in Parser.cs to the places where the parameter endNumberOnTernaryOpChars was added.
|
@TravisEz13 Your comments have been addressed. Can you please take another look? |
There was a problem hiding this comment.
Please respond to the comment about moving parsing tests to parsing.tests.ps1
There was a problem hiding this comment.
the only real blocker for me is that I believe we should keep the parser tests together, the behavioral tests can be separated, but I would like to have all the parser tests in the same location.
|
@adityapatwardhan and @JamesWTruher Your comments are addressed. I rebased the branch with the master branch. The last 2 commits are changes that addressed your comments. Please take another look, thanks! |
|
@JamesWTruher Can you update your review? |
There was a problem hiding this comment.
thanks for making the test changes
|
It seems we should have only one CL-* label. |
|
@iSazonov With experimental, we should have a second, but only experimental. |
|
🎉 Handy links: |
PR Summary
Add support to ternary operator
<condition> ? <if-true> : <if-false>. It results in aTernaryExpressionAst.Address #3239
RFC: PowerShell/PowerShell-RFC#218
The old draft PR where a lot of discussions happened: #10161
The ternary operator behaves like the simplified if-else statement. The
condition-expressionwill always be evaluated, and its result will be converted to boolean to determine which branch will be evaluated next:if-true-expressionwill execute if the condition's result is evaluated astrueif-false-expressionwill execute if the condition's result is evaluated asfalsePR Context
Ternary operator has lower precedence than binary operator, so you can write
$a -eq $b ? "Hello World" : [int]::MaxValue.In order to make the number constant expression work naturally with ternary operators, such as in
return ${succeed}?0:1, characters?and:are made start-new-token chars conditionally when scanning for numbers.When it's known for sure that we are expecting an expression, allowing a generic token like
123?is not useful and bound to result in parsing errors.In those cases, we will force to start a new token upon seeing characters
?and:when scanning for a number, so that that number constant expressions can work with ternary operators more intuitively.Note that, the implicit line continuance for
?support was reverted in the latest code change based on the feedback I received within the team.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.PSTernaryOperator