Review Request: Make the epub filter handle math formulas

Friedrich W. H. Kossebau kossebau at kde.org
Fri Dec 28 21:11:30 GMT 2012


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


Cannot really comment on the usecase, only nitpick on implementation level. Done that.

All in all I see no reason to block this from entering master. But leaving decision to someone else.


filters/words/epub/OdfParser.cpp
<http://git.reviewboard.kde.org/r/107969/#comment18423>

    Mr. Wallin, Mr. QString::chop(), may I introduce you to each other? :)
    
    Less code, possibly no malloc, faster... wants to be used here.
    
    http://qt-project.org/doc/qt-4.8/qstring.html#chop



filters/words/epub/OdtHtmlConverter.cpp
<http://git.reviewboard.kde.org/r/107969/#comment18427>

    Propose to use instead:
        href.remove(0, 2);
    



filters/words/epub/OdtHtmlConverter.cpp
<http://git.reviewboard.kde.org/r/107969/#comment18430>

    "QString &href"?
    "const " was lost here?



filters/words/epub/OdtHtmlConverter.cpp
<http://git.reviewboard.kde.org/r/107969/#comment18429>

    So this mehod has one store->open, but two store->close()? Does not look right, seems somewhere else the store is not closed after fetching stuff from it.
    So better find who forces you to clean up for him.
    
    Or add a big TODO here for now, but this smells buggy.



filters/words/epub/OdtHtmlConverter.cpp
<http://git.reviewboard.kde.org/r/107969/#comment18431>

    "QPair<QString, QString> attrPair" could be a const ref, no need to copy the content as value. (surely then could be changed in the KoUnAvailShape as well).


- Friedrich W. H. Kossebau


On Dec. 28, 2012, 2:48 a.m., Inge Wallin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107969/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2012, 2:48 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> This patch implements support for math formulas in the EPUB filter. This is the first one of the EPUB3 features that we want to add to Calligra 2.7.
> 
> This version only supports math formulas saved as an embedded document, like LibreOffice and the OpenOffice variants save it. Calligra saves math formulas as inline mathML in the frame, which is not supported by this version. I thought that I could get some initial feedback while implementing support for the Calligra way too.
> 
> 
> Diffs
> -----
> 
>   filters/words/epub/OdfParser.cpp 6069b89 
>   filters/words/epub/OdtHtmlConverter.h 68aaffa 
>   filters/words/epub/OdtHtmlConverter.cpp d4b7199 
>   filters/words/epub/exportepub2.cpp 84d8a90 
>   filters/words/epub/exporthtml.cpp bdd33e7 
> 
> Diff: http://git.reviewboard.kde.org/r/107969/diff/
> 
> 
> Testing
> -------
> 
> Created one simple odt using OOo which has a formula and some text.
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20121228/06c1a16e/attachment.htm>


More information about the calligra-devel mailing list