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

Type hint wrapping after comment #3350

Open
henryiii opened this issue Oct 22, 2022 · 3 comments · May be fixed by #3362
Open

Type hint wrapping after comment #3350

henryiii opened this issue Oct 22, 2022 · 3 comments · May be fixed by #3362
Labels
F: linebreak How should we split up lines? T: bug Something isn't working

Comments

@henryiii
Copy link
Contributor

henryiii commented Oct 22, 2022 •
edited

Describe the style change

For some reason, black weirdly wraps a type hint only if it is after a comment.

Examples in the current Black style

def get_requires_for_build_sdist(
    config_settings: dict[str, str | list[str]] | None = None
) -> list[str]:
    return ["pathspec", "pyproject_metadata"]

def get_requires_for_build_sdist(
    # pylint: disable-next=unused-argument
    config_settings: dict[str, str | list[str]]
    | None = None
) -> list[str]:
    return ["pathspec", "pyproject_metadata"]

Desired style

def get_requires_for_build_sdist(
    config_settings: dict[str, str | list[str]] | None = None
) -> list[str]:
    return ["pathspec", "pyproject_metadata"]

def get_requires_for_build_sdist(
    # pylint: disable-next=unused-argument
    config_settings: dict[str, str | list[str]] | None = None
) -> list[str]:
    return ["pathspec", "pyproject_metadata"]

Additional context

Tested with both regular black and --preview, and with target version.
@henryiii henryiii added the T: style What do we want Blackened code to look like? label Oct 22, 2022
@felix-hilden felix-hilden added F: linebreak How should we split up lines? T: bug Something isn't working and removed T: style What do we want Blackened code to look like? labels Oct 22, 2022
@felix-hilden
Copy link
Collaborator

All for it, wouldn't consider this change controversial in any way as the line length is not exceeded!

@charlie572
Copy link

The function definition is split using left_hand_split, and this yields a line that looks like "# pylint: disable-next=unused-argumentconfig_settings: dict[str, str | list[str]] | None = None". Then, this line gets split using delimiter_split, standalone_comment_split, and right_hand_split, in that order.

Changing the order to put standalone_comment_split first fixes the issue, but causes some tests to fail because some comma-separated items get put on one line where they shouldn't.

@charlie572
Copy link

Am I correct in thinking that it should have split the line using standalone_comment_split instead of delimiter_split? I'm not familiar with how black works.

charlie572 added a commit to charlie572/black that referenced this issue Oct 28, 2022
@charlie572
Add comma split before delimiter split
08c9138 
This fixes psf#3350.
@charlie572 charlie572 linked a pull request Oct 28, 2022 that will close this issue
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
F: linebreak How should we split up lines? T: bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Format type hints after comments charlie572/black
3 participants
@henryiii @felix-hilden @charlie572