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

Parameter wrapping incompatible with ktlint with long function parameter #245

Closed
bethcutler opened this issue Jul 12, 2021 · 2 comments
Closed

Comments

@bethcutler
Copy link
Contributor

Current ktfmt formatting:

private fun testWrapping(
  testParameter:
    (
      longEnoughNameToForceMultilineParams: MyVeryLongObject,
      anotherVeryLongName: ALongObject) -> Unit
) {
  // foobar
}

What ktlint expects:

private fun testWrapping(
  testParameter: (
    longEnoughNameToForceMultilineParams: MyVeryLongObject,
    anotherVeryLongName: ALongObject
  ) -> Unit
) {
  // foobar
}

I think the ktlint formatting looks better. The ) on a new line would be standard for "google-style". The ( not being on its own line is a bit more compact and it's more obvious what the parens are "attached" to. However, the current style is similar to the version with a scoping function, e.g.: (Acceptable for both ktfmt and ktlint)

private fun testWrapping(
  testParameter:
    suspend (
      longEnoughNameToForceMultilineParams: MyVeryLongObject,
      anotherVeryLongName: ALongObject,
    ) -> Unit
) {
  // foobar
}
@bethcutler
Copy link
Contributor Author

Similar issue:

Original (and ktlint's version, and my personal preference):

typealias Listener = (
  aVeryLongFoo: String,
  anotherVeryLongFoo: String,
  aThirdVeryLongFoo: String,
  aFourthVerylongFoo: String
) -> Unit

ktfmt's version (google-style, FWIW):

typealias Listener =
  (
    aVeryLongFoo: String,
    anotherVeryLongFoo: String,
    aThirdVeryLongFoo: String,
    aFourthVerylongFoo: String) -> Unit

bethcutler added a commit to bethcutler/ktfmt that referenced this issue Apr 7, 2022
facebook-github-bot pushed a commit that referenced this issue Apr 13, 2022
Summary:
Fix for #245

Pull Request resolved: #308

Reviewed By: cgrushko

Differential Revision: D35506462

Pulled By: strulovich

fbshipit-source-id: 5419130214152e6e083919935e5ae3be0cb47492
@davidtorosyan
Copy link
Contributor

Fixed by 1bebf0b, closing.

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