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] Support DjVu metadata #4314

Merged
merged 1 commit into from
Nov 9, 2018

Conversation

xelxebar
Copy link
Contributor

@xelxebar xelxebar commented Nov 9, 2018

Overview

As of f71627c, DjVu support for metadata is faked by setting the document tite to the file's basename. This is fixes that. It is the frontend companion to koreader/koreader-base#753, as such the continuous integration will inevitably fail until base integrates those changes.

If there's a better protocol for this kind of split pull request, please let me know.

Notes

This is my first foray into koreader's codebase as well as lua programming, so please forgive any boneheadeness. I tried to model the DjvuDocument:getProps() method after its namesake in frontend/document/pdfdocument.lua.

The silliness with capitalized keys versus lowercase, as mentioned in the code comment, comes from the covention mentioned in djvused(1). I considered lower()ing all props keys into a new table, but I figured that, with just six keys, it's best to be explicit about the identifications.

Anyway, if I've made obvious mistakes or there is a better way to do things, please let me know. I hope this turns out to be useful.

@Frenzie
Copy link
Member

Frenzie commented Nov 9, 2018

Awesome, I've been meaning to look into this but never got around to it! I'll have some time to review it tonight.

Frenzie pushed a commit to koreader/koreader-base that referenced this pull request Nov 9, 2018
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.
@avsej
Copy link
Contributor

avsej commented Nov 9, 2018

Is there something left to do in this PR? This is really cool feature to have. And BTW how often nightly builds are triggered?

@Frenzie
Copy link
Member

Frenzie commented Nov 9, 2018

Yes, base still needs to be bumped. So it'll actually work. :-)

Nightlies are triggered every day around 6 CET.

@xelxebar
Copy link
Contributor Author

xelxebar commented Nov 9, 2018

@Frenzie Thanks for merging base. This PR should be all working now!

@poire-z
Copy link
Contributor

poire-z commented Nov 9, 2018

Yes, base still needs to be bumped.

It has been (a few minutes after you merged base PR).
This PR looks good to be merged to me.

@Frenzie Frenzie merged commit 05e687f into koreader:master Nov 9, 2018
@xelxebar xelxebar deleted the feature/djvu-props branch November 9, 2018 18:15
@xelxebar
Copy link
Contributor Author

xelxebar commented Nov 9, 2018

You guys work quick! Thanks!

@Frenzie
Copy link
Member

Frenzie commented Nov 9, 2018

Thanks for caring about DjVu! ;-)

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

4 participants