--===============8069599160370815169== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- 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(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 > > --===============8069599160370815169== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129657/

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 December 15th, 2016, 11:15 p.m. UTC, David Faure wrote:

Review request for KDE Frameworks and Stephen Kelly.
By David Faure.

Updated Dec. 15, 2016, 11:15 p.m.

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.

Testing 

New unittest + unchecking calendar in korganizer no longer asserts.

Diffs

  • autotests/kselectionproxymodeltest.cpp (483e7c42dabab6aa560622ff0418ee7f90363e15)
  • src/kselectionproxymodel.cpp (0f57c70f05d4cbde9b14f4257bff1365ef5443f6)

View Diff
--===============8069599160370815169==--