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

Fix incorect placement of trailing stub function comments #11632

Merged
merged 4 commits into from
May 31, 2024

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented May 31, 2024

Summary

This fixes an issue in the formatter where it reorder comments or, worse, moved trailing stub function comments into the next's function body.

The problem is that the formatter uses line_suffix for the trailing comments and the line suffixes are only flushed after the next newline. However, there's no such newline directly after the comment(s) because the formatter now allows newlines to be omitted between stub functions.

The fix of this PR is to match Black's behavior to insert the empty lines if a collapsed stub has a trailing comment. I'm not very fond of the empty lines, because I think the comments should remain close to the stub body and not be separated by empty lines but it is consistent with how we format trailing function comments in other places and matches Black.

Closes #11569

Test Plan

See added tests

Copy link
Contributor

github-actions bot commented May 31, 2024

ruff-ecosystem results

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser MichaReiser added bug Something isn't working formatter Related to the formatter labels May 31, 2024
@@ -240,7 +240,8 @@ impl FormatRule<Suite, PyFormatContext<'_>> for FormatSuite {
preceding_stub.end(),
f.context().source(),
) < 2
});
})
&& !preceding_comments.has_trailing_own_line();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the actual fix

@MichaReiser MichaReiser marked this pull request as ready for review May 31, 2024 09:18
@MichaReiser MichaReiser changed the title fix function trailing comments Fix misplacement of trailing stub function comments May 31, 2024
@MichaReiser MichaReiser changed the title Fix misplacement of trailing stub function comments Fix incorecty placement of trailing stub function comments May 31, 2024
@MichaReiser MichaReiser changed the title Fix incorecty placement of trailing stub function comments Fix incorect placement of trailing stub function comments May 31, 2024
@MichaReiser MichaReiser force-pushed the fix-function-trailing-comments branch from 8dab64a to 53b7f53 Compare May 31, 2024 12:02
@MichaReiser MichaReiser enabled auto-merge (squash) May 31, 2024 12:02
@MichaReiser MichaReiser merged commit 9b6d2ce into main May 31, 2024
17 checks passed
@MichaReiser MichaReiser deleted the fix-function-trailing-comments branch May 31, 2024 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working formatter Related to the formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formatter changes order of comments.
2 participants