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

hug_parens_with_braces_and_square_brackets: Doesn't hug if all entries fit on a line #4099

Open
MichaReiser opened this issue Dec 11, 2023 · 3 comments
Labels
C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. T: style What do we want Blackened code to look like?

Comments

@MichaReiser
Copy link

MichaReiser commented Dec 11, 2023

Describe the style change

This issue is specific to the new hug_parens_with_braces_and_square_brackets preview style.

Examples in the current Black style

self._edits.append(
    {"operation": "update", "id": index, "fields": list(fields), "region_fields": []}
)

Desired style

self._edits.append({
    "operation": "update", "id": index, "fields": list(fields), "region_fields": []
})

The benefit of the desired style in my view are:

  • It minifies the diff when the length of the entires change because the formatting of the { remain unchanged:

    self._edits.append({
        "operation": "update", 
        "id": index, 
        "fields": list(fields), 
        "region_fields": [], 
        "loooger": "more",
    })

    or

    self._edits.append({ "operation": "update", "id": index, "fields": list(fields)})
  • It feels more consistent with the call expression formatting where the opening are formatted on separated lines (which is necessary to avoid invalid syntax):

    self._edits.append(
        "operation", "update", "id", index, "fields", list(fields), "region_fields", []
    )

Additional context

Originally posted in the stable 2024 style issue.

@MichaReiser MichaReiser added the T: style What do we want Blackened code to look like? label Dec 11, 2023
@JelleZijlstra JelleZijlstra added the C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. label Dec 11, 2023
@JelleZijlstra
Copy link
Collaborator

Just to confirm, your preferred style uses an 8-space indent, which is unusual and inconsistent with the rest of Black's style. Did you mean to use a 4-space indent, or are you proposing to indent further in that case?

Assuming we use 4-space indents, I'm not sure which style is better, but I'm definitely open to revisiting the style. I think I explicitly asked for the current style on one of the related PRs, because the style you're asking for seemed too disruptive.

You say (correctly) that your proposed style minimizes diffs better if entries are added to the dictionary. That's true, but your style is worse for diffs if a second argument is added to the function call. Maybe that's less common though?

@MichaReiser
Copy link
Author

Just to confirm, your preferred style uses an 8-space indent, which is unusual and inconsistent with the rest of Black's style. Did you mean to use a 4-space indent, or are you proposing to indent further in that case?

Sorry, no. It's just GitHub defaulting to tab. I need to check my settings if I can change this somewhere. The indentation should remain as is. It's only about the placement of the opening and closing curly

Assuming we use 4-space indents, I'm not sure which style is better, but I'm definitely open to revisiting the style. I think I explicitly asked for the current style on one of the related PRs, because the style you're asking for seemed too disruptive.

Thanks for the context. That makes sense from a transitioning perspective and I'm unfamiliar with how Black prioritises consistency vs disruption due to the new formatting.

You say (correctly) that your proposed style minimizes diffs better if entries are added to the dictionary. That's true, but your style is worse for diffs if a second argument is added to the function call. Maybe that's less common though?

That's true, but isn't it inherit to hug_parens_with_braces_and_square_brackets?

@hauntsaninja
Copy link
Collaborator

Yes, it's inherit to hug_parens_with_braces_and_square_brackets (and this is one of my worries about it). Note that Black will respect magic trailing commas here and not hug, so this is a way with current preview style to avoid diff noise

I think the proposed change here is probably more consistent? I think hug is already changes enough formatting that we should try to favour more consistent rules (possibly unless you think the primary benefit of hug is reducing vertical space)

JelleZijlstra added a commit to JelleZijlstra/black that referenced this issue Jan 31, 2024
Primarily because of psf#4036 (a crash) but also because of the feedback
in psf#4098 and psf#4099.
JelleZijlstra added a commit that referenced this issue Feb 2, 2024
…le (#4198)

Primarily because of #4036 (a crash) but also because of the feedback
in #4098 and #4099.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

3 participants