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.

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

Match ktlint_code_style with Ktlint documentation #2090

Closed
wants to merge 10 commits into from

Conversation

Jaehwa-Noh
Copy link

@Jaehwa-Noh Jaehwa-Noh commented Apr 7, 2024

Ktlint document says that

Starting from version 1.0, ktlint_official is the default code style. If you want to revert to another code style, then set the .editorconfig property ktlint_code_style.

But Spotless default set intellij_idea It makes developer mistake and misconception.

According Ktlint documentation, only one want to revert another code style must set the ktlint_code_style, but in Spotless need to set the ktlint_code_style who want to apply ktlint_official or else styles.

@nedtwigg
Copy link
Member

nedtwigg commented Apr 7, 2024

  • This needs changelog entries in CHANGES.md, plugin-gradle/CHANGES.md, and plugin-maven/CHANGES.md
  • These entries need to look like this:
- **BREAKING** default `ktlint` editorconfig `ktlint_code_style` changed from `intellij_idea` to `ktlint_official` to better match the ktlint docs. ([#2090](https://github.com/diffplug/spotless/pull/2090))
  - to keep the previous behavior of `intellij_idea`, you must do "blah"

CHANGES.md Outdated
@@ -19,6 +19,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
* Bump default `shfmt` version to latest `3.7.0` -> `3.8.0`. ([#2050](https://github.com/diffplug/spotless/pull/2050))
* Bump default `ktlint` version to latest `1.1.1` -> `1.2.1`. ([#2057](https://github.com/diffplug/spotless/pull/2057))
* Bump default `sortpom` version to latest `3.4.0` -> `3.4.1`. ([#2078](https://github.com/diffplug/spotless/pull/2078))
* Default `ktlint` editorconfig `ktlint_code_style` to `intellij_idea` -> `ktlint_official`. ([#2090](https://github.com/diffplug/spotless/pull/2090))
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, the context was #1808 (comment).

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can just note the default style intellij_idea in ktlint sections in docs, users can override them anyway in their configs.

Copy link
Author

@Jaehwa-Noh Jaehwa-Noh Apr 8, 2024

Choose a reason for hiding this comment

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

Yes, developer can override the style.
But the problem is that I introduced.

According Ktlint documentation, only one want to revert another code style must set the ktlint_code_style, but in Spotless need to set the ktlint_code_style who want to apply ktlint_official or else styles.

Unmatched default with ktlint documentation, makes developer mistake and misconception.

Copy link
Contributor

@Goooler Goooler Apr 8, 2024

Choose a reason for hiding this comment

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

That's the ktlint CLI's default value instead of this plugin.

Did you see the context above? This change will break users' compatibility, it's so painful and annoying.

Copy link
Author

Choose a reason for hiding this comment

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

No, I don't think that break users' compatibility.
This ktlint_official is ktlint's default value, and spotless loads ktlint plugin, so that there's no reason doesn't follow ktlint official documentation.

See below snippet

spotless {
  kotlin {
    ktlint()
  }
}

You say that spotless will apply intellij_idea style ktlint.
But the one who read ktlint documentation, will say that spotless will apply ktlint_official style ktlint.
This inconsistent situation will bring more fatigue to developer. Every developers must double check both spotless documentation and ktlint documentation what is different.
It is non-sense.

@Jaehwa-Noh
Copy link
Author

Jaehwa-Noh commented Apr 8, 2024

I add the instruction in CHANGES.md.
The test, however, failed because of default ktlint style changed. so that I need to adjust the test.
I am now changing this draft to solve the test problem.

- If don't have ktlint_code_style, ```ktlint``` set it ```ktlint_official``` as default value.
- writePomWithKotlinSteps_intellijIdea_intellijIdeaStyleKtlint
- writePomWithKotlinSteps_default_officialStyleKtlint
- target_setAndUnset_intellijIdeaStyle
- target_setAndUnset_defaultKtlintOfficialStyle
- editorConfigOverride_setExperimental_intellijIdeaStyle
- editorConfigOverride_setExperimental_defaultKtlintOfficialStyle
@Jaehwa-Noh Jaehwa-Noh marked this pull request as ready for review April 8, 2024 13:01
@Goooler
Copy link
Contributor

Goooler commented Apr 8, 2024

What you actually need to do is reverting 7de7991, but anyway I don't wanna do this breaking now. Docs will be updated in #2092, new users will see it.

@Goooler Goooler closed this Apr 8, 2024
@Jaehwa-Noh
Copy link
Author

Okay, I got it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants