Review Request 122035: Calligra filters: Enhance the odf reader library significantly

[Jarosław Staniek]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122035/#review75219
-----------------------------------------------------------

Ship it!


Cool. Minor, possibly future, notes. 

Not sure about the smart stuff like DECLARE_READER_ -- does it work ok in Qt Creator's symbol lookup for example?


filters/libodf2/KoXmlUtils.h
<https://git.reviewboard.kde.org/r/122035/#comment52023>

    How about returning bool instead (see the validation comments below)?



filters/libodf2/KoXmlUtils.cpp
<https://git.reviewboard.kde.org/r/122035/#comment52024>

    TODO: validation here and in 43, 44



filters/libodfreader/OdfChartReader.cpp
<https://git.reviewboard.kde.org/r/122035/#comment52025>

    Future: We can use QLatin1String; the same in other methods



filters/libodfreader/OdfChartReaderBackend.h
<https://git.reviewboard.kde.org/r/122035/#comment52027>

    --explicit



filters/libodfreader/OdfDrawReaderBackend.h
<https://git.reviewboard.kde.org/r/122035/#comment52028>

    --explicit



filters/libodfreader/OdfReaderBackend.h
<https://git.reviewboard.kde.org/r/122035/#comment52030>

    --explicit



filters/libodfreader/OdfReaderInternals.h
<https://git.reviewboard.kde.org/r/122035/#comment52026>

    maybe -> QLatin1String



filters/libodfreader/OdsReaderBackend.h
<https://git.reviewboard.kde.org/r/122035/#comment52032>

    --explicit



filters/libodfreader/OdtReaderBackend.h
<https://git.reviewboard.kde.org/r/122035/#comment52029>

    --explicit


- Jarosław Staniek


On Jan. 13, 2015, 3:30 p.m., Inge Wallin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122035/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2015, 3:30 p.m.)
> 
> 
> Review request for Calligra, Lassi Nieminen and Jarosław Staniek.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> Here is a little something that I hacked on in my sickbed. It's all very simple stuff, but I managed to accumulate quite a lot over the weeks.
> 
> Short summary:
>  - Some refactoring: a generic odf reader class
>  - New: support for ODS (spreadsheets)
>  - Support for many draw objects
>  - Support for frames
>  - Support for charts
>  - Much simplified code in the reader backend due to smart macros
> 
> There should be *no* API changes for external users of this library. This is tested using existing filters, mainly the docx export filter.
> 
> It would be nice to get this into master before we start on the qt5 port. The changes are limited to a very small part of the source tree and even though they are somewhat biggish, they are very simple in nature.
> 
> 
> Diffs
> -----
> 
>   filters/libodf2/CMakeLists.txt 3792771 
>   filters/libodf2/KoXmlStreamReader.h a06c8ec 
>   filters/libodf2/KoXmlUtils.h PRE-CREATION 
>   filters/libodf2/KoXmlUtils.cpp PRE-CREATION 
>   filters/libodfreader/CMakeLists.txt e4bfdda 
>   filters/libodfreader/OdfChartReader.h PRE-CREATION 
>   filters/libodfreader/OdfChartReader.cpp PRE-CREATION 
>   filters/libodfreader/OdfChartReaderBackend.h PRE-CREATION 
>   filters/libodfreader/OdfChartReaderBackend.cpp PRE-CREATION 
>   filters/libodfreader/OdfDrawReader.h PRE-CREATION 
>   filters/libodfreader/OdfDrawReader.cpp PRE-CREATION 
>   filters/libodfreader/OdfDrawReaderBackend.h PRE-CREATION 
>   filters/libodfreader/OdfDrawReaderBackend.cpp PRE-CREATION 
>   filters/libodfreader/OdfReader.h PRE-CREATION 
>   filters/libodfreader/OdfReader.cpp PRE-CREATION 
>   filters/libodfreader/OdfReaderBackend.h PRE-CREATION 
>   filters/libodfreader/OdfReaderBackend.cpp PRE-CREATION 
>   filters/libodfreader/OdfReaderInternals.h PRE-CREATION 
>   filters/libodfreader/OdfTextReader.h d6c5bc8 
>   filters/libodfreader/OdfTextReader.cpp ab6514d 
>   filters/libodfreader/OdfTextReaderBackend.h 49c25ea 
>   filters/libodfreader/OdfTextReaderBackend.cpp 30219ff 
>   filters/libodfreader/OdsReader.h PRE-CREATION 
>   filters/libodfreader/OdsReader.cpp PRE-CREATION 
>   filters/libodfreader/OdsReaderBackend.h PRE-CREATION 
>   filters/libodfreader/OdsReaderBackend.cpp PRE-CREATION 
>   filters/libodfreader/OdtReader.h 22ae935 
>   filters/libodfreader/OdtReader.cpp 48e1037 
>   filters/libodfreader/OdtReaderBackend.h ae321ab 
>   filters/libodfreader/OdtReaderBackend.cpp cdc59f1 
> 
> Diff: https://git.reviewboard.kde.org/r/122035/diff/
> 
> 
> Testing
> -------
> 
> Tested with extensive traces and existing filters that use the library.
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20150202/6f0519ac/attachment.htm>