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

Don't use subscription brackets to break long lines? #951

Closed
yurkobb opened this issue Jul 30, 2019 · 7 comments
Closed

Don't use subscription brackets to break long lines? #951

yurkobb opened this issue Jul 30, 2019 · 7 comments
Labels
F: linebreak How should we split up lines? T: style What do we want Blackened code to look like?

Comments

@yurkobb
Copy link

yurkobb commented Jul 30, 2019

Consider this input:

class Test:
    def method(self):
        ctx = super().get_context_data(**kwargs)
        ctx["shipping_methods"] = list(
            ShippingMethod.objects.all().values("id", "labels__identifier")
        )
        ctx["shipping_method_fields"] = (
            MyCheckoutForm.Meta.shipping_required_fields
        )
        return ctx

And output:

class Test:
    def method(self):
        ctx = super().get_context_data(**kwargs)
        ctx["shipping_methods"] = list(
            ShippingMethod.objects.all().values("id", "labels__identifier")
        )
        ctx[
            "shipping_method_fields"
        ] = MyCheckoutForm.Meta.shipping_required_fields
        return ctx

I would (subjectively) expect the third assignment to stay the way it was.

Do "a[...]" often contain more than 1 element between the brackets? (Ignore this argument please; of course they do if it's something other than a string literal)


Operating system:
Python version: 3.7.3
Black version: 19.3b0
Does also happen on master: yes

@yurkobb yurkobb changed the title Don't use [] brackets to break long lines? Don't use subscription brackets to break long lines? Jul 30, 2019
@zsol zsol added the T: style What do we want Blackened code to look like? label Jul 30, 2019
@JelleZijlstra
Copy link
Collaborator

I agree that this looks pretty weird. If we can find a way to fix this issue without affecting other existing formatting too much, I'd be supportive of changing it.

@zsol
Copy link
Collaborator

zsol commented Jul 31, 2019

IMO the reason this looks weird is because the line break is on the LHS of an assignment statement and the indented line has a single, trivial expression.

(
    shipping_method_fields
) = MyCheckoutForm.Meta.shipping_required_fields

Looks the same kind of weird to me.

@alexanderGerbik
Copy link

alexanderGerbik commented Oct 8, 2019

Breaking lines on subscription brackets is quite confusing, it makes it look like list literal (I don't know the better way of formatting this though):

        return self.client.post(url_for("auth.obtain_token"), json=credentials).json[
            "token"
        ]

I think the same formatting can be observed in the following issues - #500 #1048

@yurkobb
Copy link
Author

yurkobb commented Oct 8, 2019

Maybe something like this?

        return (
            self.client.post(url_for("auth.obtain_token"), json=credentials)
            .json["token"]
        )

@remytms
Copy link

remytms commented Oct 8, 2019

I would even propose :

        return (
            self.client
            .post(url_for("auth.obtain_token"), json=credentials)
            .json["token"]
        )

Following the same rule as parameters of a function. If it fits on one line keep it on one line. If not, split in multiline. Here instead of splitting on commas, it splits on dots.

@ttilberg
Copy link

ttilberg commented Dec 2, 2020

I found this issue while trying to find community comments on code that I wondered "Is this formatting intentional? Is this a bug? Should I train myself to accept this?"

Here is the example I was reviewing:

# ...
                yield {
                    # ...
                    "isAvailable": result["stock"]["relativeInventoryForSets"][
                        "isAvailable"
                    ],
                    "appointmentLeadTimeDays": result["stock"][
                        "appointmentLeadTimeDays"
                    ],
                    "stockLevel": result["stock"]["stockLevel"],
                    "zip_code": response.meta["item"]["zip_code"],
                }

It gets weirder if there's even longer keys to dig into:

                {   # ...
                    "isAvailable": result["stock"]["relativeInventoryForSets"][
                        "isAvailable"
                    ]["isLong"]["isQuiteLong"],
                }

In this specific case, refactoring to a local stock, or moving some extraction code directly to an instance method could probably improve the situation, but I wanted to bring this example to the table for debate and potential refinement. I think it's reasonable to dig into JSON keys, and I think the way it gets formatted if the json keys are long is hard to interpret.

@JelleZijlstra JelleZijlstra added the F: linebreak How should we split up lines? label May 30, 2021
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

7 participants