[Kmymoney-devel] Review Request: Highlight unwanted rows instead of deleting them on import. Improve display to avoid clipping height of rows. Standardise height of wizard pages so table widget height doesn't change. Don't move position of 'Back' button between pages. Check for valid numeric entries on investment import.

[Cristian Oneț]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106749/#review20108
-----------------------------------------------------------

Ship it!


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.


kmymoney/plugins/csvimport/csvdialog.cpp
<http://git.reviewboard.kde.org/r/106749/#comment15939>

    Is this intentional (removing 'rs' from 'Headers')?



kmymoney/plugins/csvimport/csvdialog.cpp
<http://git.reviewboard.kde.org/r/106749/#comment15940>

    ditto to what? since 'the above' was removed...



kmymoney/plugins/csvimport/csvdialog.cpp
<http://git.reviewboard.kde.org/r/106749/#comment15941>

    If you really need these magic numbers please give them some names.



kmymoney/plugins/csvimport/csvdialog.cpp
<http://git.reviewboard.kde.org/r/106749/#comment15942>

    Can this be removed? I really don't like commented code :).


- Cristian Oneț


On Oct. 6, 2012, 8:11 p.m., Allan Anderson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106749/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2012, 8:11 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   kmymoney/plugins/csvimport/bankingwizardpage.ui 168a197 
>   kmymoney/plugins/csvimport/completionwizardpage.ui 3a44e24 
>   kmymoney/plugins/csvimport/csvdialog.h 7d25407 
>   kmymoney/plugins/csvimport/csvdialog.cpp f767b79 
>   kmymoney/plugins/csvimport/csvdialog.ui 6bdfb1b 
>   kmymoney/plugins/csvimport/csvutil.h fb47100 
>   kmymoney/plugins/csvimport/csvutil.cpp c2ea436 
>   kmymoney/plugins/csvimport/introwizardpage.ui d36fd9f 
>   kmymoney/plugins/csvimport/investmentdlg.h 406a2c9 
>   kmymoney/plugins/csvimport/investmentdlg.cpp 2419aeb 
>   kmymoney/plugins/csvimport/investmentwizardpage.ui bd86e1f 
>   kmymoney/plugins/csvimport/investprocessing.h 8aea482 
>   kmymoney/plugins/csvimport/investprocessing.cpp 985d302 
>   kmymoney/plugins/csvimport/lines-datewizardpage.ui f748f2c 
>   kmymoney/plugins/csvimport/redefinedlg.cpp 0e3d9a0 
>   kmymoney/plugins/csvimport/redefinedlgdecl.ui f6847c8 
>   kmymoney/plugins/csvimport/separatorwizardpage.ui 85bb618 
>   kmymoney/plugins/csvimport/symboltabledlg.h d8ea95b 
>   kmymoney/plugins/csvimport/symboltabledlg.cpp b047f3c 
> 
> Diff: http://git.reviewboard.kde.org/r/106749/diff/
> 
> 
> Testing
> -------
> 
> Extensive testing with numerous data files of differing numbers of rows and columns, and different column widths.
> 
> 
> Thanks,
> 
> Allan Anderson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20121009/b1c1ce52/attachment-0001.html>