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

Multiple accesses are turned multiline when the last argument of a call has a trailing comma #3582

Open
halworsen opened this issue Feb 25, 2023 · 2 comments
Labels
F: linebreak How should we split up lines? T: style What do we want Blackened code to look like?

Comments

@halworsen
Copy link

I'm a little unsure whether to categorize this as a bug or a code style issue, but anyways...

Describe the style change

This is so incredibly specific that it is best explained by example. See this example in the Black playground.

When chaining dict accesses to the result of a function call that uses a single kwarg with a trailing comma, Black will reformat one of the accesses to be multiline, which does not look good.

Examples in the current Black style

For the following code:

# The return of self.q(...) is a dict containing a dict
regions = self.q(Q_REGION_LIST.format(
    innerQuery='id',
))['worldData']['regions']

Black will produce the following code:

regions = self.q(
    Q_REGION_LIST.format(
        innerQuery="id",
    )
)[
    "worldData"
]["regions"]

Desired style

Black should either:

  1. Format the code as if there was no trailing comma after the single kwarg, or
  2. Produce similar code as when using multiple kwargs with trailing commas (or as when there is only 1 dict access)

For the following code:

regions = self.q(Q_REGION_LIST.format(
    innerQuery='id',
))['worldData']['regions']

In the case of 1., Black should produce the following formatted code:

regions = self.q(Q_REGION_LIST.format(innerQuery="id"))["worldData"]["regions"]

And for 2., it should produce the following formatted code:

regions = self.q(
    Q_REGION_LIST.format(
        innerQuery="id",
    )
)["worldData"]["regions"]

Additional context

  • Black version: 23.1.0
  • OS: Windows 10
  • Python version: 3.9.7

This is an out of the box, brand-spanking-new, fresh install of Black from pip.

@halworsen halworsen added the T: style What do we want Blackened code to look like? label Feb 25, 2023
@JelleZijlstra JelleZijlstra added the F: linebreak How should we split up lines? label Feb 25, 2023
@hauntsaninja
Copy link
Collaborator

(note that you can get behaviour 1 by passing --skip-magic-trailing-comma/-C )

@halworsen
Copy link
Author

Doesn't seem to be related to kwargs, but trailing commas on function args in general. Also happens on arbitrary accesses, not just dicts with string keys.

f(a,)[1][2.56]

# Becomes...
f(
    a,
)[
    1
][2.56]

Same with f(a, b,)[1][2.56]:

f(
    a,
    b,
)[
    1
][2.56]

or any amount of arguments as long as the last one has a trailing comma.

Interestingly, if you keep chaining accesses, you can get different patterns for how Black inserts linebreaks into the accesses. Particularly, an odd number of accesses inserts the linebreaks on the even numbered accesses. An even number of accesses inserts them on the odd numbered accesses:

f(a,)['c']['d']['e']

# Becomes...
f(
    a,
)["c"][
    "d"
]["e"]

f(a,)['c']['d']['e']['f']

# Becomes...
f(
    a,
)[
    "c"
]["d"][
    "e"
]["f"]

(note that you can get behaviour 1 by passing --skip-magic-trailing-comma/-C )

This seems to work pretty well, thanks! I find that Black will sometimes still break the accesses across multiple lines in my code, but that seems to be due to line length, so it's not strictly related to this issue even if it doesn't look great...

class WorldMixin:
    ...

    def get_all_expansions(self) -> list[FFLogsExpansion]:
        '''
        Retrieves a list of all expansions supported by FFLogs.

        Returns:
            A list of FFLogsExpansions representing each expansion.
        '''
        expacs = self.q(Q_EXPANSION_LIST.format(
            innerQuery='id',
        ))['worldData']['expansions']

        return [FFLogsExpansion(id=e['id'], client=self) for e in expacs]

# Becomes...
class WorldMixin:
    ...

    def get_all_expansions(self) -> list[FFLogsExpansion]:
        """
        Retrieves a list of all expansions supported by FFLogs.

        Returns:
            A list of FFLogsExpansions representing each expansion.
        """
        expacs = self.q(Q_EXPANSION_LIST.format(innerQuery="id"))["worldData"][
            "expansions"
        ]

        return [FFLogsExpansion(id=e["id"], client=self) for e in expacs]

@halworsen halworsen changed the title Multiple dict accesses are turned multiline when a single-kwarg function call has a trailing comma Multiple accesses are turned multiline when the last argument of a call has a trailing comma Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: linebreak How should we split up lines? T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

3 participants