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

Unexpected indendation is reporting non-existent issues (Android project) #42

Closed
autonomousapps opened this issue Mar 15, 2018 · 13 comments

Comments

@autonomousapps
Copy link
Contributor

Lint error > src/main/java/com/MyClass.kt:24:9: Unexpected indentation (expected 4, actual 8)

// Class names obfuscated
class MyClass internal constructor(
        private val sharedPreferences: SharedPreferences // there are 8 spaces here
) : IMyClass {

The 8 spaces are required for continuation indents by the Android Kotlin Style Guide.

@shyiko
Copy link

shyiko commented Mar 15, 2018

@autonomousapps see pinterest/ktlint#134 (comment)

@autonomousapps
Copy link
Contributor Author

Cool. I'll update my IDE settings to reflect this and awaaaay I go. Thanks!

@autonomousapps
Copy link
Contributor Author

Although, I will note that I tried this, and it accomplished nothing:

kotlinter {
    indentSize = 4
    continuationIndentSize = 8
}

Am I doing it wrong? :)

@jeremymailen
Copy link
Owner

Thanks for the help-out @shyiko

@autonomousapps I'll look into it. The expectation was that continuationIndentSize ought to have been applicable to the parameter list indent, correct?

@shyiko
Copy link

shyiko commented Mar 15, 2018

Parameters use single indent per
https://kotlinlang.org/docs/reference/coding-conventions.html#class-header-formatting and
android/kotlin-guides#42 (not yet merged in gh-pages).

@autonomousapps
Copy link
Contributor Author

Now it's yelling at me about another indentation issue.

Lint error > src/main/java/com/login/LoginViewModel.kt:34:1: Unexpected indentation (12) (it should be 16)

    fun onLoginRequested(credentials: LoginCredentials) {
        loginManager.login(credentials)
            .disposeOnCleared() // 12 spaces indentation
            .observeOn(mainThread)
            .subscribe({
                Logger.i(TAG, "Successful login. loginData = $it")
                loginState.value = true
            }, { t ->
                Logger.e(TAG, t, "Error logging in: ${t.localizedMessage}")
                // TODO tell view about errors
                loginState.value = false
            })
    }

@autonomousapps
Copy link
Contributor Author

I can't control continuation indents for constructors and method calls independently. So if I set my IDE to use 4, it uses 4 everywhere. SIGH.

screenshot from 2018-03-15 13-55-41

@jeremymailen
Copy link
Owner

jeremymailen commented Mar 16, 2018

Thanks for the info. That is interesting that the style guide special cases class (primary) constructor parameters as regular indents but continuation indents for secondary constructors and function calls.

I get their rationale, but IntelliJ's Kotlin formatter treats the primary constructor parameters as continuation indent. I wonder if someone ought to file a bug against JetBrains since they aren't in internal agreement.

@jeremymailen
Copy link
Owner

And I've also noticed that kotlinter's default continuationIndentSize of 8 is out of date with the new standard. I'll change it and push a new release.

Now that the default will be 4 and 4 I guess the discrepancy with IntelliJ will not be troublesome.

@jeremymailen
Copy link
Owner

On your latest code example, (onLoginRequested()) it's pointing out that you should have the continuation indent of 8 there instead of the 4 extra you have. I'm assuming you still have continuationIndentSize = 8 in that case.

@jeremymailen
Copy link
Owner

@autonomousapps let me know if there's anything else I can do to help. Otherwise can we resolve the issue?

@MyDogTom
Copy link
Contributor

@autonomousapps Please take a look at my setup for Kotlin plugin. Pay attention that "use continuation indent" is checked only for: "Method call arguments", "Chained method calls" and "expression body functions".
@jeremymailen continuation indent for android is still 8, not 4. (source)

screen shot 2018-03-16 at 09 10 05

@autonomousapps
Copy link
Contributor Author

autonomousapps commented Mar 16, 2018

@jeremymailen actually, I had removed the kotlinter {} block, so it was using the default of 8 for continuationIndentSize. However, if I put the block back and specify that it should be 4, then I get a successful build. Of course, this is contrary to the current advice in the style guide, which I think still specifies 8 spaces for continuation indents (unless they're primary constructor params?!).

The issue seems to be that IntelliJ doesn't let me specify those two cases individually. But frankly, I am ok with using a continuation indent of 4. The value of the tool outweighs such minor concerns. (oh whoops, I'm wrong about this)

PS @MyDogTom thanks for that advice! I'll consider doing that, instead.

Now that I have two options two deal with this, I fully consider the issue resolved. Thanks, everyone!

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

4 participants