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

Fix episode title notifications when there are unicode characters in it. #2078

Closed
wants to merge 1 commit into from

Conversation

p0psicles
Copy link
Contributor

@p0psicles p0psicles commented Feb 1, 2017

  • PR is based on the DEVELOP branch
  • Don't send big changes all at once. Split up big PRs into multiple smaller PRs that are easier to manage and review
  • Read the contribution guide

This is reproducable by (manual) searching (and getting a result) for an episode which has unicode chars in the title.

Like for ex. Dragon ball super S04E13.
Protect Kaiō-shin Gowasu - Destroy Zamasu!

I tested this on Windows 7 x64.
Maybe it's not even an issue on linux like OS'.

image

@p0psicles p0psicles added the Bug label Feb 1, 2017
@p0psicles
Copy link
Contributor Author

Need to perform some tests on different os. To see if this is safe.

@OmgImAlexis
Copy link
Collaborator

Let me know what needs to happen and I can test on Windows 10 and OSX.

@fernandog
Copy link
Contributor

fernandog commented Feb 6, 2017

Confirm it fixed:

My concerns:

  1. if we should do in the above log to with "We have found season packs for {0}".format(self.show.name)
  2. If we should add this in the ui.py to apply to all messages: self._messages.append(Notification(title, message, MESSAGE))

ping @ratoaq2

UPDATE:
i tried changing in ui.py:
self._messages.append(Notification(title.encode('utf-8'), message.encode('utf-8'), MESSAGE))

that but same error:
UnicodeEncodeError: 'charmap' codec can't encode character u'\u014d' in position 40: character maps to <undefined>

Error without the PR:

2017-02-06 13:18:48 DEBUG    SEARCHQUEUE-MANUAL-295068 :: [51ff22a] Traceback (most recent call last):
  File "C:/Users/Fernando/Documents/Medusa\medusa\search\queue.py", line 345, in run
    ui.notifications.message("We have found results for {0}".format(self.segment[0].pretty_name()),
  File "C:\Python27\lib\encodings\cp1252.py", line 12, in encode
    return codecs.charmap_encode(input,errors,encoding_table)
UnicodeEncodeError: 'charmap' codec can't encode character u'\u014d' in position 40: character maps to <undefined>

After:
after

@fernandog fernandog requested a review from ratoaq2 February 11, 2017 11:06
@ratoaq2
Copy link
Contributor

ratoaq2 commented Feb 11, 2017

The same as in #2096
I dont think that's the right place to encode and decode. I haven't check the source for those strings, but the encode/decode should be done quite earlier in the flow.

@p0psicles
Copy link
Contributor Author

This is different then the read from file imo. We should come with a better solution for this.

@p0psicles
Copy link
Contributor Author

I was thinking about utilizing the str magic method on the series and episode objects. But then we should remove the current str mess. As I don't think it's really used anywhere.
Then we can by default encode the name using encode('utf-8').

I still have to work out some details though.

@p0psicles p0psicles closed this Feb 17, 2017
@fernandog fernandog deleted the feature/fix-pnotify-unicode branch February 21, 2017 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants