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

Fix indentation of delegated super type call entry #1305

Merged

Conversation

paul-dingemans
Copy link
Collaborator

Description

Forces the BY keyword in a super type call entry to be indented when preceded by a new line

The code sample from issue #1210 is now formatted as:

object ApplicationComponentFactory : ApplicationComponent.Factory
    by DaggerApplicationComponent.factory()

As a side effect, this also formats

class Test2 : Foo
by Bar(
    a = 1,
    b = 2,
    c = 3
)

to

class Test2 : Foo
    by Bar(
        a = 1,
        b = 2,
        c = 3
    )

Closes #1210

Checklist

  • tests are added
  • CHANGELOG.md is updated

Paul Dingemans added 2 commits December 11, 2021 18:01
Forces the BY keyword in a super type call entry to be indented when preceded by a new line

Closes pinterest#1210
@romtsn
Copy link
Collaborator

romtsn commented Dec 13, 2021

Hm, when I throw this code into IJ, it actually keeps it unchanged:

class Test2 : Foo
by Bar(
    a = 1,
    b = 2,
    c = 3
)

Should we maybe just make sure that it doesn't fail, but follow IJ's formatting in this case? Note, however, that this case is fine:

class Test2 : 
    Foo
    by Bar(
        a = 1,
        b = 2,
        c = 3
    )

@paul-dingemans
Copy link
Collaborator Author

You're right. We should not conflict with IntelliJ default of formatting case below:

class Test2 : Foo
by Bar(
    a = 1,
    b = 2,
    c = 3
)

Let me try to fix this one before merging.

…ate-super-type

# Conflicts:
#	CHANGELOG.md
#	ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt
@shashachu
Copy link
Contributor

@paul-dingemans let me know if/when this is ready for merging.

…ate-super-type

# Conflicts:
#	CHANGELOG.md
#	ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt
@paul-dingemans
Copy link
Collaborator Author

@paul-dingemans let me know if/when this is ready for merging.

@shashachu Merge conflict is resolved. It is ready to be merged.

@shashachu shashachu merged commit 3866e09 into pinterest:master Dec 20, 2021
@romtsn
Copy link
Collaborator

romtsn commented Dec 20, 2021

@paul-dingemans but have you addressed the comment I left? the test is kinda still not looking compatible with IJ
image

@paul-dingemans
Copy link
Collaborator Author

@romtsn @shashachu I am sorry. I only focussed at the merge conflict. I did not read the history carefully enough. I will address or submit a PR to revert this tomorrow.

@romtsn
Copy link
Collaborator

romtsn commented Dec 20, 2021

awesome, thanks 👍

@paul-dingemans
Copy link
Collaborator Author

I will address or submit a PR to revert this tomorrow.

@shashachu @romtsn I have it solved on my local machine. Code however is not production ready yet. Will continue tomorrow.

@paul-dingemans paul-dingemans mentioned this pull request Dec 22, 2021
2 tasks
@paul-dingemans
Copy link
Collaborator Author

@romtsn @shashachu I am sorry. I only focussed at the merge conflict. I did not read the history carefully enough. I will address or submit a PR to revert this tomorrow.

@romtsn @shashachu See #1322 for fix.

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

Successfully merging this pull request may close these issues.

Internal Error when upgrading from 0.41.0 to 0.42.1
3 participants