Review Request 117626: Implement the first version of a DOCX export filter
Jarosław Staniek
staniek at kde.org
Sat Apr 19 00:12:05 BST 2014
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117626/#review56039
-----------------------------------------------------------
CMakeLists.txt
<https://git.reviewboard.kde.org/r/117626/#comment39052>
Loosely related but while we're at it, could be:
All Words import filter -> All Words import filters
All Words export filter -> All Words export filters
filters/libodf2/KoOdfStyleManager.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39053>
coding style: space after "foreach" everywhere in this patch
filters/libodf2/KoOdfStyleManager.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39054>
Simpler and maybe faster is to use QList<T> QHash::values() const
filters/libodf2/KoOdfStyleManager.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39055>
Simpler and maybe faster is to use QList<T> QHash::values() const
filters/libodf2/KoOdfStyleManager.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39056>
Serious bug by the way, missing:
d->styles.clear();
d->defaultStyles.clear();
clear() can only be skipped in dtors.
I propose to commit this fix independently to master and 2.8.
filters/libodf2/KoOdfStyleManager.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39058>
Readability alert: please let's use a convention here by adding /*fromStylesXml*/ comment.
filters/libodf2/KoOdfStyleManager.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39059>
Readability alert like above:
add /* !fromStylesXml*/
filters/libodf2/KoOdfStyleProperties.h
<https://git.reviewboard.kde.org/r/117626/#comment39062>
couldn't the arg be const KoOdfStyleProperties &sourceProperties?
filters/words/docx/export/CMakeLists.txt
<https://git.reviewboard.kde.org/r/117626/#comment39063>
Let's use established naming convention:
calligra_filter_odt2docx
filters/words/docx/export/CMakeLists.txt
<https://git.reviewboard.kde.org/r/117626/#comment39064>
Like above, use name calligra_filter_odt2docx.desktop
filters/words/docx/export/DocxExport.h
<https://git.reviewboard.kde.org/r/117626/#comment39065>
For analogy later we'd implement MsooXmlExport inheriting KoOdfImporter inheriting KoFilter. Then inherit MsooXmlExport here.
filters/words/docx/export/DocxExport.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39066>
Note: in most filters we have a bug probably: we use ****Factory(componentName) only with componentName == "calligrafilters" and we do not use catalogName. We should use 2 args, componentName == filter name and catalogName == "calligrafilters".
filters/words/docx/export/DocxExport.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39067>
check result, exit on failure
filters/words/docx/export/DocxExport.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39068>
check result, exit on failure
filters/words/docx/export/DocxFile.h
<https://git.reviewboard.kde.org/r/117626/#comment39070>
let's use Doxygen comments everywhere
filters/words/docx/export/DocxFile.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39078>
Move all this to MsooXmlRelationships in libmsooxml (note how it has a "@todo add write methods and saving support")
filters/words/docx/export/DocxFile.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39079>
Move to MsooXmlRelationships too.
filters/words/docx/export/DocxStyleHelper.h
<https://git.reviewboard.kde.org/r/117626/#comment39080>
isn't namespace better?
filters/words/docx/export/DocxStyleHelper.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39081>
properties != 0 -> properties
filters/words/docx/export/DocxStyleWriter.h
<https://git.reviewboard.kde.org/r/117626/#comment39069>
make these attrs private, add methods before debugging becomes too hard
filters/words/docx/export/FileCollector.h
<https://git.reviewboard.kde.org/r/117626/#comment39072>
const references are better here
filters/words/docx/export/FileCollector.h
<https://git.reviewboard.kde.org/r/117626/#comment39071>
if it's a struct with public attrs, better drop m_ prefixes
filters/words/docx/export/FileCollector.h
<https://git.reviewboard.kde.org/r/117626/#comment39073>
use const ref
filters/words/docx/export/FileCollector.h
<https://git.reviewboard.kde.org/r/117626/#comment39074>
use const ref
filters/words/docx/export/FileCollector.h
<https://git.reviewboard.kde.org/r/117626/#comment39075>
use const refs
filters/words/docx/export/FileCollector.h
<https://git.reviewboard.kde.org/r/117626/#comment39076>
use const refs
filters/words/docx/export/FileCollector.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39084>
EPUB -> DOCX
filters/words/docx/export/FileCollector.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39083>
duplicated definition, not within the class FileCollector I guess
filters/words/docx/export/FileCollector.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39085>
compare result with file->m_fileContents.size() and result with error if not equal; do not forget to close before returning
filters/words/docx/export/OdfReaderDocxContext.h
<https://git.reviewboard.kde.org/r/117626/#comment39087>
replace public attrs with access methods
filters/words/docx/export/OdfTextReaderDocxBackend.h
<https://git.reviewboard.kde.org/r/117626/#comment39086>
not needed explicit here
filters/words/docx/export/OdfTextReaderDocxBackend.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39088>
No writer->endElement() paired with w:p?
How about writing and using macros analogous to *READ_* in DOCX importer
filters/words/docx/export/OdtReaderDocxBackend.h
<https://git.reviewboard.kde.org/r/117626/#comment39089>
not needed explicit here
filters/words/docx/export/OpcRelSet.h
<https://git.reviewboard.kde.org/r/117626/#comment39091>
Move to MsooXmlRelationships?
filters/words/docx/export/OpcRelSet.h
<https://git.reviewboard.kde.org/r/117626/#comment39090>
Relation -> Relationship
filters/words/docx/export/OpcRelSetManager.h
<https://git.reviewboard.kde.org/r/117626/#comment39092>
isn't this a duty of MsooXmlRelationships?
filters/words/docx/export/OpcRelSetManager.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39094>
Missing clear()
filters/words/docx/export/OpcRelSetManager.cpp
<https://git.reviewboard.kde.org/r/117626/#comment39093>
missing d->relSets.clear()
filters/words/docx/export/UnitConversions.h
<https://git.reviewboard.kde.org/r/117626/#comment39095>
move entire file to libmsooxml/MsooXmlUnits.h and use macros
filters/words/docx/export/words_docx_export.desktop
<https://git.reviewboard.kde.org/r/117626/#comment39096>
-> MS Word 2007 Export Filter for Words
filters/words/docx/export/words_docx_export.desktop
<https://git.reviewboard.kde.org/r/117626/#comment39097>
-> xxMS Word 2007 Export Filter for Wordsxx
filters/words/docx/export/words_docx_export.desktop
<https://git.reviewboard.kde.org/r/117626/#comment39098>
-> calligra_filter_odt2docx
filters/words/docx/export/words_docx_export.desktop
<https://git.reviewboard.kde.org/r/117626/#comment39099>
+
X-KDE-LibraryMajor=0
X-KDE-LibraryMinor=1
- Jarosław Staniek
On April 18, 2014, 9:38 a.m., Inge Wallin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117626/
> -----------------------------------------------------------
>
> (Updated April 18, 2014, 9:38 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 headers and normal paragraphs [This turned out not to be true when I look at the code in the RR now. But it will be before it's merged. We will concentrate on this now]. 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 78421c5
> filters/libodf2/KoOdfStyle.h 558ade9
> filters/libodf2/KoOdfStyle.cpp 2b4eb95
> filters/libodf2/KoOdfStyleManager.h 3761d38
> filters/libodf2/KoOdfStyleManager.cpp a7f18a2
> 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/words_docx_export.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/20140418/f3ae29f5/attachment.htm>
More information about the calligra-devel
mailing list