-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
simplyfy ascii escaping by using backslashreplace error handling #2734
simplyfy ascii escaping by using backslashreplace error handling #2734
Conversation
bf7d525
to
78a027e
Compare
oh fun, it seems like this is broken due to bugs on older python3's :( |
@@ -146,13 +145,7 @@ def _ascii_escaped(val): | |||
|
|||
""" | |||
if isinstance(val, bytes): | |||
if val: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should fallback to the old code in Python 3.3 and Python 3.4. It will make the current code uglier of course, but at least when we drop 3.4 we will get the chance of getting the better version.
If we don't merge this I fear we end up just forgetting about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i will declare the a function sometime later to handle the detail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good
changelog/2734.trivial
Outdated
@@ -0,0 +1 @@ | |||
- simplify ascii string escaping by using the backslashreplace error handler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: please use full sentences:
Internal refactor: simplify ascii string escaping by using the backslashreplace error handler in newer Python 3 versions.
Thanks @RonnyPfannschmidt, took the liberty of applying the change I suggested to the CHANGELOG and merging, hope you don't mind. 👍 |
thanks for fixing what i missed 👍 |
No description provided.