Skip to content
This repository was archived by the owner on Jul 10, 2021. It is now read-only.

Fix unicode messages#47

Open
Ameriks wants to merge 2 commits into
kfdm:masterfrom
Ameriks:master
Open

Fix unicode messages#47
Ameriks wants to merge 2 commits into
kfdm:masterfrom
Ameriks:master

Conversation

@Ameriks

@Ameriks Ameriks commented Jul 22, 2013

Copy link
Copy Markdown

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

kfdm commented Jul 22, 2013

Copy link
Copy Markdown
Owner

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

Ameriks commented Jul 22, 2013

Copy link
Copy Markdown
Author

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

kfdm commented Jul 22, 2013

Copy link
Copy Markdown
Owner

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

Ameriks commented Jul 22, 2013

Copy link
Copy Markdown
Author

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

@Ameriks

Ameriks commented Jul 22, 2013

Copy link
Copy Markdown
Author

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