-
Notifications
You must be signed in to change notification settings - Fork 247
Ensure newline after trailing line comments to prevent formatting issues #1015
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
base: main
Are you sure you want to change the base?
Conversation
840e43a
to
3999b52
Compare
Having to resolve this in the attribute case specifically feels like the wrong approach. There should never be a situation where a |
Thanks for the suggestion — I agree that it makes more sense to handle this through logic for |
3999b52
to
7fa247f
Compare
7fa247f
to
6447c6c
Compare
6447c6c
to
93b91f4
Compare
@@ -623,4 +623,42 @@ final class AttributeTests: PrettyPrintTestCase { | |||
|
|||
assertPrettyPrintEqual(input: input, expected: expected, linelength: 45) | |||
} | |||
|
|||
func testAttributesWithComment() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered moving the test case to CommentTests
, but it felt more appropriate to keep it where it is 🤔
@@ -479,6 +479,9 @@ public class PrettyPrinter { | |||
} | |||
} | |||
outputBuffer.write(comment.print(indent: currentIndentation)) | |||
if wasEndOfLine, (comment.kind == .line || comment.kind == .docLine) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose this works, but I'm more worried about the token stream creation itself (I should have been more specific in my earlier comment). Usually the newline behavior is controlled by what kind of break
is inserted into the token stream, and I would expect that a line comment would always be preceded by something whose newline behavior is soft
(meaning always insert a newline, unless it would contribute more than the maximum number of blank lines allowed). The original issue would seem to imply that that's not the case, and I think we need to understand why.
Resolve #1003
When arranging attributes, they were previously placed on a single line regardless of the presence of line comments.
This change preserves the original behavior of keeping attributes on a single line whenever possible, but inserts a newline when a line comment is present to prevent compilation failures.