> On Jan. 25, 2015, 10:11 p.m., Milian Wolff wrote: > > kdeui/tests/krecursivefilterproxymodeltest.cpp, line 26 > > <https://git.reviewboard.kde.org/r/122227/diff/1/?file=344391#file344391line26> > > > > is this still required in qt5? this should really be added upstream, > > imo...
I don't see this line anywhere in the Qt5 autotests, so it looks like Stephen's metatype changes made it unnecessary. In any case, this review is for Qt4 code, for now :) > On Jan. 25, 2015, 10:11 p.m., Milian Wolff wrote: > > kdeui/tests/krecursivefilterproxymodeltest.cpp, line 90 > > <https://git.reviewboard.kde.org/r/122227/diff/1/?file=344391#file344391line90> > > > > style: { on newline? fixed - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122227/#review74725 ----------------------------------------------------------- On Jan. 23, 2015, 6:11 p.m., David Faure wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122227/ > ----------------------------------------------------------- > > (Updated Jan. 23, 2015, 6:11 p.m.) > > > Review request for kdelibs and Christian Mollekopf. > > > Repository: kdelibs > > > Description > ------- > > 1) setData(false), i.e. a dataChanged that removes and item from the filter, > didn't actually lead to removal. The code was only looking at changing to > get in, not changing to get out. > > 2) On insertion, we can avoid emitting dataChanged up the chain, by > finding out before the insertion which exact ancestor will be changed > (lastHiddenAscendantForInsert). > > 3) On removal, well simplify the code (completeRemove was always true, unless > ignoreRemove was set, so we only need to keep ignoreRemove), and avoid > emitting dataChanged up the chain, by finding out which the last parent > before one that should still be visible, and hide just that one. > > 4) While at it, an obvious optimization that could have been done > since day one: filterAcceptsRow can return true as soon as a child wants > to be shown. > > > Diffs > ----- > > kdeui/itemviews/krecursivefilterproxymodel.cpp > efa286ad87ded962b20c8a581b659d1b154ebf3a > kdeui/tests/krecursivefilterproxymodeltest.cpp > 3bcb72980730cb22f887ae8fa5fbd91b5609aeb6 > > Diff: https://git.reviewboard.kde.org/r/122227/diff/ > > > Testing > ------- > > Unittest, obviously. > + KMail smoke testing. > > > Thanks, > > David Faure > >
