[Nepomuk] Review Request: Plugin to index various file formats supported by Okular

[Vishesh Handa]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107939/#review24033
-----------------------------------------------------------


Overall, I love this! It adds support for so many more types. Yay!


CMakeLists.txt
<http://git.reviewboard.kde.org/r/107939/#comment18316>

    Please change the ffmpeg site to okular's website.



services/fileindexer/indexer/main.cpp
<http://git.reviewboard.kde.org/r/107939/#comment18317>

    Please add a comment over here indicating that the okular extractor requires widgets.



services/fileindexer/indexer/okularextractor.h
<http://git.reviewboard.kde.org/r/107939/#comment18318>

    Naming Convention: All classes should be capitalized.
    
    "OkularExtractor" instead of "okularExtractor"



services/fileindexer/indexer/okularextractor.cpp
<http://git.reviewboard.kde.org/r/107939/#comment18325>

    If it's not too much trouble, could you please add a comment over here indicating all the mimetypes it handles. Approximately.



services/fileindexer/indexer/okularextractor.cpp
<http://git.reviewboard.kde.org/r/107939/#comment18320>

    Why is this required? Please document.



services/fileindexer/indexer/okularextractor.cpp
<http://git.reviewboard.kde.org/r/107939/#comment18322>

    Please use a QScopedPointer instead of manually deleting it.



services/fileindexer/indexer/okularextractor.cpp
<http://git.reviewboard.kde.org/r/107939/#comment18319>

    just do a supportedMimeTypes.removeAll(QLatin1String("application/pdf"));



services/fileindexer/indexer/okularextractor.cpp
<http://git.reviewboard.kde.org/r/107939/#comment18323>

    QScopedPointer please



services/fileindexer/indexer/okularextractor.cpp
<http://git.reviewboard.kde.org/r/107939/#comment18324>

    This page - http://api.kde.org/4.x-api/kdegraphics-apidocs/okular/html/classOkular_1_1DocumentInfo.html seems to indicate a lot more information that you can fetch. Could you please also add that?
    
    Specifically - 
    
    Producer -> nie:generator
    Copyright -> nie:copyright
    CreationDate -> nie:contentCreated
    ModificationDate -> nie:content
    Description -> nie:description? I'm not sure about this cause the nie:discription is also used in the comment widget in Dolphin.
    
    Title -> nie:title



services/fileindexer/indexer/okularextractor.cpp
<http://git.reviewboard.kde.org/r/107939/#comment18321>

    I'm slightly skeptical about this. Do you actually get the plain text?
    
    Could you check by running a query / nepomukshell / nepomukshow?
    
    I'm skeptical cause I'm not sure what happens when the document->exportToText() function tries to write to the file when you already have a fd open for the file, and then try to read data from it.


- Vishesh Handa


On Dec. 27, 2012, 11:07 a.m., Rohan Garg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107939/
> -----------------------------------------------------------
> 
> (Updated Dec. 27, 2012, 11:07 a.m.)
> 
> 
> Review request for Nepomuk.
> 
> 
> Description
> -------
> 
> Use the Okular API to extract information out of files that are supported by Okular (mobi/epub/etc) with the added exception of not indexing pdf files.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 6766e4b112258465aee93cd1d13cf619b1f3aa5e 
>   services/fileindexer/indexer/CMakeLists.txt f7b5d2a29dc4098cda3997f338adf45b211c4872 
>   services/fileindexer/indexer/main.cpp 4d6fcfdb1b5231bd8af1c606f4b13a0cae89639d 
>   services/fileindexer/indexer/nepomukokularextractor.desktop PRE-CREATION 
>   services/fileindexer/indexer/okularextractor.h PRE-CREATION 
>   services/fileindexer/indexer/okularextractor.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/107939/diff/
> 
> 
> Testing
> -------
> 
> Indexed an epub file using : nepomukindexer --data file.epub, could see all the data that was indexed on the terminal
> 
> Sample epub : https://epub-samples.googlecode.com/files/epub30-spec-20121128.epub
> 
> NOTE: Install kdegraphics-mobi to get mobi support.
> 
> 
> Thanks,
> 
> Rohan Garg
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/nepomuk/attachments/20121227/df002d0d/attachment-0001.html>