ktfmt-gradle not in sync with intellij ktfmt plugin when using googleStyle()

[blipinsk]

🐛 Describe the bug

ktfmt-gradle is not in sync with intellij ktfmt plugin when using googleStyle()

🕵️ Details

(I think I got to the bottom of this, so I'm gonna just give you the full details)

1. Both Intellij plugin (1.1.0.47) and ktfmt-gradle (0.17.0) are using ktfmt version 0.47, but...
2. intellij plugin after selecting Google (internal), directly utilizes FormattingOptions GOOGLE_FORMAT from ktfmt
   https://github.com/facebook/ktfmt/blob/6a639f00561219f3d520de271661013dc0209d4e/ktfmt_idea_plugin/src/main/java/com/facebook/ktfmt/intellij/UiFormatterStyle.java#L29

3. ktfmt-gradle does not do the same thing directly. It internally creates FormattingOptions which are intended to mimic the GOOGLE_FORMAT from ktfmt

ktfmt-gradle/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtExtension.kt

Lines 65 to 71 in c7d7b9d

	/** Sets the Google style (equivalent to set blockIndent to 2 and continuationIndent to 2). */
	@Suppress("MagicNumber")
	fun googleStyle() {
	ktfmtStyle = GOOGLE
	blockIndent.set(2)
	continuationIndent.set(2)
	}

ktfmt-gradle/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/tasks/worker/KtfmtFormatter.kt

Lines 56 to 72 in 6648a7a

	internal fun FormattingOptionsBean.toFormattingOptions(): FormattingOptions =
	FormattingOptions(
	style = style.toKtfmtStyle(),
	maxWidth = maxWidth,
	blockIndent = blockIndent,
	continuationIndent = continuationIndent,
	removeUnusedImports = removeUnusedImports,
	manageTrailingCommas = manageTrailingCommas,
	debuggingPrintOpsAfterFormatting = debuggingPrintOpsAfterFormatting
	)
	internal fun FormattingOptionsBean.Style.toKtfmtStyle(): FormattingOptions.Style =
	when (this) {
	FormattingOptionsBean.Style.FACEBOOK -> FormattingOptions.Style.FACEBOOK
	FormattingOptionsBean.Style.DROPBOX -> FormattingOptions.Style.DROPBOX
	FormattingOptionsBean.Style.GOOGLE -> FormattingOptions.Style.GOOGLE
	}

4. The problem is, even though, both of those things are using ktfmt version 0.47, the available 0.17.0 version of ktfmt-gradle is missing manageTrailingCommas handling.

ktfmt-gradle/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtExtension.kt

Lines 19 to 55 in c7d7b9d

	internal var ktfmtStyle = FACEBOOK
	/** ktfmt breaks lines longer than maxWidth. Default 100. */
	abstract val maxWidth: Property<Int>
	/**
	* blockIndent is the size of the indent used when a new block is opened, in spaces.
	*
	* For example,
	* ```
	* fun f() {
	* //
	* }
	* ```
	*/
	abstract val blockIndent: Property<Int>
	/**
	* continuationIndent is the size of the indent used when a line is broken because it's too
	* long, in spaces.
	*
	* For example,
	* ```
	* val foo = bar(
	* 1)
	* ```
	*/
	abstract val continuationIndent: Property<Int>
	/** Whether ktfmt should remove imports that are not used. */
	abstract val removeUnusedImports: Property<Boolean>
	/**
	* Print the Ops generated by KotlinInputAstVisitor to help reason about formatting (i.e.,
	* newline) decisions
	*/
	abstract val debuggingPrintOpsAfterFormatting: Property<Boolean>

5. And manageTrailingCommas is false by default in ktfmt

https://github.com/facebook/ktfmt/blob/6a639f00561219f3d520de271661013dc0209d4e/core/src/main/java/com/facebook/ktfmt/format/FormattingOptions.kt#L58-L65

6. Which creates an inconsistency between FormattingOptions because Intellij Plugin uses GOOGLE_FORMAT which has manageTralingCommas = true

https://github.com/facebook/ktfmt/blob/6a639f00561219f3d520de271661013dc0209d4e/core/src/main/java/com/facebook/ktfmt/format/Formatter.kt#L48-L50

7. and ktfmt-gradle uses custom FormattingOptions without overwriting the default value of manageTrailingCommas which ends up being false.

ktfmt-gradle/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtExtension.kt

Lines 67 to 71 in c7d7b9d

	fun googleStyle() {
	ktfmtStyle = GOOGLE
	blockIndent.set(2)
	continuationIndent.set(2)
	}

❓ Question

I can see you already added manageTrailingCommas handling here: #272
Is there an ETA for releasing a new version of ktfmt-gradle with that included?

🔗 Related

facebook/ktfmt#444 (potentially)
facebook/ktfmt#455

[gino-m]

@blipinsk +1. I originally posted this in #264 but was informed this was likely a ktfmt issue so I moved it over to facebook/ktfmt#444.

[cortinico]

@blipinsk +1. I originally posted this in #264 but was informed this was likely a ktfmt issue so I moved it over to facebook/ktfmt#444.

Yup closing for the same reason

[blipinsk]

@cortinico did you have the chance to go through my issue? It's clearly a problem with ktfmt-gradle. It says it provides googleStyle matching 0.47 from kfmt while it really doesn't. The internal FormatterOptions are not in sync with kfmt.

The intellij plugin provides what it says it does - the GOOGLE_FORMAT from kfmt. ktfmt-gradle provides a limited version of it.

[blipinsk]

I mean, maybe @gino-m's issue is not exactly the same, idk. Maybe that one presents a problem with kfmt.
The thing I'm pointing out is clearly an incomplete implementation of the 0.47 API, which causes issues.
So the question is: is there an ETA for shipping the manageTrailingCommas API?

[cortinico]

@cortinico did you have the chance to go through my issue?

#272 manageTrailingCommas support has been added here 3 weeks ago.

[blipinsk]

@cortinico I'd appreciate if you read through my issue.
I am aware that support was added, and I mentioned that in the issue description.
It is not released (not available in 0.17.0), and that's the problem.

[cortinico]

I will be making a release in the coming minutes.

@cortinico I'd appreciate if you read through my issue.

@blipinsk I'd also appreciate if you make a search before opening a new issue. You would have found #260 which was already closed and could have asked there for a release there instead of forcing me to read through a whole new issue. Thank you 🙏

[blipinsk]

@cortinico

1. I read that issue beforehand. To me googleStyle trailing commas do not work is not the same as gradle plugin is not in sync with intellij plugin and it causes problems. That's why I wanted to give you the full details on why the issue is there (with a deep-down exploration I already did).
2. I'm really sorry you found it useless.
3. It's pretty hurtful to disregard somebody's initiative in finding a confusing issue between two tools (which should work in sync), by blaming ktfmt and not even reading one section of the whole issue- "questions". Really makes me not want to try to help in the future.

Have a great day and thank you for taking the time to make a release 👍

[cortinico]

3. It's pretty hurtful to disregard somebody's initiative in finding a confusing issue between two tools (which should work in sync), by blaming ktfmt and not even reading one section of the whole issue- "questions". Really makes me not want to try to help in the future.

I'm sorry for mis-triaging this one. I receive thousands of issues every day, and this looked really similar to another issue that was opened some days ago.

After all, the issue was already solved, and we do have a release out with the fix already 👍