-
Notifications
You must be signed in to change notification settings - Fork 21
Adds option to override the theme's icons through monochrome icons and fixes string encoding issues #25
Conversation
It has been observerd that Zotero encodes its strings *sometimes* twice recursively. The modifications of this commit try to cope with that by decoding each string up to three times.
Hi, Thanks for your input. I'll take a look at it soon. The encoding fix looks pretty particularly interesting--that has defeated me for a long time! Cheers, |
Fixes the project version from 2.2.1 to 2.1.1 because it makes more sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for this. The encoding issue is not fixed yet, see my comment below. What do you think?
@@ -26,6 +26,33 @@ | |||
import time | |||
from libzotero.zotero_item import zoteroItem as zotero_item | |||
|
|||
def decode_zotero_str(raw, max_iters=3): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, this completely breaks the PDF attachments. This appears to be due to my own weird way of trying to fix the encoding. If I comment the line below out (from LibZotero.update()
), it works again on my system.
item_attachment = item_attachment.encode('latin-1').decode('utf-8')
However, the fact that a patch that for you seems to fix things, breaks things for me makes me a bit worried. On what operating system do you work? It's not unthinkable that the encoding differs between operating systems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm on Linux. I have that fork running on 3 different systems,
- two of which are Ubuntu 14.04 based Elementary OS Freya
- and one is Ubuntu 16.04 based Elementary OS Loki.
What operating system are you on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What operating system are you on?
I'm on Ubuntu 16.04.
It has been observerd that Zotero encodes its strings sometimes twice recursively. The modifications of this commit try to cope with that by decoding each string up to three times.
What makes you think this? Is this a conclusion based on the fact the decoding multiple times resolves the encoding issues, at least in some cases? Or is it a known bug in the Zotero code?
I'll think about this. The combination of my odd hack (clearly dating from before understood how encoding really works) and your fix is nonsensical; but it may be that your fix on its own is correct. But I'd like to do some more tests before I push another fix that actually doesn't fix things.
(As you may have noticed, I don't have that much time to work Qnotero anymore. But I'll get to it!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds very reasonable!
To answer your question: My fix was based on the observation that Zotero encoded 'Öf'
(a two-letters string, where the first letter is "Ö", that is a non-ASCII) as the five-bytes sequence \xc3\x83\xc2\x96\x66
. This is strange, because UTF-8 encodes ASCII letters as single bytes and non-ASCII characters as a sequence of two bytes. Empirically, I ruled out the possibility of that being a UTF-16 encoding. So why are two letters encoded as five bytes?
In [14]: '\xc3\x83\xc2\x96\x66'.decode('utf-8')
Out[14]: u'\xc3\x96f'
Turns out, that \xc3\x83\xc2\x96\x66
is the UTF-8 encoding for the three-characters-string \xC3\x96f
(note the f
at the end). The first two bytes \xC3\x96
are in turn the UTF-8 representation of Ö
. So I suspect that this is an odd behaviour from Zotero.
Update: In an earlier version of this, I miscounted the number of bytes in the exemplary sequence and wrote that it were 4, although it are 5.
@@ -41,7 +41,7 @@ def __init__(self, qnotero): | |||
self.setWindowProperties() | |||
self.setScrollBars() | |||
|
|||
def icon(self, iconName): | |||
def icon(self, iconName, overrideIconExt=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other themes should be updated as well. Right now this breaks the chameleon theme.
i use ubuntu16.04, like |
I would appreciate it very much if you could push these changes to Launchpad.
#1. Fixes #23
#2. Fixes #24
It has been observerd that Zotero encodes its strings sometimes twice recursively. The modifications of this commit try to cope with that by decoding each string up to three times.
Probably this also fixes #1?