<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/108913/">http://git.reviewboard.kde.org/r/108913/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The xmlWriter & mainStyles parameters pre-date the introduction of OdfSavingContext (and KoShapeSavingContext), so yeah, I think it is a good idea to clean this up a bit.</pre>
<br />
<p>- Marijn</p>
<br />
<p>On February 11th, 2013, 8:20 p.m. UTC, Friedrich W. H. Kossebau wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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 and Marijn Kruisselbrink.</div>
<div>By Friedrich W. H. Kossebau.</div>
<p style="color: grey;"><i>Updated Feb. 11, 2013, 8:20 p.m.</i></p>
<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;">It confused me when I read the code of saving a Sheet document to ODF:
"KoXmlWriter& xmlWriter" and "KoGenStyles &mainStyles" are passed explicitly as parameters to Sheet::saveOdfColRowCell(...), Sheet::saveOdfCells(...), and Cell::saveOdf(...), even though they are already members of the also passed "OdfSavingContext& tableContext". By just looking at the method it seemed that xmlWriter and mainStyles were different objects to those which are part of tableContext, which has such objects via the shapeContext member. Only when going up the callstack it could be found that these references are just duplicates.
This confusion seems also the reason why in Cell::saveOdf(...) a shapeContext was recreated with these objects, instead of using the shapeContext of tableContext.
Attached patch changes the signatures of those methods to only pass "OdfSavingContext& tableContext", like also done in similar methods, and to simply create helper references to xmlWriter and mainStyles in the method bodies, as needed. It also removes the unneeded additional creation of a shapeContext.
Resulting code should be less confusing :)
(Came across the code during of my currently done big tuning of KoOdfWriteStore)</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;">all Sheets tests pass as before, richtext in cell roundtrip also worked</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>sheets/Sheet.cpp <span style="color: grey">(1b11a6f)</span></li>
<li>sheets/Sheet.h <span style="color: grey">(9d9f1a7)</span></li>
<li>sheets/Cell.cpp <span style="color: grey">(fef7846)</span></li>
<li>sheets/Cell.h <span style="color: grey">(015e847)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108913/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>