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

[Feature] Implement getMetadata() function for DjVu documents #753

Merged
merged 1 commit into from
Nov 9, 2018

Conversation

xelxebar
Copy link
Contributor

@xelxebar xelxebar commented Nov 9, 2018

Overview

Pretty much what the title says. This implements the getMetadata() function for DjVu documents, following in the style of the same for the PDF implementation.

This just dumbly copies all the metadata key-value pairs into a lua table. The intent is to give the frontend everything we possibly can and let it figure out what to do with the info, an example of which can be found in the companion pull request koreader/koreader#4314.

Notes

This is my first foray into koreader's code, not to mention anything lua-related. I tried to copy the surrounding style as much as I could, and I tested this out on my actual kindle. However, I'd not be surprised if I'm missing something obvious. Anyway, I hope this turns out to be helpful.

@poire-z
Copy link
Contributor

poire-z commented Nov 9, 2018

Nice to see fixes on the base/libraries side :)

The gcc builds fail with:

djvu.c: In function 'getMetadata':
djvu.c:172:2: error: 'for' loop initial declarations are only allowed in C99 or C11 mode
  for (int i = 0; keys[i] != miniexp_nil; i++) {
  ^
djvu.c:172:2: note: use option -std=c99, -std=gnu99, -std=c11 or -std=gnu11 to compile your code

or just move int i; outside (https://stackoverflow.com/questions/24881/how-do-i-fix-for-loop-initial-declaration-used-outside-c99-mode-gcc-error)

@Frenzie
Copy link
Member

Frenzie commented Nov 9, 2018

We can just go for c99 I'd say. :-)

@xelxebar
Copy link
Contributor Author

xelxebar commented Nov 9, 2018

Oh. Huh. Didn't get that on my machine. Extracted the declaration.

// we have the choice of returning either `nil` or
// the empty table back to lua. Here we prefer the latter.
lua_newtable(L);
if (!keys) return 1;
Copy link
Contributor Author

@xelxebar xelxebar Nov 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was bothering me. The docs indicate that keys should never be null; however, looking at the djvulibre source malloc can fail to allocate our array---which is quite obvious in retrospect.

To that effect, I ammended the original PR with this extra null check. Since we have a choice on how to handle it and the ordering with lua_newtable is important, I went ahead and threw in a comment.

@Frenzie Frenzie merged commit b027efb into koreader:master Nov 9, 2018
@Frenzie
Copy link
Member

Frenzie commented Nov 9, 2018

Thanks, looks good!

@xelxebar xelxebar deleted the feature/djvu-props branch November 9, 2018 16:53
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.

None yet

3 participants