<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/128817/">https://git.reviewboard.kde.org/r/128817/</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have not compiled nor run it. Simply visual inspection.</p></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/128817/diff/2/?file=475983#file475983line217" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/plugins/csvimport/csvdialog.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">216</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">amount</span> <span class="o">=</span> <span class="n">MyMoneyMoney</span><span class="p">(</span><span class="s">"0"</span> <span class="o">+</span> <span class="n"><span class="hl">m_wiz</span></span><span class="o"><span class="hl">-></span></span><span class="n"><span class="hl">m_</span>decimalSymbol</span> <span class="o">+</span> <span class="s">"00"</span><span class="p">);</span> <span class="c1">// both fields are empty and zero so set amount to zero</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">196</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">amount</span> <span class="o">=</span> <span class="n">MyMoneyMoney</span><span class="p">(</span><span class="s">"0"</span> <span class="o">+</span> <span class="n">decimalSymbol</span> <span class="o">+</span> <span class="s">"00"</span><span class="p">);</span> <span class="c1">// both fields are empty and zero so set amount to zero</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">amount = MyMoneyMoney();</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">does exactly the same and is much easier.</p></pre>
</div>
</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/128817/diff/2/?file=475985#file475985line622" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/plugins/csvimport/csvwizard.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">584</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">newTxt</span> <span class="o">=</span> <span class="n">m_parse</span><span class="o">-></span><span class="n">possiblyReplaceSymbol</span><span class="p">(</span><span class="n">txt</span><span class="p">);</span> <span class="c1">// update data</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">573</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QString</span> <span class="n">pattern</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">QLocale</span><span class="p">().</span><span class="n">currencySymbol</span><span class="p">());</span> <span class="c1">// strip of all signs, currency symbols, decimal symbols, and whitespace</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Don't we know about a lot more currency symbols in the engine (MyMoneyFile::instance()->currencyList())? The method would be tradingSymbol(). See KMyMoneyView::loadDefaultCurrencies() for the list of known currencies. Why do some of them are hard coded here?</p></pre>
</div>
</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/128817/diff/2/?file=475985#file475985line625" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/plugins/csvimport/csvwizard.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">587</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ui</span><span class="o">-></span><span class="n">tableWidget</span><span class="o">-></span><span class="n">item</span><span class="p">(</span><span class="n">row</span><span class="p">,</span> <span class="n">col</span><span class="p">)</span><span class="o">-></span><span class="n">setForeground</span><span class="p">(</span><span class="n">m_colorBrushText</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">576</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">re</span><span class="p">.</span><span class="n">exactMatch</span><span class="p">(</span><span class="n">txt</span><span class="p">))</span> <span class="c1">// if string is pure numerical then go forward...</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Doesn't that match an empty txt as well?</p></pre>
</div>
</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/128817/diff/2/?file=475985#file475985line2245" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/plugins/csvimport/csvwizard.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1995</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">CompletionPage</span><span class="o">::</span><span class="n">slotImportValid</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">1890</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">bool</span> <span class="n">FormatsPage</span><span class="o">::</span><span class="n">isComplete</span><span class="p">()</span> <span class="k">const</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Can't this be written as</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">bool FormatsPage::isComplete() const
{
const bool enable = m_isDecimalSymbolOK && m_isDateFormatOK;
wizard()->button(QWizard::CustomButton2)->setEnabled(enable);
wizard()->button(QWizard::CustomButton3)->setEnabled(enable);
return enable;
}</p></pre>
</div>
</div>
<br />
<p>- Thomas Baumgart</p>
<br />
<p>On September 4th, 2016, 8:29 nachm. CEST, Łukasz Wojniłowicz wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KMymoney.</div>
<div>By Łukasz Wojniłowicz.</div>
<p style="color: grey;"><i>Updated Sept. 4, 2016, 8:29 nachm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kmymoney
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I actually decided to get rid of completion page and restructure LinesDate page. In my opinion it's more logical now and better structured for new feature I introduced ie. calculate fee column on fly.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">1) shortened updateDecimalSymbol and renamed it to
validateDecimalSymbol,
2) introduced detectDecimalSymbol for autodetecting decimal symbol,
3) simplified createStatement,
4) simplified and deduplicated slotImportClicked,
5) new decimal symbol option 'auto' for columns with various decimal
symbols,
6) decimal symbol and date format get checked every time formats page is
being opened,
7) removed completion page,
8) introduced rows page after separator page which is usefull for
calculate fee
9) introduced formats page which groups all validity checks in one
place,
10) checking securities existence is now part of
InvestmentPage::validatePage(),
11) validateDateFormat operates on tableWidget and not on m_lines so it
can be faster,
12) renamed page labels so they represent the content better,
13) Rows and Formats page have layout equal to Separator page, so look is more consistent.</p></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>kmymoney/plugins/csvimport/CMakeLists.txt <span style="color: grey">(216c79c)</span></li>
<li>kmymoney/plugins/csvimport/completionwizardpage.ui <span style="color: grey">(616e0c3)</span></li>
<li>kmymoney/plugins/csvimport/csvdialog.h <span style="color: grey">(59e6898)</span></li>
<li>kmymoney/plugins/csvimport/csvdialog.cpp <span style="color: grey">(ed9e1d8)</span></li>
<li>kmymoney/plugins/csvimport/csvwizard.h <span style="color: grey">(ecec5b0)</span></li>
<li>kmymoney/plugins/csvimport/csvwizard.cpp <span style="color: grey">(b576dea)</span></li>
<li>kmymoney/plugins/csvimport/csvwizard.ui <span style="color: grey">(723e128)</span></li>
<li>kmymoney/plugins/csvimport/investprocessing.h <span style="color: grey">(902f9bb)</span></li>
<li>kmymoney/plugins/csvimport/investprocessing.cpp <span style="color: grey">(f050441)</span></li>
<li>kmymoney/plugins/csvimport/separatorwizardpage.ui <span style="color: grey">(2ad5abd)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128817/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>