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

Additional parent classes are forced to be on a new line always. Is this intentional? #1838

Closed
ZakTaccardi opened this issue Mar 1, 2023 · 4 comments

Comments

@ZakTaccardi
Copy link

Is reformatting:

import kotlinx.coroutines.CoroutineScope

internal abstract class MyParentClass(
    param1: String,
    param2: String
)

internal class MyChildClass(
    scope: CoroutineScope
) : MyParentClass(
    param1 = "one",
    param2 = "two"
), CoroutineScope by scope

to be:

import kotlinx.coroutines.CoroutineScope

internal abstract class MyParentClass(
    param1: String,
    param2: String
)

internal class MyChildClass(
    scope: CoroutineScope
) : MyParentClass(
    param1 = "one",
    param2 = "two"
),
    CoroutineScope by scope

a bug? the error is Missing newline after "," and violates the wrapping rule.

My take here is that the following format looks...off (bc it can fit on the same line):

),
    CoroutineScope by scope

Using version 0.48.2

@paul-dingemans
Copy link
Collaborator

I am not sure whether to classify it as a bug or not. On one hand, I do agree that it looks less pretty than the original code. The argument bc it can fit on the same line is not a convincing argument as that depends on the size of your indent. Also the Kotlin coding conventions do not provide an example that fits with your example. Based on the examples, I think that it should be formatted something like:

internal class MyChildClass(
    scope: CoroutineScope
) : MyParentClass(
        param1 = "one",
        param2 = "two"
    ),
    CoroutineScope by scope

This can best be resolved when implementing #1349.

@ZakTaccardi
Copy link
Author

internal class MyChildClass(
    scope: CoroutineScope
) : MyParentClass(
        param1 = "one",
        param2 = "two"
    ),
    CoroutineScope by scope

actually, yeah, this looks good 👍

@paul-dingemans paul-dingemans added this to the 1.0 (Yeah!) milestone Apr 16, 2023
@paul-dingemans paul-dingemans self-assigned this Jul 3, 2023
@paul-dingemans
Copy link
Collaborator

Implemented by #1349

@paul-dingemans
Copy link
Collaborator

See #2119

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants