Skip to content

Match ktlint_code_style with Ktlint documentation#2090

Closed
Jaehwa-Noh wants to merge 10 commits intodiffplug:mainfrom
Jaehwa-Noh:match-ktlint
Closed

Match ktlint_code_style with Ktlint documentation#2090
Jaehwa-Noh wants to merge 10 commits intodiffplug:mainfrom
Jaehwa-Noh:match-ktlint

Conversation

@Jaehwa-Noh
Copy link
Copy Markdown

@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
Copy Markdown
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
* 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
Copy Markdown
Member

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
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
Member

@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
Copy Markdown
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
Copy Markdown
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.

@Jaehwa-Noh Jaehwa-Noh marked this pull request as draft April 8, 2024 03:25
- 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
Copy Markdown
Member

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
Copy Markdown
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.

3 participants