diff --git a/src/collection_notifications.cpp b/src/collection_notifications.cpp index cec46d73..59c835ad 100644 --- a/src/collection_notifications.cpp +++ b/src/collection_notifications.cpp @@ -146,17 +146,7 @@ void CollectionChangeBuilder::merge(CollectionChangeBuilder&& c) insertions.erase_at(c.deletions); insertions.insert_at(c.insertions); - // Look for moves which are now no-ops, and remove them plus the associated - // insert+delete. Note that this isn't just checking for from == to due to - // that rows can also be shifted by other inserts and deletes - IndexSet to_remove; - moves.erase(remove_if(begin(moves), end(moves), [&](auto const& move) { - if (move.from - deletions.count(0, move.from) != move.to - insertions.count(0, move.to)) - return false; - deletions.remove(move.from); - insertions.remove(move.to); - return true; - }), end(moves)); + clean_up_stale_moves(); modifications.erase_at(c.deletions); modifications.shift_for_insert_at(c.insertions); @@ -166,6 +156,20 @@ void CollectionChangeBuilder::merge(CollectionChangeBuilder&& c) verify(); } +void CollectionChangeBuilder::clean_up_stale_moves() +{ + // Look for moves which are now no-ops, and remove them plus the associated + // insert+delete. Note that this isn't just checking for from == to due to + // that rows can also be shifted by other inserts and deletes + moves.erase(remove_if(begin(moves), end(moves), [&](auto const& move) { + if (move.from - deletions.count(0, move.from) != move.to - insertions.count(0, move.to)) + return false; + deletions.remove(move.from); + insertions.remove(move.to); + return true; + }), end(moves)); +} + void CollectionChangeBuilder::modify(size_t ndx) { modifications.add(ndx); diff --git a/src/collection_notifications.hpp b/src/collection_notifications.hpp index 95c1fe04..019ee907 100644 --- a/src/collection_notifications.hpp +++ b/src/collection_notifications.hpp @@ -86,6 +86,7 @@ public: bool sort); void merge(CollectionChangeBuilder&&); + void clean_up_stale_moves(); void insert(size_t ndx, size_t count=1); void modify(size_t ndx); diff --git a/src/impl/list_notifier.cpp b/src/impl/list_notifier.cpp index fd6a4ffe..5d69f4e3 100644 --- a/src/impl/list_notifier.cpp +++ b/src/impl/list_notifier.cpp @@ -101,7 +101,6 @@ void ListNotifier::run() end(m_change.moves)); for (size_t i = 0; i < m_lv->size(); ++i) { - // FIXME: may need to mark modifications even for inserts (for moves?) if (m_change.insertions.contains(i) || m_change.modifications.contains(i)) continue; if (m_info->row_did_change(m_lv->get_target_table(), m_lv->get(i).get_index())) diff --git a/src/impl/transact_log_handler.cpp b/src/impl/transact_log_handler.cpp index 19617c14..87c18c25 100644 --- a/src/impl/transact_log_handler.cpp +++ b/src/impl/transact_log_handler.cpp @@ -445,6 +445,14 @@ public: change->modify(row); } + void parse_complete() + { + for (auto& list : m_info.lists) + { + list.changes->clean_up_stale_moves(); + } + } + bool select_link_list(size_t col, size_t row, size_t) { mark_dirty(row, col); diff --git a/tests/collection_change_indices.cpp b/tests/collection_change_indices.cpp index dacaefad..2fac5fa9 100644 --- a/tests/collection_change_indices.cpp +++ b/tests/collection_change_indices.cpp @@ -295,6 +295,13 @@ TEST_CASE("[collection_change] move()") { c.move(15, 10); REQUIRE_MOVES(c, {5, 11}, {15, 10}); } + + SECTION("collapses redundant swaps of adjacent rows to a no-op") { + c.move(7, 8); + c.move(7, 8); + c.clean_up_stale_moves(); + REQUIRE(c.empty()); + } } TEST_CASE("[collection_change] calculate() unsorted") {