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

Formatter --preview add trailing comma #9029

Open
hoxbro opened this issue Dec 6, 2023 · 7 comments
Open

Formatter --preview add trailing comma #9029

hoxbro opened this issue Dec 6, 2023 · 7 comments
Labels
formatter Related to the formatter preview Related to preview mode features style How should formatted code look

Comments

@hoxbro
Copy link
Contributor

hoxbro commented Dec 6, 2023

I have the following code:

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

Which I run with ruff format --preview and gets

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

I would expect it to behave like this based on the new rule hug_parens_with_braces_and_square_brackets

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

Playground example: https://play.ruff.rs/6d3bf559-edf0-47f9-b1fd-1fc3418d7a46

@charliermarsh
Copy link
Member

Black seems to leave this code unmodified in preview.

@charliermarsh charliermarsh added bug Something isn't working formatter Related to the formatter labels Dec 6, 2023
@MichaReiser
Copy link
Member

MichaReiser commented Dec 7, 2023

Black seems to leave this code unmodified in preview.

Not entirely. Black hugs the parentheses, but formats all entries on a single line

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

The fix here should be simple, similar to what we do in FormatArgs: Wrap the content (the entries) in a group. This gives us a two-staged formatting: Try to fit the entire dictionary on a line, try to fit the entries on a single line but break the {, }, and last, split each entry on a single line

--- a/crates/ruff_python_formatter/src/expression/expr_dict.rs	(revision Staged)
+++ b/crates/ruff_python_formatter/src/expression/expr_dict.rs	(date 1701914528224)
@@ -60,7 +60,7 @@
             joiner.finish()
         });
 
-        parenthesized("{", &format_pairs, "}")
+        parenthesized("{", &group(&format_pairs), "}")
             .with_dangling_comments(open_parenthesis_comments)
             .fmt(f)
     }

@MichaReiser
Copy link
Member

It seems they changed this between Stable and Main. Stable gives you the formatting I shared. Main leaves it as is. I don't like the change. Makes it rather magic and doesn't save on vertical space.

@MichaReiser MichaReiser added this to the Formatter: Stable milestone Dec 22, 2023
@MichaReiser
Copy link
Member

MichaReiser commented Feb 12, 2024

Note: The pattern where the "huggable" fully fits on its own line seems somewhat common in Airflow's codebase apache/airflow#37355

@MichaReiser
Copy link
Member

Related black issue psf/black#4099

There's the option to format this as:

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

or

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

The former probably requires the use of best_fitting![expression.format(), block_indent(&expression.format()), expression.format()] where it should be sufficient to group the pairs for the second one.

@MichaReiser
Copy link
Member

I remove this from the stable formatter milestone because we decided not to ship the hugging preview style

@MichaReiser MichaReiser removed this from the Formatter: Stable milestone Feb 14, 2024
@MichaReiser MichaReiser added preview Related to preview mode features style How should formatted code look and removed bug Something isn't working labels Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter preview Related to preview mode features style How should formatted code look
Projects
None yet
Development

No branches or pull requests

3 participants