-
Notifications
You must be signed in to change notification settings - Fork 506
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
Double indentation of class body when superclass is wrapped #2423
Comments
This is intended behavior for code style |
What is the rationale behind re-indenting hundreds of lines based only on a very few lines of class header? I guess that part of the answer is that it matches some level of logical indentation, but which part? (I'm really curious about this!) Here's a reason for not doing this: When I'm looking at any random code at line 230, and see 2 levels of indentation it gives implicit context that the code is nested inside something. This rule takes away this crutch and requires anyone reading any code formatted with ktlint_official to actually have to look at the top level definition (which might be at the top of the file, or might be at line 145) to figure out if this is the case. This requires a lot of mental and physical (scrolling) gymnastics for something that's a given in 99.9% of auto-formatted code styles in a plethora of languages not just Kotlin. Note: I might be actually fine with this, because I always advocate for the clearly structured:
style which gives a clean and immediate overview without the need for brain-parsing the code on a single line into disparate pieces. And this style doesn't reproduce this issue. It just feels weird to reformat a whole file based on the placement of
The class body logically belongs to the |
The idea is to write all class headers in a consistent way. There are a few pain points when doing so:
Is it worth to indent the entire class body compared to the size of the class header? That depends on your context and the size of your classes. For small classes it doesn't hurt at all. For big classes (code smell) it might actually hurt. |
I also agree with @TWiStErRob the content belongs to the class and not the superclass so have the double indent makes it less clear were the code belongs to. Also I don't see a bigger class means that it is automatically a code smell(at which point is it actually a bigger class). I'm all in for consistency but at the same time we should also not forget that the code should not get more complicated to read for humans. Coming from python this indentiation would also be wrong so depends from where you come. But having it with a single indent is i believe in all languages the same |
I have to agree with the OP, and even go a step further, I think this makes ktlint basically unusable for common scenarios. With test code like this class Test(
val mockService: Service
) : WordSpec({
val test = 0
}) which you will often have when using Spring, the body is double indented with no way to disable that behavior other than disabling the standard indent rule. Having thousands of lines of test code double intended is just not an option, especially since Kotest already adds another level of indention with most specs. |
Expected Behavior
Input is valid. Class body is correctly indented. Final brace is aligned with class opener.
Maybe the
SuperClass(primaryParam) {
line indented, but not the whole body. IntelliJ also just does this one line indent.Observed Behavior
Steps to Reproduce
ktlint "MyClass.kt" --format
Note: when I unwrap the first line:
the indentation does not change at all, neither with:
But this header also reproduces this issue:
Your Environment
.editorconfig
settings: noneThe text was updated successfully, but these errors were encountered: