<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="http://git.reviewboard.kde.org/r/100629/">http://git.reviewboard.kde.org/r/100629/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 13th, 2011, 5:09 a.m., <b>Thorsten Zachmann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="http://git.reviewboard.kde.org/r/100629/diff/1/?file=8912#file8912line165" style="color: black; font-weight: bold; text-decoration: underline;">braindump/src/SectionsIO.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bool SectionsIO::SaveContext::saveSection(SectionsIO* sectionsIO )</pre></td>
</tr>
</tbody>
<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">165</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">not</span> <span class="n">embeddedFileSaver</span><span class="p">.</span><span class="n">saveEmbeddedFiles</span><span class="p">(</span><span class="n">documentContext</span><span class="p">))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">please don't use not use !</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I didn't put it there, i just copied the code just above it. You have to tell Cyrille to stop putting them there. The code above has since been fixed by somebody to make it compile on Solaris, I think. All this said, of course I'll remove it.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 13th, 2011, 5:09 a.m., <b>Thorsten Zachmann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="http://git.reviewboard.kde.org/r/100629/diff/1/?file=8923#file8923line213" style="color: black; font-weight: bold; text-decoration: underline;">libs/flake/KoUnavailShape.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KoUnavailShape::drawNull(QPainter &painter) const</pre></td>
</tr>
</tbody>
<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">211</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QByteArray</span> <span class="n">xmlArray</span><span class="p">(</span><span class="n">d</span><span class="o">-></span><span class="n">frameContents</span><span class="p">.</span><span class="n">value</span><span class="p">(</span><span class="n">i</span><span class="p">));</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Any reason to copy the data here instead of using the parameters directly when used.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">To simplify the code below (the cost is not that big since it's implicitly shared). But I can remove it if you think it's too expensive.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 13th, 2011, 5:09 a.m., <b>Thorsten Zachmann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="http://git.reviewboard.kde.org/r/100629/diff/1/?file=8923#file8923line335" style="color: black; font-weight: bold; text-decoration: underline;">libs/flake/KoUnavailShape.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KoUnavailShape::drawNull(QPainter &painter) const</pre></td>
</tr>
</tbody>
<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">322</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span><span class="kt">int</span> <span class="n">j</span> <span class="o">=</span> <span class="mi">0</span><span class="p">;</span> <span class="n">j</span> <span class="o"><</span> <span class="n">manifest</span><span class="p">.</span><span class="n">size</span><span class="p">();</span> <span class="o">++</span><span class="n">j</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">how about adding a method to KoOdfLoadingContext to get the manifestEntry per path. This removes the need to iterate over all manifest entries.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I can do that, but I think we should instead create a real Manifest class that could have these methods.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 13th, 2011, 5:09 a.m., <b>Thorsten Zachmann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="http://git.reviewboard.kde.org/r/100629/diff/1/?file=8941#file8941line56" style="color: black; font-weight: bold; text-decoration: underline;">libs/odf/KoEmbeddedFileSaver.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<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">QByteArray</span> <span class="n">contents</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Please pass contents as a const & </pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">No, I don't think we should do that, and there is a good reason for it: When the call to saveFile and embedFile is done, there is no way for the KoEmbeddedFileSaver to know what the shape does with the QByteArray. The QByteArray is implicitly shared, meaning it is ref counted and by creating a copy this way I protect against the contents being deleted between the call to saveFile and when the contents is actually saved to the store. But I should really add a comment about that.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 13th, 2011, 5:09 a.m., <b>Thorsten Zachmann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="http://git.reviewboard.kde.org/r/100629/diff/1/?file=8942#file8942line82" style="color: black; font-weight: bold; text-decoration: underline;">libs/odf/KoEmbeddedFileSaver.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QString KoEmbeddedFileSaver::getFilename(const QString &prefix)</pre></td>
</tr>
</tbody>
<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">82</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">prefix</span> <span class="o">+</span> <span class="n">QString</span><span class="p">(</span><span class="s">"%1"</span><span class="p">).</span><span class="n">arg</span><span class="p">(</span><span class="n">index</span><span class="p">,</span> <span class="mi">4</span><span class="p">,</span> <span class="mi">10</span><span class="p">,</span> <span class="n">QChar</span><span class="p">(</span><span class="sc">'0'</span><span class="p">));</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">any reason to make the number at least 4 chars long?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It just felt right.</pre>
<br />
<p>- Inge</p>
<br />
<p>On February 11th, 2011, 9:36 a.m., Inge Wallin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/images/review_request_box_top_bg.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>
<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 saving in the KoUnavailShape.
The method it uses is to introduce a new class, KoEmbeddedFileSaver, in koodf. This class is a bit like the KoEmbeddedObjectSaver class, and is created and instantiated in the same places. It is also part of the KoShapeSavingContext.
As a bonus, this patch also implements saving for the Vector shape, since I used that for testing in early stages.</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;">Tested with a simple odt file with an embedded spreadsheet.</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>braindump/src/SectionsIO.cpp <span style="color: grey">(9d8949b)</span></li>
<li>karbon/common/KarbonDocument.cpp <span style="color: grey">(1a6a2a5)</span></li>
<li>kformula/flake/FormulaDocument.cpp <span style="color: grey">(84872fa)</span></li>
<li>kformula/flake/KoFormulaTool.cpp <span style="color: grey">(d9eada1)</span></li>
<li>krita/ui/flake/kis_shape_layer.cc <span style="color: grey">(f695b8a)</span></li>
<li>krita/ui/flake/kis_shape_selection.cpp <span style="color: grey">(f5e2f28)</span></li>
<li>libs/flake/KoDrag.cpp <span style="color: grey">(8ed2656)</span></li>
<li>libs/flake/KoDragOdfSaveHelper.h <span style="color: grey">(b1e6212)</span></li>
<li>libs/flake/KoDragOdfSaveHelper.cpp <span style="color: grey">(727e6c5)</span></li>
<li>libs/flake/KoShapeSavingContext.h <span style="color: grey">(a06e040)</span></li>
<li>libs/flake/KoShapeSavingContext.cpp <span style="color: grey">(fd8fb41)</span></li>
<li>libs/flake/KoUnavailShape.cpp <span style="color: grey">(3f10acf)</span></li>
<li>libs/kokross/KoScriptingOdf.cpp <span style="color: grey">(17a4f7d)</span></li>
<li>libs/kopageapp/KoPADocument.cpp <span style="color: grey">(2ef4ac1)</span></li>
<li>libs/kopageapp/KoPAOdfPageSaveHelper.h <span style="color: grey">(d8f064a)</span></li>
<li>libs/kopageapp/KoPAOdfPageSaveHelper.cpp <span style="color: grey">(c60d0d8)</span></li>
<li>libs/kopageapp/KoPAPastePage.cpp <span style="color: grey">(01bafe0)</span></li>
<li>libs/kopageapp/KoPASavingContext.h <span style="color: grey">(c678e2c)</span></li>
<li>libs/kopageapp/KoPASavingContext.cpp <span style="color: grey">(52524bf)</span></li>
<li>libs/kotext/KoTextDrag.cpp <span style="color: grey">(f30cb5c)</span></li>
<li>libs/kotext/KoTextOdfSaveHelper.h <span style="color: grey">(155aecd)</span></li>
<li>libs/kotext/KoTextOdfSaveHelper.cpp <span style="color: grey">(d012557)</span></li>
<li>libs/kotext/KoTextShapeSavingContext.h <span style="color: grey">(4d27bc4)</span></li>
<li>libs/kotext/KoTextShapeSavingContext.cpp <span style="color: grey">(d5ade20)</span></li>
<li>libs/kotext/opendocument/tests/TestChangeLoading.cpp <span style="color: grey">(ac42ddd)</span></li>
<li>libs/kotext/opendocument/tests/TestLoading.cpp <span style="color: grey">(e6a790a)</span></li>
<li>libs/main/KoDocument.cpp <span style="color: grey">(956e179)</span></li>
<li>libs/odf/CMakeLists.txt <span style="color: grey">(1a6a131)</span></li>
<li>libs/odf/KoEmbeddedDocumentSaver.cpp <span style="color: grey">(a8e13d8)</span></li>
<li>libs/odf/KoEmbeddedFileSaver.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/odf/KoEmbeddedFileSaver.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/odf/KoOdfDocument.h <span style="color: grey">(c242776)</span></li>
<li>libs/odf/KoOdfLoadingContext.h <span style="color: grey">(023b92f)</span></li>
<li>libs/odf/KoOdfLoadingContext.cpp <span style="color: grey">(ce53992)</span></li>
<li>libs/odf/KoOdfManifest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/odf/KoOdfManifest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/chartshape/ChartDocument.cpp <span style="color: grey">(f85bb19)</span></li>
<li>plugins/chartshape/ChartShape.cpp <span style="color: grey">(edce6be)</span></li>
<li>plugins/vectorshape/VectorShape.cpp <span style="color: grey">(9965d68)</span></li>
<li>tables/Cell.cpp <span style="color: grey">(7aa8306)</span></li>
<li>tables/DocBase.cpp <span style="color: grey">(5ccaef3)</span></li>
<li>words/part/KWDocument.cpp <span style="color: grey">(f3dc5d4)</span></li>
<li>words/part/KWOdfWriter.h <span style="color: grey">(4264e99)</span></li>
<li>words/part/KWOdfWriter.cpp <span style="color: grey">(b507bbf)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100629/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>