Skip to content

Commit

Permalink
Don't break for short variables names (even for simple chains)
Browse files Browse the repository at this point in the history
Summary:
## Context

After the changes in D33399531 (5164ad9), we were left with a few hard to understand variables which would get in the way of changing the newline logic. This aims to remove the `simple` boolean.

## Change

WARNING: This isn't a refactor - the final formatting is different (but for the better).

As described in the docs for `textLength`, we don't break if the first reference is short enough:
```
// shorter than 4 characters
z123.red
    .orange
    .yellow
    .green
    .blue
    .indigo
    .violet
    .cyan
    .magenta
    .key
```

However, this logic wasn't used whenever there were multiple invocations:
```
// shorter than 4 characters...
// but ends with a lambda and has more 2 invocations
// why should that matter?
z123
    .shine()
    .bright()
    .z { it }
```

Now it's done uniformly:
```
// shorter than 4 characters
z123.shine()
    .bright()
    .z { it }
```

Reviewed By: strulovich

Differential Revision: D33488790

fbshipit-source-id: 5eb47fd952d976bd0899a85e112fb4fa08eee379
  • Loading branch information
davidtorosyan authored and facebook-github-bot committed Jan 13, 2022
1 parent d74f46a commit fd042ed
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -669,16 +669,6 @@ class KotlinInputAstVisitor(
// is the last expression a lambda? e.g.`foo.bar.apply { ... }`
val hasTrailingLambda = chunks.last().expressions.last().isLambda()

// Do we have any expressions that should be kept on the same line?
// True example: `rainbow.red.orange.shine()`
// False example: `rainbow.red.orange`
// Note that in this example the function invocation is what causes the chunk to be a "prefix",
// but there are other ways defined in [breakIntoChunks].
val hasPrefixes = chunks.first().shouldKeepOnSameLine

// simple chain with no meaningful groups or trailing lambda, e.g `rainbow.red.orange`
val simple = !hasPrefixes && !hasTrailingLambda

// When the last chunk is meant to be on one line, reduce the indentation of arguments:
// ```
// rainbow.shine(
Expand Down Expand Up @@ -754,7 +744,7 @@ class KotlinInputAstVisitor(
if (chunkIndex > 0 || itemIndex > 0) {

// break if there's a lambda, or the line is long enough
if (!simple || textLength > options.continuationIndent || item.isLambda()) {
if (textLength > options.continuationIndent || item.isLambda()) {
val fillMode =
if (chunk.shouldKeepOnSameLine) Doc.FillMode.INDEPENDENT else Doc.FillMode.UNIFIED
builder.breakOp(fillMode, "", ZERO, Optional.of(nameTag))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4945,8 +4945,7 @@ class FormatterTest {
assertFormatted(
"""
|-------------
|z123
| .shine()
|z123.shine()
| .bright()
| .z { it }
|""".trimMargin(),
Expand Down

0 comments on commit fd042ed

Please sign in to comment.