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

[3889] Fix long lines with power operator(s) getting splitted before line length #3942

Merged
merged 9 commits into from
Oct 16, 2023

Conversation

henriholopainen
Copy link
Contributor

@henriholopainen henriholopainen commented Oct 13, 2023

Description

Hugging power operators had a bug, where operators that end up hugging were still counted as not hugging when deciding to split the line. The bug is described here. The whole issue is a bit circular as we need to know the line length before we start to apply transformations. Calculating the hugging of power operators at the beginning of transform_line seems to do the trick. Ideally the hug_power_op transform would be done only once, but switching the order of transforms broke a lot of other places, so I don't know if there's any other good options here.

Checklist - did you ...

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

@github-actions
Copy link

github-actions bot commented Oct 13, 2023

diff-shades results comparing this PR (ba2cc11) to main (6f84f65). The full diff is available in the logs under the "Generate HTML diff report" step.

╭─────────────────────── Summary ────────────────────────╮
│ 3 projects & 10 files changed / 56 changes [+14/-42]   │
│                                                        │
│ ... out of 2 497 808 lines, 11 746 files & 23 projects │
╰────────────────────────────────────────────────────────╯

Differences found.

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

@henriholopainen
Copy link
Contributor Author

diff-shades results comparing this PR (43fcdee) to main (935f303). The full diff is available in the logs under the "Generate HTML diff report" step.

╭─────────────────────── Summary ────────────────────────╮
│ 3 projects & 10 files changed / 56 changes [+14/-42]   │
│                                                        │
│ ... out of 2 496 979 lines, 11 745 files & 23 projects │
╰────────────────────────────────────────────────────────╯

Differences found.

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

These all seem to be cases of erroneous line splitting getting fixed.

@henriholopainen henriholopainen changed the title Issue 3889 [3889] Fix long lines with power operator(s) getting splitted before line length Oct 13, 2023
Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. This looks correct, but we'll have to make the change in the preview style only for now.

@henriholopainen
Copy link
Contributor Author

Thanks. This looks correct, but we'll have to make the change in the preview style only for now.

Ok, will do. Does this mean that the tests should also be reverted, as they will fail after the fix is behind --preview?

@JelleZijlstra
Copy link
Collaborator

You can add # flags: --preview to the top of the test so they get run in preview mode.

@JelleZijlstra JelleZijlstra merged commit 1648ac5 into psf:main Oct 16, 2023
37 of 41 checks passed
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 this pull request may close these issues.

2 participants