From 5901d2e06b7c43fd0e6156a49761b81b747ea908 Mon Sep 17 00:00:00 2001 From: Maxime Coste Date: Sat, 17 Jun 2023 17:30:43 +1000 Subject: [PATCH] Revert "Switch undo storage from a tree to a plain list" Moving across history moved to / to keep / for selection undo/redo This reverts commit e0d33f51b36c9f0be7ae2467dab455d211bbf561. --- README.asciidoc | 2 + doc/pages/expansions.asciidoc | 16 ++- doc/pages/keys.asciidoc | 6 + src/buffer.cc | 173 ++++++++++++++++-------- src/buffer.hh | 23 ++-- src/buffer_utils.cc | 10 ++ src/main.cc | 3 +- src/normal.cc | 25 ++++ test/compose/history/kak_quoted_history | 2 +- test/compose/history/rc | 2 +- 10 files changed, 184 insertions(+), 78 deletions(-) diff --git a/README.asciidoc b/README.asciidoc index 93b6d93e..61766773 100644 --- a/README.asciidoc +++ b/README.asciidoc @@ -540,7 +540,9 @@ Changes * ``: append command output after each selection * `u`: undo last change + * ``: move backward in history * `U`: redo last change + * ``: move forward in history * `&`: align selections, align the cursor of selections by inserting spaces before the first character of the selection diff --git a/doc/pages/expansions.asciidoc b/doc/pages/expansions.asciidoc index 6d935a3f..7bfd3894 100644 --- a/doc/pages/expansions.asciidoc +++ b/doc/pages/expansions.asciidoc @@ -285,13 +285,15 @@ The following expansions are supported (with required context _in italics_): *%val{history}*:: _in buffer, window scope_ + - the full change history of the buffer, as a list of - `committed modification0 modification1 ...` entries. - _committed_ is a count in seconds from Kakoune's steady clock's epoch of - the creation of the history entry, and each _modification_ is presented - as in `%val{uncommitted_modifications}`. - Boundaries in the list can be identified knowing that _committed_ is a - plain integer, and each _modification_ starts with `+` or `-`. + the full change history of the buffer, including undo forks, in terms + of `parent committed redo_child modification0 modification1 ...` + entries, where _parent_ is the index of the entry's predecessor (entry + 0, which is the root of the history tree, will always have `-` here), + _committed_ is a count in seconds from Kakoune's steady clock's epoch + of the creation of the history entry, _redo_child_ is the index of the + child which will be visited for `U` (always `-` at the leaves of the + history), and each _modification_ is presented as in + `%val{uncommitted_modifications}`. *%val{history_id}*:: _in buffer, window scope_ + diff --git a/doc/pages/keys.asciidoc b/doc/pages/keys.asciidoc index 84ed4260..eaadd17f 100644 --- a/doc/pages/keys.asciidoc +++ b/doc/pages/keys.asciidoc @@ -308,6 +308,12 @@ Yanking (copying) and pasting use the *"* register by default (See <*:: + move forward in changes history + +**:: + move forward in changes history + **:: undo last selection change diff --git a/src/buffer.cc b/src/buffer.cc index 8baa7423..c968690f 100644 --- a/src/buffer.cc +++ b/src/buffer.cc @@ -20,8 +20,8 @@ namespace Kakoune { -Buffer::HistoryNode::HistoryNode() - : committed{Clock::now()} +Buffer::HistoryNode::HistoryNode(HistoryId parent) + : parent{parent}, committed{Clock::now()} {} Buffer::Buffer(String name, Flags flags, BufferLines lines, @@ -31,9 +31,9 @@ Buffer::Buffer(String name, Flags flags, BufferLines lines, m_name{(flags & Flags::File) ? real_path(parse_filename(name)) : std::move(name)}, m_display_name{(flags & Flags::File) ? compact_path(m_name) : m_name}, m_flags{flags | Flags::NoUndo}, - m_history{{HistoryNode{}}}, - m_history_id{0}, - m_last_save_history_id{0}, + m_history{{HistoryId::Invalid}}, + m_history_id{HistoryId::First}, + m_last_save_history_id{HistoryId::First}, m_fs_status{fs_status} { #ifdef KAK_DEBUG @@ -114,7 +114,7 @@ bool Buffer::set_name(String name) if (m_flags & Buffer::Flags::File and not file_exists(m_name)) { m_flags |= Buffer::Flags::New; - m_last_save_history_id = InvalidHistoryId; + m_last_save_history_id = HistoryId::Invalid; } } else @@ -201,9 +201,9 @@ void Buffer::reload(BufferLines lines, ByteOrderMark bom, EolFormat eolformat, F if (not record_undo) { // Erase history about to be invalidated history - m_history_id = 0; - m_last_save_history_id = 0; - m_history = {HistoryNode{}}; + m_history_id = HistoryId::First; + m_last_save_history_id = HistoryId::First; + m_history = {HistoryNode{HistoryId::Invalid}}; m_changes.push_back({ Change::Erase, {0,0}, line_count() }); static_cast(m_lines) = std::move(lines); @@ -273,20 +273,12 @@ void Buffer::commit_undo_group() if (m_current_undo_group.empty()) return; - // walk back to current position in history, and append reverse events - for (size_t current = m_history.size() - 1; current != m_history_id ; --current) - { - m_history.emplace_back(); - m_history.back().undo_group = m_history[current].undo_group - | reverse() - | transform([](auto& modif) { return modif.inverse(); }) - | gather(); - } - m_history.emplace_back(); + const HistoryId id = next_history_id(); + m_history.push_back({m_history_id}); m_history.back().undo_group = std::move(m_current_undo_group); - m_current_undo_group.clear(); - m_history_id = m_history.size() - 1; + current_history_node().redo_child = id; + m_history_id = id; } bool Buffer::undo(size_t count) @@ -295,15 +287,15 @@ bool Buffer::undo(size_t count) commit_undo_group(); - if ((m_history_id - 1) == InvalidHistoryId) + if (current_history_node().parent == HistoryId::Invalid) return false; - while (count-- != 0 and (m_history_id - 1) != InvalidHistoryId) + while (count-- != 0 and current_history_node().parent != HistoryId::Invalid) { for (const Modification& modification : current_history_node().undo_group | reverse()) apply_modification(modification.inverse()); - m_history_id--; + m_history_id = current_history_node().parent; } return true; @@ -313,19 +305,79 @@ bool Buffer::redo(size_t count) { throw_if_read_only(); - if (m_history_id == (m_history.size() - 1)) + if (current_history_node().redo_child == HistoryId::Invalid) return false; kak_assert(m_current_undo_group.empty()); - while (count-- != 0 and m_history_id != (m_history.size() - 1)) + while (count-- != 0 and current_history_node().redo_child != HistoryId::Invalid) { - m_history_id++; + m_history_id = current_history_node().redo_child; for (const Modification& modification : current_history_node().undo_group) apply_modification(modification); } + return true; +} +bool Buffer::move_to(HistoryId id) +{ + if (id >= next_history_id()) + return false; + + throw_if_read_only(); + + commit_undo_group(); + + auto find_lowest_common_parent = [this](HistoryId a, HistoryId b) { + auto depth_of = [this](HistoryId id) { + size_t depth = 0; + for (; history_node(id).parent != HistoryId::Invalid; id = history_node(id).parent) + ++depth; + return depth; + }; + auto depthA = depth_of(a), depthB = depth_of(b); + + for (; depthA > depthB; --depthA) + a = history_node(a).parent; + for (; depthB > depthA; --depthB) + b = history_node(b).parent; + + while (a != b) + { + a = history_node(a).parent; + b = history_node(b).parent; + } + + kak_assert(a == b and a != HistoryId::Invalid); + return a; + }; + + auto parent = find_lowest_common_parent(m_history_id, id); + + // undo up to common parent + for (auto id = m_history_id; id != parent; id = history_node(id).parent) + { + for (const Modification& modification : history_node(id).undo_group | reverse()) + apply_modification(modification.inverse()); + } + + static void (*apply_from_parent)(Buffer&, HistoryId, HistoryId) = + [](Buffer& buffer, HistoryId parent, HistoryId id) { + if (id == parent) + return; + + auto& node = buffer.history_node(id); + apply_from_parent(buffer, parent, node.parent); + + buffer.history_node(node.parent).redo_child = id; + + for (const Modification& modification : node.undo_group) + buffer.apply_modification(modification); + }; + + apply_from_parent(*this, parent, id); + m_history_id = id; return true; } @@ -594,7 +646,7 @@ void Buffer::run_hook_in_own_context(Hook hook, StringView param, String client_ Optional Buffer::last_modification_coord() const { - if (m_history_id == 0) + if (m_history_id == HistoryId::First) return {}; return current_history_node().undo_group.back().coord; } @@ -682,49 +734,50 @@ UnitTest test_undo{[]() buffer.insert(2_line, "tchou\n"); // change 3 buffer.commit_undo_group(); buffer.undo(); - buffer.insert(2_line, "mutch\n"); // change 4 (inverse of 3) and 5 + buffer.insert(2_line, "mutch\n"); // change 4 buffer.commit_undo_group(); - buffer.erase({2, 1}, {2, 5}); // change 6 + buffer.erase({2, 1}, {2, 5}); // change 5 buffer.commit_undo_group(); buffer.undo(2); buffer.redo(2); buffer.undo(); - buffer.replace(2_line, buffer.end_coord(), "foo"); // change 7 (inverse of 6) and 8 + buffer.replace(2_line, buffer.end_coord(), "foo"); // change 6 buffer.commit_undo_group(); - kak_assert(buffer.history().size() == 9); + kak_assert((int)buffer.line_count() == 3); + kak_assert(buffer[0_line] == "allo ?\n"); + kak_assert(buffer[1_line] == "mais que fais la police\n"); + kak_assert(buffer[2_line] == "foo\n"); - auto check_content = [&](const Vector& lines) { - kak_assert(buffer.line_count() == lines.size()); - for (size_t i = 0; i < lines.size(); ++i) - kak_assert(buffer[i] == lines[i] + "\n"); - }; + buffer.move_to((Buffer::HistoryId)3); + kak_assert((int)buffer.line_count() == 5); + kak_assert(buffer[0_line] == "allo ?\n"); + kak_assert(buffer[1_line] == "mais que fais la police\n"); + kak_assert(buffer[2_line] == "tchou\n"); + kak_assert(buffer[3_line] == " hein ?\n"); + kak_assert(buffer[4_line] == " youpi\n"); - kak_assert(not buffer.redo(1)); - check_content({ "allo ?", "mais que fais la police", "foo" , }); - kak_assert(buffer.undo(1)); - check_content({ "allo ?", "mais que fais la police", "mutch" , " hein ?", " youpi", }); - kak_assert(buffer.undo(1)); - check_content({ "allo ?", "mais que fais la police", "m" , " hein ?", " youpi", }); - kak_assert(buffer.undo(1)); - check_content({ "allo ?", "mais que fais la police", "mutch" , " hein ?", " youpi", }); - kak_assert(buffer.undo(1)); - check_content({ "allo ?", "mais que fais la police", " hein ?", " youpi" , }); - kak_assert(buffer.undo(1)); - check_content({ "allo ?", "mais que fais la police", "tchou" , " hein ?", " youpi", }); - kak_assert(buffer.undo(1)); - check_content({ "allo ?", "mais que fais la police", " hein ?", " youpi" , }); - kak_assert(buffer.undo(1)); - check_content({ "allo ?", "mais que fais la police", " hein ?", " youpi" , "kanaky", }); - kak_assert(buffer.undo(1)); - check_content({ "allo ?", "mais que fais la police", " hein ?", " youpi" , }); - kak_assert(not buffer.undo(1)); + buffer.move_to((Buffer::HistoryId)4); + kak_assert((int)buffer.line_count() == 5); + kak_assert(buffer[0_line] == "allo ?\n"); + kak_assert(buffer[1_line] == "mais que fais la police\n"); + kak_assert(buffer[2_line] == "mutch\n"); + kak_assert(buffer[3_line] == " hein ?\n"); + kak_assert(buffer[4_line] == " youpi\n"); - kak_assert(buffer.redo(1000)); - check_content({ "allo ?", "mais que fais la police", "foo" , }); + buffer.move_to(Buffer::HistoryId::First); + kak_assert((int)buffer.line_count() == 4); + kak_assert(buffer[0_line] == "allo ?\n"); + kak_assert(buffer[1_line] == "mais que fais la police\n"); + kak_assert(buffer[2_line] == " hein ?\n"); + kak_assert(buffer[3_line] == " youpi\n"); + kak_assert(not buffer.undo()); - kak_assert(buffer.undo(1000)); - check_content({ "allo ?", "mais que fais la police", " hein ?", " youpi" , }); + buffer.move_to((Buffer::HistoryId)5); + kak_assert(not buffer.redo()); + + buffer.move_to((Buffer::HistoryId)6); + kak_assert(not buffer.redo()); }}; } diff --git a/src/buffer.hh b/src/buffer.hh index 415d32da..7b671c0e 100644 --- a/src/buffer.hh +++ b/src/buffer.hh @@ -123,6 +123,8 @@ public: }; friend constexpr bool with_bit_ops(Meta::Type) { return true; } + enum class HistoryId : size_t { First = 0, Invalid = (size_t)-1 }; + Buffer(String name, Flags flags, BufferLines lines, ByteOrderMark bom = ByteOrderMark::None, EolFormat eolformat = EolFormat::Lf, @@ -148,7 +150,9 @@ public: void commit_undo_group(); bool undo(size_t count = 1); bool redo(size_t count = 1); - size_t current_history_id() const noexcept { return m_history_id; } + bool move_to(HistoryId id); + HistoryId current_history_id() const noexcept { return m_history_id; } + HistoryId next_history_id() const noexcept { return (HistoryId)m_history.size(); } String string(BufferCoord begin, BufferCoord end) const; StringView substr(BufferCoord begin, BufferCoord end) const; @@ -241,14 +245,14 @@ public: struct HistoryNode : UseMemoryDomain { - HistoryNode(); + HistoryNode(HistoryId parent); + HistoryId parent; + HistoryId redo_child = HistoryId::Invalid; TimePoint committed; UndoGroup undo_group; }; - static constexpr size_t InvalidHistoryId = (size_t)-1; - const Vector& history() const { return m_history; } const UndoGroup& current_undo_group() const { return m_current_undo_group; } @@ -259,6 +263,7 @@ private: BufferCoord do_erase(BufferCoord begin, BufferCoord end); void apply_modification(const Modification& modification); + void revert_modification(const Modification& modification); struct LineList : BufferLines { @@ -284,12 +289,14 @@ private: Flags m_flags; Vector m_history; - size_t m_history_id = InvalidHistoryId; - size_t m_last_save_history_id = InvalidHistoryId; + HistoryId m_history_id = HistoryId::Invalid; + HistoryId m_last_save_history_id = HistoryId::Invalid; UndoGroup m_current_undo_group; - HistoryNode& current_history_node() { return m_history[m_history_id]; } - const HistoryNode& current_history_node() const { return m_history[m_history_id]; } + HistoryNode& history_node(HistoryId id) { return m_history[(size_t)id]; } + const HistoryNode& history_node(HistoryId id) const { return m_history[(size_t)id]; } + HistoryNode& current_history_node() { return m_history[(size_t)m_history_id]; } + const HistoryNode& current_history_node() const { return m_history[(size_t)m_history_id]; } Vector m_changes; diff --git a/src/buffer_utils.cc b/src/buffer_utils.cc index 9ff1f4ce..47f9a88b 100644 --- a/src/buffer_utils.cc +++ b/src/buffer_utils.cc @@ -301,6 +301,14 @@ void write_to_debug_buffer(StringView str) } +auto to_string(Buffer::HistoryId id) +{ + using Result = decltype(to_string(size_t{})); + if (id == Buffer::HistoryId::Invalid) + return Result{1, "-"}; + return to_string(static_cast(id)); +} + static String modification_as_string(const Buffer::Modification& modification) { return format("{}{}.{}|{}", @@ -315,7 +323,9 @@ Vector history_as_strings(const Vector& history) for (auto& node : history) { auto seconds = std::chrono::duration_cast(node.committed.time_since_epoch()); + res.push_back(to_string(node.parent)); res.push_back(to_string(seconds.count())); + res.push_back(to_string(node.redo_child)); for (auto& modification : node.undo_group) res.push_back(modification_as_string(modification)); }; diff --git a/src/main.cc b/src/main.cc index 69bcc2f5..ac4a5625 100644 --- a/src/main.cc +++ b/src/main.cc @@ -45,7 +45,8 @@ struct { StringView notes; } constexpr version_notes[] = { { 0, - "» History is now stored linearly instead of in a tree\n" + "» Selection undo/redo moved to {+b}{}/{+b}{} " + "and moving in history tree to {+b}{}/{+b}c-j>{}\n" "» {+b}{} and {+b}{} now undo selection history\n" "» {+u}%exp\\{...}{} expansions provide flexible quoting for expanded " "strings (as double quoted strings)\n" diff --git a/src/normal.cc b/src/normal.cc index 68392199..08ce028d 100644 --- a/src/normal.cc +++ b/src/normal.cc @@ -2010,6 +2010,29 @@ void redo(Context& context, NormalParams params) throw runtime_error("nothing left to redo"); } +template +void move_in_history(Context& context, NormalParams params) +{ + Buffer& buffer = context.buffer(); + size_t timestamp = buffer.timestamp(); + const int count = std::max(1, params.count); + const int history_id = (size_t)buffer.current_history_id() + direction * count; + const int max_history_id = (int)buffer.next_history_id() - 1; + if (buffer.move_to((Buffer::HistoryId)history_id)) + { + auto ranges = compute_modified_ranges(buffer, timestamp); + if (not ranges.empty()) + context.selections_write_only() = std::move(ranges); + + context.print_status({ format("moved to change #{} ({})", + history_id, max_history_id), + context.faces()["Information"] }); + } + else + throw runtime_error(format("no such change: #{} ({})", + history_id, max_history_id)); +} + template void undo_selection_change(Context& context, NormalParams params) { @@ -2339,6 +2362,8 @@ static constexpr HashMap { {'u'}, {"undo", undo} }, { {'U'}, {"redo", redo} }, + { {ctrl('k')}, {"move backward in history", move_in_history} }, + { {ctrl('j')}, {"move forward in history", move_in_history} }, { {alt('u')}, {"undo selection change", undo_selection_change} }, { {alt('U')}, {"redo selection change", undo_selection_change} }, diff --git a/test/compose/history/kak_quoted_history b/test/compose/history/kak_quoted_history index 2e78a548..5265a633 100644 --- a/test/compose/history/kak_quoted_history +++ b/test/compose/history/kak_quoted_history @@ -1 +1 @@ -'$timestamp' '$timestamp' '+0.5|m' '+0.6|i' '+0.7|d' '+0.8|d' '+0.9|l' '+0.10|e' '-0.8|dle' +'-' '$timestamp' '1' '0' '$timestamp' '-' '+0.5|m' '+0.6|i' '+0.7|d' '+0.8|d' '+0.9|l' '+0.10|e' '-0.8|dle' diff --git a/test/compose/history/rc b/test/compose/history/rc index f8a37fda..da4c3cd7 100644 --- a/test/compose/history/rc +++ b/test/compose/history/rc @@ -1,6 +1,6 @@ # Make our expansion have a predictable timestamp hook global ClientClose .* %{ nop %sh{ - kak -f 'ghlwc$timestamp1flwc$timestamp' kak_quoted_history + kak -f 'ghflwc$timestamp3flwc$timestamp' kak_quoted_history } }