Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Match indent to continuation indent? #196

Closed
ScottPierce opened this issue Apr 24, 2018 · 6 comments
Closed

Match indent to continuation indent? #196

ScottPierce opened this issue Apr 24, 2018 · 6 comments

Comments

@ScottPierce
Copy link

ScottPierce commented Apr 24, 2018

The documentation for the most recent version currently states:

change Continuation indent to the same value as Indent (4 by default).

Isn't this against the Kotlin and Android style guides? Both style guides have / use continuation indents currently, and the continuation indents are expected to be double the normal indent (defaulting to 8). In order to compensate, many things that would normally use continuation indents, no longer use continuation indents. i.e. constructor properties, or method chaining:

data class Example(
    val a: String, // <-- normal indent, not continuation indent
    val b: String
)

The solution of making the continuation indent to match the normal indent seems wrong. Instead, ktlint needs to be updated to stop expecting continuation indents in locations where the style guide prohibits them.

@igorwojda
Copy link

I kind of like this change, but I think it's just a matter of personal preference.

Whatever team decides I want to point out that ktlint --apply-to-idea-project --android does not set continuation indent to 4, so this must be set manually (currently official ktlint settings are conflicting with Android Studio formatting settings)

@ScottPierce
Copy link
Author

It doesn't change the fact that now ktlint has deviated from the official kotlin and android style guides.

The official Kotlin style guide takes things like constructor parameters and makes them use single indents instead of continuation indents (as they would normally). ktlint should just support this properly by changing the expected indent there from a continuation indent to a normal indent. Instead of supporting it properly though, the recommendation is to just take continuation indent == to normal indent. This solution is against both style guides, as both style guides still have continuation indents that are expected to be twice the normal indent.

Another example is method chaining. The kotlin style guide specifies that chained methods should be used with normal indents, not continuation indents. I'm sure there are others. Ktlint should match these changes in the same way.

@shyiko
Copy link
Collaborator

shyiko commented May 1, 2018

  1. continuation_indent_size=4 was the default in ktlint pretty much from the beginning.
    At some point it was changed to match Android Style Guide's "8" (note that official Kotlin Style Guide doesn't say a thing about continuation indent, it never did) but was swiftly reverted back to 4 after continuation indent was replaced with indent in most of the cases (constructor/method parameters, method chaining) (simply to avoid all the back and forth).
  2. In general, 4 as a default is not as bad as you might think:
    (at least not while ident rule is dumb)
  • ktlint obeys value specified in .editorconfig. e.g. if continuation_indent_size=8 - ktlint --apply-to-idea-project will set "8" as continuation indent.

  • if you upgrade from ktlint@0.1.0 all the way up to the latest ktlint@0.22.0, unless you had a custom (and by custom I mean anything but continuation_indent_size=4 & continuation_indent_size=8) you shouldn't notice a thing (as far as indent rule goes).

  • no matter what you have in .editorconfig the following is true:

    • constructor/method parameters are expected to be "single-indent"ed
    • everything else is at least "single-indent"ed

    (as a direct result if you run ktlint against any codebase without continuation_indent_size in .editorconfig no errors will be printed regardless of whether the code is using single or double-indent)

  1. Default continuation indent for Android should be 8 not 4 #120 is still open. 4 vs 8 default is by no means resolved.
  2. If Kotlin Style Guide enumerates all the cases where continuation_indent must be used (which I really hope they find the wisdom not to do as continuation indent brings zero value and only complicates the style guide) ktlint will be updated to match that recommendation.

@igorwojda
Copy link

Good explanation.

We have twp separate discussions here...getting back to my point
I think that ktlint --apply-to-idea-project should also override IDE settings to 4 if there is no .editorconfig file. Otherwise, ktlint setting conflicts with Android Studio default setting with is 8. Now after downloading ktlint and running ktlint --apply-to-idea-project --android ktlint is complaining about default Android Studio continuation_indent formatting (I applied config, but this setting still needs to be changed manually).

@shyiko
Copy link
Collaborator

shyiko commented May 1, 2018

Thanks for the clarification, @igorwojda. I'll look into it.

@shyiko
Copy link
Collaborator

shyiko commented May 3, 2018

Closing in favor of #120.

@igorwojda it seems like ktlint --apply-to-idea-project changes continuation indent to match the indent after all (tested on latest Android Studio). Feel free to open a separate ticket if that's not what you observe. Thank you.

@shyiko shyiko closed this as completed May 3, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants