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

Formatter: Pragma comments #6197

Closed
3 tasks done
Tracked by #4798 ...
MichaReiser opened this issue Jul 31, 2023 · 10 comments
Closed
3 tasks done
Tracked by #4798 ...

Formatter: Pragma comments #6197

MichaReiser opened this issue Jul 31, 2023 · 10 comments
Assignees
Labels
formatter Related to the formatter

Comments

@MichaReiser
Copy link
Member

MichaReiser commented Jul 31, 2023

@MichaReiser MichaReiser added the formatter Related to the formatter label Jul 31, 2023
@MichaReiser MichaReiser added this to the Formatter: Alpha milestone Jul 31, 2023
@MichaReiser MichaReiser self-assigned this Jul 31, 2023
@davidszotten
Copy link
Contributor

do you have any examples of this happening? (how did you find them)

@MichaReiser
Copy link
Member Author

MichaReiser commented Aug 4, 2023

do you have any examples of this happening? (how did you find them)

I haven't looked closely into it but there's some related Black logic:

https://github.com/psf/black/blob/2fd9d8b339e1e2e1b93956c6d68b2b358b3fc29d/src/black/lines.py#L227-L294

and

https://github.com/psf/black/blob/268dcb677ce80331e0ef104d8335c2ada32872fb/src/black/comments.py#L324-L335

I don't know if this is a valid noqa placement, but splitting by the arguments could change what it annotates

def test(a, b, # noqa: RUFXXX
	c, d): pass

# Formatted
def test(
	a, 
	b, # noqa: RUFXXX
	c, 
	d
): pass

This could cause RUFXXX to trigger now if it suppressed a or test and not b. Playground

@roshanjrajan-zip
Copy link

roshanjrajan-zip commented Aug 6, 2023

Hi! This is one of the problems we have in our repo. However, we use # pyright: ignore instead of # type: ignore. It would be great to have this as dynamic or something to support different use cases. Please lmk if there is anything I can do to provide more info or any help - psf/black#3661

@MichaReiser
Copy link
Member Author

@charliermarsh
Copy link
Member

My one-second reaction is that I think we probably do a reasonably good job of avoiding the uncollapsible comment case since we don't really collapse comments (comments always expand the parent), but I don't think we handle the unsplittable line case at all.

@MichaReiser
Copy link
Member Author

@roshanjrajan-zip The example you brought up on the Black issue is

some_long_variable_name = some_long_dict_name[0] # pyright: ignore[reportGeneralTypeIssues]

How would you preferred formatting look like?

Would the following work for you?

some_long_variable_name = ( # pyright: ignore[reportGeneralTypeIssues]
	some_long_dict_name[0] 
)
# Or
some_long_variable_name = some_long_dict_name[  # pyright: ignore[reportGeneralTypeIssues]
	0
] 

It would still require manual intervention (you have to move the comment to the opening parentheses), but it would allow you to suppress the issue.

@MichaReiser
Copy link
Member Author

@MichaReiser
Copy link
Member Author

See #6670

@roshanjrajan-zip
Copy link

roshanjrajan-zip commented Aug 19, 2023

Hi @MichaReiser,

Definitely the first one is much better imo and my team agrees! Thanks so much for your work on the proposal. Taking a look at it!

some_long_variable_name = ( # pyright: ignore[reportGeneralTypeIssues]
	some_long_dict_name[0] 
)

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

No branches or pull requests

4 participants