Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TTOzzi
Copy link
Member

@TTOzzi TTOzzi commented May 11, 2025

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.

@TTOzzi TTOzzi force-pushed the fix-arrangeAttributeList branch from 840e43a to 3999b52 Compare May 11, 2025 09:13
@allevato
Copy link
Member

Having to resolve this in the attribute case specifically feels like the wrong approach. There should never be a situation where a // or /// comment is followed by a break that would do anything but force a newline. Can you check that instead?

@TTOzzi
Copy link
Member Author

TTOzzi commented May 14, 2025

Having to resolve this in the attribute case specifically feels like the wrong approach. There should never be a situation where a // or /// comment is followed by a break that would do anything but force a newline. Can you check that instead?

Thanks for the suggestion — I agree that it makes more sense to handle this through logic for // or /// comments rather than treating the attribute case separately. That should also help cover other cases we haven’t identified yet.
I’ll look into it again!

@TTOzzi TTOzzi force-pushed the fix-arrangeAttributeList branch from 3999b52 to 7fa247f Compare May 14, 2025 15:26
@TTOzzi TTOzzi changed the title Fixes incorrect formatting when attributes contain line comments by preserving linebreaks Ensure newline after trailing line comments to prevent formatting issues May 14, 2025
@TTOzzi TTOzzi force-pushed the fix-arrangeAttributeList branch from 7fa247f to 6447c6c Compare May 14, 2025 15:27
@TTOzzi TTOzzi force-pushed the fix-arrangeAttributeList branch from 6447c6c to 93b91f4 Compare May 14, 2025 15:30
@@ -623,4 +623,42 @@ final class AttributeTests: PrettyPrintTestCase {

assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
}

func testAttributesWithComment() {
Copy link
Member Author

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) {
Copy link
Member

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.

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.

Import with attribute with trailing comment incorrectly formatted
2 participants