<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 />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 7th, 2016, 8:07 vorm. CEST, <b>Thomas Baumgart</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="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>
<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>
</blockquote>
<p>On September 7th, 2016, 8:28 nachm. CEST, <b>Łukasz Wojniłowicz</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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 use d following code to harvest potential currencies.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"> QList<span style="color: #666666"><</span>MyMoneyAccount<span style="color: #666666">></span> accountList;
MyMoneyFile<span style="color: #666666">::</span>instance()<span style="color: #666666">-></span>accountList(accountList);
QList<span style="color: #666666"><</span>MyMoneyAccount<span style="color: #666666">::</span>accountTypeE<span style="color: #666666">></span> accountTypes;
accountTypes <span style="color: #666666"><<</span> MyMoneyAccount<span style="color: #666666">::</span>Checkings <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Savings <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Liability <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Checkings <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Savings <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Cash <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>CreditCard <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Loan <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Asset <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Liability;
QString filteredCurrencies;
QList<span style="color: #666666"><</span>MyMoneyAccount<span style="color: #666666">>::</span>ConstIterator account;
<span style="color: #008000; font-weight: bold">for</span> (account <span style="color: #666666">=</span> accountList.cbegin(); account <span style="color: #666666">!=</span> accountList.cend(); <span style="color: #666666">++</span>account)
<span style="color: #008000; font-weight: bold">if</span> (accountTypes.contains((<span style="color: #666666">*</span>account).accountType()) <span style="color: #666666">&&</span> <span style="color: #408080; font-style: italic">// account must acctually have currency property</span>
<span style="color: #666666">!</span>filteredCurrencies.contains((<span style="color: #666666">*</span>account).currencyId())) <span style="color: #408080; font-style: italic">// currency must not be on the list already</span>
filteredCurrencies <span style="color: #666666">+=</span> (<span style="color: #666666">*</span>account).currencyId();
</pre></div>
</p></pre>
</blockquote>
<p>On September 7th, 2016, 8:36 nachm. CEST, <b>Łukasz Wojniłowicz</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">Disregard previous response. It should be this.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"> QList<span style="color: #666666"><</span>MyMoneyAccount<span style="color: #666666">></span> accountList;
MyMoneyFile<span style="color: #666666">::</span>instance()<span style="color: #666666">-></span>accountList(accountList);
QList<span style="color: #666666"><</span>MyMoneyAccount<span style="color: #666666">::</span>accountTypeE<span style="color: #666666">></span> accountTypes;
accountTypes <span style="color: #666666"><<</span> MyMoneyAccount<span style="color: #666666">::</span>Checkings <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Savings <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Liability <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Checkings <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Savings <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Cash <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>CreditCard <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Loan <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Asset <span style="color: #666666"><<</span>
MyMoneyAccount<span style="color: #666666">::</span>Liability;
QString filteredCurrencies;
QList<span style="color: #666666"><</span>MyMoneyAccount<span style="color: #666666">>::</span>ConstIterator account;
<span style="color: #008000; font-weight: bold">for</span> (account <span style="color: #666666">=</span> accountList.cbegin(); account <span style="color: #666666">!=</span> accountList.cend(); <span style="color: #666666">++</span>account) {
<span style="color: #008000; font-weight: bold">if</span> (accountTypes.contains((<span style="color: #666666">*</span>account).accountType()) <span style="color: #666666">&&</span> <span style="color: #408080; font-style: italic">// account must actually have currency property</span>
<span style="color: #666666">!</span>filteredCurrencies.contains((<span style="color: #666666">*</span>account).currencyId(), Qt<span style="color: #666666">::</span>CaseInsensitive)) { <span style="color: #408080; font-style: italic">// currency id must not be on the list already</span>
filteredCurrencies <span style="color: #666666">+=</span> (<span style="color: #666666">*</span>account).currencyId();
QString tradingSymbol <span style="color: #666666">=</span> MyMoneyFile<span style="color: #666666">::</span>instance()<span style="color: #666666">-></span>currency((<span style="color: #666666">*</span>account).currencyId()).tradingSymbol();
<span style="color: #008000; font-weight: bold">if</span> (<span style="color: #666666">!</span>filteredCurrencies.contains(tradingSymbol, Qt<span style="color: #666666">::</span>CaseInsensitive )) <span style="color: #408080; font-style: italic">// currency trading symbol must not be on the list already</span>
filteredCurrencies <span style="color: #666666">+=</span> tradingSymbol;
}
}
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think it's better to remove currency symbols based on what user has in his accounts, than to hunt for over 180 currency symbols/ids from (MyMoneyFile::instance()->currencyList()).</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'd use a QSet<QString> to add all trading symbols w/o the hassle of checking if it already exists. QSet does not create duplicates. At the end</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QSet<QString> symbols;</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">// do the collection of the symbols over the accounts here</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QString filteredCurrencies = QStringList(symbols.values()).join("");</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Should make the code more readable.</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 7th, 2016, 8:07 vorm. CEST, <b>Thomas Baumgart</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="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>
<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>
</blockquote>
<p>On September 7th, 2016, 8:28 nachm. CEST, <b>Łukasz Wojniłowicz</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">Yes it does and couple of lines above is my recipe for it</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"> QString txt <span style="color: #666666">=</span> ui<span style="color: #666666">-></span>tableWidget<span style="color: #666666">-></span>item(row, col)<span style="color: #666666">-></span>text();
<span style="color: #008000; font-weight: bold">if</span> (txt.isEmpty()) <span style="color: #408080; font-style: italic">// nothing to process, so go to next row</span>
<span style="color: #008000; font-weight: bold">continue</span>;
</pre></div>
</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Never mind. I did not see that. Then it's not a problem.</p></pre>
<br />
<p>- Thomas</p>
<br />
<p>On September 7th, 2016, 8:31 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. 7, 2016, 8:31 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/formatswizardpage.ui <span style="color: grey">(PRE-CREATION)</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/lines-datewizardpage.ui <span style="color: grey">(84c1c91)</span></li>
<li>kmymoney/plugins/csvimport/rowswizardpage.ui <span style="color: grey">(PRE-CREATION)</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>