facebook / ktfmt

A program that reformats Kotlin source code to comply with the common community standard for Kotlin code conventions.
https://facebook.github.io/ktfmt/
Apache License 2.0
925 stars 78 forks source link

Kotlin DSL formatting looks messy #206

Open steventrouble opened 3 years ago

steventrouble commented 3 years ago

Actual Behavior

foo = 
  fooDsl {
    bar += 
      barDsl {
        baz = 
          bazDsl {
            bal = 1
          }
      }
  }

Expected Behavior

foo = fooDsl {
  bar += barDsl {
    baz = bazDsl {
      bal = 1
    }
  }
}
steventrouble commented 3 years ago

I have a fix ready and tested for this, but need to get set up to contribute.

cgrushko commented 3 years ago

Thanks for the report. Expected behavior looks good to me, and improves the consistency of scoping function formatting. (changing to val foo = puts the first lambda on the same line).

Looking forward to your patch :)

cgrushko commented 3 years ago

@steventrouble , iirc we had to roll this back; did you by chance look at this again?

steventrouble commented 3 years ago

Hi Carmi, thanks for the ping! I'm on vacation rn and some things are up in the air.

Right now, it doesn't look promising. This task may be difficult to complete without introducing other bugs, at least until there's a better abstraction for handling trailing lambdas. Given that (plus the "things up in the air"), feel free to close this bug if nobody else will have time to fix it.

I wish I had better news to give!

JavierSegoviaCordoba commented 2 years ago

@cgrushko I want to update this. Currently, I am doing an API for tree nodes

It looks so when the tree is simple

val tree = tree(1) { node(2) { node(3) } }

But it should be

val tree =
    tree(1) {
        node(2) {
            node(3)
        }
    }

But that format is correct only when it is a DSL, and not for every lambda.

Because ktfmt doesn't analyze deeply to know if Tree can have a specific annotation like DslMaker to understand that it, indeed, is a DSL, which are the possible options to get a fix for this?

Those options have to be compatible with Gradle DSL, so we can solve the non-standard KTS formatting.

One solution is marking top-level lambdas as DSL as default, so we only do one line lambdas when we are chaining them.

Related: https://github.com/facebookincubator/ktfmt/issues/181

I want to cc active contributors that are not participating in this issue: @strulovich @nreid260

nreid260 commented 2 years ago

I have no problem with the single-line formatting. I suspect that in examples where a single line becomes difficult to read, there will be enough characters to force additional lines.

I don't oppose the multi-line formatting, but don't think it's categorically better.

JavierSegoviaCordoba commented 2 years ago

I have no problem with the single-line formatting. I suspect that in examples where a single line becomes difficult to read, there will be enough characters to force additional lines.

I don't oppose the multi-line formatting, but don't think it's categorically better.

With Gradle kts it is pretty usual and it looks really weird because nobody does one line lambdas with Kotlin KTS or Groovy generally.

For example you can check Google Android docs which are using KTS in new samples.