This repository has been archived by the owner on Sep 16, 2019. It is now read-only.
-
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
Closed
Closed
Adds option to override the theme's icons through monochrome icons and fixes string encoding issues #25
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.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,
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.
I'm on Ubuntu 16.04.
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?Turns out, that
\xc3\x83\xc2\x96\x66
is the UTF-8 encoding for the three-characters-string\xC3\x96f
(note thef
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.