Skip to content
This repository was archived by the owner on Jul 27, 2023. It is now read-only.

Add Decorator node - #7

Merged
MichaReiser merged 1 commit into
mainfrom
decorator-node
Jun 8, 2023
Merged

Add Decorator node#7
MichaReiser merged 1 commit into
mainfrom
decorator-node

Conversation

@MichaReiser

@MichaReiser MichaReiser commented Jun 6, 2023

Copy link
Copy Markdown
Member

This PR introduces a new Decorator node. The node wraps an expression and its range includes the @.

Having a dedicated node for Decorators simplifies the implementation of the Formatter when handling leading comments:

@deco1
# leading 2
@deco2(with_args=True)
# leading 3
@deco3

# leading 4 that already has an empty line
def decorated_with_split_leading_comments():
    pass

The leading comment should be attached to the whole decorator and not just the expression coming after the @. It's also necessary that we distinguish between comments attached to the decorator vs comments attached to the decorator's expression:

@(
    # leading 3
    deco3
)

This is straightforward when the decorator and the expression are different nodes.

This change has the added benefit that the Linter now gets the full range of the decorator. For example, we have fixes that replace remove the whole decorator but they incorrectly assume that its safe to subtract 1 from the start (offset of the @). However, this doesn't account for:

@         deco2(with_args=True)
def decorated1():
    pass
    

which is valid python

@MichaReiser

MichaReiser commented Jun 6, 2023

Copy link
Copy Markdown
Member Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser
MichaReiser changed the base branch from named-expr-range to include-parentheses-in-arguments-range June 6, 2023 12:39
@MichaReiser
MichaReiser marked this pull request as ready for review June 6, 2023 12:46
@MichaReiser
MichaReiser requested a review from charliermarsh June 7, 2023 13:48

@charliermarsh charliermarsh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great change.

Base automatically changed from include-parentheses-in-arguments-range to main June 7, 2023 20:26
@MichaReiser
MichaReiser merged commit 0dc8fdf into main Jun 8, 2023
@MichaReiser
MichaReiser deleted the decorator-node branch June 8, 2023 05:44
charliermarsh pushed a commit that referenced this pull request Jun 19, 2023
zanieb added a commit that referenced this pull request Jul 10, 2023
zanieb added a commit that referenced this pull request Jul 17, 2023
zanieb pushed a commit that referenced this pull request Jul 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants