Google style with trailing comma does not use maxWidth correctly
Hello!
There seems to be an off-by-one error when using Google-style and trailing commas. I just tried out the Google Style with trailing commas and saw the following scenario:
@Test
fun testShowBillingErrorPaymentButton() =
composeExtension.use {
// Arrange
setContentWithTheme {
AccountScreen(
state =
AccountUiState.default()
.copy(billingPaymentState = PaymentState.Error.Billing),
uiSideEffect = MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
)
}
// Assert
onNodeWithText("Add 30 days time").assertExists()
}
The line:
uiSideEffect = MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
ends up being 101 characters even though I set maxWidth to 100.
If i change the named parameter to uiSideEffects, we see the following behavior, where it correctly wraps the line:
@Test
fun testShowBillingErrorPaymentButton() =
composeExtension.use {
// Arrange
setContentWithTheme {
AccountScreen(
state =
AccountUiState.default()
.copy(billingPaymentState = PaymentState.Error.Billing),
uiSideEffects =
MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
)
}
// Assert
onNodeWithText("Add 30 days time").assertExists()
}
My configuration is as follows:
configure<com.ncorti.ktfmt.gradle.KtfmtExtension> {
googleStyle()
blockIndent.set(4)
continuationIndent.set(4)
maxWidth.set(100)
removeUnusedImports.set(true)
}
The 1.0 issue mentioned merging trailing-comma management into all styles. If we do that, I think this issue is unblocked.
We could switch the default behaviour of trailing-commas to do "nothing", rather than today where they trigger a line-break. Once that's done, management becomes:
- Insert trailing-commas everywhere
- Pretty print
- Remove unnecessary trailing-commas
Since the insertion happens before pretty-print, all the elements are already at their maximum length. We might get some lines that are unnecessarily wrapped after removal, but that's much less noticeable than exceeding max-width.
As of v0.52 trailing comma management appears to be on for all styles. Is there any. The proposal above can be implemented. @hick209
This issue also happens with --kotlinglang-style now that this style enables trailing comma management. Any chance this can be fixed soon? It makes ktlint unhappy and complain about line width in AOSP :-)
I implemented the idea in https://github.com/facebook/ktfmt/issues/472#issuecomment-2166182024 and its not good.
Imagine a case like foo.bar(a, b).baz(a, b).fiz(a, b). There are 3 param lists, so 3 commas are preemptively inserted, which might push the length past the max line length. That would be sort of mediocre on its own, but worse, pretty printing breaks as the .s not the ,s, so all the inserted commas end up removed. Then it's surprising why this code, which should fit on one line, is forced to break.
I'd like to revive https://github.com/facebook/ktfmt/pull/473. It's got less surprising output and was also easier to implement.
I'd like to revive https://github.com/facebook/ktfmt/pull/473. It's got less surprising output and was also easier to implement.
@hick209 Does Nick's comment above change your perspective on #472?
(I'm also facing this issue after upgrading to 0.52)
Bump @hick209
Friendly ping @hick209 :-)
I would be curious if this issue still persists when using the new TrailingCommaManagementStrategy.ONLY_ADD. Unfortunately, didn't have the time to try it out yet (But just might do so sometime today).