8000 Fix unicode messages by Ameriks · Pull Request #47 · kfdm/gntp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Jul 10, 2021. It is now read-only.

Fix unicode messages #47

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix unicode messages #47

wants to merge 2 commits into from

Conversation

Ameriks
Copy link
@Ameriks Ameriks commented Jul 22, 2013

There was problem with unicode messages. That is fixed by verifying that incoming variable is basestring (not str) and in case it is not, then we convert variable to unicode (not to str).

There was problem with unicode messages. That is fixed by verifying that incoming variable is basestring (not str) and in case it is not, then we convert variable to unicode (not to str).
@kfdm
Copy link
Owner
kfdm commented Jul 22, 2013

Thanks for the catch. Which version of Python were you using ? Did you run the unit tests through Tox to test all the versions? I'll try to run through the tests myself sometime this week.

@Ameriks
Copy link
Author
Ameriks commented Jul 22, 2013

Sorry, I didn't run tests on all versions, but basestring was first introduced in version 2.3, but it is no longer available in Python 3.

I will update code to pass in all versions of python you have defined in Tox.

@kfdm
Copy link
Owner
kfdm commented Jul 22, 2013

I know the Six library does a few things to check basestring vs str

https://bitbucket.org/gutworth/six/src/e3da7fd96039a6ed89493f89d121c4f3797e6713/six.py?at=default#cl-35

Not sure how we would want to do this here without poking at it some myself. I wonder why it wasn't caught by my existing unicode test

https://github.com/kfdm/gntp/blob/master/gntp/test/basic_tests.py#L47

Do you have an example message that failed ?

@Ameriks
Copy link
Author
Ameriks commented Jul 22, 2013

I'm pretty sure that this will fail, but I was using different text: u"Šaursliežu dzelzceļš"

@Ameriks
Copy link
Author
Ameriks commented Jul 22, 2013

BTW, I didn't use six library, but instead try except. I didn't test it with Tox as I don't have it installed.

Let me know if there is anything else I can help.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0