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

Added clarification for match expression multiline case #40762

Closed
wants to merge 1 commit into from
+10 −2
Closed

Added clarification for match expression multiline case #40762

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 10 additions & 2 deletions docs/fsharp/style-guide/formatting.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1092,16 +1092,24 @@ match l with
| [] -> failwith "Couldn't find David"
```

If the expression on the right of the pattern matching arrow is too large, move it to the following line, indented one step from the `match`/`|`.
If one of the expressions on the right of the pattern matching arrow is too large, move it to the following line, indented one step from the `match`/`|`. Move all the other expressions to the next line as well for consistency.
Copy link
Member

Choose a reason for hiding this comment

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

I don't interpret Don's last comment in the suggestion as setting this as the default, but rather an option.
I think the wording should reflect that (or the suggestion discussion continued about the default-ness of this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dsyme set the label style-guide-decided, so I regard it as a marker that style guide is decided (final decision). As I understand, Don was not sure about tooling changes needed to support this style, but it should not affect the style and the recommendation itself.

Copy link
Member

Choose a reason for hiding this comment

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

The label is fine.
What I do not see confirmed is that "defaultness" of this, I interpret the context of the suggestion that this should still remain a possible option only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, looks like it can't be resolved without Don

Copy link
Member

@abonie abonie May 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I agree with @T-Gro , here is what Don said

I agree with @isaacabraham that at option (4) listed above should be an option.

(emphasis mine)
cc: @vzarytovskii


```fsharp
// ✔️ OK
match lam with
| Var v -> 1
| Var v ->
1
| Abs(x, body) ->
1 + sizeLambda body
| App(lam1, lam2) ->
sizeLambda lam1 + sizeLambda lam2

// ❌ Not OK, see above for preferred formatting
match lam with
| Var v -> 1
| Abs(x, body) -> 1 + sizeLambda body
| App(lam1, lam2) ->
sizeLambda lam1 + sizeLambda lam2
```

Similar to large if conditions, if a match expression is multiline or exceeds the default tolerance of the single-line, the match expression should use one indentation and a new line.
Expand Down
Loading