-
Notifications
You must be signed in to change notification settings - Fork 789
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 match! to structure #5473
Add match! to structure #5473
Conversation
@@ -259,7 +260,7 @@ module Structure = | |||
| SynExpr.LetOrUse (_,_,bindings, body, _) -> | |||
parseBindings bindings | |||
parseExpr body | |||
| SynExpr.Match (seqPointAtBinding,_expr,clauses,_,r) -> | |||
| SynExpr.Match (seqPointAtBinding,_expr,clauses,_,r) | SynExpr.MatchBang (seqPointAtBinding, _expr, clauses, _, r) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m sure there no such formatting in the whole repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cartermp,
From a readability perspective, I think we should prefer the fall through match, I agree with @vasily-kirichenko.
It is a certainty that we can, using this repo, point to an example of every type of formatting style that could possibly exist. For sure fall through matches with the separate cases per line is the most common, and I think readable format.
Kevin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can change it. I'll also change the other case in the compiler that does this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks mate
tests/service/StructureTests.fs
Outdated
| Some _ -> () // 7 | ||
| None -> // 8 | ||
let x = () // 9 | ||
() // 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example code is not realistical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just like the other tests. Do you have a suggested improvement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe {
match! Some None with
| Some _ ->
()
| None ->
match None with
| Some _ -> ()
| None ->
let x = ()
()
}
@@ -259,7 +260,7 @@ module Structure = | |||
| SynExpr.LetOrUse (_,_,bindings, body, _) -> | |||
parseBindings bindings | |||
parseExpr body | |||
| SynExpr.Match (seqPointAtBinding,_expr,clauses,_,r) -> | |||
| SynExpr.Match (seqPointAtBinding,_expr,clauses,_,r) | SynExpr.MatchBang (seqPointAtBinding, _expr, clauses, _, r) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cartermp,
From a readability perspective, I think we should prefer the fall through match, I agree with @vasily-kirichenko.
It is a certainty that we can, using this repo, point to an example of every type of formatting style that could possibly exist. For sure fall through matches with the separate cases per line is the most common, and I think readable format.
Kevin
Thanks mate. |
Also rebased atop master. |
Fixes #5456