Refactor KMyMoney to use just AmountEdit
Details
- Reviewers
tbaumgart - Group Reviewers
KMyMoney - Commits
- R261:8841ae39d15b: Change KMyMoneyEdit to AmountEdit
"make test" and explore the interface
Diff Detail
- Repository
- R261 KMyMoney
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
I just did a visual inspection of the code and have not compiled nor tested it.
kmymoney/dialogs/kmymoneysplittable.cpp | ||
---|---|---|
955 | This can be improved to new AmountEdit(nullptr) or better new AmountEdit | |
kmymoney/dialogs/knewaccountdlg.cpp | ||
426 | Better widget->setText(QString()); | |
kmymoney/dialogs/knewequityentrydlg.cpp | ||
70 | Better d->ui->edtFraction->setText(QLatin1String("100")); | |
kmymoney/dialogs/stdtransactioneditor.cpp | ||
1010 | Better setText(QString()) | |
1012 | Better setText(QString()) | |
1029 | see above | |
1031 | see above | |
kmymoney/kmymoney.cpp | ||
3559–3560 | This just duplicates the previous line. Remove it. | |
kmymoney/widgets/kguiutils.cpp | ||
237 | This does not work :( because AmountEdit::text() returns something different than KMyMoneyEdit::text(). AmountEdit::text() is identical to KMyMoneyEdit::lineedit()->text() I think if (!(qobject_cast<AmountEdit*>(widget))->value().isZero()) { could do the trick. | |
kmymoney/wizards/newaccountwizard/kloandetailspage.cpp | ||
131 | This might have some unwanted influence on the results which I cannot oversee right now. Please make sure to do thorough testing of the loan calculations. | |
210 | Instead of MyMoneyMoney(0) use MyMoneyMoney(). Any MyMoneyMoney object is initialized to zero by default. | |
219 | see above, also next line | |
242 | Why don't you use d->ui->m_balloonAmount->setValue(val); directly? There's no need to create a local copy just for the setValue() call. | |
259–260 | see above, also next lines | |
270–271 | Again, why don't you use val directly now that it is a MyMoneyMoney object? | |
kmymoney/wizards/newloanwizard/keditloanwizard.cpp | ||
138–139 | m_nameEdit is not of type AmountEdit. Why did you change it? |
Cool. This looks good. I have noticed one anomaly which I have not figured out why it happens.
Using master (w/o your change), I open the ledger view and start creating a new transaction using the form at the bottom of the view. I simply TAB through until I reach the amount field. The result is: all characters in the amount field are selected. So pressing any number replaces what is in there. This is the current behavior and OK.
Applying D20887 the cursor is located to the right of the amount and the existing characters are not selected. Pressing any number does not replace the 0,00 in the edit field.
What is more astonishing: Selecting Tools/Prices, selecting a price and pressing the Modify button and then TAB into the amount field selects the amount as I expect it. Strange.
We need to figure out what is going on here and fix it. I expect the old transaction editor to cause this as it also works OK with the new one (needs -DENABLE_UNFINISHEDFEATURES=ON). Can you check all other locations where this widget is used, please?
Let's move forward with this one. Except this one instance I did not see any problem and we can fix it. The part where the problem is located will be replaced eventually anyway. In case you don't have a dev account for the KDE repo let me know and I'll do the commit.
I tried to debug this odd behavior last weekend with no luck.
Not sure if it's good to get technical debt on this.
Hi.
you are right, let's move on. I tried a "arc land" with no success:
PUSHING Pushing changes to "origin/master".
fatal: remote error: service not enabled: /kmymoney
Usage Exception: Push failed! Fix the error and run "arc land" again.
I don't think that I have write access to kmymoney repository.