-
Notifications
You must be signed in to change notification settings - Fork 276
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
FormatOps: handle fewer braces and refined types #3446
Conversation
5c9ad12
to
e5788a0
Compare
def f(): Unit = | ||
List(1, 2, 3).foo: a => | ||
a + 2 | ||
.apply: | ||
12 + 3 |
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.
def f(): Unit = | |
List(1, 2, 3).foo: a => | |
a + 2 | |
.apply: | |
12 + 3 | |
def f(): Unit = | |
List(1, 2, 3).foo: a => | |
a + 2 | |
.apply: | |
12 + 3 |
?
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.
it comes from the rule which adds indent to first NL before a dot (so that it's indented relative to the beginning of the full expression), and your proposal will be inconsistent with the formatting so far:
// short maxColumn
def f(): Unit =
List(1, 2, 3).foo(a =>
a + 2)
.apply(
12 + 3)
// longer maxColumn
def f(): Unit =
List(1, 2, 3).foo(a => a + 2)
.apply(12 + 3)
// there's no NL before a dot
def f(): Unit =
List(1, 2, 3).foo { a =>
a + 2
}.apply {
12 + 3
}
// there's a NL before a dot
def f(): Unit =
List(1, 2, 3).foo { a =>
a + 2
} //
.apply {
12 + 3
}
i didn't like this formatting for fold
(no proper unindent before select) and forced NL before the first dot if there's a second select. i could do the same for classic
(but not keep
).
i could try formatting it like this if you prefer:
// short maxColumn
def f(): Unit =
List(1, 2, 3).foo: a =>
a + 2
.apply:
12 + 3)
// but, should we still do this?
def f(): Unit =
List(1, 2, 3).foo: a =>
a + 2
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.
Ach right, I didn't realize it.
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.
slightly modified, tried testing with significant indentation smaller than main indentation, and ran into incorrect parsing problems, so ensure that body is indented relative to subsequent select.
scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat
Outdated
Show resolved
Hide resolved
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.
LGTM!
Fixes #3447.