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.

Sign up for GitHub

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

Jump to bottom

Allow single-line if expressions #648

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Allow single-line if expressions #648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
657c586
Allow single-line `if` expressions
rlefevre Nov 16, 2019
d9404ad
Add consecutive comments tests for `if` expressions
rlefevre Nov 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion parser/src/AST/Expression.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ data Expr'
| AccessFunction LowercaseIdentifier

| Lambda [(Comments, Pattern.Pattern)] Comments Expr Bool
| If IfClause [(Comments, IfClause)] (Comments, Expr)
| If IfClause [(Comments, IfClause)] (Comments, Expr) Multiline
| Let [LetDeclaration] Comments Expr
| Case (Commented Expr, Bool) [(Commented Pattern.Pattern, (Comments, Expr))]

Expand Down
2 changes: 1 addition & 1 deletion parser/src/AST/Json.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ instance ToJSON Expr where
, ("body", showJSON body)
]

If (Commented _ cond' _, Commented _ thenBody' _) rest' (_, elseBody) ->
If (Commented _ cond' _, Commented _ thenBody' _) rest' (_, elseBody) _ ->
let
ifThenElse :: Expr -> Expr -> [(Comments, IfClause)] -> JSValue
ifThenElse cond thenBody rest =
Expand Down
13 changes: 8 additions & 5 deletions parser/src/Parse/Expression.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,14 @@ ifExpr elmVersion =
>> whitespace
in
do
first <- ifClause elmVersion
rest <- many (try $ (,) <$> elseKeyword <*> ifClause elmVersion)
final <- (,) <$> elseKeyword <*> expr elmVersion

return $ E.If first rest final
(first, firstMultiline) <- trackNewline $ ifClause elmVersion
(rest, restMultiline) <- trackNewline $ many (try $ (,) <$> elseKeyword <*> ifClause elmVersion)
(final, finalMultiline) <- trackNewline $ (,) <$> elseKeyword <*> expr elmVersion

return $ E.If first rest final $
case (firstMultiline, restMultiline, finalMultiline) of
(JoinAll, JoinAll, JoinAll) -> JoinAll
_ -> SplitAll


ifClause :: ElmVersion -> IParser E.IfClause
Expand Down
4 changes: 2 additions & 2 deletions src/AST/MapExpr.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ instance MapExpr Expr' where
AccessFunction _ -> expr
Lambda params pre body multi ->
Lambda params pre (mapExpr f body) multi
If c1 elseIfs els ->
If (mapExpr f c1) (mapExpr f elseIfs) (mapExpr f els)
If c1 elseIfs els multiline ->
If (mapExpr f c1) (mapExpr f elseIfs) (mapExpr f els) multiline
Let decls pre body ->
Let (mapExpr f decls) pre body
Case cond branches ->
Expand Down
160 changes: 109 additions & 51 deletions src/ElmFormat/Render/Box.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1407,57 +1407,9 @@ formatExpression' elmVersion importInfo context aexpr =
(fmap (formatPreCommentedExpression elmVersion importInfo SpaceSeparated) args)
|> expressionParens SpaceSeparated context

AST.Expression.If if' elseifs (elsComments, els) ->
let
opening key cond =
case (key, cond) of
(SingleLine key', SingleLine cond') ->
line $ row
[ key'
, space
, cond'
, space
, keyword "then"
]
_ ->
stack1
[ key
, cond |> indent
, line $ keyword "then"
]

formatIf (cond, body) =
stack1
[ opening (line $ keyword "if") $ formatCommentedExpression elmVersion importInfo SyntaxSeparated cond
, indent $ formatCommented_ True (formatExpression elmVersion importInfo SyntaxSeparated) body
]

formatElseIf (ifComments, (cond, body)) =
let
key =
case (formatHeadCommented id (ifComments, line $ keyword "if")) of
SingleLine key' ->
line $ row [ keyword "else", space, key' ]
key' ->
stack1
[ line $ keyword "else"
, key'
]
in
stack1
[ blankLine
, opening key $ formatCommentedExpression elmVersion importInfo SyntaxSeparated cond
, indent $ formatCommented_ True (formatExpression elmVersion importInfo SyntaxSeparated) body
]
in
formatIf if'
|> andThen (map formatElseIf elseifs)
|> andThen
[ blankLine
, line $ keyword "else"
, indent $ formatCommented_ True (formatExpression elmVersion importInfo SyntaxSeparated) (AST.Commented elsComments els [])
]
|> expressionParens AmbiguousEnd context
AST.Expression.If if' elseifs els multiline ->
formatIfExpression elmVersion importInfo if' elseifs els multiline
|> expressionParens AmbiguousEnd context

AST.Expression.Let defs bodyComments expr ->
let
Expand Down Expand Up @@ -1595,6 +1547,112 @@ formatExpression' elmVersion importInfo context aexpr =
]


formatIfExpression ::
ElmVersion
-> ImportInfo
-> AST.Expression.IfClause
-> [(AST.Comments, AST.Expression.IfClause)]
-> (AST.Comments, AST.Expression.Expr)
-> AST.Multiline
-> Box
formatIfExpression elmVersion importInfo if' elseifs (elsComments, els) multiline =
let
(cond, (AST.Commented preThen thenExpr postThen)) =
if'

cond' =
formatIfOpening (line $ keyword "if") $
formatCommentedExpression elmVersion importInfo SyntaxSeparated cond

then' =
concat $
[ Maybe.maybeToList $ formatComments preThen
, [ formatExpression elmVersion importInfo SyntaxSeparated thenExpr ]
, Maybe.maybeToList $ formatComments postThen
]

else' =
(Maybe.maybeToList $ formatComments elsComments)
++ [ formatExpression elmVersion importInfo SyntaxSeparated els ]
in
case
( multiline
, isLine cond'
, allSingles then'
, fmap (formatElseIf elmVersion importInfo) elseifs
, allSingles else'
)
of
(AST.JoinAll, Right singleCond, Right singleThens, [], Right singleElses) ->
line $ row $ concat
[ [singleCond, space]
, List.intersperse space singleThens
, [ space, keyword "else", space ]
, List.intersperse space singleElses
]

(_, ifCond, thenBody, elseIfs', elseBody) ->
stack1
[ boxOrLineToBox ifCond
, indent $ boxesOrLinesToBox True thenBody
]
|> andThen elseIfs'
|> andThen
[ blankLine
, line $ keyword "else"
, indent $ boxesOrLinesToBox True elseBody
]


formatIfOpening :: Box -> Box -> Box
formatIfOpening key cond =
case (key, cond) of
(SingleLine key', SingleLine cond') ->
line $ row [ key', space, cond', space, keyword "then" ]

_ ->
stack1
[ key
, cond |> indent
, line $ keyword "then"
]


formatElseIf :: ElmVersion -> ImportInfo -> (AST.Comments, AST.Expression.IfClause) -> Box
formatElseIf elmVersion importInfo (ifComments, (cond', body')) =
let
key =
case (formatHeadCommented id (ifComments, line $ keyword "if")) of
SingleLine key' ->
line $ row [ keyword "else", space, key' ]
key' ->
stack1
[ line $ keyword "else"
, key'
]
in
stack1
[ blankLine
, formatIfOpening key $ formatCommentedExpression elmVersion importInfo SyntaxSeparated cond'
, indent $ formatCommented_ True (formatExpression elmVersion importInfo SyntaxSeparated) body'
]


boxOrLineToBox :: Either Box Line -> Box
boxOrLineToBox boxOrLine =
case boxOrLine of
Right singleLine -> line singleLine
Left box -> box


boxesOrLinesToBox :: Bool -> Either [Box] [Line] -> Box
Copy link
Contributor Author

@rlefevre rlefevre Nov 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe we could put this function and boxOrLineToBox into Box.hs?
I'm not sure because of the use of ElmStructure.forceableSpaceSepOrStack1.

boxesOrLinesToBox forceMultiline boxesOrLines =
ElmStructure.forceableSpaceSepOrStack1 forceMultiline $
case boxesOrLines of
Right singleLines -> fmap line singleLines
Left boxes -> boxes


formatCommentedExpression :: ElmVersion -> ImportInfo -> ExpressionContext -> AST.Commented AST.Expression.Expr -> Box
formatCommentedExpression elmVersion importInfo context (AST.Commented pre e post) =
let
Expand Down
8 changes: 4 additions & 4 deletions tests/Parse/ExpressionTest.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,10 +259,10 @@ tests =
]

, testGroup "if statement"
[ example "" "if x then y else z" $ at 1 1 1 19 (If (Commented [] (at 1 4 1 5 (VarExpr (VarRef [] $ LowercaseIdentifier "x"))) [],Commented [] (at 1 11 1 12 (VarExpr (VarRef [] $ LowercaseIdentifier "y"))) []) [] ([],at 1 18 1 19 (VarExpr (VarRef [] $ LowercaseIdentifier "z"))))
, example "comments" "if{-A-}x{-B-}then{-C-}y{-D-}else{-E-}if{-F-}x_{-G-}then{-H-}y_{-I-}else{-J-}z" $ at 1 1 1 78 (If (Commented [BlockComment ["A"]] (at 1 8 1 9 (VarExpr (VarRef [] $ LowercaseIdentifier "x"))) [BlockComment ["B"]],Commented [BlockComment ["C"]] (at 1 23 1 24 (VarExpr (VarRef [] $ LowercaseIdentifier "y"))) [BlockComment ["D"]]) [([BlockComment ["E"]],(Commented [BlockComment ["F"]] (at 1 45 1 47 (VarExpr (VarRef [] $ LowercaseIdentifier "x_"))) [BlockComment ["G"]],Commented [BlockComment ["H"]] (at 1 61 1 63 (VarExpr (VarRef [] $ LowercaseIdentifier "y_"))) [BlockComment ["I"]]))] ([BlockComment ["J"]],at 1 77 1 78 (VarExpr (VarRef [] $ LowercaseIdentifier "z"))))
, example "else if" "if x1 then y1 else if x2 then y2 else if x3 then y3 else z" $ at 1 1 1 59 (If (Commented [] (at 1 4 1 6 (VarExpr (VarRef [] $ LowercaseIdentifier "x1"))) [],Commented [] (at 1 12 1 14 (VarExpr (VarRef [] $ LowercaseIdentifier "y1"))) []) [([],(Commented [] (at 1 23 1 25 (VarExpr (VarRef [] $ LowercaseIdentifier "x2"))) [],Commented [] (at 1 31 1 33 (VarExpr (VarRef [] $ LowercaseIdentifier "y2"))) [])),([],(Commented [] (at 1 42 1 44 (VarExpr (VarRef [] $ LowercaseIdentifier "x3"))) [],Commented [] (at 1 50 1 52 (VarExpr (VarRef [] $ LowercaseIdentifier "y3"))) []))] ([],at 1 58 1 59 (VarExpr (VarRef [] $ LowercaseIdentifier "z"))))
, example "newlines" "if\n x\n then\n y\n else\n z" $ at 1 1 6 3 (If (Commented [] (at 2 2 2 3 (VarExpr (VarRef [] $ LowercaseIdentifier "x"))) [],Commented [] (at 4 2 4 3 (VarExpr (VarRef [] $ LowercaseIdentifier "y"))) []) [] ([],at 6 2 6 3 (VarExpr (VarRef [] $ LowercaseIdentifier "z"))))
[ example "" "if x then y else z" $ at 1 1 1 19 (If (Commented [] (at 1 4 1 5 (VarExpr (VarRef [] $ LowercaseIdentifier "x"))) [],Commented [] (at 1 11 1 12 (VarExpr (VarRef [] $ LowercaseIdentifier "y"))) []) [] ([],at 1 18 1 19 (VarExpr (VarRef [] $ LowercaseIdentifier "z"))) JoinAll)
, example "comments" "if{-A-}x{-B-}then{-C-}y{-D-}else{-E-}if{-F-}x_{-G-}then{-H-}y_{-I-}else{-J-}z" $ at 1 1 1 78 (If (Commented [BlockComment ["A"]] (at 1 8 1 9 (VarExpr (VarRef [] $ LowercaseIdentifier "x"))) [BlockComment ["B"]],Commented [BlockComment ["C"]] (at 1 23 1 24 (VarExpr (VarRef [] $ LowercaseIdentifier "y"))) [BlockComment ["D"]]) [([BlockComment ["E"]],(Commented [BlockComment ["F"]] (at 1 45 1 47 (VarExpr (VarRef [] $ LowercaseIdentifier "x_"))) [BlockComment ["G"]],Commented [BlockComment ["H"]] (at 1 61 1 63 (VarExpr (VarRef [] $ LowercaseIdentifier "y_"))) [BlockComment ["I"]]))] ([BlockComment ["J"]],at 1 77 1 78 (VarExpr (VarRef [] $ LowercaseIdentifier "z"))) JoinAll)
, example "else if" "if x1 then y1 else if x2 then y2 else if x3 then y3 else z" $ at 1 1 1 59 (If (Commented [] (at 1 4 1 6 (VarExpr (VarRef [] $ LowercaseIdentifier "x1"))) [],Commented [] (at 1 12 1 14 (VarExpr (VarRef [] $ LowercaseIdentifier "y1"))) []) [([],(Commented [] (at 1 23 1 25 (VarExpr (VarRef [] $ LowercaseIdentifier "x2"))) [],Commented [] (at 1 31 1 33 (VarExpr (VarRef [] $ LowercaseIdentifier "y2"))) [])),([],(Commented [] (at 1 42 1 44 (VarExpr (VarRef [] $ LowercaseIdentifier "x3"))) [],Commented [] (at 1 50 1 52 (VarExpr (VarRef [] $ LowercaseIdentifier "y3"))) []))] ([],at 1 58 1 59 (VarExpr (VarRef [] $ LowercaseIdentifier "z"))) JoinAll)
, example "newlines" "if\n x\n then\n y\n else\n z" $ at 1 1 6 3 (If (Commented [] (at 2 2 2 3 (VarExpr (VarRef [] $ LowercaseIdentifier "x"))) [],Commented [] (at 4 2 4 3 (VarExpr (VarRef [] $ LowercaseIdentifier "y"))) []) [] ([],at 6 2 6 3 (VarExpr (VarRef [] $ LowercaseIdentifier "z"))) SplitAll)
]

, testGroup "let statement"
Expand Down
96 changes: 81 additions & 15 deletions tests/test-files/good/Elm-0.19/AllSyntax/Expressions.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,16 @@ lambdaWithMultilinePattern =
ifStatement =
let
a =
if True then 1 else 2
rlefevre marked this conversation as resolved.
Show resolved Hide resolved

b =
if True then
1

else
2

c =
if True then
1

Expand All @@ -282,44 +292,100 @@ ifStatement =
else
3

b =
if {- C -} True {- D -} then
{- E -}
d =
if {- A -} True {- B -} then {- C -} 1 {- D -} else {- E -} 2

e =
if {- A -} {- B -} True {- C -} {- D -} then {- E -} {- F -} 1 {- G -} {- H -} else {- I -} {- J -} 2

f =
if {- A -} True {- B -} then
{- C -}
1
{- F -}
{- D -}

else {- E -} if {- F -} False {- G -} then
{- H -}
2
{- I -}

else {- G -} if {- H -} False {- I -} then
else
{- J -}
3

g =
if {- A -} {- B -} True {- C -} {- D -} then
{- E -} {- F -}
1
{- G -} {- H -}

else {- I -} {- J -} if {- K -} {- L -} False {- M -} {- N -} then
{- O -} {- P -}
2
{- K -}
{- Q -} {- R -}

else
{- L -}
{- S -} {- T -}
3

c =
h =
if
--A
True
--B
then
--C
1
--D

else
--E
if
--F
False
--G
then
--H
2
--I

else
--J
3

i =
if
--A
--B
True
--C
--D
then
--E
1
--F
1
--G
--H

else
--G
--I
--J
if
--H
--K
--L
False
--I
--M
--N
then
--J
--O
--P
2
--K
--Q
--R

else
--L
--S
--T
3
in
{}
Expand Down
3 changes: 2 additions & 1 deletion tests/test-files/transform/Elm-0.18/Examples.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ ratio = graphHeight / (if range == 0 then 0.1 else toFloat range)

-- foo=(case x of {True->1;False->3})

bar = (if if a then True else False then "a" else "b")
bar = (if if a then
True else False then "a" else "b")

multilineList = [1,2,3
]
8 changes: 1 addition & 7 deletions tests/test-files/transform/Elm-0.18/Examples.formatted.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,7 @@ module Main exposing (bar, multilineList, ratio)


ratio =
graphHeight
/ (if range == 0 then
0.1

else
toFloat range
)
graphHeight / (if range == 0 then 0.1 else toFloat range)
avh4 marked this conversation as resolved.
Show resolved Hide resolved



Expand Down
1 change: 1 addition & 0 deletions tests/test-files/transform/Elm-0.19/IfElseIfAlwaysMultiLine.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
x = if True then 1 else if False then 2 else 3
Copy link
Owner

Choose a reason for hiding this comment

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

👍

Loading