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

Router: indent trailing fewer braces, too #3514

Merged
merged 1 commit into from
Mar 28, 2023
Merged

Router: indent trailing fewer braces, too #3514

Show file tree
Hide file tree
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
16 changes: 10 additions & 6 deletions scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1726,12 +1726,13 @@ class Router(formatOps: FormatOps) {
val expire = getLastEnclosedToken(expireTree)
val indentLen = style.indent.main

def checkFewerBraces(tree: Tree) = tree match {
case p: Term.Apply => isFewerBraces(p)
case p: Term.Match => !tokenBefore(p.cases).left.is[T.LeftBrace]
case _ => false
}
val nextDotIfSig = nextSelect.flatMap { ns =>
val ok = ns.qual match {
case p: Term.Apply => isFewerBraces(p)
case p: Term.Match => !tokenBefore(p.cases).left.is[T.LeftBrace]
case _ => false
}
val ok = checkFewerBraces(ns.qual)
if (ok) Some(tokenBefore(ns.nameToken)) else None
}
def forcedBreakOnNextDotPolicy = nextSelect.map { selectLike =>
Expand Down Expand Up @@ -1903,7 +1904,10 @@ class Router(formatOps: FormatOps) {
if (nextNonCommentSameLine(tokens(t, 2)).right.is[T.Comment])
baseSplits.map(_.withIndent(nlIndent)) // will break
else {
val spcIndent = nextDotIfSig.fold(Indent.empty) { x =>
val spcIndent = nextDotIfSig.fold {
val ok = nextSelect.isEmpty && checkFewerBraces(expireTree)
if (ok) nlIndent else Indent.empty
} { x =>
Indent(indentLen, x.left, ExpiresOn.Before)
}
baseSplits.map { s =>
Expand Down
8 changes: 4 additions & 4 deletions scalafmt-tests/src/test/resources/rewrite/RedundantParens.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,12 +554,12 @@ object a {
>>>
object a {
val b = c + (d.match
case true => true
case false => false
case true => true
case false => false
)
val b = c + (d.match
case true => true
case false => false
case true => true
case false => false
) + e
val b = c + (d match
case true => true
Expand Down
22 changes: 11 additions & 11 deletions scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3609,20 +3609,20 @@ object a:
object a:
def f(): Unit =
List(1, 2, 3).foldLeft(1):
case (a, b) => a + b
case (a, b) => a + b
<<< coloneol in fewer braces 1, main > sig
maxColumn = 80
indent.main = 4
===
object a:
def f(): Unit =
List(1, 2, 3).foldLeft(1):
case (a, b) => a + b
case (a, b) => a + b
>>>
object a:
def f(): Unit =
List(1, 2, 3).foldLeft(1):
case (a, b) => a + b
case (a, b) => a + b
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like way too much indentation. Looks like it's applied 3 times, wouldn't really one be enough? The alternative with {} will have single indentation:

  object a:
    def f(): Unit =
      List(1, 2, 3).foldLeft(1) {
        case (a, b) => a + b
        case (a, b) => a + b
      }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this example uses 4 as the main indent. so there's only one additional level added, to make way for a potential select continuation after it.

Copy link
Contributor

Choose a reason for hiding this comment

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

We haven't yet agreed on the dotty PR, no? I having doubts about whether we should even merge it. If we don't does this change still make sense?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i don't know. it fixes an inconsistency but you're right that both are not yet supported.

but if the dotty change isn't merged, we'll have a problem...

are you opposed to the change martin proposed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that change might hard to nail especially with the wierd 1-space rule.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i asked about that space as it seemed unexpected. no documentation mentions that one can be within one space of a significant indentation rather than matching it exactly... does the code not match it exactly?

if he agrees with the half-open interval you're proposing, would that unblock it?

Copy link
Contributor

Choose a reason for hiding this comment

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

if he agrees with the half-open interval you're proposing, would that unblock it?

I think that would be the most logical yeah.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tgodzik i would like to merge this change anyway. i have another pr ready, which makes this configurable, so if the compiler change is made, this is ready, and if not, there's a currently compatible alternative. wdyt?

<<< coloneol in fewer braces 2
maxColumn = 80
===
Expand Down Expand Up @@ -3679,8 +3679,8 @@ object a:
object a:
def f(): Unit =
List(1, 2, 3).foo:
case a: Int =>
case _ =>
case a: Int =>
case _ =>
otherTerm()
<<< coloneol in fewer braces 3, main > sig
maxColumn = 80
Expand All @@ -3696,8 +3696,8 @@ object a:
object a:
def f(): Unit =
List(1, 2, 3).foo:
case a: Int =>
case _ =>
case a: Int =>
case _ =>
otherTerm()
<<< match with eol
maxColumn = 80
Expand Down Expand Up @@ -3865,7 +3865,7 @@ class test:
>>>
class test:
baz.qux:
2 + 2
2 + 2
<<< #3489 3
class test:
foo.bar:
Expand Down Expand Up @@ -3923,7 +3923,7 @@ class test:
bar(
2 + 2
).baz.qux:
3 + 3
3 + 3
<<< #3489 8
class test:
bar.baz:
Expand All @@ -3945,8 +3945,8 @@ class test:
>>>
class test:
foo.match
case bar => ""
case baz => ""
case bar => ""
case baz => ""
<<< #3489 10
class test:
foo.match
Expand Down
20 changes: 10 additions & 10 deletions scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3433,7 +3433,7 @@ object a:
>>>
object a:
def f(): Unit = List(1, 2, 3).foldLeft(1):
case (a, b) => a + b
case (a, b) => a + b
<<< coloneol in fewer braces 1, main > sig
maxColumn = 80
indent.main = 4
Expand All @@ -3445,7 +3445,7 @@ object a:
>>>
object a:
def f(): Unit = List(1, 2, 3).foldLeft(1):
case (a, b) => a + b
case (a, b) => a + b
<<< coloneol in fewer braces 2
maxColumn = 80
===
Expand Down Expand Up @@ -3498,8 +3498,8 @@ object a:
object a:
def f(): Unit =
List(1, 2, 3).foo:
case a: Int =>
case _ =>
case a: Int =>
case _ =>
otherTerm()
<<< coloneol in fewer braces 3, main > sig
maxColumn = 80
Expand All @@ -3515,8 +3515,8 @@ object a:
object a:
def f(): Unit =
List(1, 2, 3).foo:
case a: Int =>
case _ =>
case a: Int =>
case _ =>
otherTerm()
<<< match with eol
maxColumn = 80
Expand Down Expand Up @@ -3674,7 +3674,7 @@ class test:
>>>
class test:
baz.qux:
2 + 2
2 + 2
<<< #3489 3
class test:
foo.bar:
Expand Down Expand Up @@ -3729,7 +3729,7 @@ class test:
>>>
class test:
bar(2 + 2).baz.qux:
3 + 3
3 + 3
<<< #3489 8
class test:
bar.baz:
Expand All @@ -3750,8 +3750,8 @@ class test:
>>>
class test:
foo.match
case bar => ""
case baz => ""
case bar => ""
case baz => ""
<<< #3489 10
class test:
foo.match
Expand Down
18 changes: 9 additions & 9 deletions scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3591,7 +3591,7 @@ object a:
object a:
def f(): Unit =
List(1, 2, 3).foldLeft(1):
case (a, b) => a + b
case (a, b) => a + b
<<< coloneol in fewer braces 1, main > sig
maxColumn = 80
indent.main = 4
Expand All @@ -3604,7 +3604,7 @@ object a:
object a:
def f(): Unit =
List(1, 2, 3).foldLeft(1):
case (a, b) => a + b
case (a, b) => a + b
<<< coloneol in fewer braces 2
maxColumn = 80
===
Expand Down Expand Up @@ -3659,8 +3659,8 @@ object a:
object a:
def f(): Unit =
List(1, 2, 3).foo:
case a: Int =>
case _ =>
case a: Int =>
case _ =>
otherTerm()
<<< coloneol in fewer braces 3, main > sig
maxColumn = 80
Expand All @@ -3676,8 +3676,8 @@ object a:
object a:
def f(): Unit =
List(1, 2, 3).foo:
case a: Int =>
case _ =>
case a: Int =>
case _ =>
otherTerm()
<<< match with eol
maxColumn = 80
Expand Down Expand Up @@ -3841,7 +3841,7 @@ class test:
>>>
class test:
baz.qux:
2 + 2
2 + 2
<<< #3489 3
class test:
foo.bar:
Expand Down Expand Up @@ -3920,8 +3920,8 @@ class test:
>>>
class test:
foo.match
case bar => ""
case baz => ""
case bar => ""
case baz => ""
<<< #3489 10
class test:
foo.match
Expand Down
28 changes: 14 additions & 14 deletions scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3732,22 +3732,22 @@ object a:
object a:
def f(): Unit =
List(1, 2, 3).foldLeft(1):
case (a, b) =>
a + b
case (a, b) =>
a + b
<<< coloneol in fewer braces 1, main > sig
maxColumn = 80
indent.main = 4
===
object a:
def f(): Unit =
List(1, 2, 3).foldLeft(1):
case (a, b) => a + b
case (a, b) => a + b
>>>
object a:
def f(): Unit =
List(1, 2, 3).foldLeft(1):
case (a, b) =>
a + b
case (a, b) =>
a + b
<<< coloneol in fewer braces 2
maxColumn = 80
===
Expand Down Expand Up @@ -3802,8 +3802,8 @@ object a:
object a:
def f(): Unit =
List(1, 2, 3).foo:
case a: Int =>
case _ =>
case a: Int =>
case _ =>
otherTerm()
<<< coloneol in fewer braces 3, main > sig
maxColumn = 80
Expand All @@ -3819,8 +3819,8 @@ object a:
object a:
def f(): Unit =
List(1, 2, 3).foo:
case a: Int =>
case _ =>
case a: Int =>
case _ =>
otherTerm()
<<< match with eol
maxColumn = 80
Expand Down Expand Up @@ -3994,7 +3994,7 @@ class test:
>>>
class test:
baz.qux:
2 + 2
2 + 2
<<< #3489 3
class test:
foo.bar:
Expand Down Expand Up @@ -4075,10 +4075,10 @@ class test:
>>>
class test:
foo.match
case bar =>
""
case baz =>
""
case bar =>
""
case baz =>
""
<<< #3489 10
class test:
foo.match
Expand Down