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

Future of string_processing #4208

Open
JelleZijlstra opened this issue Feb 5, 2024 · 2 comments
Open

Future of string_processing #4208

JelleZijlstra opened this issue Feb 5, 2024 · 2 comments
Labels
C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. T: bug Something isn't working

Comments

@JelleZijlstra
Copy link
Collaborator

We merged string processing in May 2020 (#1132), almost four years ago. A few months later, I put the feature behind a flag because there were too many related bugs (#1609). Four years later, we're still in basically the same state: most of the open issues in C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. are about string processing.

I'm also increasingly convinced that string_processing's core feature, splitting up long strings, would be exceedingly controversial if we ever did turn it on by default. It is just too hard for a tool to figure out how to split a string in a way that makes sense to readers of the code. However, there are other things that string_processing does, and some of those are valuable and could perhaps be split off.

So my proposal is:

  • Give up on splitting strings automatically; Black will simply not do that
  • Split out any other useful features from string-processing into separate preview features that we can reasonably hope to enable in next year's stable style.

If you're interested in getting any part of string_processing into stable Black, please contribute a PR moving it to a separate preview feature.

I ran string_processing on my work codebase and noticed a few specific changes that could be split out. I'll list them in a separate comment.

@JelleZijlstra JelleZijlstra added T: bug Something isn't working C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. labels Feb 5, 2024
@JelleZijlstra
Copy link
Collaborator Author

Here are some examples of changes that string-processing makes in my codebase that I'd consider accepting in a standalone preview feature:

  • Add parentheses around a string so it fits in the line limit
         xxxxxx = xx.xxxxxxx.XXXXXxxxxXxxXxfxxxxxx(
-            xxxxx_xxxxxxx_x3_xxxx_xxxxxxxx=f"x3://{XXX_X3_XXXXXX}/{XXXX_XXXXXXX_X3_XXXX_XXXXXXXX}",
+            xxxxx_xxxxxxx_x3_xxxx_xxxxxxxx=(
+                f"x3://{XXX_X3_XXXXXX}/{XXXX_XXXXXXX_X3_XXXX_XXXXXXXX}"
+            ),
             xxxxx_xxx=[
  • Merge implicitly concatenated strings if the result fits in the line limit
     xxx_xxxxxxx_xxxxxxxx = (
-        "XXX(XX(xx XXXXXXX '{xxxxx_xx}' XXX '{xxx_xx}', {xxx}, 0)) "
-        "XX {xxx}_x{xxxxxx}"
+        "XXX(XX(xx XXXXXXX '{xxxxx_xx}' XXX '{xxx_xx}', {xxx}, 0)) XX {xxx}_x{xxxxxx}"
     )
  • Combination of both, looks much nicer
     name_to_base_expression = {
-        "view": f"CASE WHEN (playtime >= {VIEW_THRESHOLD} "
-        "or has_unmute) THEN 1 ELSE 0 END",
+        "view": (
+            f"CASE WHEN (playtime >= {VIEW_THRESHOLD} or has_unmute) THEN 1 ELSE 0 END"
+        ),
     }
  • Remove f from parts of an implicitly concatenated f-string that don't have placeholders
             [
-                f"xxxxxx /xxxx/xxxxxxx/xxxx_xxxxxxx_xxxxx_xx.xx "
+                "xxxxxx /xxxx/xxxxxxx/xxxx_xxxxxxx_xxxxx_xx.xx "
                 f"--xxxxxxxx_xxxx {xxxxx_xxxxxxx_xxxxx} "
                 f"--xxxxxx_xxx {xxxxx_xxxxxx_xxxx} "
-                f"--xx_xxxx "
+                "--xx_xxxx "
                 f" {xxxxxx_xxxxxxx_xxxxx_xxx} "
                 f" {xx_xxx_fxxxx} "
             ]

@zanieb
Copy link

zanieb commented Feb 18, 2024

My 2-cents: I switched to Black's preview mode just for this feature. Otherwise, string processing is the vast majority of the time I spend manually formatting code. I would love to see this reach a place where it can be stabilized. I never had any problems with it, although I do understand there are many edge cases. I think splitting it into individual features makes a lot of sense so you can make incremental progress. I find splitting long strings really valuable, but understand it's a tough problem — we've been hesitant to tackle it in Ruff as well.

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: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants