FormatOps: improve binpacking parentConstructors #3094
Conversation
| object CoreReflection extends { | ||
| val u: ru.type = ru | ||
| val mirror: u.Mirror = u.runtimeMirror(classOf[scala.meta.Tree].getClassLoader) | ||
| } with scala.meta.internal.trees.Reflection with scala.meta.internal.tokens.Reflection |
There was a problem hiding this comment.
it only fixes extends position.
early stats and } with .. shouldn't be indented
There was a problem hiding this comment.
why shouldn't they be indented? everything between the beginning of a template and the opening brace of its body should be indented.
in your case,
... {
val u: ru.type = ru
val mirror: u.Mirror = u.runtimeMirror(classOf[scala.meta.Tree].getClassLoader)
}
is not the body of CoreReflection but of some other (parent) construct. definitely has to be indented.
There was a problem hiding this comment.
here's another example, existing behaviour:
your proposal is contrary to that.
There was a problem hiding this comment.
If we treat that parentConstructor doesn't affect early init body and work only for extends and with chain it wouldn't be contrary.
Let me try to fix it it properly, I didn't noticed that there is such setting at the moment when I was preparing a fix.
There was a problem hiding this comment.
can you explain how init body is different? other than indentation, what else would you like to do?
| @@ -54,8 +54,7 @@ private class Encoder extends CharsetEncoder( | |||
| } | |||
| >>> | |||
| object a { | |||
| private class Encoder | |||
| extends CharsetEncoder(UTF_16_Common.this, 2.0f, 2.0f, | |||
| private class Encoder extends CharsetEncoder(UTF_16_Common.this, 2.0f, 2.0f, | |||
There was a problem hiding this comment.
yes, same explanation is above. always attempts to binpack as much as possible unless there was an explicit break.
| case Bar | ||
| extends Foo(1) with X | ||
| with Y with Z | ||
| case Bar extends Foo(1) |
There was a problem hiding this comment.
This seems wrong now. With binPack.parentConstructors = Always wasn't the previous state correct?
There was a problem hiding this comment.
the new approach allows space if source is ignored (i.e. fold/unfold) or space was used in the source. in this example (line 128), there was no break here, so extends is also binpacked.
Relax single-line formatting to exclude delimited ranges (parent ctor args or early-init block) in a few cases.
|
just for visual effect, here's a version which avoids indenting early ctors: in my view, the formatting is not especially readable or pleasing, the dissonance with subsequent parents is prominent: |
Alternative to #3091.