Review Request 117626: Implement the first version of a DOCX export filter

Mon Apr 21 21:24:52 BST 2014

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

Much better!

filters/words/docx/export/DocxStyleHelper.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39154>

    -1 by default would be nice

filters/words/docx/export/DocxStyleHelper.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39152>

    checking conversion to double would be nice too: toDouble(&ok), returning e.g. -1 at error; in such case kWarning would be useful

filters/words/docx/export/DocxStyleHelper.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39153>

    checking conversion to double would be nice too

filters/words/docx/export/DocxStyleHelper.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39151>

    else {} would be nice, e.g. returning -1

    in such case kWarning would be useful

filters/words/docx/export/DocxStyleHelper.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39156>

    to reduce # of nested better use 

    if (!properties) {
      return;
    }

    and Q_ASSERT() before it would be nice too

filters/words/docx/export/DocxStyleHelper.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39155>

    check success?
    This is pedantic but we're doing this using apprach such as STRING_TO_INT at filters/libmsooxml/MsooXmlReader_p.h

filters/words/docx/export/DocxStyleHelper.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39157>

    See the comment for handleTextStyles()

filters/words/docx/export/DocxStyleWriter.h
<https://git.reviewboard.kde.org/r/117626/#comment39158>

    make it const

filters/words/docx/export/FileCollector.h
<https://git.reviewboard.kde.org/r/117626/#comment39159>

    Let's move the impl. to FileCollector.cpp - that's what I meant initially

filters/words/docx/export/FileCollector.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39160>

    Nice note but I recommend using //! @todo - I learned to do that everywhere

filters/words/docx/export/OdfReaderDocxContext.h
<https://git.reviewboard.kde.org/r/117626/#comment39162>

    Add //! here and everywhere

filters/words/docx/export/OdfReaderDocxContext.h
<https://git.reviewboard.kde.org/r/117626/#comment39163>

    Add Add //!< here and everywhere

filters/words/docx/export/OdfReaderDocxContext.h
<https://git.reviewboard.kde.org/r/117626/#comment39161>

    not needed private:

filters/words/docx/export/OdfTextReaderDocxBackend.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39164>

    >=0 more readable?

filters/words/docx/export/OpcRelSetManager.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39165>

    ok but a note for future, much nicer place for it is ~Private()

filters/words/docx/export/words_docx_export.desktop
<https://git.reviewboard.kde.org/r/117626/#comment39168>

    words_docx_export.desktop not needed since there is calligra_filter_odt2docx.desktop?

filters/words/docx/export/words_docx_export.desktop
<https://git.reviewboard.kde.org/r/117626/#comment39167>

    OK, I propose "MS Word DOCX Export Filter" 

libs/odf/KoEncryptedStore.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39169>

    BTW, return false on failure as we do everywhere

filters/libodf2/KoOdfStyleProperties.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39149>

    isn't this simpler, faster? :

    d->attributes = sourceProperties.d->attributes;

filters/words/docx/export/DocxExport.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39150>

    Unusual indentation in this block...

filters/words/docx/export/calligra_filter_odt2docx.desktop
<https://git.reviewboard.kde.org/r/117626/#comment39166>

    I guess it's general purpose filter, I'd be able to use it in Kexi for some needs, so "MS Word DOCX Export Filter" would name be enough
    (We'll need to fix calligra_filter_docx2odt this way too)

- Jarosław Staniek

On April 20, 2014, 8:11 a.m., Inge Wallin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117626/
> -----------------------------------------------------------
> 
> (Updated April 20, 2014, 8:11 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> This patch implements the first simple version of a docx export filter. It has support for simple character formatting and paragraph formatting, including named styles. It can also distinguish between headings and normal paragraphs. Other than that it's an unwritten page.  A special thanks to Lassi Nieminen who helped me with converting the styles.
> 
> The patch itself is very straightforward and mostly self contained in the filters/words/docx subdirectory. It builds on my previous work with libodfreader and libodf2, which are both in filters/. The only problem was that the KoZipStore and KoEncryptedStore backends create a file called "mimetype" automatically when a KoStore is created in write mode. I tried to work around this with as little impact as possible to the code in libs/odf and with full source compatibility with the previous API. If you think there is a better way to solve this problem than the one I implemented, then please tell me.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 9eedc3d 
>   filters/libodf2/KoOdfStyle.h 558ade9 
>   filters/libodf2/KoOdfStyle.cpp 2b4eb95 
>   filters/libodf2/KoOdfStyleManager.h 3761d38 
>   filters/libodf2/KoOdfStyleManager.cpp 6e8f5b55 
>   filters/libodf2/KoOdfStyleProperties.h 1bfbb5c 
>   filters/libodf2/KoOdfStyleProperties.cpp 186e971 
>   filters/libodfreader/OdtReader.h 64e0584 
>   filters/libodfreader/OdtReader.cpp 6fa8ce6 
>   filters/words/docx/CMakeLists.txt f38a2bb 
>   filters/words/docx/export/CMakeLists.txt PRE-CREATION 
>   filters/words/docx/export/DocxExport.h PRE-CREATION 
>   filters/words/docx/export/DocxExport.cpp PRE-CREATION 
>   filters/words/docx/export/DocxFile.h PRE-CREATION 
>   filters/words/docx/export/DocxFile.cpp PRE-CREATION 
>   filters/words/docx/export/DocxStyleHelper.h PRE-CREATION 
>   filters/words/docx/export/DocxStyleHelper.cpp PRE-CREATION 
>   filters/words/docx/export/DocxStyleWriter.h PRE-CREATION 
>   filters/words/docx/export/DocxStyleWriter.cpp PRE-CREATION 
>   filters/words/docx/export/FileCollector.h PRE-CREATION 
>   filters/words/docx/export/FileCollector.cpp PRE-CREATION 
>   filters/words/docx/export/OdfReaderDocxContext.h PRE-CREATION 
>   filters/words/docx/export/OdfReaderDocxContext.cpp PRE-CREATION 
>   filters/words/docx/export/OdfTextReaderDocxBackend.h PRE-CREATION 
>   filters/words/docx/export/OdfTextReaderDocxBackend.cpp PRE-CREATION 
>   filters/words/docx/export/OdtReaderDocxBackend.h PRE-CREATION 
>   filters/words/docx/export/OdtReaderDocxBackend.cpp PRE-CREATION 
>   filters/words/docx/export/OpcContentTypes.h PRE-CREATION 
>   filters/words/docx/export/OpcContentTypes.cpp PRE-CREATION 
>   filters/words/docx/export/OpcRelSet.h PRE-CREATION 
>   filters/words/docx/export/OpcRelSet.cpp PRE-CREATION 
>   filters/words/docx/export/OpcRelSetManager.h PRE-CREATION 
>   filters/words/docx/export/OpcRelSetManager.cpp PRE-CREATION 
>   filters/words/docx/export/README PRE-CREATION 
>   filters/words/docx/export/UnitConversions.h PRE-CREATION 
>   filters/words/docx/export/UnitConversions.cpp PRE-CREATION 
>   filters/words/docx/export/calligra_filter_odt2docx.desktop PRE-CREATION 
>   libs/odf/KoDirectoryStore.h 19c059d 
>   libs/odf/KoDirectoryStore.cpp c893d47 
>   libs/odf/KoEncryptedStore.h 0edd892 
>   libs/odf/KoEncryptedStore.cpp 315df1a 
>   libs/odf/KoStore.h dadecd1 
>   libs/odf/KoStore.cpp fd42378 
>   libs/odf/KoStore_p.h 2e518c1 
>   libs/odf/KoTarStore.h d99f09b 
>   libs/odf/KoTarStore.cpp 6829f34 
>   libs/odf/KoZipStore.h 90ffcb0 
>   libs/odf/KoZipStore.cpp 4235134 
> 
> Diff: https://git.reviewboard.kde.org/r/117626/diff/
> 
> 
> Testing
> -------
> 
> Testing with a number of odt files. Lassi did all the testing involving MS Office since I don't have that.
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

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