----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128817/#review98964 -----------------------------------------------------------
I have not compiled nor run it. Simply visual inspection. kmymoney/plugins/csvimport/csvdialog.cpp (line 196) <https://git.reviewboard.kde.org/r/128817/#comment66627> amount = MyMoneyMoney(); does exactly the same and is much easier. kmymoney/plugins/csvimport/csvwizard.cpp (line 573) <https://git.reviewboard.kde.org/r/128817/#comment66628> 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? kmymoney/plugins/csvimport/csvwizard.cpp (line 576) <https://git.reviewboard.kde.org/r/128817/#comment66629> Doesn't that match an empty txt as well? kmymoney/plugins/csvimport/csvwizard.cpp (line 1890) <https://git.reviewboard.kde.org/r/128817/#comment66631> Can't this be written as 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; } - Thomas Baumgart On Sept. 4, 2016, 8:29 nachm., Łukasz Wojniłowicz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128817/ > ----------------------------------------------------------- > > (Updated Sept. 4, 2016, 8:29 nachm.) > > > Review request for KMymoney. > > > Repository: kmymoney > > > Description > ------- > > 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. > > 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. > > > Diffs > ----- > > kmymoney/plugins/csvimport/CMakeLists.txt 216c79c > kmymoney/plugins/csvimport/completionwizardpage.ui 616e0c3 > kmymoney/plugins/csvimport/csvdialog.h 59e6898 > kmymoney/plugins/csvimport/csvdialog.cpp ed9e1d8 > kmymoney/plugins/csvimport/csvwizard.h ecec5b0 > kmymoney/plugins/csvimport/csvwizard.cpp b576dea > kmymoney/plugins/csvimport/csvwizard.ui 723e128 > kmymoney/plugins/csvimport/investprocessing.h 902f9bb > kmymoney/plugins/csvimport/investprocessing.cpp f050441 > kmymoney/plugins/csvimport/separatorwizardpage.ui 2ad5abd > > Diff: https://git.reviewboard.kde.org/r/128817/diff/ > > > Testing > ------- > > > Thanks, > > Łukasz Wojniłowicz > >
