> On Jan. 1, 2014, 4:15 p.m., Thomas Baumgart wrote: > > kmymoney/wizards/newloanwizard/keditloanwizard.cpp, line 470 > > <https://git.reviewboard.kde.org/r/114767/diff/1/?file=228526#file228526line470> > > > > Think about using "void institutionList(QList<MyMoneyInstitution>& > > list) const" here. Avoids another copy operation. > > Jeremy Whiting wrote: > I copied this bit of code directly from KNewAccountDlg.cpp, I'll update > my patch, but that code I copied from ought to be fixed also probably.
Yes. It's not wrong per se, but new code should use the method I mentioned. > On Jan. 1, 2014, 4:15 p.m., Thomas Baumgart wrote: > > kmymoney/wizards/newloanwizard/loanattributeswizardpage.cpp, line 84 > > <https://git.reviewboard.kde.org/r/114767/diff/1/?file=228530#file228530line84> > > > > Better use > > > > if(dlg->exec() && dlg != 0) > > > > here. See http://blogs.kde.org/node/3919 for details. > > Jeremy Whiting wrote: > Noted. > > I do have one other question about this task. I can't seem to get the > institutions to be sorted in the combobox alphabetically. I set the > insertPolicy of the combobox to be alphabetical, yet the entries appear in > the order in which they appear in the xml file I guess. The institution list > in the KNewAccountDlg is sorted alphabetically, but I don't see what's > sorting it. Any ideas? Check NewAccountWizard::InstitutionPage::slotLoadWidgets() which contains a qsort() statement. KNewAccountDlg::slotLoadInstitutions(const QString& name) should probably also receive the sorting feature. - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114767/#review46544 ----------------------------------------------------------- On Dec. 31, 2013, 6:37 p.m., Jeremy Whiting wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/114767/ > ----------------------------------------------------------- > > (Updated Dec. 31, 2013, 6:37 p.m.) > > > Review request for KMymoney. > > > Bugs: 257619 > http://bugs.kde.org/show_bug.cgi?id=257619 > > > Repository: kmymoney > > > Description > ------- > > Add ability to modify loan institution attribute. > > Note this is definitely a work in progress, the QWizardPage title > "Attributes" is not best probably, and the page order doesn't make much sense > either imo. It probably should go after the radio buttons to ask what you > want to change. I'd like to add a fourth option to just modify attributes if > that makes sense to everyone, since it's a bit cumbersome to recalculate > interest, payoff, etc. just to change the institution the loan belongs to. > > > Diffs > ----- > > kmymoney/wizards/newloanwizard/CMakeLists.txt > 939d569c2bba56ccfd3c9d6b28f8b90d414b9332 > kmymoney/wizards/newloanwizard/keditloanwizard.cpp > 5590a359ffc16c92fc3dc24b6484aac1180b1507 > kmymoney/wizards/newloanwizard/knewloanwizard.h > 6b3f049912605f4f9cb0e78ee83b14e2767586b8 > kmymoney/wizards/newloanwizard/knewloanwizarddecl.ui > 831d85ce388b51a904c76cd998f54ca6a099b347 > kmymoney/wizards/newloanwizard/loanattributeswizardpage.h PRE-CREATION > kmymoney/wizards/newloanwizard/loanattributeswizardpage.cpp PRE-CREATION > kmymoney/wizards/newloanwizard/loanattributeswizardpagedecl.ui PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/114767/diff/ > > > Testing > ------- > > It works here to set and to show the current institution. > > > Thanks, > > Jeremy Whiting > >
_______________________________________________ KMyMoney-devel mailing list KMyMoney-devel@kde.org https://mail.kde.org/mailman/listinfo/kmymoney-devel
