Skip to content

Comments

Preserve newlines after nested compound statements#7608

Merged
charliermarsh merged 2 commits intomainfrom
charlie/if-else
Sep 25, 2023
Merged

Preserve newlines after nested compound statements#7608
charliermarsh merged 2 commits intomainfrom
charlie/if-else

Conversation

@charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Sep 22, 2023

Summary

Given:

if True:
    if True:
        pass
    else:
        pass
        # a

        # b
        # c

else:
    pass

We want to preserve the newline after the # c (before the else). However, the last_node ends at the pass, and the comments are trailing comments on the pass, not trailing comments on the last_node (the if). As such, when counting the trailing newlines on the outer if, we abort as soon as we see the comment (# a).

This PR changes the logic to skip all comments (even those with newlines between them). This is safe as we know that there are no "leading" comments on the else, so there's no risk of skipping those accidentally.

Closes #7602.

Test Plan

No change in compatibility.

Before:

project similarity index total files changed files
cpython 0.76083 1789 1631
django 0.99983 2760 36
transformers 0.99963 2587 319
twine 1.00000 33 0
typeshed 0.99979 3496 22
warehouse 0.99967 648 15
zulip 0.99972 1437 21

After:

project similarity index total files changed files
cpython 0.76083 1789 1631
django 0.99983 2760 36
transformers 0.99963 2587 319
twine 1.00000 33 0
typeshed 0.99983 3496 18
warehouse 0.99967 648 15
zulip 0.99972 1437 21

@charliermarsh charliermarsh added the formatter Related to the formatter label Sep 22, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 22, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

@charliermarsh charliermarsh force-pushed the charlie/if-else branch 2 times, most recently from 8388c0a to 2395178 Compare September 23, 2023 16:42
@charliermarsh charliermarsh enabled auto-merge (squash) September 25, 2023 14:12
@charliermarsh charliermarsh merged commit 17ceb5d into main Sep 25, 2023
@charliermarsh charliermarsh deleted the charlie/if-else branch September 25, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

formatter Related to the formatter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Formatter instability: trailing clause comments in if statement

3 participants