Skip to content
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

Add grammar for "if (test) then {expr}" with no else #284

Merged
merged 3 commits into from
Jan 17, 2023

Conversation

michaelhkay
Copy link
Contributor

As discussed in issue #234. In reviewing this PR, I suggest we consider it together with the existing proposals for ternary conditionals (x ?? y !! z) and the "otherwise" operator.

Copy link
Contributor

@ChristianGruen ChristianGruen left a comment

Choose a reason for hiding this comment

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

If curly braces are used, I would strongly vote for allowing the else branch. Due to the braces, we now have an unambiguous assignment of conditions and branches. Examples:

Expression Equivalent expression
if($a) { if($b) { $c } } if($a) then if ($b) then $c else () else ()
if($a) { if($b) { $c } else { $d } } if($a) then if ($b) then $c else $d else ()
if($a) { if($b) { $c } } else { $e } if($a) then if ($b) then $c else () else $e
if($a) { if($b) { $c } else { $d } } else { $e } if($a) then if ($b) then $c else $d else $e

@ChristianGruen
Copy link
Contributor

In addition, we should allow else if:

if($a) { $b } else if($c) { $d } else { $e }

@ChristianGruen
Copy link
Contributor

ChristianGruen commented Jan 10, 2023

Thanks, @michaelhkay, for adding else. I have revised my first comment to make it better accessible.

In my second comment, I proposed to also support else if chains:

if($a) { $b }
else if($c) { $d }
else if($e) { $f }

The grammar rules for else if could look as follows:

IfExpr       ::= "if" "(" Expr ")" (("then" ExprSingle "else" ExprSingle) | BracedIfExpr)
BracedIfExpr ::= EnclosedExpr ("else" (EnclosedExpr | ("if" "(" Expr ")" BracedIfExpr)))?

Without the extended grammar, we would need to use additional curly braces:

if($a) {
  $b
} else {
  if($c) {
    $d
  } else {
    if($e) {
      $f
    }
  }
}

@liamquin
Copy link

i like elsif or elif but i think there's probably no ambiguity (neither for humans nor for the parser) with else if, so sounds OK here.

@michaelhkay
Copy link
Contributor Author

Of all the languages I've used, I much preferred Algol68's IF expr (THEN expr)? (ELSE expr)? FI. Sigh.

@michaelhkay
Copy link
Contributor Author

As regards the grammar, we tend to prefer iteration over recursion. So perhaps

IfExpr       ::= "if" "(" Expr ")" (UnbracedActions | BracedActions)
UnbracedActions ::= "then" ExprSingle "else" ExprSingle 
BracedActions ::= EnclosedExpr ("else" "if "(" Expr ")" EnclosedExpr)*  ("else" EnclosedExpr)?

@ChristianGruen
Copy link
Contributor

ChristianGruen commented Jan 10, 2023

+1 for the iterative variant of the grammar.

The year 1968 is something I wasn't blessed to experience personally, but when I read FI, I get reminded of good old machine code (beq/bne).

@ndw
Copy link
Contributor

ndw commented Jan 17, 2023

Accepted at meeting 018

@ndw ndw merged commit 3c41ffb into qt4cg:master Jan 17, 2023
ndw pushed a commit to ndw/qtspecs-xslt4 that referenced this pull request Jan 31, 2023
@michaelhkay michaelhkay deleted the Issue234-If-without-else branch July 8, 2023 22:07
@michaelhkay michaelhkay added Enhancement A change or improvement to an existing feature Tests Added Tests have been added to the test suites labels Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement A change or improvement to an existing feature Tests Added Tests have been added to the test suites
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants