<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/104626/">http://git.reviewboard.kde.org/r/104626/</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;">I like it very much, and I found only a few issues. Before we merge this however, I think it should be made public in a branch sopeople can test build it beforehand. After all there may be parts of the code that you don't build and thus hasn't found.</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="http://git.reviewboard.kde.org/r/104626/diff/1/?file=56917#file56917line50" style="color: black; font-weight: bold; text-decoration: underline;">libs/kopageapp/dialogs/KoPAConfigureDialog.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; ">KoPAConfigureDialog::KoPAConfigureDialog(KoPAView* parent)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">50</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">connect</span><span class="p">(</span><span class="n">m_miscPage</span><span class="p">,</span> <span class="n">SIGNAL</span><span class="p">(</span><span class="n">unitChanged</span><span class="p">(</span><span class="kt"><span class="hl">in</span>t</span><span class="p">)),</span> <span class="n">m_gridPage</span><span class="p">,</span> <span class="n">SLOT</span><span class="p">(</span><span class="n">slotUnitChanged</span><span class="p">(</span><span class="kt"><span class="hl">in</span>t</span><span class="p">)));</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">50</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">connect</span><span class="p">(</span><span class="n">m_miscPage</span><span class="p">,</span> <span class="n">SIGNAL</span><span class="p">(</span><span class="n">unitChanged</span><span class="p">(</span><span class="n"><span class="hl">KoUni</span>t</span><span class="p">)),</span> <span class="n">m_gridPage</span><span class="p">,</span> <span class="n">SLOT</span><span class="p">(</span><span class="n">slotUnitChanged</span><span class="p">(</span><span class="n"><span class="hl">KoUni</span>t</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;">uhm is this correct?</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="http://git.reviewboard.kde.org/r/104626/diff/1/?file=56924#file56924line155" style="color: black; font-weight: bold; text-decoration: underline;">libs/koreport/wrtembed/reportscene.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 ReportScene::contextMenuEvent(QGraphicsSceneContextMenuEvent * e)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">151</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">KoUnit</span><span class="o">::</span><span class="n">unitName</span><span class="p">(</span><span class="n">m_unit</span><span class="p">)</span> <span class="o">!=</span> <span class="n">KoUnit</span><span class="o">::</span><span class="n">unitName</span><span class="p">(</span><span class="n">m_rd</span><span class="o">-></span><span class="n">pageUnit</span><span class="p">()))</span> <span class="p">{</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">155</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// </span><span class="cs">TODO</span><span class="c1">: Again? Copy&Paste error?</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;">uhm no, at least it doesn't look like it. i mean it has just be set one line above</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="http://git.reviewboard.kde.org/r/104626/diff/1/?file=56935#file56935line80" style="color: black; font-weight: bold; text-decoration: underline;">libs/odf/KoUnit.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></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">80</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">enum</span> <span class="n">ListFilter</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;">Maybe make this into QFlags, if we want as you Say to have imperial vs metric too;
btw for the ui list it may be an idea to have a SortImperialFirst which we can set depending of locale (just an idea)</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="http://git.reviewboard.kde.org/r/104626/diff/1/?file=56938#file56938line119" style="color: black; font-weight: bold; text-decoration: underline;">libs/widgets/KoUnitDoubleSpinBox.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; ">QValidator::State KoUnitDoubleSpinBox::validate(QString &input, int &pos) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">119</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KoUnit</span> <span class="n">unit</span> <span class="o">=</span> <span class="n">KoUnit</span><span class="o">::</span><span class="n"><span class="hl">unit</span></span><span class="p">(</span> <span class="n">unitName</span><span class="p">,</span> <span class="o">&</span><span class="n">ok</span> <span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">119</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="hl"> </span><span class="k"><span class="hl">const</span></span> <span class="n">KoUnit</span> <span class="n">unit</span> <span class="o">=</span> <span class="n">KoUnit</span><span class="o">::</span><span class="n"><span class="hl">fromSymbol</span></span><span class="p">(</span> <span class="n">unitName</span><span class="p">,</span> <span class="o">&</span><span class="n">ok</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;">in the future when editing a line with wrong hacking style please take the oppotunity to fix that too</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="http://git.reviewboard.kde.org/r/104626/diff/1/?file=56946#file56946line449" style="color: black; font-weight: bold; text-decoration: underline;">sheets/part/dialogs/PreferenceDialog.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 PreferenceDialog::slotReset()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">448</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KoUnit</span> <span class="n">unit</span><span class="p">((</span><span class="n">KoUnit</span><span class="o">::</span><span class="n">Unit</span><span class="p">)</span><span class="n">d</span><span class="o">-></span><span class="n">interfaceOptions</span><span class="p">.</span><span class="n">m_unit</span><span class="o">-></span><span class="n">itemData</span><span class="p">(</span><span class="n">index</span><span class="p">).</span><span class="n">toInt</span><span class="p">());</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">442</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">const</span> <span class="n">KoUnit</span> <span class="n">unit</span> <span class="o">=</span> <span class="n">KoUnit</span><span class="o">::</span><span class="n">fromListForUi</span><span class="p">(</span><span class="n">index</span><span class="p">,</span> <span class="n">KoUnit</span><span class="o">::</span><span class="n">ListAll</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;">Hmmm do we really want pixel listed in Sheets?</pre>
</div>
<br />
<p>- C.</p>
<br />
<p>On April 16th, 2012, 9:27 p.m., Friedrich W. H. Kossebau 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 and C. Boemann.</div>
<div>By Friedrich W. H. Kossebau.</div>
<p style="color: grey;"><i>Updated April 16, 2012, 9:27 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;">Sigh... so the proposal from https://git.reviewboard.kde.org/r/104607/ from boemann
"I'd say fix the code in KoUnit so it reports the unit in the order you like, and use KoUnit everywhere to define the order"
turned into quite some hacking and in the end I found myself redoing the KoUnit API partially (because the old confused me too often).
So let's have some feedback on the current state, to see what is welcome and what is not, and what else could/should be done :)
Changed:
* rename KoUnit::unit(...) to KoUnit::fromSymbol(...) <- more Qt'ish
* turn static KoUnit::unitName(KoUnit) into member method KoUnit::symbol() <- as typical use-case is on existing KoUnit instance, also shorter and more OOed
* rename KoUnit::Unit to KoUnit::Type <- "type" feels a better term here
* added KoUnit::type() and KoUnit::setFactor(...) <- useful in a few places
* remove KoUnit::unitDescription(...) from API <- not used outside
* rename KoUnit::PixelVisibility to KoUnit::ListFilter <- more general, some might want to add other flags like HideNoneMetrics
Fixes:
* ensure the same order of unit types in all unit type selectors in the UI
* update the page layout dialog on a change of the document's unit property
* update the changeUnitActions on a change of the document's unit property
</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;">Played with (hopefully) all touched widgets, seems to work.</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>filters/karbon/image/ImageExportOptionsWidget.cpp <span style="color: grey">(2b5d541)</span></li>
<li>karbon/ui/KarbonPart.cpp <span style="color: grey">(5e6a958)</span></li>
<li>krita/plugins/extensions/imagesize/dlg_imagesize.cc <span style="color: grey">(96490c9)</span></li>
<li>krita/plugins/tools/defaulttools/kis_tool_measure.cc <span style="color: grey">(5e9afa3)</span></li>
<li>krita/ui/widgets/kis_custom_image_widget.cc <span style="color: grey">(5b00fb9)</span></li>
<li>libs/kopageapp/KoPADocument.cpp <span style="color: grey">(240171a)</span></li>
<li>libs/kopageapp/dialogs/KoPAConfigureDialog.cpp <span style="color: grey">(8912db3)</span></li>
<li>libs/koproperty/editors/spinbox.cpp <span style="color: grey">(d923c6e)</span></li>
<li>libs/koreport/common/KoReportItemBase.cpp <span style="color: grey">(7f6a575)</span></li>
<li>libs/koreport/common/krsectiondata.cpp <span style="color: grey">(38c14c8)</span></li>
<li>libs/koreport/wrtembed/KoReportDesigner.cpp <span style="color: grey">(68a61f9)</span></li>
<li>libs/koreport/wrtembed/KoReportDesignerItemBase.cpp <span style="color: grey">(f3ff8dc)</span></li>
<li>libs/koreport/wrtembed/KoReportDesignerItemLine.cpp <span style="color: grey">(ce54e7c)</span></li>
<li>libs/koreport/wrtembed/reportscene.cpp <span style="color: grey">(dd32f7a)</span></li>
<li>libs/koreport/wrtembed/reportsection.cpp <span style="color: grey">(52447cf)</span></li>
<li>libs/main/KoDocument.h <span style="color: grey">(66bf3ac)</span></li>
<li>libs/main/KoDocument.cpp <span style="color: grey">(831ed9d)</span></li>
<li>libs/main/KoRuler.cpp <span style="color: grey">(4272b9b)</span></li>
<li>libs/main/KoView.cpp <span style="color: grey">(81dafd3)</span></li>
<li>libs/main/KoView_p.h <span style="color: grey">(9b3dff2)</span></li>
<li>libs/main/config/KoConfigGridPage.h <span style="color: grey">(01373e9)</span></li>
<li>libs/main/config/KoConfigGridPage.cpp <span style="color: grey">(a3e1d6f)</span></li>
<li>libs/main/config/KoConfigMiscPage.h <span style="color: grey">(2c36996)</span></li>
<li>libs/main/config/KoConfigMiscPage.cpp <span style="color: grey">(7f54ef1)</span></li>
<li>libs/odf/KoUnit.h <span style="color: grey">(1f035fe)</span></li>
<li>libs/odf/KoUnit.cpp <span style="color: grey">(43cc908)</span></li>
<li>libs/widgets/KoPageLayoutWidget.cpp <span style="color: grey">(c9f0fc0)</span></li>
<li>libs/widgets/KoUnitDoubleSpinBox.cpp <span style="color: grey">(f9f00da)</span></li>
<li>plugins/paragraphtool/Ruler.cpp <span style="color: grey">(3053696)</span></li>
<li>plugins/textshape/dialogs/ParagraphBulletsNumbers.cpp <span style="color: grey">(87c1b85)</span></li>
<li>sheets/DocBase.cpp <span style="color: grey">(a9812c6)</span></li>
<li>sheets/dialogs/LayoutDialog.cpp <span style="color: grey">(d1090f4)</span></li>
<li>sheets/part/Doc.cpp <span style="color: grey">(4c40b87)</span></li>
<li>sheets/part/HeaderItems.cpp <span style="color: grey">(5fc3cfa)</span></li>
<li>sheets/part/HeaderWidgets.cpp <span style="color: grey">(3fac4cc)</span></li>
<li>sheets/part/dialogs/PreferenceDialog.cpp <span style="color: grey">(fae954a)</span></li>
<li>words/part/KWApplicationConfig.cpp <span style="color: grey">(b5fd980)</span></li>
<li>words/part/KWOdfLoader.cpp <span style="color: grey">(238c7fe)</span></li>
<li>words/part/dialogs/KWPageSettingsDialog.h <span style="color: grey">(0a028a9)</span></li>
<li>words/part/dialogs/KWPageSettingsDialog.cpp <span style="color: grey">(f577353)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104626/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>