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

I001: Support for magic trailing commas #1200

Closed
Jackenmen opened this issue Dec 11, 2022 · 10 comments · Fixed by #1363
Closed

I001: Support for magic trailing commas #1200

Jackenmen opened this issue Dec 11, 2022 · 10 comments · Fixed by #1363
Labels
isort Related to import sorting

Comments

@Jackenmen
Copy link
Contributor

Black auto-explodes imports, collections, function calls, etc. when they contain a trailing comma:
https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#the-magic-trailing-comma

This means that even though this would fit into one line:

from seven_dwwarfs import (
    Grumpy,
    Happy,
    Sleepy,
    Bashful,
    Sneezy,
    Dopey,
    Doc,
)

Black will NOT convert it into this:

from seven_dwwarfs import Grumpy, Happy, Sleepy, Bashful, Sneezy, Dopey, Doc

As long as the import list contains a trailing comma.

Ruff currently does not have the magic trailing comma handling as far as I can tell and will collapse the import list if it can fit within the line length limit.

See relevant fixed (but not released) issue in isort: PyCQA/isort#1683

@charliermarsh charliermarsh added the isort Related to import sorting label Dec 11, 2022
@charliermarsh
Copy link
Member

Yeah this would be nice to support.

@colin99d
Copy link
Contributor

I am going to attempt this one.

@charliermarsh
Copy link
Member

Sweet. We'll probably need some logic to detect whether an import block ends in a trailing comma, then propagate that through as we normalize and combine the import blocks in the isort module, then thread through a flag to format_import_from to take the format_multi_line path if we have a trailing comma.

@jaxwagner
Copy link

Hi! I'm encountering some strange behavior where the formatter will:

  • add the comma if there is a single argument that takes up multiple lines
  • remove the comma if single arg on one line
  • add the comma if multiple args on multiple lines
    in particular, the first bullet seems strange to me. I have the following configs:
...
[lint.isort]
...
split-on-trailing-comma = false
...
[format]
skip-magic-trailing-comma = true

is this expected behavior? Thanks so much!

@charliermarsh
Copy link
Member

@jaxwagner - Are you able to include a Python snippet to demonstrate the behavior you're seeing, and what you find surprising? Happy to help if so :)

@jaxwagner
Copy link

jaxwagner commented Nov 29, 2023

def _func(
    input_values: Sequence[
        Union[CxxxxxxxxxxxxxxxxxxxxxxT, PxxxxxxxxxxxT, CxxxxxxxxxxxxT, BcccT]
    ],
) -> Type:

here's an example where the comma was added. this only happens when there is a single argument in the function that spans multiple lines. is this expected? thanks so much @charliermarsh ! :)

in this case, there was an existing comma that was taken away:

async def cxxxxxxxxxxxxxxxxxxxxxxxxx_js(
    txxxxxxxxxxxir: Vxxxxxxxxxxxxxxxxxxxxy  # used to be a comma at end of this line
) -> bool:

@charliermarsh
Copy link
Member

@jaxwagner - Ahh yes, that first example is intended. However, if you shorten the annotation, the formatter should re-collapse it given that you have skip-magic-trailing-comma = true set.

I can't reproduce that second example -- for me, the formatter is always adding a comma there (https://play.ruff.rs/b777c7bf-78e5-492d-aa3a-0869b930d565).

@jaxwagner
Copy link

here's an example where the comma is removed:
https://play.ruff.rs/8c27c3f5-1f36-48b5-b4d0-dac32a05ce02

i'm seeing a ton like this

@charliermarsh
Copy link
Member

Ooh interesting, thank you! It's the trailing comment that makes the difference: https://play.ruff.rs/0c1f8900-2880-402e-a68b-0537d73e7b61

I'll file a ticket to explore this (it may be working as intended but it deserves a second look).

@jaxwagner
Copy link

thanks @charliermarsh!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
isort Related to import sorting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants