Skip to content
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

handle unicode strings properly #167

Closed
wants to merge 2 commits into from
Closed

Conversation

jodok
Copy link

@jodok jodok commented Jun 1, 2018

@wiggin15
Copy link
Collaborator

This change is not compatible with Python 3 (see Travis build). Do you think you can fix it? Thanks

@Delgan
Copy link
Contributor

Delgan commented Jun 16, 2018

Hi @jodok

Are you sure the issue is from colorama side?

Encoding in Python is notoriously known to be hard.

I think the problem may come from environment or terminal.

I could not reproduce your issue, but as you linked to #36, I tried to print s = u'í' with and without calling colorama.init().
In both cases, the unicode í character was properly displayed.
Then I tried with your patch, and when printing í after colorama.init(), it was replaced by an empty string ''. 😕

I guess my Windows terminal is for some reason correctly configured to handle unicode, I would be disappointed not to be able to enjoy it with colorama.

@jodok
Copy link
Author

jodok commented Jun 17, 2018

my root cause is this issue: Azure/azure-cli#6408 - and there the issue only occurs if it is called in an non-interactive session (e.g. from jenkins).

@jodok
Copy link
Author

jodok commented Jul 20, 2018

@wiggin15 are you happy with my fixup?

@wiggin15
Copy link
Collaborator

Hi @jodok . The fixup for Python 3 looks good but I'm not sure about the solution. If I understand correctly, this change will skip and ignore non-ascii character, which may be a problem, for two reasons:

  • The underlying terminal may support non-ascii characters. In this case we can write them and don't need to ignore them.
  • This can cause strange output - e.g. when the user prints "Bokmål" (or, say, u'Bokm\xe5l') we will write "Bokml" which doesn't look right.

Perhaps we should consider trying to encode with self.stream.encoding instead of ascii?
(by the way, I am also unable to reproduce this reliably)

@tartley
Copy link
Owner

tartley commented Oct 13, 2020

Hey. FYI, yesterday I created a PR to test releases before we push them to PyPI. When that is merged, I'll be more confident about resuming merges and releases. I'll try to look at this PR soon. Thank you for creating it!

@tartley
Copy link
Owner

tartley commented Oct 7, 2021

Hey. Huge thanks for this idea. But I'm going to close this for now, because:

  • In order to be considered for merging, any changes needs to come with accompanying tests that demonstrate the problem that they fix.
  • wiggin's reservations here don't seem to have been addressed.

Please do re-open or re-submit if you think I'm making a mistake, or if you address the above problems. Many thanks!

@tartley tartley closed this Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants