> On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote: > > kmymoney/dialogs/investactivities.cpp, line 394 > > <http://git.reviewboard.kde.org/r/112947/diff/1/?file=192722#file192722line394> > > > > does the ending '///' mean anything? if this is a placeholder to search > > for maybe a textual comment would be better suited > > Allan Anderson wrote: > Oops. Yes, it's a placeholder, that I use to search for changes I've > made. I tend not to use it much nowadays. I should have removed it, but it > was getting very late and I slipped up. There may be one or two others as > well. > > Cristian Oneț wrote: > Yes there are other I only signaled this one, no need to rush with the > patch :).
This still needs to be fixed (remove markers or add a better comment). > On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote: > > kmymoney/dialogs/investactivities.cpp, line 701 > > <http://git.reviewboard.kde.org/r/112947/diff/1/?file=192722#file192722line701> > > > > Please run astyle-kmymoney.sh before pushing the commit. > > Allan Anderson wrote: > I did have it the 'correct' way at first, but noticed that there was 'if > (w) w->hide()' a few lines above, so changed it because it looked 'wrong' in > that context. I'll change them both. I usually use {} even for a one-liner, > but don't change existing code. There are too many. This still needs to be fixed - run astyle-kmymoney.sh. > On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote: > > kmymoney/dialogs/investtransactioneditor.cpp, line 528 > > <http://git.reviewboard.kde.org/r/112947/diff/1/?file=192724#file192724line528> > > > > Could this: > > if (cond) {} else if (cond) {} > > be written in a more readable manner? I would write something like: > > if (cond) {} else {} > > since all activity types are either in one or the other codition. > > Allan Anderson wrote: > In general, then that is what I would do. I decided not to do it here > because it provided immediate information on what was dealt with where, with > there being so many different activity types. > However, I could make it a comment instead, but I would like to leave the > information there, for clarity. > Perhaps also remove the existing comment above which doesn't seem to > apply any more? > > Cristian Oneț wrote: > Please feel free to remove any comments which are not valid anymore (the > same applies for commented code). This needs to be solved. I do not agree that each Activity should be checked individually on the else block. - Cristian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112947/#review40849 ----------------------------------------------------------- On Sept. 26, 2013, 12:03 a.m., Allan Anderson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112947/ > ----------------------------------------------------------- > > (Updated Sept. 26, 2013, 12:03 a.m.) > > > Review request for KMymoney. > > > Bugs: 322768 > http://bugs.kde.org/show_bug.cgi?id=322768 > > > Repository: kmymoney > > > Description > ------- > > - Interest category disappears - > Steps to Reproduce: > 1.Open a new Dividend transaction. > 2.Enter an interest category and amount. > 3.Enter a new fee category. > 4.On accepting the new category, the interest category and amount have been > cleared. > One line fix in kmymoney/dialogs/transactioneditor.cpp. > > - Fixes for KEditWidget visibility issues. > When editing an investment transaction, interest or fee edit widgets show or > hide incorrectly. This is a fairly long-standing issue, and there have been > attempts to fix by delaying the show() or hide() instructions. This became > more pronounced during work to allow column resizing, and Cristian produced a > fix for the root cause. This fix is included here. > With the above fix in place, it became necessary to revert the delayed show() > and hide() calls, and this has been done. > Of course, nothing is ever as simple as that, and another couple of issues > emerged. Whether or not an interest or fee amount widget is shown depends on > the presence or absence of the associated category's text. It transpired > that if, say, an existing Reinvest transaction was edited to be, say a Buy > transaction, the text from the Reinvest interest category was seen by the > new Buy entry and resulted in the interest-amount widget being visible when > none should appear. Similarly, if a transaction with a fee set is edited to > be a type with no fee expected, for instance, an Add or RemoveShares, then > the fee-amount widget became visible when not needed. > It was necessary to rework the slotUpdateFeeVisibility() and > slotUpdateInterestVisibility() functions to take account of the new > transaction type. > > > Diffs > ----- > > kmymoney/dialogs/investactivities.cpp 50f33ed > kmymoney/dialogs/investtransactioneditor.h 3e62c2a > kmymoney/dialogs/investtransactioneditor.cpp e9f87fb > kmymoney/dialogs/transactioneditor.cpp 39049cf > kmymoney/widgets/transactioneditorcontainer.h f77b195 > > Diff: http://git.reviewboard.kde.org/r/112947/diff/ > > > Testing > ------- > > Entering and editing various transaction types to ensure only the appropriate > widgets became visible or hidden when required. > > > Thanks, > > Allan Anderson > >
_______________________________________________ KMyMoney-devel mailing list KMyMoney-devel@kde.org https://mail.kde.org/mailman/listinfo/kmymoney-devel
