Skip to content

Attempt to fix carriage return issues #4674

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MeGaGiGaGon
Copy link
Collaborator

Description

This is my attempt at fixing the carriage return problems. In combination with tusharsadhwani/pytokens#1 , this should fix the hypothesis errors around \r.

I'm making both PRs drafts since my changes were made with the consideration of a sledgehammer, and were just focused on getting the issue to go away, so I'm uncertain if I've accidentally broken other things. All the black tests pass, as well as a local hypothesis run gave no problems, but I might have messed something up.

pytokens changes

My general change is based around the fact that I don't understand how you could fix issue_128233 without making \rs behave properly as newlines, so that's what this PR does. If issue_128233_handling is enabled, \rs are treated as newlines instead of whitespace.

With this change, I came across issues where a lone \r newline would cause a double advance to happen, so I made is_newline return an int with the length of chars that should be removed. This keeps the old behavior intact, since 0 is still falsey and 1/2 are truthy, but also allows the methods that depend on \r\n handling to work correctly through checking if the result is 2.

black changes

While working on the other hypothesis issues, I noticed that \rs weren't accounted for properly in the driver, so this adds those checks, leaving the \n ones first to hopefully leave \r\n behavior unchanged.

I also have no clue how to handle the cross-repository things that would make the tests show as passing, since they don't have the full effects without the other repo's changes.

One other note, I wonder if it would be easier to side-step all these problems completely and normalize the newlines early like how CPython does, probably in src/black/__init__.py:format_str

Checklist - did you ...

  • Add an entry in CHANGES.md if necessary?
  • Add / update tests if necessary?
    • Still unsure how to do the return type sensitive tests
  • Add new / update outdated documentation?
    • Documentation not affected

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.

1 participant