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

Merged
merged 3 commits into from Sep 6, 2017

Conversation

Projects
None yet
3 participants
@RonnyPfannschmidt
Member

RonnyPfannschmidt commented Aug 30, 2017

No description provided.

@RonnyPfannschmidt RonnyPfannschmidt requested a review from nicoddemus Aug 30, 2017

@RonnyPfannschmidt RonnyPfannschmidt changed the title from [wip] simplyfy ascii escaping by using backslashreplace error handling to simplyfy ascii escaping by using backslashreplace error handling Aug 30, 2017

@coveralls

This comment has been minimized.

coveralls commented Aug 30, 2017

Coverage Status

Coverage decreased (-0.004%) to 92.111% when pulling 78a027e on RonnyPfannschmidt:simplify-string-safening into 488bbd2 on pytest-dev:features.

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Aug 30, 2017

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:

This comment has been minimized.

@nicoddemus

nicoddemus Aug 31, 2017

Member

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.

This comment has been minimized.

@RonnyPfannschmidt

RonnyPfannschmidt Aug 31, 2017

Member

i will declare the a function sometime later to handle the detail

This comment has been minimized.

@nicoddemus

nicoddemus Aug 31, 2017

Member

Sounds good

@@ -0,0 +1 @@
- simplify ascii string escaping by using the backslashreplace error handler

This comment has been minimized.

@nicoddemus

nicoddemus Aug 31, 2017

Member

Nit: please use full sentences:

Internal refactor: simplify ascii string escaping by using the backslashreplace error handler in newer Python 3 versions.

@RonnyPfannschmidt RonnyPfannschmidt self-assigned this Aug 31, 2017

@coveralls

This comment has been minimized.

coveralls commented Sep 4, 2017

Coverage Status

Coverage decreased (-0.04%) to 92.071% when pulling 13eac94 on RonnyPfannschmidt:simplify-string-safening into 488bbd2 on pytest-dev:features.

@nicoddemus

This comment has been minimized.

Member

nicoddemus commented Sep 5, 2017

Thanks @RonnyPfannschmidt, took the liberty of applying the change I suggested to the CHANGELOG and merging, hope you don't mind. 👍

@coveralls

This comment has been minimized.

coveralls commented Sep 6, 2017

Coverage Status

Coverage decreased (-0.06%) to 92.054% when pulling 3d70727 on RonnyPfannschmidt:simplify-string-safening into 488bbd2 on pytest-dev:features.

@nicoddemus nicoddemus merged commit e1f2254 into pytest-dev:features Sep 6, 2017

1 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Sep 6, 2017

thanks for fixing what i missed 👍

@RonnyPfannschmidt RonnyPfannschmidt deleted the RonnyPfannschmidt:simplify-string-safening branch Nov 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment