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.

Sign up for GitHub

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

Jump to bottom

Refine edge-case comment handling for PatternMatchClass #6880

Closed
charliermarsh opened this issue Aug 25, 2023 · 0 comments · Fixed by #6881
Closed

Refine edge-case comment handling for PatternMatchClass #6880

charliermarsh opened this issue Aug 25, 2023 · 0 comments · Fixed by #6881
Assignees
@charliermarsh
Labels
bug Something isn't working formatter Related to the formatter
Milestone
Formatter: Alpha

Comments

@charliermarsh
Copy link
Member

In this case, comments a and b are leading on 2, but should be leading on a node for the entire b=2 (this requires an AST change):

    case A(
        b # b
        = # c
        2 # d
        # e
    ):
        pass

In this case, the comment should be an open parenthesis comment on the arguments, but is currently a leading comment on x due to our generic parenthesized comment handling (which thinks x is parenthesized, rather than in a call) (this requires an AST change):

    case Point2D(  # end of line line
            x
            ):
        ...
@charliermarsh charliermarsh self-assigned this Aug 25, 2023
@charliermarsh charliermarsh added bug Something isn't working formatter Related to the formatter labels Aug 25, 2023
This was referenced Aug 25, 2023
Merged
Merged
charliermarsh added a commit that referenced this issue Aug 26, 2023
@charliermarsh
Introduce AST nodes for PatternMatchClass arguments (#6881)
15b73bd 
## Summary

This PR introduces two new AST nodes to improve the representation of
`PatternMatchClass`. As a reminder, `PatternMatchClass` looks like this:

```python
case Point2D(0, 0, x=1, y=2):
  ...
```

Historically, this was represented as a vector of patterns (for the `0,
0` portion) and parallel vectors of keyword names (for `x` and `y`) and
values (for `1` and `2`). This introduces a bunch of challenges for the
formatter, but importantly, it's also really different from how we
represent similar nodes, like arguments (`func(0, 0, x=1, y=2)`) or
parameters (`def func(x, y)`).

So, firstly, we now use a single node (`PatternArguments`) for the
entire parenthesized region, making it much more consistent with our
other nodes. So, above, `PatternArguments` would be `(0, 0, x=1, y=2)`.

Secondly, we now have a `PatternKeyword` node for `x=1` and `y=2`. This
is much more similar to the how `Keyword` is represented within
`Arguments` for call expressions.

Closes #6866.

Closes #6880.
@MichaReiser MichaReiser added this to the Formatter: Alpha milestone Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@charliermarsh charliermarsh

Labels
bug Something isn't working formatter Related to the formatter
Projects
None yet
Milestone
Formatter: Alpha
Development

Successfully merging a pull request may close this issue.

Introduce AST nodes for PatternMatchClass arguments astral-sh/ruff
2 participants
@MichaReiser @charliermarsh