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

Drop requirement that code formatted by Ktlint is compatible with IntelliJ IDEA default formatting #1526

Closed
paul-dingemans opened this issue Jun 26, 2022 · 16 comments
Labels
conflict-with-default-intellij-formatting Code produced by KtLint is not accepted by the IntelliJ default formatter

Comments

@paul-dingemans
Copy link
Collaborator

paul-dingemans commented Jun 26, 2022

ktlint code tries to be compatible with the IntelliJ IDEA default formatter. This to prevent that the IntelliJ IDEA format is changing code which is formatted by ktlint. For this reason it is recommended in ktlint documentation to avoid using both formatters.

So far, I know of one use case in which both formatters are used. In this case the developer does not want to install and run ktlint on the local development machine although ktlint is running in the CI pipelines of the project. The developer relies on IntelliJ IDEA default formatter and manual fixing of violations reported in the CI pipeline.

More and more issues however are found in which it is really hard to format code nicely without that it conflicts with the IntelliJ IDEA default formatting. For this, I do believe that we should replace the strict interpretation of "not conflicting with the IntelliJ IDEA default formatter" to a guideline of "strives (in reasonable bounds) to be compatible with IntelliJ IDEA default formatter".

In the last couple of months, I have tagged issues ( conflict-with-default-intellij-formatting Code produced by KtLint is not accepted by the IntelliJ default formatter ) whenever they conflicted with IntelliJ IDEA default formatter:

@JLLeitschuh
Copy link
Contributor

Can you elaborate a bit more on the "why" behind this proposed change?

@paul-dingemans
Copy link
Collaborator Author

Can you elaborate a bit more on the "why" behind this proposed change?

  • The requirement is conflicting with the recommendation not to use them both. Reason for this recommendation is that it can result in conflicts.
  • It is "less motivating" to write rules that format code in an "ugly" way as the "nicer" version is rejected by bugs in the IntelliJ IDEA default formatter. Especially because those issue really cost a lot of time to fix. I do not enjoy to make those fixes. Nor does anyone else contribute such kind of fixes.
  • In some case, an opionion was asked from Jetbrains but we did not get any reply on them. I just encountered for example issue experimental:multiline-if-else treats all if statements as multiline #812 in which after almost 2 years, still no response is received.

@eygraber
Copy link
Contributor

I think it would be ok if compatibility was relaxed very slightly (e.g. not so frequent cases where it's very difficult to maintain compatibility).

But if that would lead to growing incompatibility then I'd be against it. I almost always rely on Intellij for formatting while coding, and I rely on ktlint for CI.

@paul-dingemans
Copy link
Collaborator Author

I think it would be ok if compatibility was relaxed very slightly (e.g. not so frequent cases where it's very difficult to maintain compatibility).

I agree that it should be used in restricted cases only. But that is a kind of an arbitrary decision. What would your judgement be for cases mentioned before?

@paul-dingemans paul-dingemans added the conflict-with-default-intellij-formatting Code produced by KtLint is not accepted by the IntelliJ default formatter label Jun 27, 2022
@eygraber
Copy link
Contributor

Maybe by using telemetry to measure usage patterns? Although that's a whole separate issue.

Maybe introduce the changes behind a flag and ask for community input before making it default.

@paul-dingemans
Copy link
Collaborator Author

Maybe by using telemetry to measure usage patterns? Although that's a whole separate issue.

Yeah, indeed. I consider this totally out of scope for this project.

Maybe introduce the changes behind a flag and ask for community input before making it default.

I am trying to get rid of flags and configuration options when possible. Flags tend to make the code more complicated. This issue was just meant to make it simpler to resolve issue by dropping the compatibility requirement.

Unfortunately, you did not answer (or misunderstood) my question. Suppose that you would be the voice of the community. What would be your judgement reqarding each of the issues mentions in the initial post? The choices you can make for each issue are:

  1. drop the issue because code formatted by ktlint must be compatible with IntelliJ IDEA formatter;
    or
  2. accept that code formatted by ktlint is not compatible with IntelliJ IDEA default formatter

@eygraber
Copy link
Contributor

In those particular cases, my choice would be #2

@Kantis
Copy link
Contributor

Kantis commented Jul 1, 2022

I'd vote for 1. The on-boarding experience when everything just works and is compliant out of the box is great. I think adopting Ktlint would've been a harder sell if the team were required to disable intellij rules to align with Ktlint rules.

@paul-dingemans
Copy link
Collaborator Author

I'd vote for 1. The on-boarding experience when everything just works and is compliant out of the box is great.

Sure, if possible against resaonable costs. In case of #1324 I have not found how to achieve compatibility although I spent a few days on it. Dropping the issue does not make ktlint compatible with IntelliJ IDEA.

For issues #1350 and #1217 there seems to be common understanding that those issues should be fixed in IntelliJ IDEA. If ktlint now would comply to IntelliJ IDEA formatting and if the issues would be fixed in IntelliJ IDEA than ktlint needs to be changed again to comply with fixed formatting.

... to disable intellij rules ...

What do you mean with this? IntelliJ does not have rules like ktlint but they have a huge amount of configuation options that affect formatting.

@JLLeitschuh
Copy link
Contributor

I'd like to remind everyone of the original intent of Ktlint.

The goal of the project is to be anti-bike shedding. Going down this route seems to be very bike shedding.

If there's a problem with the formatting that IJ outputs, maybe that is best pushed upstream to IJ as a fix that happens there first?

@paul-dingemans
Copy link
Collaborator Author

I'd like to remind everyone of the original intent of Ktlint.

The goal of the project is to be anti-bike shedding. Going down this route seems to be very bike shedding.

I have never understood what is meant by this sentence. Maybe it is because I am not a native speaker.

If there's a problem with the formatting that IJ outputs, maybe that is best pushed upstream to IJ as a fix that happens there first?

Do you mean that an issue is to be created or a fix? For one of the problems an issue has been created two years ago and still there is no response. So, to me that does not seem a viable route.

@JLLeitschuh
Copy link
Contributor

BikeShedding:

https://en.m.wiktionary.org/wiki/bikeshedding

@paul-dingemans
Copy link
Collaborator Author

BikeShedding:

https://en.m.wiktionary.org/wiki/bikeshedding

Tnx this at least explain what is meant with bikeshedding.

But why do you think it is bike-shedding to discuss about dropping a requirement. The main goal of ktlint is to lint and format code in line with the Kotlin coding conventions and/or Android Kotlin style guide. As far as I know, none of them require to use default IntelliJ IDEA formatting.

@magneticflux-
Copy link

Based on the Kotlin Coding Conventions page, I believe that the IntelliJ "Kotlin style guide" preset (the default for new installs) represents an absolute minimum bar for formatters to adhere to if they purport to follow the official convention:

No configuration required
ktlint aims to capture the official Kotlin coding conventions and Android Kotlin Style Guide.

If the IntelliJ default formatter, the Kotlin coding conventions, or the Android Kotlin Style Guide are ever in direct conflict, I believe they should be prioritized in that order.

In short: deliberately producing code that IntelliJ rejects/modifies goes against the point of an anti-bikeshedding formatter.

@paul-dingemans
Copy link
Collaborator Author

Like KtLint, the default IntelliJ IDEA formatter is an implementation of the "Kotlint style guide". As of that it can contain inconsistencies or unwanted formatting as well. As of that, I do not think that the IntelliJ IDEA way of formatting should have the highest priority.

This issue is about the situation in which it either is impossible or rather time consuming to be consistent with IntelliJ IDEA default formatter or in case the code formatted by IntelliJ is just bad (which happens only rarely).

@paul-dingemans
Copy link
Collaborator Author

Closed in favour of #1699

@paul-dingemans paul-dingemans closed this as not planned Won't fix, can't repro, duplicate, stale Nov 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict-with-default-intellij-formatting Code produced by KtLint is not accepted by the IntelliJ default formatter
Projects
None yet
Development

No branches or pull requests

5 participants