<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/106749/">http://git.reviewboard.kde.org/r/106749/</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;">Otherwise it look good. I haven't tried to build the patch but I think you can safely push changes that are in the cvsimport plugin since you are the author of it.</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/106749/diff/1/?file=88556#file88556line36" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/plugins/csvimport/csvdialog.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; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">36</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">// QT Heade<span class="hl">rs</span></span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">36</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">// QT Heade</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;">Is this intentional (removing 'rs' from 'Headers')?</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/106749/diff/1/?file=88556#file88556line215" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/plugins/csvimport/csvdialog.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 CSVDialog::init()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">207</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_page<span class="hl">Investment</span></span><span class="o">-></span><span class="n">ui</span><span class="o">-></span><span class="n">comboBox<span class="hl">Inv_fee</span>Col</span><span class="o">-></span><span class="n">setCurrentIndex</span><span class="p">(</span><span class="o">-</span><span class="mi">1</span><span class="p">);</span> <span class="c1">// <span class="hl"> This col might not get selected, so clear it</span></span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">213</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_page<span class="hl">Banking</span></span><span class="o">-></span><span class="n">ui</span><span class="o">-></span><span class="n">comboBox<span class="hl">Bnk_memo</span>Col</span><span class="o">-></span><span class="n">setCurrentIndex</span><span class="p">(</span><span class="o">-</span><span class="mi">1</span><span class="p">);</span> <span class="c1">// <span class="hl">ditto</span></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;">ditto to what? since 'the above' was removed...</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/106749/diff/1/?file=88556#file88556line250" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/plugins/csvimport/csvdialog.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 CSVDialog::init()</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">242</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setMinimumHeight</span><span class="p">(</span><span class="mi">595</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;">If you really need these magic numbers please give them some names.</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/106749/diff/1/?file=88556#file88556line3756" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/plugins/csvimport/csvdialog.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; ">bool LinesDatePage::validatePage()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">3234</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_dlg</span><span class="o">-></span><span class="n">m_importError</span> <span class="o">=</span> <span class="kc">false</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">3563</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="hl"> </span><span class="c1"><span class="hl">//</span> m_dlg->m_importError = false;<span class="hl"> Needed in completion()</span></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;">Can this be removed? I really don't like commented code :).</pre>
</div>
<br />
<p>- Cristian</p>
<br />
<p>On October 6th, 2012, 8:11 p.m., Allan Anderson 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 KMymoney.</div>
<div>By Allan Anderson.</div>
<p style="color: grey;"><i>Updated Oct. 6, 2012, 8:11 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;">Firstly, an apology regarding the size of this change. The initial intent was to highlight any header or trailer rows chosen by the user to be excluded from the import, instead of deleting them, in order to improve confidence that all was as intended. This change was not major. However, during testing, I began more and more to dislike the untidy way the ui appeared, with top and/or bottom rows of the table widget possibly having their height clipped. Unfortunately, this turned out to involve far more than I expected, for instance when dealing with differing column widths within the same file, with the horizontal scrollbar, when present, clipping rows. Also, when moving between wizard pages, the pages had differing heights, and this too affected the table appearance.
Also, a small change was made to fix the position of the 'Back' button on the wizard, so it wasn't shifted when other buttons appeared or disappeared.
Then, it was discovered that there were some difficulties in dealing with field delimiter selection changes. The best way to deal with this turned out to be to determine the appropriate delimiter to use.
Finally, another small change, in the investment wizard, where the validity of numeric fields was not checked during import, but only when the decimal symbol was selected.
</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;">Extensive testing with numerous data files of differing numbers of rows and columns, and different column widths.</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/bankingwizardpage.ui <span style="color: grey">(168a197)</span></li>
<li>kmymoney/plugins/csvimport/completionwizardpage.ui <span style="color: grey">(3a44e24)</span></li>
<li>kmymoney/plugins/csvimport/csvdialog.h <span style="color: grey">(7d25407)</span></li>
<li>kmymoney/plugins/csvimport/csvdialog.cpp <span style="color: grey">(f767b79)</span></li>
<li>kmymoney/plugins/csvimport/csvdialog.ui <span style="color: grey">(6bdfb1b)</span></li>
<li>kmymoney/plugins/csvimport/csvutil.h <span style="color: grey">(fb47100)</span></li>
<li>kmymoney/plugins/csvimport/csvutil.cpp <span style="color: grey">(c2ea436)</span></li>
<li>kmymoney/plugins/csvimport/introwizardpage.ui <span style="color: grey">(d36fd9f)</span></li>
<li>kmymoney/plugins/csvimport/investmentdlg.h <span style="color: grey">(406a2c9)</span></li>
<li>kmymoney/plugins/csvimport/investmentdlg.cpp <span style="color: grey">(2419aeb)</span></li>
<li>kmymoney/plugins/csvimport/investmentwizardpage.ui <span style="color: grey">(bd86e1f)</span></li>
<li>kmymoney/plugins/csvimport/investprocessing.h <span style="color: grey">(8aea482)</span></li>
<li>kmymoney/plugins/csvimport/investprocessing.cpp <span style="color: grey">(985d302)</span></li>
<li>kmymoney/plugins/csvimport/lines-datewizardpage.ui <span style="color: grey">(f748f2c)</span></li>
<li>kmymoney/plugins/csvimport/redefinedlg.cpp <span style="color: grey">(0e3d9a0)</span></li>
<li>kmymoney/plugins/csvimport/redefinedlgdecl.ui <span style="color: grey">(f6847c8)</span></li>
<li>kmymoney/plugins/csvimport/separatorwizardpage.ui <span style="color: grey">(85bb618)</span></li>
<li>kmymoney/plugins/csvimport/symboltabledlg.h <span style="color: grey">(d8ea95b)</span></li>
<li>kmymoney/plugins/csvimport/symboltabledlg.cpp <span style="color: grey">(b047f3c)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106749/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>