<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/103152/">http://git.reviewboard.kde.org/r/103152/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 17th, 2011, 12:33 p.m., <b>Jarosław Staniek</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/103152/diff/2/?file=41211#file41211line103" style="color: black; font-weight: bold; text-decoration: underline;">libs/flake/KoLineBorder.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">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">103</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="n">paintBorder</span><span class="p">(</span><span class="n">KoShape</span> <span class="o">*</span><span class="n">shape</span><span class="p">,</span> <span class="n">QPainter</span> <span class="o">&</span><span class="n">painter</span><span class="p">,</span> <span class="k">const</span> <span class="n">QPen</span> <span class="o">&</span><span class="n">pen</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;">A small proposal. Can we start using passing via QPainter* recommended by Qt API design guidelines? This change would have to be adopted by other methods in this class and elsewhere but complying with the guideline in the new code would reduce amount of work in the future.
The same note applies to other methods you added in this patch.</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 think we should not start to use 2 different ways in our code. If we want to change it then we should to do it in one go. However I think using a QPainter& makes more sense as you don't need to check for it as if you would pass a pointer as it always expects a object to be there.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 17th, 2011, 12:33 p.m., <b>Jarosław Staniek</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/103152/diff/2/?file=41214#file41214line92" style="color: black; font-weight: bold; text-decoration: underline;">libs/flake/KoMarker.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 KoMarker::saveOdf(KoShapeSavingContext &context) 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">92</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QString</span> <span class="n">name</span> <span class="o">=</span> <span class="n">QString</span><span class="p">(</span><span class="n">QUrl</span><span class="o">::</span><span class="n">toPercentEncoding</span><span class="p">(</span><span class="n">d</span><span class="o">-></span><span class="n">name</span><span class="p">,</span> <span class="s">""</span><span class="p">,</span> <span class="s">" "</span><span class="p">)).</span><span class="n">replace</span><span class="p">(</span><span class="sc">'%'</span><span class="p">,</span> <span class="sc">'_'</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;">I am afraid this expression does not give unique name. To make it unique one would have to escape '_' character as well.</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;">Not sure I understand you comment here. This just makes sure that only valid characters are used in the identifier. The uniqness of the name is done one line down when inserting it into the styles.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 17th, 2011, 12:33 p.m., <b>Jarosław Staniek</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/103152/diff/2/?file=41215#file41215line37" style="color: black; font-weight: bold; text-decoration: underline;">libs/flake/KoMarkerCollection.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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">37</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">virtual</span> <span class="o">~</span><span class="n">KoMarkerCollection</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;">no need for virtual here...</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;">As the base class has already a virtual destructor the class automatically gets a virtual destructor. And I prefer to also show that in the code.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 17th, 2011, 12:33 p.m., <b>Jarosław Staniek</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/103152/diff/2/?file=41227#file41227line229" style="color: black; font-weight: bold; text-decoration: underline;">libs/flake/KoShapeSavingContext.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">QString KoShapeSavingContext::imageHref(QImage &image)</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">229</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">QString</span> <span class="n">KoShapeSavingContext</span><span class="o">::</span><span class="n">markerRef</span><span class="p">(</span><span class="k">const</span> <span class="n">KoMarker</span> <span class="o">*</span><span class="n">marker</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;">const KoMarker &</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'm interested in the pointer here.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 17th, 2011, 12:33 p.m., <b>Jarosław Staniek</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/103152/diff/2/?file=41227#file41227line232" style="color: black; font-weight: bold; text-decoration: underline;">libs/flake/KoShapeSavingContext.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">QString KoShapeSavingContext::imageHref(QImage &image)</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">232</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">it</span> <span class="o">==</span> <span class="n">d</span><span class="o">-></span><span class="n">markerRefs</span><span class="p">.</span><span class="n">end</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;">it's easier to use: !d->markerRefs.contains(marker)</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;">Using !d->markerRefs.contains(marker) would mean an additional lookup in the hash which is avoided by the above code. So this code is faster then using contains.</pre>
<br />
<p>- Thorsten</p>
<br />
<p>On November 16th, 2011, 5:51 a.m., Thorsten Zachmann wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/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 Thorsten Zachmann.</div>
<p style="color: grey;"><i>Updated Nov. 16, 2011, 5:51 a.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;">At the sprint it was suggested to have this feature/bug fix in 2.4. Therefore positing for review. It adds one new string.
This add support for markers to calligra.
Loading, saving, manipulation, work without problems
At the moment the document loaded need to have markers so that the user can select a marker on a path shape. I'm working on adding markers per default to the docker at the moment.</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 various documents with markers and they all worked without problems. Tested manipulation of path shapes that have a marker attached.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=260421">260421</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=260423">260423</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=260431">260431</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>libs/flake/CMakeLists.txt <span style="color: grey">(5face45)</span></li>
<li>libs/flake/KoDocumentResourceManager.h <span style="color: grey">(2d6c8c0)</span></li>
<li>libs/flake/KoLineBorder.h <span style="color: grey">(7f7d088)</span></li>
<li>libs/flake/KoLineBorder.cpp <span style="color: grey">(a2f0645)</span></li>
<li>libs/flake/KoMarker.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/flake/KoMarker.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/flake/KoMarkerCollection.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/flake/KoMarkerCollection.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/flake/KoMarkerData.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/flake/KoMarkerData.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/flake/KoMarkerSharedLoadingData.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/flake/KoMarkerSharedLoadingData.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/flake/KoPathShape.h <span style="color: grey">(5b5b91b)</span></li>
<li>libs/flake/KoPathShape.cpp <span style="color: grey">(1898862)</span></li>
<li>libs/flake/KoPathShapeFactory.cpp <span style="color: grey">(f3a98b8)</span></li>
<li>libs/flake/KoPathShape_p.h <span style="color: grey">(e1e2843)</span></li>
<li>libs/flake/KoShapeLoadingContext.cpp <span style="color: grey">(9d3d1da)</span></li>
<li>libs/flake/KoShapeSavingContext.h <span style="color: grey">(a06e040)</span></li>
<li>libs/flake/KoShapeSavingContext.cpp <span style="color: grey">(34f55c8)</span></li>
<li>libs/flake/commands/KoPathShapeMarkerCommand.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/flake/commands/KoPathShapeMarkerCommand.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/widgets/CMakeLists.txt <span style="color: grey">(473a264)</span></li>
<li>libs/widgets/KoMarkerItemDelegate.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/widgets/KoMarkerItemDelegate.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/widgets/KoMarkerModel.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/widgets/KoMarkerModel.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/widgets/KoMarkerSelector.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/widgets/KoMarkerSelector.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/widgets/KoStrokeConfigWidget.h <span style="color: grey">(3ae44b9)</span></li>
<li>libs/widgets/KoStrokeConfigWidget.cpp <span style="color: grey">(9b2532f)</span></li>
<li>marker_todo.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/dockers/strokedocker/StrokeDocker.h <span style="color: grey">(dde8db7)</span></li>
<li>plugins/dockers/strokedocker/StrokeDocker.cpp <span style="color: grey">(82f18b5)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103152/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>