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

Don't move type param opening parenthesis comments #8162

Closed
dhruvmanila opened this issue Oct 24, 2023 · 1 comment · Fixed by #8163
Closed

Don't move type param opening parenthesis comments #8162

dhruvmanila opened this issue Oct 24, 2023 · 1 comment · Fixed by #8163
Assignees
@dhruvmanila
Labels
bug Something isn't working formatter Related to the formatter
Milestone
Formatter: Beta

Comments

@dhruvmanila
Copy link
Member

Given:

type foo[  # comment
    a,
    b
] = ...

We format it as:

type foo[a, b] = ...  # comment

But, the list formatting is from:

foo = [  # comment 0
    a,
    b
]

To:

foo = [  # comment 0
    a,
    b,
]
@dhruvmanila dhruvmanila added the formatter Related to the formatter label Oct 24, 2023
@konstin
Copy link
Member

konstin commented Oct 24, 2023

I think the correct solution is to keep type param opening parentheses comment on the opening parenthesis

@konstin konstin changed the title Inconsistent formatting of dangling comments for type params Don't move type param opening parenthesis comments Oct 24, 2023
@konstin konstin added the bug Something isn't working label Oct 24, 2023
@dhruvmanila dhruvmanila self-assigned this Oct 24, 2023
@dhruvmanila dhruvmanila mentioned this issue Oct 24, 2023
Merged
dhruvmanila added a commit that referenced this issue Oct 24, 2023
@dhruvmanila
Don't move type param opening parenthesis comment (#8163)
2e81b9c 
## Summary

This PR fixes the issue to avoid collapsing the type param declaration
if
there's a comment after the opening parenthesis. For example,

```python
type foo[  # comment
    A,
    B
] = int
```

Here, we'll preserve the comment on the same line as is being done for
other
similar type of nodes.

## Test Plan

Add a new test case for it, update the snapshots, and validate the
ecosystem
check.

### Formatter ecosystem

#### `main`

| project | similarity index | total files | changed files |

|----------------|------------------:|------------------:|------------------:|
| cpython | 0.75803 | 1799 | 1647 |
| django | 0.99983 | 2772 | 34 |
| home-assistant | 0.99953 | 10596 | 186 |
| poetry | 0.99891 | 317 | 17 |
| transformers | 0.99966 | 2657 | 330 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99978 | 3669 | 20 |
| warehouse | 0.99977 | 654 | 13 |
| zulip | 0.99970 | 1459 | 22 |

#### `dhruv/type-params`

| project | similarity index | total files | changed files |

|----------------|------------------:|------------------:|------------------:|
| cpython | 0.75803 | 1799 | 1647 |
| django | 0.99983 | 2772 | 34 |
| home-assistant | 0.99953 | 10596 | 186 |
| poetry | 0.99891 | 317 | 17 |
| transformers | 0.99966 | 2657 | 330 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99978 | 3669 | 20 |
| warehouse | 0.99977 | 654 | 13 |
| zulip | 0.99970 | 1459 | 22 |

fixes: #8162
@MichaReiser MichaReiser added this to the Formatter: Beta milestone Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dhruvmanila dhruvmanila

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

Successfully merging a pull request may close this issue.

Don't move type param opening parenthesis comment astral-sh/ruff
3 participants
@MichaReiser @konstin @dhruvmanila