Skip to content

Use bytes or %r to handle paths and process output #766

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

Closed
wants to merge 3 commits into from

Conversation

theotherjimmy
Copy link
Contributor

Paths are not ASCII, paths are not Unicode. Paths are bytes.
This PR handles paths like they are bytes, well at least the
current working directory. Handling paths and process output as
if they are unicode will introduce errors when paths are not
valid unicode. I tested this by creating a directory named

�abc\uXXXX

Which is not valid unicode, and tricks most python string
decoders into throwing an exception.

Fixes #710, maybe

Copy link
Contributor

@screamerbg screamerbg left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks @theotherjimmy. You might want @toyowata to check this with his Japanese dev environment before merging.

@theotherjimmy
Copy link
Contributor Author

I should get it working with python3 as well.

@theotherjimmy
Copy link
Contributor Author

theotherjimmy commented Oct 3, 2018

This is very involved. Paths and other bytes being treated interchangeably with strings is everywhere in this code base.

@theotherjimmy
Copy link
Contributor Author

Closing in favor of #767

@screamerbg screamerbg closed this Oct 23, 2018
@theotherjimmy theotherjimmy deleted the non-unicode-cwd branch October 23, 2018 14:32
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