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

Suggestions: show annotation in FM on long tap #5956

Closed
mergen3107 opened this issue Mar 14, 2020 · 26 comments · Fixed by #5980
Closed

Suggestions: show annotation in FM on long tap #5956

mergen3107 opened this issue Mar 14, 2020 · 26 comments · Fixed by #5980
Milestone

Comments

@mergen3107
Copy link
Contributor

  • KOReader version: from new release
  • Device: Kindle PW3 (5.9.7)

It would be nice if there was an option to read a book's annotation (for epub, fb2 at least) via File Manager's long tap on a book. It would make it easier to choose a book without opening and rendering it.

What do you think guys?

@mergen3107 mergen3107 changed the title Suggestions: show annotation in FM on lang tap Suggestions: show annotation in FM on long tap Mar 15, 2020
@poire-z
Copy link
Contributor

poire-z commented Mar 16, 2020

It would make it easier to choose a book without opening

What do you call "a book's annotation"? Bookmarks/highlights you made, or the book description/metadata?

@mergen3107
Copy link
Contributor Author

The book’s short description. In fb2 books it is usually enclosed in tag somewhere in the beginning of the XML.

@poire-z
Copy link
Contributor

poire-z commented Mar 16, 2020

For EPUB, you have it: long-press > Book information - or if you have any of the CoverBrowser mode: long-press > View book description directly (and among its option, you can set it to show an indicator, a small rectangle poping out on the right of the cover, for book that have a description).
(Dunno if it's extracted for FB2 - but it definitely is for EPUBs)

@hius07
Copy link
Member

hius07 commented Mar 16, 2020

Unfortunately, for fb2 we have very short information.
Please see comparison with CoolReader here
https://www.mobileread.com/forums/showpost.php?p=3959834&postcount=1058

@hius07
Copy link
Member

hius07 commented Mar 16, 2020

'Annotation' field is shown in the beginning of the book. Other fields (such as 'translator') are hidden by fb2.fss default settings.
I have a User style tweak (translator: { display: block; }) to show them after annotation (it is not very nice).

@poire-z
Copy link
Contributor

poire-z commented Mar 16, 2020

That's what we fetch and fill for EPUB: https://github.com/koreader/crengine/blob/c1700e3798d2563123b303485f3bcb340017f66b/crengine/src/epubfmt.cpp#L782-L827

That's what we fetch and fill for FB2 (and other formats that are parsed and made to look like a FB2 XML tree):
https://github.com/koreader/crengine/blob/c1700e3798d2563123b303485f3bcb340017f66b/crengine/src/lvdocview.cpp#L4628-L4635
with the help of https://github.com/koreader/crengine/blob/c1700e3798d2563123b303485f3bcb340017f66b/crengine/src/lvtinydom.cpp#L8798-L8857

So, we have the 2 slots we added for EPUBs (and didn't bother filling for other formats): "keywords" and "description" - that we could fill from FB2 fields, possibly multiple if we concatenate them.

If you don't write C and can't tackle it yourself, I'm gonna need you to do the thinking :) and give me some pseudocode about what to parse, and what to put in which field, and in which order, and if they can be zero, 1 or more occurence, like:
DESCRIPTION = /FictionBook/description/title-info/annotation (only one) + "\n" + /FictionBook/description/title-info/date (possibly multiple)

@Frenzie
Copy link
Member

Frenzie commented Mar 16, 2020

Also I think references to the relevant spec are always helpful. ;-)

@Frenzie
Copy link
Member

Frenzie commented Mar 16, 2020

I meant a reference to the specific relevant section the way I did, not "the spec" as some abstract entity. ;-) (Although that too can be useful.)

In this case, presumably http://www.fictionbook.org/index.php/%D0%AD%D0%BB%D0%B5%D0%BC%D0%B5%D0%BD%D1%82_annotation

So that someone reading doesn't have to waste time trying to find that, even if it doesn't take long.

@hius07
Copy link
Member

hius07 commented Mar 16, 2020

I am not sure my writings are useful, anyway, I would be happy to see the following information from fb2 description section as Book information:

Book info

 Title = /FictionBook/description/title-info/book-title
 Author = /FictionBook/description/title-info/author/first-name +
          /FictionBook/description/title-info/author/middle-name +
          /FictionBook/description/title-info/author/last-name
 Date = /FictionBook/description/title-info/date
 Series = /FictionBook/description/title-info/sequence:name
 Series number = /FictionBook/description/title-info/sequence:number
 Genres = /FictionBook/description/title-info/genre
 Translator = /FictionBook/description/title-info/translator/first-name +
              /FictionBook/description/title-info/translator/middle-name +
              /FictionBook/description/title-info/translator/last-name

Publication info

 Publication name = /FictionBook/description/publish-info/book-name
 Publisher = /FictionBook/description/publish-info/publisher
 Publisher city = /FictionBook/description/publish-info/city
 Publication year = /FictionBook/description/publish-info/year
 ISBN = /FictionBook/description/publish-info/isbn
 
Document info

 Document author = /FictionBook/description/document-info/author/first-name +
                   /FictionBook/description/document-info/author/middle-name +
                   /FictionBook/description/document-info/author/last-name
 Document date = /FictionBook/description/document-info/date

@hius07
Copy link
Member

hius07 commented Mar 16, 2020

Regarding /FictionBook/description/title-info/annotation.
Annotation is long (sometimes a full page). It is displayed in the book and is not needed to be in the Book information.
The topic starter would like to see annotation in the File Manager, so, if the same module is used to display Book information while reading and from FM, annotation would need a scrollable part of Book information window or separate popup window ("Tap to view annotation", similar to book cover).

@poire-z
Copy link
Contributor

poire-z commented Mar 16, 2020

Thanks for that reference.

Book information while reading and from FM, annotation would need a scrollable window or popup window ("Tap to view annotation", similar to book cover).

We already have that for any long metadata (that gets a truncation ellipsis ...): long-press on it to display it in a full screen TextViewer:

image

image

So, the length is not an issue.
What's complicated is to bring many new individual metadata names into that Book information panel (and these new metadata names would be empty for most of all other ebook formats).

What's less complicated would be to merge/concatenate the metadata you list into the ones we already have:

authors (could contains translators or doc authors)
keywords (concatenated small set of words, maybe "date", "genres")
description (annotation + any of your publisher info concatenated).

with, possibly, no prefix to tell what they are (because of translations, unless you (and @Frenzie :) are fine with having them in english).

Note that this little indicator tells you that the book has a "Description":
image

@hius07
Copy link
Member

hius07 commented Mar 16, 2020

What's less complicated would be to merge/concatenate the metadata you list into the ones we already have:

It would be perfect.

@Frenzie
Copy link
Member

Frenzie commented Mar 16, 2020

unless you (and @Frenzie :) are fine with having them in english).

You can probably still concat translations?

@poire-z
Copy link
Contributor

poire-z commented Mar 16, 2020

That would be build (annotation + publisher name + publisher city + ...) by crengine, and given to frontend as a single string:

local description = book_props.description
if description == "" or description == nil then
description = _("N/A")
else
-- Description may (often in EPUB, but not always) or may not (rarely
-- in PDF) be HTML.
description = util.htmlToPlainTextIfHtml(book_props.description)
end
-- (We don't BD wrap description: it may be multi-lines, and the value we set
-- here may be viewed in a TextViewer that has auto_para_direction=true, which
-- will show the right thing, that'd we rather not mess with BD wrapping.)
table.insert(kv_pairs, { _("Description:"), description })

I really don't want to have to put some tags (###PUBLISHER_CITY###) for them in crengine, and have them replaced in frontend with translated strings :)
description = description:sub("###PUBLISHER_CITY###", _("Publisher city:"))

@Frenzie
Copy link
Member

Frenzie commented Mar 16, 2020

Eh, should be fine.

@poire-z
Copy link
Contributor

poire-z commented Mar 18, 2020

OK, pasting some code, that should hopefully be readable by non-programmers.
Tell me if the ordering and aggregation is fine, or suggest some alternative.
(So, with hardcoded non-translatable english bits - @Frenzie: please suggest if you see some better international-english wordings - this will be only for FB2 books)

lString16 extractDocKeywords( ldomDocument * doc )
{
    lString16 res;
    // Year
    res << doc->createXPointer(L"/FictionBook/description/title-info/date").getText().trim();
    // Genres
    for ( int i=0; i<16; i++) {
        lString16 path = cs16("/FictionBook/description/title-info/genre[") + fmt::decimal(i+1) + "]";
        ldomXPointer genre = doc->createXPointer(path);
        if ( !genre ) {
            break;
        }
        if ( !res.empty() )
            res << "\n";
        res << genre.getText().trim();
    }
    return res;
}

lString16 extractDocDescription( ldomDocument * doc )
{
    // We put all other FB2 meta info in this description
    lString16 res;

    // Annotation (description)
    res << doc->createXPointer(L"/FictionBook/description/title-info/annotation").getText().trim();

    // Translators
    lString16 translators;
    int nbTranslators = 0;
    for ( int i=0; i<16; i++) {
        lString16 path = cs16("/FictionBook/description/title-info/translator[") + fmt::decimal(i+1) + "]";
        ldomXPointer ptranslator = doc->createXPointer(path);
        if ( !ptranslator ) {
            break;
        }
        lString16 firstName = ptranslator.relative( L"/first-name" ).getText().trim();
        lString16 lastName = ptranslator.relative( L"/last-name" ).getText().trim();
        lString16 middleName = ptranslator.relative( L"/middle-name" ).getText().trim();
        lString16 translator = firstName;
        if ( !translator.empty() )
            translator += " ";
        if ( !middleName.empty() )
            translator += middleName;
        if ( !lastName.empty() && !translator.empty() )
            translator += " ";
        translator += lastName;
        if ( !translators.empty() )
            translators << "\n";
        translators << translator;
        nbTranslators++;
    }
    if ( !translators.empty() ) {
        if ( !res.empty() )
            res << "\n\n";
        if ( nbTranslators > 1 )
            res << "Translators:\n" << translators;
        else
            res << "Translator: " << translators;
    }

    // Publication info & publisher
    ldomXPointer publishInfo = doc->createXPointer(L"/FictionBook/description/publish-info");
    if ( !publishInfo.isNull() ) {
        lString16 publisher = publishInfo.relative( L"/publisher" ).getText().trim();
        lString16 pubcity = publishInfo.relative( L"/city" ).getText().trim();
        lString16 pubyear = publishInfo.relative( L"/year" ).getText().trim();
        lString16 isbn = publishInfo.relative( L"/isbn" ).getText().trim();
        lString16 bookName = publishInfo.relative( L"/book-name" ).getText().trim();
        lString16 publication;
        if ( !publisher.empty() || !pubcity.empty() ) {
            if ( !publisher.empty() ) {
                publication << publisher;
            }
            if ( !pubcity.empty() ) {
                if ( !!publisher.empty() ) {
                    publication << ", ";
                }
                publication << pubcity;
            }
        }
        if ( !pubyear.empty() || !isbn.empty() ) {
            if ( !publication.empty() )
                publication << "\n";
            if ( !pubyear.empty() ) {
                publication << pubyear;
            }
            if ( !isbn.empty() ) {
                if ( !pubyear.empty() ) {
                    publication << ", ";
                }
                publication << isbn;
            }
        }
        if ( !bookName.empty() ) {
            if ( !publication.empty() )
                publication << "\n";
            publication << bookName;
        }
        if ( !publication.empty() ) {
            if ( !res.empty() )
                res << "\n\n";
            res << "Publication:\n" << publication;
        }
    }

    // Document info
    ldomXPointer pDocInfo = doc->createXPointer(L"/FictionBook/description/document-info");
    if ( !pDocInfo.isNull() ) {
        lString16 docInfo;
        lString16 docAuthors;
        int nbAuthors = 0;
        for ( int i=0; i<16; i++) {
            lString16 path = cs16("/FictionBook/description/document-info/author[") + fmt::decimal(i+1) + "]";
            ldomXPointer pdocAuthor = doc->createXPointer(path);
            if ( !pdocAuthor ) {
                break;
            }
            lString16 firstName = pdocAuthor.relative( L"/first-name" ).getText().trim();
            lString16 lastName = pdocAuthor.relative( L"/last-name" ).getText().trim();
            lString16 middleName = pdocAuthor.relative( L"/middle-name" ).getText().trim();
            lString16 docAuthor = firstName;
            if ( !docAuthor.empty() )
                docAuthor += " ";
            if ( !middleName.empty() )
                docAuthor += middleName;
            if ( !lastName.empty() && !docAuthor.empty() )
                docAuthor += " ";
            docAuthor += lastName;
            if ( !docAuthors.empty() )
                docAuthors << "\n";
            docAuthors << docAuthor;
            nbAuthors++;
        }
        if ( !docAuthors.empty() ) {
            if ( nbAuthors > 1 )
                docInfo << "Authors:\n" << docAuthors;
            else
                docInfo << "Author: " << docAuthors;
        }
        lString16 docPublisher = pDocInfo.relative( L"/publisher" ).getText().trim();
        lString16 docId = pDocInfo.relative( L"/id" ).getText().trim();
        lString16 docVersion = pDocInfo.relative( L"/version" ).getText().trim();
        lString16 docDate = pDocInfo.relative( L"/date" ).getText().trim();
        lString16 docHistory = pDocInfo.relative( L"/history" ).getText().trim();
        lString16 docSrcUrl = pDocInfo.relative( L"/src-url" ).getText().trim();
        lString16 docSrcOcr = pDocInfo.relative( L"/src-ocr" ).getText().trim();
        lString16 docProgramUsed = pDocInfo.relative( L"/program-used" ).getText().trim();
        if ( !docPublisher.empty() ) {
            if ( !docInfo.empty() )
                docInfo << "\n";
            docInfo << "Publisher: " << docPublisher;
        }
        if ( !docId.empty() ) {
            if ( !docInfo.empty() )
                docInfo << "\n";
            docInfo << "Id: " << docId;
        }
        if ( !docVersion.empty() ) {
            if ( !docInfo.empty() )
                docInfo << "\n";
            docInfo << "Version: " << docVersion;
        }
        if ( !docDate.empty() ) {
            if ( !docInfo.empty() )
                docInfo << "\n";
            docInfo << "Date: " << docDate;
        }
        if ( !docHistory.empty() ) {
            if ( !docInfo.empty() )
                docInfo << "\n";
            docInfo << "History: " << docHistory;
        }
        if ( !docSrcUrl.empty() ) {
            if ( !docInfo.empty() )
                docInfo << "\n";
            docInfo << "Url: " << docSrcUrl;
        }
        if ( !docSrcOcr.empty() ) {
            if ( !docInfo.empty() )
                docInfo << "\n";
            docInfo << "OCR: " << docSrcOcr;
        }
        if ( !docProgramUsed.empty() ) {
            if ( !docInfo.empty() )
                docInfo << "\n";
            docInfo << "Program: " << docProgramUsed;
        }
        if ( !docInfo.empty() ) {
            if ( !res.empty() )
                res << "\n\n";
            res << "Document:\n" << docInfo;
        }
    }

    return res;
}

Which would give:
image
image
image

image
image
image

@hius07
Copy link
Member

hius07 commented Mar 18, 2020

It's the best decision I dreamt of.
A few points:

  • «genre» is not very interesting to read (isn't for automated books sorting?), I would change "Keywords" to "Date" or "Year" and display «date» only
  • as the Description is long, displaying its first words has no sense. "Tap to display" would be OK.
    The above will allow to justify-left all the data displayed.

@poire-z
Copy link
Contributor

poire-z commented Mar 18, 2020

I would change "Keywords" to "Date" or "Year" and display «date» only

We have a field already named "keywords", and it will stay displayed "Keywords". I can put small stuff in it, or let it blank. But I guess it's good to put the date in it (and first), so you see it without having to open Description. The "genre" small words will be in there too, because they are there for other formats (would be strange to drop them for FB2 only).

We have these fields Keywords and Description because some other book formats have slots for them (both PDF & EPUB). FB2 has a lot more metadata, but adding new fields is tedious (I don't want to add new columns in the cache database...), and these would be empty for most other book formats).

as the Description is long, displaying its first words has no sense

At least, its first words tell you if it is in a language you can read :)
Well, we use some generic "Key: Value" widget, so that's how it will be :)

@hius07
Copy link
Member

hius07 commented Mar 18, 2020

Thank you, I like it very much.

@mergen3107
Copy link
Contributor Author

This is great! Thank you for your kind consideration!

@Frenzie
Copy link
Member

Frenzie commented Mar 18, 2020

Seems fine. Btw, I think PDF calls the program used the creator.

@poire-z
Copy link
Contributor

poire-z commented Mar 18, 2020

My Acrobat Reader uses "Application", which feels a bit less rough than "program".

@Frenzie
Copy link
Member

Frenzie commented Mar 18, 2020

Either way, it's a difference without distinction. An application is a program. A program is an application. Application's more French/Latin, program's more Greek.

@hius07
Copy link
Member

hius07 commented Mar 22, 2020

v2020.03.2-16
I love new detailed Description!

@hius07
Copy link
Member

hius07 commented Mar 24, 2020

One more point: Book information for .fb2.zip shows Format: ZIP.
It'd be better to show FB2 or FB2.ZIP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants