Fix OneCasePerLine stranding the case keyword on its own line#1208
Conversation
When the elements of a multi-element case declaration are spread across
continuation lines (each element after a trailing comma on its own line),
splitting them into separate case declarations kept the leading newline
trivia on the element name. With respectsExistingLineBreaks enabled (the
default), the pretty printer honored that newline and emitted the case
keyword alone on one line with the element on the next, e.g.
case
b = 1
which is also harder to read. Drop the leading vertical whitespace from
the first element of each generated case declaration while preserving any
comments attached to it.
| expected: """ | ||
| enum Foo: Int { | ||
| case a = 1 | ||
| case // This should stay with `b`. |
There was a problem hiding this comment.
I think in this situation, we could do even better: Move the comment/trivia to precede the newly inserted case keyword.
Another question: What if the comment is an end-of-line comment on the previous line, like this?
enum Foo: Int {
case a = 1, // a comment about 'a'
b = 2
}In that case, since the comment is at the end of the line associated with a, it should stay on that line. Can you add a test for that as well and make sure it behaves as described?
Refine the trivia handling so a comment that documents a binding on a continuation line moves to precede the newly inserted case keyword instead of being stranded after it. An end-of-line comment that sits on the previous binding's line now stays attached to that binding rather than migrating to the new case. Previously the comment was left as the leading trivia of the element, so it ended up between case and the binding name. The split now hoists any documentation comment ahead of the case keyword. Separately, removing a trailing comma used to discard the comma's trailing trivia, which lost an end-of-line comment like `case a = 1, // about a`. That trivia is now preserved on the element so the comment keeps its place. Adds tests for the hoisted leading comment, the retained end-of-line comment, and the combination of both on a single split.
|
Good calls on both, thanks. I pushed a revision. For the documentation comment that lives on its own continuation line, I now hoist it so it precedes the newly inserted case a = 1
// This should stay with `b`.
case b = 2For the end-of-line comment case you raised, that comment sits in the trailing trivia of the previous binding's comma, and the rule was dropping it when it stripped the comma. I now carry that trailing trivia over onto the element before removing the comma, so it stays on the same line as the binding it follows: case a = 1, // a comment about 'a'
b = 2becomes case a = 1 // a comment about 'a'
case b = 2I added a test for that ( |
Fixes #1009.
When a multi-element
casedeclaration has its elements spread across continuation lines,OneCasePerLineleft the leading newline trivia on each element's name when splitting them into separate declarations. WithrespectsExistingLineBreaksenabled (the default), the pretty printer honored that newline, so thecasekeyword ended up alone on its own line:was reformatted to
Fix
When building each generated case declaration, drop the leading vertical whitespace (newlines and surrounding indentation) from its first element so the element stays on the same line as the
casekeyword. Comments attached to the element are preserved rather than dropped, so something likestill keeps the comment with
b.Testing
Added
testElementsOnContinuationLinesAreMovedOntoCaseKeywordcovering the reported case andtestCommentBetweenElementsOnContinuationLinesIsPreservedfor the comment edge case.swift testpasses (915 tests, the one pre-existing skip unchanged).