black icon indicating copy to clipboard operation
black copied to clipboard

fix: bug where the doublestar operation had inconsistent formatting.

Open wannieman98 opened this issue 2 years ago • 6 comments

Description

Fixes #4149, which according to the ternary operation expression was not correctly being formatted. This was caused by the existing black's logic to determine whether the base of the exponential expression is simple or not, meaning whether it does not have parenthesis or brackets and such and just accepting naming or dotted lookups. Essentially, the existing logic was to loop over the direction (base backwards, exponential forward) and find whether a bracket or parenthesis existed. This logic worked for the exponential case since it doesn't need to care about chained expressions. This PR is to involve checking whether base is a chained expression and check the expression is simple.

Just to make sure, I added some more complex cases that would cause the bug other than the case mentioned in the issue.

Checklist - did you ...

  • [x] Add an entry in CHANGES.md if necessary?
  • [x] Add / update tests if necessary?
  • [x] Add new / update outdated documentation?

wannieman98 avatar Jan 16 '24 09:01 wannieman98

Hi! Seems like diff-shades check failed with "6 semaphore objects missing" user warning. From my experience this was a result of memory issue rather than the test actually failing. Could we try running this step again? Thanks!

wannieman98 avatar Jan 16 '24 19:01 wannieman98

diff-shades reports zero changes comparing this PR (d9d12793f540896abd9ac11c3729eefbf53ccb08) to main (7edb50f5a0afc56bb648dc14640ced144366b43a).


What is this? | Workflow run | diff-shades documentation

github-actions[bot] avatar Jan 17 '24 02:01 github-actions[bot]

Thanks! I haven't looked in detail yet, but this likely needs to go into the preview style only if it changes formatting for existing code. (diff-shades didn't find any, but that might just be because the patterns where this makes a difference are rare.)

JelleZijlstra avatar Jan 19 '24 00:01 JelleZijlstra

Review would be appreciated!

wannieman98 avatar Jan 25 '24 07:01 wannieman98

Thanks, I'm planning to look at this after the 24.1 release goes out (hopefully today/tomorrow). There's enough in that release already :)

JelleZijlstra avatar Jan 25 '24 19:01 JelleZijlstra

Hi @JelleZijlstra, I have addressed the comments, could you kindly review again? Thanks!

wannieman98 avatar Jan 31 '24 02:01 wannieman98

@JelleZijlstra Thanks for the review! Addressed the comments hope this works out :)

wannieman98 avatar Feb 05 '24 04:02 wannieman98