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

Format multiple trailing closures. #230

Merged
merged 1 commit into from
Sep 17, 2020

Conversation

allevato
Copy link
Member

When a function call has multiple trailing closures, every closure is forced to wrap.


There are two potentially surprising things about the behavior here:

  1. Unlike an if/else statement, which can be formatted like this if it would fit entirely on one line:

    if foo { bar } else { baz }

    ...we do not allow the same thing to occur for multiple trailing closures at this time:

    // this
    foo(x) { bar() } label: { baz() }
    // will always become
    foo(x) {
      bar()
    } label: {
      baz()
    }
  2. The forced break occurs after the closure signature { arg, arg, arg in‸ if it's present; otherwise, it occurs after the open brace. This lets the closure signature stay on the same line as the open brace if it fits, but it will still wrap (via the normal break that occurs here {‸arg, arg, arg in) if it wouldn't fit.

    However, because the break after in‸ is forced, that means that if the break after {‸ also fires, then even a single-statement closure will have a line break after the signature, when regular closures otherwise wouldn't. In other words,

    // possible
    reallyLongFunctionName(x) {
      x in bar(x)
    }
    
    // NOT possible
    reallyLongFunctionName(x) {
      x in bar(x)
    } secondLabelThatIsAlsoLong: {
      y in baz(y)
    }
    
    // ...it becomes this:
    reallyLongFunctionName(x) {
      x in
      bar(x)
    } secondLabelThatIsAlsoLong: {
      y in
      baz(y)
    }

I can live with (1); I don't think it's a huge problem because I imagine the number of function calls with multiple trailing closures that would fit entirely on one line are exceedingly rare to begin with, and I think in the case of multiple trailing closures it's more readable to have them wrapped anyway so that the label isn't hidden in the middle of the line.

I'm less happy about (2), since it's a weird inconsistency, but I'm not sure our current model is flexible enough to say "given {₁arg, arg, arg in₂, force break 2 only if we didn't already break at 1". It's almost like a reset, but not quite the same because we're not conditioning it on continuation line state.

@dylansturg Any ideas here?

When a function call has multiple trailing closures,
every closure is forced to wrap.
@allevato
Copy link
Member Author

Going to go ahead and merge this to get it into the 5.3 branch; I'll revisit improvements to the newline behavior above later.

@allevato allevato merged commit d4bba6e into swiftlang:master Sep 17, 2020
@allevato allevato deleted the multiple-trailing-closures branch September 17, 2020 21:20
allevato added a commit to allevato/swift-format that referenced this pull request Sep 17, 2020
allevato added a commit that referenced this pull request Sep 17, 2020
aaditya-chandrasekhar pushed a commit to val-verde/swift-format that referenced this pull request May 20, 2021
* Build documentation for external types.

Implements swiftlang#122.

* Display extensions in definition list

Remove unnecessary style rules for extensions

* Fix false positives for external types.

* Better check for external symbols.

* Refactor isExternalSymbol to perform more general symbol resolution

* Remove unnecessary parameter

* Add resolution for nested types through typealiases

Refactor implementation of ID

* Use typealias resolution when creating relationships

* Add tests for extensions on typealiases.

* Add changelog entry for swiftlang#230

Co-authored-by: Mattt <mattt@me.com>
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.

2 participants