Skip to content

Param ids fix #1092

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

Merged
merged 5 commits into from
Oct 1, 2015
Merged

Param ids fix #1092

merged 5 commits into from
Oct 1, 2015

Conversation

nicoddemus
Copy link
Member

Fix #1087
Fix #1085

@nicoddemus nicoddemus added the type: bug problem that needs to be addressed label Sep 29, 2015
@nicoddemus nicoddemus added this to the 2.8.2 milestone Sep 29, 2015
@@ -1064,7 +1068,7 @@ def _escape_bytes(val):
is a full ascii string, otherwise escape it into its binary form.
"""
try:
return val.encode('ascii')
return val.decode('ascii')
except UnicodeDecodeError:
return val.encode('string-escape')
Copy link
Member

Choose a reason for hiding this comment

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

im not 100% sure off-hand is string-escape correctly used that way?

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems so:

Python 2.7.10 (default, May 23 2015, 09:40:32) [MSC v.1500 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> u'ação'
u'a\xe7\xe3o'
>>> u'ação'.encode('utf-8')
'a\xc3\xa7\xc3\xa3o'
>>> s=_
>>> s.encode('string-escape')
'a\\xc3\\xa7\\xc3\\xa3o'

return encoded_bytes.decode('ascii')
if val:
# source: http://goo.gl/bGsnwC
import codecs
Copy link
Contributor

Choose a reason for hiding this comment

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

i guess it makes sense to move "import codecs" outside the function (within the py3 if section)

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@nicoddemus
Copy link
Member Author

Anything else guys?

@RonnyPfannschmidt
Copy link
Member

FML, now it has conflicts,
The travisci failure was a hickup of travis

@nicoddemus
Copy link
Member Author

Fixed merge conflict. 😅

@RonnyPfannschmidt
Copy link
Member

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug problem that needs to be addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants