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

Setting the 2024 stable style #4042

Closed
hauntsaninja opened this issue Nov 13, 2023 · 33 comments · Fixed by #4106
Closed

Setting the 2024 stable style #4042

hauntsaninja opened this issue Nov 13, 2023 · 33 comments · Fixed by #4106

Comments

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Nov 13, 2023

See issue for 2023 here: #3407

Here's what this issue is:

Here's the current call to action:

  • Any comments / opinions / vibes on these features is very welcome
  • Please test your codebase with --preview and let us know your thoughts!
  • If you have a specific problem with a preview feature, especially something that feels like an implementation issue, it's best to open a separate issue (that we can then link to from here)

For many of the changes, diff-shades logs have expired. We should find a way to keep these around for longer!

It's currently November, so there's still time to make more uncontroversial changes and hopefuly iron out some issues in the not uncontroversial ones.

Not uncontroversial

string_processing

This one is controversial, vast majority of preview style issues are related to string processing
Open issues I believe are caused by string_processing: #2334, #2314, #3466, #3497, #3409, #3424, #3428, #3210, #3410, #3747, #3665, #3663, #2553, #3855, #3802

hug_parens_with_braces_and_square_brackets (#3964, #3992)

Causes O(10000) changes, although counting lines changed is unfavourable to this change.
A very substantial change in style for Black. We can't revert once it's in stable.
Outstanding crash: #4036

parenthesize_conditional_expressions (#2278)

Causes O(1000) changes. Need to look at diff
Has unfortunate interactions with comments: #3555

wrap_long_dict_values_in_parens (#3440)

Causes O(100) changes. Need to look at diff
Outstanding crash: #3758
Report of worse splitting: #3452
Unnecessary nesting: #4129
Removes parentheses: #4158

multiline_string_handling (#1879)

Causes O(1000) changes. Need to look at diff
Maybe has some synergy with hug_parens_with_braces_and_square_brackets.
Unfortunate white space preservation: #4159

dummy_implementations (#3796)

I can't find up-to-date diff-shades, but maybe O(1000) changes
Requires a change in flake8 config: #3887
Can result in an extra line, but no change expected to be made: #3872

prefer_splitting_right_hand_side_of_assignments (#3368)

Causes O(100) changes.

hex_codes_in_unicode_sequences (#2916)

Causes O(100) changes
Open issue whether lowercase or uppercase is better #3568

allow_empty_first_line_before_new_block_or_comment (#3967)

Causes O(1) changes.
This change itself is fine, but it's related to a controversial and churn causing change in 2023 style. I think we should probably revert that 2023 change; I posted a longer issue about this over here #4043

parenthesize_long_type_hints (#3899)

Causes O(100) changes

module_docstring_newlines (#3932)

Causes O(100) changes

Uncontroversial

respect_magic_trailing_comma_in_return_type (#3916)

O(10) changes.
Shouldn't affect existing formatting too much, unless there's a buggy trailing comma
All changes in diff-shades are incidental, but are good and what you'd expect Black to do

no_blank_line_before_class_docstring (#3692)

O(100) changes. Makes consistent with function behaviour
Blank lines have been a little controversial, but I think this should be totally fine

wrap_multiple_context_managers_in_parens (#3489)

O(100) changes. Naively, looks great to me

fix_power_op_line_length (#3942)

O(10) changes. A bug fix

add_trailing_comma_consistently (#3393)

O(1) changes. A bug fix

skip_magic_trailing_comma_in_subscript (#3209)

O(1) changes. A bug fix, affects a non-default option

blank_line_after_nested_stub_class (#3564)

Only affects stubs, adds some blank lines in an uncommon case

blank_line_between_nested_and_def_stub_file (#3862)

Only affects stubs, adds some blank lines in even less common case

accept_raw_docstrings (#3947)

O(1) changes. Just a bug fix

long_case_block_line_splitting (#4024)

0 changes. Seems strictly good

walrus_subscript (#3823)

0 changes. Straightforward style change

improved_async_statements_handling (#3609)

0 changes. More consistent

single_line_format_skip_with_multiple_comments (#3959)

0 changes. Makes life better

@hauntsaninja
Copy link
Collaborator Author

hauntsaninja commented Nov 13, 2023

My vibes:

  • I feel very comfortable with shipping dummy_implementations and lower
  • I think parenthesize_conditional_expressions / wrap_long_dict_values_in_parens / multiline_string_handling are also all good, but I want to look over diff and address the existing issues
  • I feel we shouldn't ship string_processing in its current state
  • I'm fairly wary of hug_parens_with_braces_and_square_brackets. It's a big change to Black's style! I'm very interested in hearing from folks on this one; if we don't feel like we get much feedback on it we should strongly consider keeping in --preview
  • I'll evaluate some more on large work codebase and post any findings

@AlexWaygood
Copy link
Contributor

My ¢2 as a Black user:

  1. I generally agree with @hauntsaninja's vibes/analysis overall!

  2. I ran Black with --preview on a few projects, and in those projects, I'm sad to say that I dislike most of the string-formatting-related changes the preview style make.

  3. I really, really like hug_parens_with_braces_and_square_brackets and hope it gets in! One of my biggest gripes with black is how annoying its formatting of multiline frozensets etc. has been in the past, and this addresses that. The one thing I'd say is that it surprised me that the preview style does this:

    -    return AnnotationStats(
    -        *[random.randint(0, 1000) for _ in AnnotationStats.__annotations__]
    -    )
    +    return AnnotationStats(*[
    +        random.randint(0, 1000) for _ in AnnotationStats.__annotations__
    +    ])

    I prefer the non-preview style there -- I believe this is basically the same thing @JelleZijlstra's is saying in Fix #4011: Handle more huggable immediately nested parens/brackets. #4012 (comment)

@JelleZijlstra
Copy link
Collaborator

Thanks, this is great!

We should try to make a 24.1a1 soon that implements a draft stable style, so we can get more feedback.

I haven't thought too hard about each change, but a few thoughts:

  • It might be time to pull the plug on string_processing, since it's been years and the feature is still buggy. Possibly we can keep a more limited version, e.g. only merging adjacent string literals that are on the same line.
  • I like the hug_parens change personally but I appreciate it's going to be controversial. Especially since it went into the preview style pretty late in the year, it might be better to hold off on it for now and gather more feedback.

@yilei
Copy link
Contributor

yilei commented Nov 14, 2023

On string_processing:

We have been using it internally (Google) since the beginning of this year and it has been working well for us. So I'm motivated to keep this and help graduate it to the stable style (too late for the 2024 stable style though).

Possibly we can keep a more limited version, e.g. only merging adjacent string literals that are on the same line.

This is a good strategy, I think we can split string_processing into a few separate style changes. Moving all of them at once is hard to reason about and is too disruptive.

@tungol
Copy link
Contributor

tungol commented Nov 15, 2023

I tested preview on a code base of mine, and it mostly looked good. The only change I was really surprised by was from string processing; the same issue as #3663 and #3210 . A little testing showed that it's from the string_paren_strip transformer. I like the idea of that part of string_processing, but that's a pretty ugly downside. It seems like it's stripping the parentheses and then re-inserting them at a worse place, which is consistent behavior with non-preview style if I manually remove the parentheses first. Not strictly a problem with string_paren_strip, I guess, but not great.

For hug_parens, I like it in theory but it does feel less consistent than the current behavior, at least in some cases. I'm not totally sure about it. I can understand why you'd compare it and multiline_string_handling, but I don't see any issues with the latter - that one looks great to me.

I think that parenthesize_conditional_expressions is a big improvement for conditionals in the original context of function arguments, but I don't think I like what it's doing in other contexts.

@JelleZijlstra
Copy link
Collaborator

I started putting together a PR to prepare an alpha for the 2024 stable style, and here's a few thoughts:

  • hex_codes_in_unicode_sequences: I think there are good reasons to prefer lowercase. I would propose to keep this out of the 2024 stable style and make the 2024 preview style use lowercase.
  • allow_form_feeds was just added (Permit standalone form feed characters at the module level #4021) as another preview feature. It should be low-impact, so I would favor adding it to the new stable style.
  • I wouldn't be comfortable shipping parenthesize_conditional_expressions and wrap_long_dict_values_in_parens with the open issues about them. Nevertheless, I'll include them initially so we can gather more feedback. It's better if we put something in the alpha and then walk it back, instead of adding a new style change after the alpha.
  • dummy_implementation is surprisingly disruptive in our test suite, but that's atypical code.
  • The test suite found a new crash with parenthesize_long_type_hints: Crash with preview style + line length 1 #4062
  • Reported suboptimal formatting with dummy_implementation: Preview style: Don't collapse dummy implementation with comment #4063

@DanielNoord
Copy link
Contributor

Is there a chance of module_docstring_newlines making it in? As far as I can see there has only been support for the feature since it was first requested in 2020 (#1872) and I haven't seen issues about it being opened since it was added to preview.

If it means anything it was also quickly picked up by ruff in astral-sh/ruff#7995 without any discussion about it being potentially controversial.

It seems to me to be controversial as it would create a lot of diffs across projects, but doesn't change styling in any way that users would say "hey, what's black doing there". I would really like to see this change make it in to standardize these newlines across the projects I work on (and which don't want to run with preview style).

@JelleZijlstra
Copy link
Collaborator

@DanielNoord yes, that one is expected to go in (unless either lots of people speak up and say they don't like it, or we discover bugs caused by it).

@MichaReiser
Copy link

MichaReiser commented Nov 28, 2023

I'm still writing it up but we track Ruff's preview style compatibility in astral-sh/ruff#8678
Ruff's stable style already includes some of the mentioned uncontroversial preview styles.

Regarding multiline_string_handling. Ruff stable already formats

MULTILINE = """
foobar
""".replace("\n", "")

and we've generally received positive feedback. I haven't yet come around to implement the special casing inside call expressions. So I can't say how people feel about that.

I personally like hug_parens_with_braces_and_square_brackets that that's probably because I'm biased from Prettier ;)

Ruff formats long dict values slightly different from black (wrap_long_dict_values_in_parens). Parenthesizing the values would align the behavior. Although it sets a new precedence of removing parentheses from sub-expression, something Black so far very explicitly avoids (in comparison to Prettier that normalises all parentheses). There's a similar issue around slice indexes that may benefit from parenthesizing as well astral-sh/ruff#7316

I'll share more feedback when I had time to take a closer look at the changes and thanks for drafting this up, it helps to understand how Black's styleguide will change.

@takanakahiko
Copy link

Release notes states the following regarding #3445 :

Please try out the preview style with black --preview and tell us your feedback. All changes in the preview style are expected to become part of Black’s stable style in January 2024.

Isn't this a topic for this discussion?

@JelleZijlstra
Copy link
Collaborator

@takanakahiko I'm not sure exactly what you're asking. This issue is definitely the right place to discuss what should go into the 2024 stable style. The default is that everything in the preview style will move to stable, unless there's some reason not to. As discussed above, string processing (which is what #3445 affects) probably will not go into the 2024 stable style because there are many open bugs around it.

@takanakahiko
Copy link

takanakahiko commented Nov 28, 2023

@JelleZijlstra I may have had a problem with my English, sorry.

I thought all Preview styles were listed because it said the following.

I've listed all the features we have enabled under --preview

However, I was concerned that my interest #3445 was not listed, so I asked my question.

Thanks for the answer.
I understand that it will not be introduced in the stable style because it is related to the string processing issue.

@jakkdl
Copy link
Contributor

jakkdl commented Dec 6, 2023

I started putting together a PR to prepare an alpha for the 2024 stable style, and here's a few thoughts:

* The test suite found a new crash with `parenthesize_long_type_hints`: [Crash with preview style + line length 1 #4062](https://github.com/psf/black/issues/4062)

Fixed the crash, feel free to tag me if any other issues with either of the type-hint features I added cause any other problems :)

@Fatal1ty
Copy link

Fatal1ty commented Dec 7, 2023

Here is my feedback. TLDR: I like the changes with --preview flag more than without it.

I was testing black 23.11.0 with --preview on https://github.com/Fatal1ty/mashumaro

I like this idea with Ellipsis on the same line (there is no longer a need to exclude it from coverage)

-    def decode(self, data: Any) -> T:  # pragma: no cover
-        ...
+    def decode(self, data: Any) -> T: ...

It found an unnecessary string concatenation:

         raise ValueError(
-            f"Time zone {s} must be either UTC " f"or in format UTC[+-]hh:mm"
+            f"Time zone {s} must be either UTC or in format UTC[+-]hh:mm"
         )

This became more readable:

         schema = JSONObjectSchema(
-            additionalProperties=_get_schema_or_none(
-                instance.derive(type=args[1]), ctx
-            )
-            if args
-            else None,
-            propertyNames=_get_schema_or_none(
-                instance.derive(type=args[0]), ctx
-            )
-            if args
-            else None,
+            additionalProperties=(
+                _get_schema_or_none(instance.derive(type=args[1]), ctx)
+                if args
+                else None
+            ),
+            propertyNames=(
+                _get_schema_or_none(instance.derive(type=args[0]), ctx)
+                if args
+                else None
+            ),
         )

Special thanks for this change, I like it a lot:

     serialize_by_alias: Union[
-        bool, Literal[Sentinel.MISSING]
-    ] = Sentinel.MISSING
-    namedtuple_as_dict: Union[
-        bool, Literal[Sentinel.MISSING]
+        bool, Literal[Sentinel.MISSING], float, List[str]
     ] = Sentinel.MISSING
+    namedtuple_as_dict: Union[bool, Literal[Sentinel.MISSING]] = (
+        Sentinel.MISSING
+    )

What interesting is that ruff turns all this changes back, so all the credits go to black!

@MichaReiser
Copy link

Some feedback to the most recent changes to hug_parens_with_braces_and_square_brackets after having implemented some of them in Ruff. I'm not sure if this is the right place to discuss these. Let me know if not and I'll move my comments.

It seems that Black now recursively collapses (hugs) parentheses. We believe that this makes the code harder to read. Like here, it becomes difficult to tell which parentheses belong together

                                     )
                                 )
                                 if commented_entity:
-                                    entity_map = CommentedMap(
-                                        [(entity["entity"], entity["value"])]
-                                    )
+                                    entity_map = CommentedMap([(
+                                        entity["entity"],
+                                        entity["value"],
+                                    )])
                                     entity_map.yaml_add_eol_comment(
                                         commented_entity, entity["entity"]
                                     )
                                     entities.append(entity_map)
                                 else:
                                     entities.append(
-                                        OrderedDict(
-                                            [(entity["entity"], entity["value"])]
-                                        )
+                                        OrderedDict([(
+                                            entity["entity"],
+                                            entity["value"],
+                                        )])
                                     )
                     else:
                         entities.append(

But maybe that's just us not being used to that style.

We were surprised to learn that Black now leaves the following snipped as is

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

and doesn't format it to

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

We prefer the second style because it leads to fewer changes when the length of the dictionary changes:

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

The parentheses and the first entry remain unchanged. Making it easier to spot the differences

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

Again, the opening parentheses remains unchanged.

It also feels more consistent with function calls where the parentheses (for good reasons) are kept on the first line:

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

What interesting is that ruff turns all this changes back, so all the credits go to black!

Ruff doesn't implement Black's preview style yet. But yes, all credit for Ruff's style guide goes to Black, because Ruff follows Black's style guide.

@JelleZijlstra
Copy link
Collaborator

Replying to a few recent posts (including my own):

hex_codes_in_unicode_sequences: I think there are good reasons to prefer lowercase. I would propose to keep this out of the 2024 stable style and make the 2024 preview style use lowercase.

I misread something here, we already use lowercase. We should keep this in the 2024 style.

@Fatal1ty

I like this idea with Ellipsis on the same line (there is no longer a need to exclude it from coverage)

This actually looks concerning to me, because Black shouldn't go around removing comments. I can't reproduce this behavior, though.

@MichaReiser thanks for the feedback! I think we'll exclude the hug_parens changes from the stable style this time because they came in late of the year and are likely to be controversial. Could you perhaps make a new issue?

@jakkdl
Copy link
Contributor

jakkdl commented Dec 10, 2023

Replying to a few recent posts (including my own):

@Fatal1ty

I like this idea with Ellipsis on the same line (there is no longer a need to exclude it from coverage)

This actually looks concerning to me, because Black shouldn't go around removing comments. I can't reproduce this behavior, though.

I'm pretty sure they meant that they removed the comment themselves.

@Fatal1ty
Copy link

Replying to a few recent posts (including my own):
@Fatal1ty

I like this idea with Ellipsis on the same line (there is no longer a need to exclude it from coverage)

This actually looks concerning to me, because Black shouldn't go around removing comments. I can't reproduce this behavior, though.

I'm pretty sure they meant that they removed the comment themselves.

Yes, you're right. Black didn't remove the comment, I did.

@MichaReiser
Copy link

I plan to create PRs for larger projects using Ruff's formatter and ask them for feedback on the preview style once I've implemented all preview styles (some of them are tricky to get right!). It will probably be January until I find the time but I'll share the feedback here if you're interested (and/or maybe that's something you want to do as well)?

@JelleZijlstra
Copy link
Collaborator

Thanks, that's great! I won't have much time until January. I'm planning to put out an alpha release today or tomorrow with the new style, then finish things up in January.

@yuhrichard
Copy link

yuhrichard commented Dec 12, 2023

Hello, we’re engineers from Lyft and want to provide some initial feedback from testing out the new features from the alpha (https://github.com/psf/black/releases/tag/24.1a1):

Bulk of the observed changes have been parenthesizing conditional expressions, splitting RHS of assignments, wrapping long dict values in parentheses, and docstring formatting which have looked good overall and are big improvements in readability. From the changelog, it shows that parenthesizing conditionals may or may not be included, but so far, we definitely liked this feature overall.

We found this case where black added parentheses around a dictionary (after topLevelBase and secondaryBase), causing some extra nesting that seemed unnecessary, although it’s still overall readable (playground link):

Wanted to confirm if this formatting is due to wrap_long_dict_value_in_parens?

# In
class Random:
    def func():
        random_service.status.active_states.inactive = (
            make_new_top_level_state_from_dict(
                {
                    "topLevelBase": {
                        "secondaryBase": {
                            "timestamp": 1234,
                            "latitude": 1,
                            "longitude": 2,
                            "actionTimestamp": Timestamp(
                                seconds=1530584000, nanos=0
                            ).ToJsonString(),
                        }
                    },
                }
            )
        )

# Out: Black formatted
class Random:
    def func():
        random_service.status.active_states.inactive = (
            make_new_top_level_state_from_dict({
                "topLevelBase": (
                    {
                        "secondaryBase": (
                            {
                                "timestamp": 1234,
                                "latitude": 1,
                                "longitude": 2,
                                "actionTimestamp": (
                                    Timestamp(
                                        seconds=1530584000, nanos=0
                                    ).ToJsonString()
                                ),
                            }
                        )
                    }
                ),
            })
        )

Also, is there a sense when to expect the major release/2024 stable style (ie) mid or end of Jan)? We’ll continue to do some more testing and provide additional feedback towards the end of the year. Thanks for all of the development work!

@cgohlke
Copy link

cgohlke commented Dec 17, 2023

In this case the trailing/inline comment is moved to the wrong line:

a = "".join(
    (
        "",  # comment
        "" if True else "",
    )
)

becomes

a = "".join((
    "",
    "" if True else "",  # comment
))

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Dec 18, 2023
## 23.12.0

### Highlights

It's almost 2024, which means it's time for a new edition of _Black_'s stable style!
Together with this release, we'll put out an alpha release 24.1a1 showcasing the draft
2024 stable style, which we'll finalize in the January release. Please try it out and
[share your feedback](psf/black#4042).

This release (23.12.0) will still produce the 2023 style. Most but not all of the
changes in `--preview` mode will be in the 2024 stable style.

### Stable style

- Fix bug where `# fmt: off` automatically dedents when used with the `--line-ranges`
  option, even when it is not within the specified line range. (#4084)
- Fix feature detection for parenthesized context managers (#4104)

### Preview style

- Prefer more equal signs before a break when splitting chained assignments (#4010)
- Standalone form feed characters at the module level are no longer removed (#4021)
- Additional cases of immediately nested tuples, lists, and dictionaries are now
  indented less (#4012)
- Allow empty lines at the beginning of all blocks, except immediately before a
  docstring (#4060)
- Fix crash in preview mode when using a short `--line-length` (#4086)
- Keep suites consisting of only an ellipsis on their own lines if they are not
  functions or class definitions (#4066) (#4103)

### Configuration

- `--line-ranges` now skips _Black_'s internal stability check in `--safe` mode. This
  avoids a crash on rare inputs that have many unformatted same-content lines. (#4034)

### Packaging

- Upgrade to mypy 1.7.1 (#4049) (#4069)
- Faster compiled wheels are now available for CPython 3.12 (#4070)

### Integrations

- Enable 3.12 CI (#4035)
- Build docker images in parallel (#4054)
- Build docker images with 3.12 (#4055)
@hauntsaninja
Copy link
Collaborator Author

@yuhrichard
Thanks for the feedback! Yes, the behaviour you're seeing is from wrap_long_dict_value_in_parens. I've filed #4129 for your issue. I'm not sure when exactly the release would happen, but I'd definitely guess second half of January or later

@cgohlke
Thanks for the report! I think this is the same issue as #3555 , relates to the parenthesize_conditional_expressions style

@AlexeyDmitriev
Copy link

AlexeyDmitriev commented Dec 28, 2023

I really like hug_parens_with_braces_and_square_brackets, hope it gets merged in 2024 so that we don't have to wait a year.
It really made code look better in all cases I saw.

As for multiline string processing, I am not a big fan, I find implicit concatenation of string literals somewhat confusing and would prefer single (but long) literal.

One case where I find it actively worse then it were:

Original:

assert x == "looooooooooooooooooooooooooooooo o o o oo o o oo oo o o oo o oo o oo ooo o o oo o oo  o oo o oo  o o o o o o oooo oo o oo ng text"

Old black: (clunky but ok)

assert (
    x
    == "looooooooooooooooooooooooooooooo o o o oo o o oo oo o o oo o oo o oo ooo o o oo o oo  o oo o oo  o o o o o o oooo oo o oo ng text"
)

New black (with preview): == is so unnoticable here

assert (
    x
    == "looooooooooooooooooooooooooooooo o o o oo o o oo oo o o oo o oo o oo ooo o o oo"
    " o oo  o oo o oo  o o o o o o oooo oo o oo ng text"
)

Ideal result (with new hugging style) considering we need to have splitting

assert x == (
    "looooooooooooooooooooooooooooooo o o o oo o o oo oo o o oo o oo o oo ooo o o oo"
    " o oo  o oo o oo  o o o o o o oooo oo o oo ng text"
)

Probably good enough is this:

assert (
    x == (
        "looooooooooooooooooooooooooooooo o o o oo o o oo oo o o oo o oo o oo ooo o o oo"
        " o oo  o oo o oo  o o o o o o oooo oo o oo ng text"
    )
)

Also, I think multiline string should almost always be in parens (except in cases where it's already in cases when they are already in parens from the function call. There are cases when it's not this way (in asserts as above), in list comprehensions:

original:

lst = [
    "looooooooooooooooooooooooooooooo o o o oo o o oo oo o o oo o oo o oo ooo o o oo o oo  o oo o oo  o o o o o o oooo oo o oo ng text"
    for line in range(5)
]

black with preview

 lst = [
    "looooooooooooooooooooooooooooooo o o o oo o o oo oo o o oo o oo o oo ooo o o oo o"
    " oo  o oo o oo  o o o o o o oooo oo o oo ng text"
    for line in range(5)
]

@phlogistonjohn
Copy link

I hope this is the correct place to comment on this. I "accidentally" ran the alpha version on my project and I think the new dummy_implementations rule made the following change(s) (slightly simplified to highlight a certain type of change):

# Before
class Parser(typing.Protocol):
    def set_defaults(self, **kwargs: typing.Any) -> None:
        ...  # pragma: no cover
    
# After
class Parser(typing.Protocol):
    def set_defaults(
        self, **kwargs: typing.Any
    ) -> None: ...  # pragma: no cover
    

This doesn't seem to me to be much of an improvement in readability. I think I understand the intent of the rule to move the elipsis onto the same line as the def but doing so at the expense of then having to split the definition across multiple lines seems much less pleasant than just leaving it as-is IMO. Thanks for listening!

@JelleZijlstra
Copy link
Collaborator

@phlogistonjohn that's issue #3872. As I wrote on the issue, I think it's still a net positive on balance, but there's definitely some risk, especially in cases where there is a comment after the ....

@JelleZijlstra
Copy link
Collaborator

@AlexeyDmitriev string splitting won't be in the 2024 stable style.

(And for everyone commenting, thanks for your feedback!)

@AlexeyDmitriev
Copy link

@JelleZijlstra thanks for your feedback.
What is the expected time when black 2024.* is published?

@AlexeyDmitriev
Copy link

@JelleZijlstra I was looking at #4106, Do I understand correctly that parens hugging will not make it either?

@JelleZijlstra
Copy link
Collaborator

We'll publish it by the end of the month and the hugging changes will not be included.

@amyreese
Copy link
Contributor

amyreese commented Jan 13, 2024

Sorry if these pieces have already been covered, but I ran the preview style (from 24.1a1) over a small portion of Meta's codebase and have some feedback. Most of the style changes seem like a positive change, but I think the changes to collapsing/compacting elements and especially multiline strings is a net-negative.

First some changes I'm happy to see:

# before
for (a, b) in things:
    ...

# after
for a, b in things:
    ...
# before
value: Optional[
    int
] = None  # some really long comment

# after
value: Optional[int] = (
    None  # some really long comment
)

Love it!

# before
func(
    arg1,
    "long string"
    if condition
    else "other string",
)

# after
func(
    arg1,
    (
        "long string"
        if condition
        else "other string"
    ),
)

Much clearer, exactly how I would have rewritten it myself! 💜


Now, some examples where I think the changes result in less-readable code, particularly where black takes aligned triple quotes and then unaligns them:

# before 
Thing(
    """
    value
    """
)

# after
Thing("""
    value
    """)

or worse:

# before
VALUE = [
    Thing(
        """
        value
        """
    )
]

# after
VALUE = [Thing("""
    value
    """)]
# before
func(
    (
        value
        + """
        something
        """
    ),
    argument,
)

# after
func(
    (value + """
        something
        """),
    argument,
)

Not pretty, and not really any more readable than before? ¯\(ツ)

Is that related to the "paren hugging" you mentioned, or is this still scheduled for the 2024 style?

@amyreese
Copy link
Contributor

Is that related to the "paren hugging" you mentioned

Maybe not, because I just found this change as well:

# before
def foo():
    result = (
        """
        something
        """
        + value
    )

# after
def foo():
    result = """
        something
        """ + value

Distinctly less readable IMO.

@JelleZijlstra
Copy link
Collaborator

Update:

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

Successfully merging a pull request may close this issue.

16 participants