add maxWidth option to IDEA plugin, fixes #351

[robinpokorny]

This add a new override option for maxWidth, one of the most changed configs, see #351.

Unfortunately, working with Kotlin data classes is less than ideal from Java, so new FormattingOptions have to be created.

[simonhauck]

If this is merged, it should in my opinion also be available via the CLI.

If not, people that are not using the IntelliJ plugin can not achive the same formatting as IntelliJ users. Also it could be relevant for CI applications.

[robinpokorny]

If this is merged, it should in my opinion also be available via the CLI.

If not, people that are not using the IntelliJ plugin can not achive the same formatting as IntelliJ users. Also it could be relevant for CI applications.

The Gradle plugin supports changing the maxWidth. While I personally think it's unfortunate, that this option exists, my change is actually brining more consistency.

[simonhauck]

I am familiar with the gradle plugin and also spotless is offering this configuration. Even in the web playground is this option available.

That being said, those are in my opinion "third party" tools (expect the playground, but that's just a "playground") and not defining the "ktfmt"-standard, which is tracked in this repository. In the README is written:

Note: There is no configurability as to the formatter's algorithm for formatting (apart from the different styles). This is a deliberate design decision to unify our code formatting on a single format.

That indicates for me, that even though both plugins offer this functionality, it is not part of the public API - if that is the case (and I could be wrong), then the official IntelliJ plugin should not offer this option.

If the above mentioned design decision is changed, it should be documented in the README is and then also be available in all artifacts from this repository - the CLI tools and the IntelliJ plugin.

Technical side

From the technical point of view: I know that at least the gradle plugin is mapping their options to the FormattingOptions.kt, which is a non internal class. From a quick look, it seems that the internal modifier is not used consistently in this project, so I would not count on this being part of the public API. If it is, then why shouldn't all options in this class be part of the public API. As you mentioned on slack, this would logically be the next step.

Summary

For me this PR is not just about adding a setting to the plugin. It is a decision if configuration options are introduced in the official API - which should be a conscious and documented decision.

If it is decided, that ktfmt has more options then:

1. The setting should also implemented for the CLI, the documentation changed, this PR accepted
2. In the long term the other options should also be available to the CLI and IntelliJ plugin

If it is a clear decision, that this should not be part of ktfmt

1. Consider making the classes internal, so that plugins can not overwrite it. This is for the plugins a breaking change, but be more aligned with the goal of ktfmt.

In the end this is just my opinion and with my assumptions. I would be very helpful to her the opinion of the maintainers and others as well.

Edit: Here this was discussed for the CLI and rejected: https://github.com/facebook/ktfmt/pull/470

[hick209]

This is failing the spotless check (just formatting apparently). Would you mind pushing changes to fix that, @robinpokorny ?

I apologize the response delay. I was busy shipping Kotlin 2.0 internally

[rock3r]

After #502 is merged, this PR will get severely out of sync with the main branch as everything is converted to Kotlin (and the UI to the Kotlin UI DSL). I was planning to expose this and the other parameters as a "custom" option in the dropdown, revealing additional UI, in a follow up. I just noticed that there was a PR open for part of it... and I don't want to look like I'm taking someone else's idea :) Happy to leave that to you, if you're ok with doing the rest of what I was planning too, or to do everything myself — likely over the weekend, or next week.

[simonhauck]

@hick209 Did you discuss internally if these options should be exposed? And if yes, can we add them also to the CLI?

[hick209]

We have discussed the idea internally and have come to the conclusion that we do not want to expose this option via IntelliJ plugin. The reason for this is the same reason as to why the CLI is not configurable, but Gradle/maven plugins are, which is share-ability and also this principle. Let me try to explain the share-ability part.

One of the goals of ktfmt (and also most formatters) is to reduce bikeshedding discussions related to code formatting. We assume code is shared among developers and for that end we only allow configurations that could be shared as well.

It is an intentional decision to allow for Gradle and Maven plugins, since those are usually checked in files that are part of the codebase. This means there shouldn't be a conflict where one person is formatting a file with 100 line width and another with 120 line width, since a change like that would have to touch a file which would have to be committed to the repository and for that reviewed.

From our experience IntelliJ configuration sharing is not easily done within an organization/codebase and for that reason we do not intent to allow the current IJ plugin to be more configuration than the CLI tool, which follows the same principle.

I'll try to update the README to reflect the information shared here. Thank you so much for probing us regarding this matter and causing the improvement of our documentation.

[hick209]

@rock3r actually shared with us that this could be shared with the file .idea/ktfmt.xml file, therefore we should be good to accept the changes. I'll take #503 though as that one is more complete.

[robinpokorny]

@hick209, thanks for the explanation.

I wholeheartedly agree with reducing the bikeshedding. On the other hand, I'm not convinced that Gradle and Maven plugins should have a special treatment. The main reason for my PR was that using the official ktfmt editor plugin could not be configured in a compatible way with CI checks using Gradle. This made the plugin not only unusable, but it IMO removes the benefit of using a strict common formatter.

I'd say the proper solution would either disallow those options completely, or store them in a usage-independent way (for example, like .editorconfig or .prettierrc).

In the end, I'm happy there is now a way to configure the plugin!

[rock3r]

It was the driving force behind my pr as well! Hopefully they come up with a universal settings mechanism that's supported by all ktfmt incarnations, building on top of what's here now