----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129657/#review101464 -----------------------------------------------------------
Ship it! Thanks for working on this! I noticed that I could remove the change in beginRemoveRows without any unit test failing. Please add this test on top of your patch: diff --git a/autotests/kselectionproxymodeltest.cpp b/autotests/kselectionproxymodeltest.cpp index e17fa53..68c8678 100644 --- a/autotests/kselectionproxymodeltest.cpp +++ b/autotests/kselectionproxymodeltest.cpp @@ -611,6 +611,13 @@ void KSelectionProxyModelTest::removeRows_data() << 1 << QStringList{"9", "9"} << 2; ++testNumber; + + QTest::newRow(QByteArray("test" + QByteArray::number(testNumber)).data()) + << static_cast<int>(kspm_mode) << connectSelectionModelFirst << false + << QStringList{"6", "8", "11"} << 4 + << 0 + << QStringList{"8", "8"} << 4; + ++testNumber; } } - Stephen Kelly On Dec. 15, 2016, 11:15 p.m., David Faure wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129657/ > ----------------------------------------------------------- > > (Updated Dec. 15, 2016, 11:15 p.m.) > > > Review request for KDE Frameworks and Stephen Kelly. > > > Repository: kitemmodels > > > Description > ------- > > After > int proxyEndRemove = proxyStartRemove; > proxyEndRemove += rc; was adding 0 (empty root) > and then --proxyEndRemove; was making us end up with proxyEndRemove < > proxyStartRemove. > > > Diffs > ----- > > autotests/kselectionproxymodeltest.cpp > 483e7c42dabab6aa560622ff0418ee7f90363e15 > src/kselectionproxymodel.cpp 0f57c70f05d4cbde9b14f4257bff1365ef5443f6 > > Diff: https://git.reviewboard.kde.org/r/129657/diff/ > > > Testing > ------- > > New unittest + unchecking calendar in korganizer no longer asserts. > > > Thanks, > > David Faure > >