<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/128511/">https://git.reviewboard.kde.org/r/128511/</a>
     </td>
    </tr>
   </table>
   <br />




<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 July 31, 2016, 6:14 p.m.</i></p>





<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Summary (updated)</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;">Major rewrite of CSV Importer part 1</pre>
  </td>
 </tr>
</table>





<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  (updated)</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;">First diff:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Purpose of displayLine in tableWidget shouldn't differ whether it's
investment or banking statement, so take of responsibilities of parsing
data into columns and creating memo field from displayLine and put them
into separate functions.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In csvdialog.cpp and investprocessing.cpp, I introduced createMemoField (it simplifies memo concatenating and allows to remove some redundant boolean variables), which in both places looks the same. I think it will be easy to move it into csvwizard.cpp after trying to move e.g. readFile there, so that will be next target.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Second diff:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">csvdialog.cpp and investprocessing.cpp have methods duplicated, 
complicated and not structured for easy maintenance and development.
Due to this methods and variables were simplified, unified, 
deduplicated and put in main file i.e. csvwizard.cpp
New/rewritten methods:
    "readfile" - loads file into buffer and gets its boundaries
    "displayLines" - for displaying lines from buffer in table
    "getMaxColumnCount" - for detecting field delimiter
    "validateDateFormat" - for validating date
    "createMemoField" - for creating memo out of copied columns
    "startLineChanged" - moved to csvwizard.cpp
    "endLineChanged" - moved to csvwizard.cpp
    "dateFormatSelected" - moved to csvwizard.cpp
    "readSettings" - for reading settings and setting variables only</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Sorry for this patch for being so big, but changing something in CSV importer code was kind of domino effect. The code is so unstructured that it makes no sense making it step by step, so I was editing obviously wrong code on the spot. I decided to publish it now because all began to work on new codepath. 
I think the next step could be rewriting parsers for investment and banking lines which look very obfuscated to me.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Banking and investment statement CSV imports; with and without setup.</p></pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>kmymoney/plugins/csvimport/completionwizardpage.ui <span style="color: grey">(ddc064b)</span></li>

 <li>kmymoney/plugins/csvimport/csvdialog.h <span style="color: grey">(69cca6e)</span></li>

 <li>kmymoney/plugins/csvimport/csvdialog.cpp <span style="color: grey">(fa70b04)</span></li>

 <li>kmymoney/plugins/csvimport/csvutil.h <span style="color: grey">(e27199b)</span></li>

 <li>kmymoney/plugins/csvimport/csvutil.cpp <span style="color: grey">(9c0929d)</span></li>

 <li>kmymoney/plugins/csvimport/csvwizard.h <span style="color: grey">(28eea62)</span></li>

 <li>kmymoney/plugins/csvimport/csvwizard.cpp <span style="color: grey">(7aee196)</span></li>

 <li>kmymoney/plugins/csvimport/investmentdlg.cpp <span style="color: grey">(b68da8e)</span></li>

 <li>kmymoney/plugins/csvimport/investprocessing.h <span style="color: grey">(38f622c)</span></li>

 <li>kmymoney/plugins/csvimport/investprocessing.cpp <span style="color: grey">(328a79b)</span></li>

 <li>kmymoney/plugins/csvimport/lines-datewizardpage.ui <span style="color: grey">(d844da5)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/128511/diff/" style="margin-left: 3em;">View Diff</a></p>






  </td>
 </tr>
</table>



  </div>
 </body>
</html>