----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122227/#review74744 -----------------------------------------------------------
Ship it! I tested this over the weekend. It fixes the problem originally addressed and seems to fix the crash I was having. - Christian Mollekopf 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 > >
