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

One parameter per line if function prototype doesn't fit entirely in one line #13

Closed
Trigon opened this issue Dec 31, 2019 · 9 comments
Closed
Labels
formatting-discussions Discussions about how formatting should look like, when there's no clear consensus.

Comments

@Trigon
Copy link

Trigon commented Dec 31, 2019

private fun someMethod(
      metaData: JsonNode,
      interfaceIdex: Int,
      registryIdex: Int
  ): ArrayNode? {

For this one, it should be the right format, but after I use ktfmt, it changes to

private fun getMethodsOrderedImplementationsWithMethods(
      metaData: JsonNode, interfaceIdex: Int, registryIdex: Int
  ): ArrayNode? {

Free feel to talk to me directly via workchat wyl@fb

@cgrushko
Copy link
Contributor

  1. Are you missing 2 spaces before private fun…?

I'd expect the output to be:

  private fun getMethodsOrderedImplementationsWithMethods(
      metaData: JsonNode, interfaceIdex: Int, registryIdex: Int
  ): ArrayNode? {
  1. The parameters fit in one line, so ktfmt doesn't break them. Should it break? do you prefer the first version? are you aware of a style guide that mandates it? how should ktfmt decide whether to break the parameter list or not?

@cgrushko cgrushko changed the title Format for function def One parameter per line if function prototype doesn't fit entirely in one line Dec 31, 2019
@Trigon
Copy link
Author

Trigon commented Dec 31, 2019

@cgrushko
Sorry, the 2 spaces are my issue, the output is the same as what you expect.
After ktfmt makes the parameters be in the same line, the lint on phabricator complains about that.
I think the style guide should be consistent between ktfmt and the lint in phabricator

@cgrushko
Copy link
Contributor

ktfmt and detekct/ktlint disagree, so some friction between the linters is expected (we plan to disable detekt's formatting on files that ktfmt formats).

Again, we could change this formatting decision. I don't have a good sense of what's right to do.
cc @strulovich

@strulovich
Copy link
Contributor

I think I agree with Carmi's expected style. We should disable some of Detekt's checks to match with this.

@sgrimm
Copy link
Contributor

sgrimm commented Jan 2, 2020

Google's Kotlin style guide for Android says arguments should be split across multiple lines if the function signature doesn't fit on one line. JetBrains' official Kotlin style guide is fuzzier; it says "closely related" arguments should be placed on the same line, but obviously ktfmt can't determine that.

@cgrushko
Copy link
Contributor

cgrushko commented Jan 3, 2020 via email

@Trigon
Copy link
Author

Trigon commented Jan 3, 2020

I agree with being similar to java. Would the detekct changes the rule OR ktfmt replace detekct as the lint in diff tool?

@cgrushko
Copy link
Contributor

cgrushko commented Jan 3, 2020 via email

@cgrushko cgrushko added the formatting-discussions Discussions about how formatting should look like, when there's no clear consensus. label Jan 7, 2020
@cgrushko
Copy link
Contributor

cgrushko commented Mar 4, 2020

I'll close this for now. We can revisit this if we run into examples where it's very clearly better to break before each parameter.

@cgrushko cgrushko closed this as completed Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatting-discussions Discussions about how formatting should look like, when there's no clear consensus.
Projects
None yet
Development

No branches or pull requests

4 participants