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

Does not match Kotlin code style for Chained Wrapping Calls #171

Closed
ScottPierce opened this issue Mar 9, 2018 · 5 comments
Closed

Does not match Kotlin code style for Chained Wrapping Calls #171

ScottPierce opened this issue Mar 9, 2018 · 5 comments

Comments

@ScottPierce
Copy link

I have the following code:

fun test() {
  listOf(1, 2, 3)
    .map {
      it.toString()
    }
    .filter {
      it.isNotBlank()
    }
    .map { "alsdfjadsf" }
    .map { "adsjfhasdf" }
}

This matches the style described by the Kotlin Style Guide: https://kotlinlang.org/docs/reference/coding-conventions.html#chained-call-wrapping

ktlint doesn't match the with a single indent part of the style guidelines for chained call wrapping.

Here is my .editorconfig:

[{*.kt,*.kts}]
indent_size=2
insert_final_newline=true

Here is my error when running ktlint:

<project dir>/features/base-android/src/main/java/KtLintTest.kt:4:1: Unexpected indentation (4) (it should be 6)
<project dir>/features/base-android/src/main/java/KtLintTest.kt:7:1: Unexpected indentation (4) (it should be 6)
<project dir>/features/base-android/src/main/java/KtLintTest.kt:10:1: Unexpected indentation (4) (it should be 6)
<project dir>/features/base-android/src/main/java/KtLintTest.kt:11:1: Unexpected indentation (4) (it should be 6)

According to the style guideline, an indent of 4 is correct, but it seems ktlint wants a continuation indent.

@shyiko
Copy link
Collaborator

shyiko commented Mar 9, 2018

Hi Scott.
In addition to indent_size=2 you need to set continuation_indent_size=2 (otherwise ktlint assumes 4).

@ScottPierce
Copy link
Author

@shyiko The kotlin style guide isn't that continuation indents are the same as normal indents, is it?

It's just in the case of chained styles that the continuation indent isn't used, right?

@ScottPierce
Copy link
Author

@shyiko Did that make sense? Here is what the style guidelines say:

When wrapping chained calls, put the . character or the ?. operator on the next line, with a single indent

You are suggesting we make the continuation indent equal to the normal indent. That would violate other styles though. The style guide effectively says to NOT use a continuation indent when chaining method calls. This can be represented in IntelliJ settings like this:

screen shot 2018-03-10 at 10 35 40 am

The Android style guidelines are that continuation indents are still twice the normal indent:

When line-wrapping, each line after the first (each continuation line) is indented at least +8 from the original line.

Do you see what I'm saying? What should I be doing in this case?

@shyiko
Copy link
Collaborator

shyiko commented Mar 10, 2018

Sure. It does make sense. It's going to be fixed in the next release (this is also why I haven't closed the ticket).

Having said that, Coding Conventions do not mention continuation indent (at all), which means the default (in form of indent_size=4 and continuation_indent_size=4) is technically "legal". As for Android Style Guide, see #134 (comment).

I'd say stick with the defaults. All this indent != continuation_indent business helps no one and only creates confusion. If you really want 2 space indent then #171 (comment) is the way to go.

@shyiko
Copy link
Collaborator

shyiko commented Apr 22, 2018

Fixed in 0.22.0.

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

No branches or pull requests

2 participants