From a50cb5f6e7969183c69b56578ad63cb61fe06227 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 3 Dec 2022 16:50:29 +0100 Subject: Share logic for undo/redo selection changes I will suggest a few changes to this code. No functional change. --- src/context.cc | 59 ++++++++++++++++++++-------------------------------------- 1 file changed, 20 insertions(+), 39 deletions(-) (limited to 'src/context.cc') diff --git a/src/context.cc b/src/context.cc index fe9bd346..3446c2e4 100644 --- a/src/context.cc +++ b/src/context.cc @@ -216,8 +216,10 @@ void Context::SelectionHistory::end_edition() m_staging.reset(); } +template void Context::SelectionHistory::undo() { + static constexpr bool backward = direction == Backward; if (in_edition()) throw runtime_error("selection undo is only supported at top-level"); kak_assert(not empty()); @@ -226,43 +228,24 @@ void Context::SelectionHistory::undo() kak_assert(current_history_node().selections == m_staging->selections); end_edition(); }); - HistoryId parent = current_history_node().parent; - if (parent == HistoryId::Invalid) - throw runtime_error("no selection change to undo"); - auto select_parent = [&, parent] { - HistoryId before_undo = m_history_id; - m_history_id = parent; - current_history_node().redo_child = before_undo; - m_staging = current_history_node(); - }; - if (&history_node(parent).selections.buffer() == &m_context.buffer()) - select_parent(); + HistoryId next; + if constexpr (backward) + next = current_history_node().parent; else - m_context.change_buffer(history_node(parent).selections.buffer(), { std::move(select_parent) }); - // }); -} - -void Context::SelectionHistory::redo() -{ - if (in_edition()) - throw runtime_error("selection redo is only supported at top-level"); - kak_assert(not empty()); - begin_edition(); - auto end = on_scope_end([&] { - kak_assert(current_history_node().selections == m_staging->selections); - end_edition(); - }); - HistoryId child = current_history_node().redo_child; - if (child == HistoryId::Invalid) - throw runtime_error("no selection change to redo"); - auto select_child = [&, child] { - m_history_id = child; + next = current_history_node().redo_child; + if (next == HistoryId::Invalid) + throw runtime_error(backward ? "no selection change to undo" : "no selection change to redo"); + auto select_next = [&, next] { + HistoryId previous_id = m_history_id; + m_history_id = next; + if constexpr (backward) + current_history_node().redo_child = previous_id; m_staging = current_history_node(); }; - if (&history_node(child).selections.buffer() == &m_context.buffer()) - select_child(); + if (&history_node(next).selections.buffer() == &m_context.buffer()) + select_next(); else - m_context.change_buffer(history_node(child).selections.buffer(), { std::move(select_child) }); + m_context.change_buffer(history_node(next).selections.buffer(), { std::move(select_next) }); } void Context::SelectionHistory::forget_buffer(Buffer& buffer) @@ -376,15 +359,13 @@ SelectionList& Context::selections(bool update) return m_selection_history.selections(update); } +template void Context::undo_selection_change() { - m_selection_history.undo(); -} - -void Context::redo_selection_change() -{ - m_selection_history.redo(); + m_selection_history.undo(); } +template void Context::undo_selection_change(); +template void Context::undo_selection_change(); SelectionList& Context::selections_write_only() { -- cgit v1.2.3 From 02d0584e0f29abab29a0397f582c0c521c6c0470 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 3 Dec 2022 16:52:08 +0100 Subject: Extract variable in selection undo No functional change. --- src/context.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/context.cc') diff --git a/src/context.cc b/src/context.cc index 3446c2e4..2afa278a 100644 --- a/src/context.cc +++ b/src/context.cc @@ -242,10 +242,11 @@ void Context::SelectionHistory::undo() current_history_node().redo_child = previous_id; m_staging = current_history_node(); }; - if (&history_node(next).selections.buffer() == &m_context.buffer()) + Buffer& destination_buffer = history_node(next).selections.buffer(); + if (&destination_buffer == &m_context.buffer()) select_next(); else - m_context.change_buffer(history_node(next).selections.buffer(), { std::move(select_next) }); + m_context.change_buffer(destination_buffer, { std::move(select_next) }); } void Context::SelectionHistory::forget_buffer(Buffer& buffer) -- cgit v1.2.3 From 016e1be77fd34610cae082fd8d90e846ed8af28e Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 3 Dec 2022 12:36:33 +0100 Subject: Extract variable and add comment in selection change recording No functional change. --- src/context.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'src/context.cc') diff --git a/src/context.cc b/src/context.cc index 2afa278a..568142c6 100644 --- a/src/context.cc +++ b/src/context.cc @@ -204,9 +204,11 @@ void Context::SelectionHistory::end_edition() if (m_history_id != HistoryId::Invalid and current_history_node().selections == m_staging->selections) { - auto& sels = m_history[(size_t)m_history_id].selections; - sels.force_timestamp(m_staging->selections.timestamp()); - sels.set_main_index(m_staging->selections.main_index()); + // No change, except maybe the index of the main selection. + // Update timestamp to potentially improve interaction with content undo. + auto& node = current_history_node(); + node.selections.force_timestamp(m_staging->selections.timestamp()); + node.selections.set_main_index(m_staging->selections.main_index()); } else { -- cgit v1.2.3 From 8427379a5dfcf35af35fd5a7b26803e8727476e6 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Thu, 22 Dec 2022 18:09:45 +0100 Subject: Tweak selection-undo interaction with WinDisplay hooks Each selection undo operation is surrounded by pair of begin_edition()/end_edition() calls. The original reason for adding these was that in one of my preliminary versions, a WinDisplay hook could break an undo chain, even if the hook did not affect selections at all. This has since been fixed. By surrounding the undo with begin_edition()/end_edition(), try to ensure that any selection modification that happens in a WinDisplay hook would not break the undo chain. Essentially this means that, after using to undo a buffer change, this was meant to make sure that could redo that buffer change. However, it turns out this actually doesn't work. The attached test case triggers an assertion. As described in the first paragraph, the only real-world motivation for this is gone, so let's simplify the behavior. The assertion fix means that we can test the next commit better. --- src/context.cc | 6 ------ 1 file changed, 6 deletions(-) (limited to 'src/context.cc') diff --git a/src/context.cc b/src/context.cc index 568142c6..3a0aadb4 100644 --- a/src/context.cc +++ b/src/context.cc @@ -225,11 +225,6 @@ void Context::SelectionHistory::undo() if (in_edition()) throw runtime_error("selection undo is only supported at top-level"); kak_assert(not empty()); - begin_edition(); - auto end = on_scope_end([&] { - kak_assert(current_history_node().selections == m_staging->selections); - end_edition(); - }); HistoryId next; if constexpr (backward) next = current_history_node().parent; @@ -242,7 +237,6 @@ void Context::SelectionHistory::undo() m_history_id = next; if constexpr (backward) current_history_node().redo_child = previous_id; - m_staging = current_history_node(); }; Buffer& destination_buffer = history_node(next).selections.buffer(); if (&destination_buffer == &m_context.buffer()) -- cgit v1.2.3 From 516759bb2fd95d02134bc84130bf9060e0354310 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Thu, 22 Dec 2022 18:09:45 +0100 Subject: Make selection undo skip over entries that are nop after buffer change After buffer modification - in particular after deletion - adjacent selection history entries may correspond to the same effective selection when applied to the current buffer. This means that we sometimes need to press multiple times to make one visible change. This is not what the user expects, so let's keep walking the selection history until we hit an actual change. Alternatively, we could minimize the selection history after buffer changes but I think that would make the it worse after content undo+redo. --- src/context.cc | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) (limited to 'src/context.cc') diff --git a/src/context.cc b/src/context.cc index 3a0aadb4..0c1b8aad 100644 --- a/src/context.cc +++ b/src/context.cc @@ -225,24 +225,29 @@ void Context::SelectionHistory::undo() if (in_edition()) throw runtime_error("selection undo is only supported at top-level"); kak_assert(not empty()); + SelectionList old_selections = selections(); HistoryId next; - if constexpr (backward) - next = current_history_node().parent; - else - next = current_history_node().redo_child; - if (next == HistoryId::Invalid) - throw runtime_error(backward ? "no selection change to undo" : "no selection change to redo"); - auto select_next = [&, next] { - HistoryId previous_id = m_history_id; - m_history_id = next; + do + { if constexpr (backward) - current_history_node().redo_child = previous_id; - }; - Buffer& destination_buffer = history_node(next).selections.buffer(); - if (&destination_buffer == &m_context.buffer()) - select_next(); - else - m_context.change_buffer(destination_buffer, { std::move(select_next) }); + next = current_history_node().parent; + else + next = current_history_node().redo_child; + if (next == HistoryId::Invalid) + throw runtime_error(backward ? "no selection change to undo" : "no selection change to redo"); + auto select_next = [&, next] { + HistoryId previous_id = m_history_id; + m_history_id = next; + if constexpr (backward) + current_history_node().redo_child = previous_id; + }; + Buffer& destination_buffer = history_node(next).selections.buffer(); + if (&destination_buffer == &m_context.buffer()) + select_next(); + else + m_context.change_buffer(destination_buffer, { std::move(select_next) }); + } + while (selections() == old_selections); } void Context::SelectionHistory::forget_buffer(Buffer& buffer) -- cgit v1.2.3