<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/117626/">https://git.reviewboard.kde.org/r/117626/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hi Jaroslaw. Thanks for a super thorough review, this is much appreciated!
When it comes to your comments to merge with libmsooxml, I suggest the following course: Keep it as it is for now, and then in a separate patch identify common functionality and merge them. Reasons:
- The patch is already quite big and would be even bigger if we start to hack in libooxml too. I suggest to keep each patch focused.
- The ooxml library is very heavily slanted towards reading ooxml, not writing it. I remember looking at it early and finding that changing the code there would be bigger. Some data structures in libmsooxml need to be changed too for dual purpose (read+write).
- We should move everything that has to do with OPC out into it's own library which can also be reused for other filters.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/117626/diff/1/?file=266851#file266851line233" style="color: black; font-weight: bold; text-decoration: underline;">CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">233</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">calligra_define_product</span><span class="p">(</span><span class="s">FILTER_ODT_TO_DOCX</span> <span class="s2">"ODT to DOCX filter"</span> <span class="s">NEEDS</span> <span class="s">LIB_KOODFREADER</span> <span class="s">CALLIGRA_LIBS</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Bug: Also needs LIB_KOODF2</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/117626/diff/1/?file=266875#file266875line112" style="color: black; font-weight: bold; text-decoration: underline;">filters/words/docx/export/OdfTextReaderDocxBackend.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">112</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">writer</span><span class="o">-></span><span class="n">endElement</span><span class="p">();</span> <span class="c1">// w:pPr</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">There is a paired endElement(). Look below in the else branch. This backend method is called twice by the reader: When the tag starts and when it ends. So we need to write <w:p> into the docx file when the text:p starts in the odt, and </w:p> when the text:p ends in the odt. But in between we need to convert the contents of the text:p, which is done by other methods in this backend.
I have thought of developing macros like your READ_* in the ooxml import filters. But you have hard coded your readers for the filters and we have split up the process into a reader class and a backend class. This makes the reader reusable with other backends (see the ascii export filter for an example) and this makes the macros only usable in the reader itself.
I'm all for it if you can find a good set of macros but I don't think it's in the scope of this patch.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/117626/diff/1/?file=266880#file266880line56" style="color: black; font-weight: bold; text-decoration: underline;">filters/words/docx/export/OpcRelSet.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">56</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">class</span> <span class="n">OpcRelSet</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Or the other way around? These are not actually only ooxml relationships, they are general relationships for the Open Packaging Conventions. I suggest keeping it as it is for this patch and then merge the two into a general OPC library. This library will also be usable in filters for other formats that use Opc, which is a bunch.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/117626/diff/1/?file=266887#file266887line3" style="color: black; font-weight: bold; text-decoration: underline;">filters/words/docx/export/words_docx_export.desktop</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">3</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">Name=Words DOCX Export Filter</pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Why only 2007? It's not clear which version of ooxml we are targetting. I suggest not changing this until we have decided.</pre>
</div>
<br />
<p>- Inge Wallin</p>
<br />
<p>On April 18th, 2014, 9:38 a.m. UTC, Inge Wallin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Calligra.</div>
<div>By Inge Wallin.</div>
<p style="color: grey;"><i>Updated April 18, 2014, 9:38 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
calligra
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Testing with a number of odt files. Lassi did all the testing involving MS Office since I don't have that.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(78421c5)</span></li>
<li>filters/libodf2/KoOdfStyle.h <span style="color: grey">(558ade9)</span></li>
<li>filters/libodf2/KoOdfStyle.cpp <span style="color: grey">(2b4eb95)</span></li>
<li>filters/libodf2/KoOdfStyleManager.h <span style="color: grey">(3761d38)</span></li>
<li>filters/libodf2/KoOdfStyleManager.cpp <span style="color: grey">(a7f18a2)</span></li>
<li>filters/libodf2/KoOdfStyleProperties.h <span style="color: grey">(1bfbb5c)</span></li>
<li>filters/libodf2/KoOdfStyleProperties.cpp <span style="color: grey">(186e971)</span></li>
<li>filters/libodfreader/OdtReader.h <span style="color: grey">(64e0584)</span></li>
<li>filters/libodfreader/OdtReader.cpp <span style="color: grey">(6fa8ce6)</span></li>
<li>filters/words/docx/CMakeLists.txt <span style="color: grey">(f38a2bb)</span></li>
<li>filters/words/docx/export/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/DocxExport.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/DocxExport.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/DocxFile.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/DocxFile.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/DocxStyleHelper.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/DocxStyleHelper.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/DocxStyleWriter.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/DocxStyleWriter.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/FileCollector.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/FileCollector.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/OdfReaderDocxContext.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/OdfReaderDocxContext.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/OdfTextReaderDocxBackend.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/OdfTextReaderDocxBackend.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/OdtReaderDocxBackend.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/OdtReaderDocxBackend.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/OpcContentTypes.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/OpcContentTypes.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/OpcRelSet.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/OpcRelSet.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/OpcRelSetManager.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/OpcRelSetManager.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/README <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/UnitConversions.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/UnitConversions.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/words/docx/export/words_docx_export.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/odf/KoDirectoryStore.h <span style="color: grey">(19c059d)</span></li>
<li>libs/odf/KoDirectoryStore.cpp <span style="color: grey">(c893d47)</span></li>
<li>libs/odf/KoEncryptedStore.h <span style="color: grey">(0edd892)</span></li>
<li>libs/odf/KoEncryptedStore.cpp <span style="color: grey">(315df1a)</span></li>
<li>libs/odf/KoStore.h <span style="color: grey">(dadecd1)</span></li>
<li>libs/odf/KoStore.cpp <span style="color: grey">(fd42378)</span></li>
<li>libs/odf/KoStore_p.h <span style="color: grey">(2e518c1)</span></li>
<li>libs/odf/KoTarStore.h <span style="color: grey">(d99f09b)</span></li>
<li>libs/odf/KoTarStore.cpp <span style="color: grey">(6829f34)</span></li>
<li>libs/odf/KoZipStore.h <span style="color: grey">(90ffcb0)</span></li>
<li>libs/odf/KoZipStore.cpp <span style="color: grey">(4235134)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/117626/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>