xenia/kakoune
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Switch undo storage from a tree to a plain list

Browse Source 
Whenever a new history node is committed after some undo steps, instead
of creating a new branch in the undo graph, we first append the inverse
modifications starting from the end of the undo list up to the current
position before adding the new node.

For example let's assume that the undo history is A-B-C, that a single undo
has been done (bringing us to state B) and that a new change D is committed.
Instead of creating a new branch starting at B, we add the inverse of C
(noted ^C) at the end, and D afterwards. This results in the undo history
A-B-C-^C-D. Since C-^C collapses to a null change, this is equivalent to
A-B-D but without having lost the C branch of the history.

If a new change is committed while no undo has been done, the new history
node is simply appended to the list, as was the case previously.

This results in a simplification of the user interaction, as two bindings
are now sufficient to walk the entire undo history, as opposed to needing
extra bindings to switch branches whenever they occur.
The <a-u> and <a-U> bindings are now free.

It also simplifies the implementation, as the graph traversal and
branching code are not needed anymore. The parent and child of a node are
now respectively the previous and the next elements in the list, so there
is no need to store their ID as part of the node.
Only the committing of an undo group is slightly more complex, as inverse
history nodes need to be added depending on the current position in the
undo list.

The following article was the initial motivation for this change:
https://github.com/zaboople/klonk/blob/master/TheGURQ.md
This commit is contained in:
Olivier Perret 2023-04-17 10:25:51 +02:00
parent 019fbc5439
commit e0d33f51b3
9 changed files with 77 additions and 182 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
README.asciidoc
View File

@ -540,9 +540,7 @@ Changes
* `<a-!>`: append command output after each selection * `<a-!>`: append command output after each selection
* `u`: undo last change * `u`: undo last change
* `<a-u>`: move backward in history
* `U`: redo last change * `U`: redo last change
* `<a-U>`: move forward in history
* `&`: align selections, align the cursor of selections by inserting * `&`: align selections, align the cursor of selections by inserting
spaces before the first character of the selection spaces before the first character of the selection

16
doc/pages/expansions.asciidoc
View File

@ -285,15 +285,13 @@ The following expansions are supported (with required context _in italics_):
*%val{history}*:: *%val{history}*::
_in buffer, window scope_ + _in buffer, window scope_ +
the full change history of the buffer, including undo forks, in terms the full change history of the buffer, as a list of
of `parent committed redo_child modification0 modification1 ...` `committed modification0 modification1 ...` entries.
entries, where _parent_ is the index of the entry's predecessor (entry _committed_ is a count in seconds from Kakoune's steady clock's epoch of
0, which is the root of the history tree, will always have `-` here), the creation of the history entry, and each _modification_ is presented
_committed_ is a count in seconds from Kakoune's steady clock's epoch as in `%val{uncommitted_modifications}`.
of the creation of the history entry, _redo_child_ is the index of the Boundaries in the list can be identified knowing that _committed_ is a
child which will be visited for `U` (always `-` at the leaves of the plain integer, and each _modification_ starts with `+` or `-`.
history), and each _modification_ is presented as in
`%val{uncommitted_modifications}`.
*%val{history_id}*:: *%val{history_id}*::
_in buffer, window scope_ + _in buffer, window scope_ +

6
doc/pages/keys.asciidoc
View File

@ -305,15 +305,9 @@ Yanking (copying) and pasting use the *"* register by default (See <<registers#,
*u*:: *u*::
undo last change undo last change
*<a-u>*::
move backward in history
*U*:: *U*::
redo last change redo last change
*<a-U>*::
move forward in history
*<c-h>*:: *<c-h>*::
undo last selection change undo last selection change

173
src/buffer.cc
View File

@ -20,8 +20,8 @@
namespace Kakoune namespace Kakoune
{ {
Buffer::HistoryNode::HistoryNode(HistoryId parent) Buffer::HistoryNode::HistoryNode()
: parent{parent}, committed{Clock::now()} : committed{Clock::now()}
{} {}
Buffer::Buffer(String name, Flags flags, BufferLines lines, 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_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_display_name{(flags & Flags::File) ? compact_path(m_name) : m_name},
m_flags{flags | Flags::NoUndo}, m_flags{flags | Flags::NoUndo},
m_history{{HistoryId::Invalid}}, m_history{{HistoryNode{}}},
m_history_id{HistoryId::First}, m_history_id{0},
m_last_save_history_id{HistoryId::First}, m_last_save_history_id{0},
m_fs_status{fs_status} m_fs_status{fs_status}
{ {
#ifdef KAK_DEBUG #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)) if (m_flags & Buffer::Flags::File and not file_exists(m_name))
{ {
m_flags |= Buffer::Flags::New; m_flags |= Buffer::Flags::New;
m_last_save_history_id = HistoryId::Invalid; m_last_save_history_id = InvalidHistoryId;
} }
} }
else else
@ -201,9 +201,9 @@ void Buffer::reload(BufferLines lines, ByteOrderMark bom, EolFormat eolformat, F
if (not record_undo) if (not record_undo)
{ {
// Erase history about to be invalidated history // Erase history about to be invalidated history
m_history_id = HistoryId::First; m_history_id = 0;
m_last_save_history_id = HistoryId::First; m_last_save_history_id = 0;
m_history = {HistoryNode{HistoryId::Invalid}}; m_history = {HistoryNode{}};
m_changes.push_back({ Change::Erase, {0,0}, line_count() }); m_changes.push_back({ Change::Erase, {0,0}, line_count() });
static_cast<BufferLines&>(m_lines) = std::move(lines); static_cast<BufferLines&>(m_lines) = std::move(lines);
@ -273,12 +273,20 @@ void Buffer::commit_undo_group()
if (m_current_undo_group.empty()) if (m_current_undo_group.empty())
return; return;
const HistoryId id = next_history_id(); // walk back to current position in history, and append reverse events
m_history.push_back({m_history_id}); 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<UndoGroup>();
}
m_history.emplace_back();
m_history.back().undo_group = std::move(m_current_undo_group); m_history.back().undo_group = std::move(m_current_undo_group);
m_current_undo_group.clear(); m_current_undo_group.clear();
current_history_node().redo_child = id; m_history_id = m_history.size() - 1;
m_history_id = id;
} }
bool Buffer::undo(size_t count) bool Buffer::undo(size_t count)
@ -287,15 +295,15 @@ bool Buffer::undo(size_t count)
commit_undo_group(); commit_undo_group();
if (current_history_node().parent == HistoryId::Invalid) if ((m_history_id - 1) == InvalidHistoryId)
return false; return false;
while (count-- != 0 and current_history_node().parent != HistoryId::Invalid) while (count-- != 0 and (m_history_id - 1) != InvalidHistoryId)
{ {
for (const Modification& modification : current_history_node().undo_group | reverse()) for (const Modification& modification : current_history_node().undo_group | reverse())
apply_modification(modification.inverse()); apply_modification(modification.inverse());
m_history_id = current_history_node().parent; m_history_id--;
} }
return true; return true;
@ -305,79 +313,19 @@ bool Buffer::redo(size_t count)
{ {
throw_if_read_only(); throw_if_read_only();
if (current_history_node().redo_child == HistoryId::Invalid) if (m_history_id == (m_history.size() - 1))
return false; return false;
kak_assert(m_current_undo_group.empty()); kak_assert(m_current_undo_group.empty());
while (count-- != 0 and current_history_node().redo_child != HistoryId::Invalid) while (count-- != 0 and m_history_id != (m_history.size() - 1))
{ {
m_history_id = current_history_node().redo_child; m_history_id++;
for (const Modification& modification : current_history_node().undo_group) for (const Modification& modification : current_history_node().undo_group)
apply_modification(modification); 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; return true;
} }
@ -646,7 +594,7 @@ void Buffer::run_hook_in_own_context(Hook hook, StringView param, String client_
Optional<BufferCoord> Buffer::last_modification_coord() const Optional<BufferCoord> Buffer::last_modification_coord() const
{ {
if (m_history_id == HistoryId::First) if (m_history_id == 0)
return {}; return {};
return current_history_node().undo_group.back().coord; return current_history_node().undo_group.back().coord;
} }
@ -734,50 +682,49 @@ UnitTest test_undo{[]()
buffer.insert(2_line, "tchou\n"); // change 3 buffer.insert(2_line, "tchou\n"); // change 3
buffer.commit_undo_group(); buffer.commit_undo_group();
buffer.undo(); buffer.undo();
buffer.insert(2_line, "mutch\n"); // change 4 buffer.insert(2_line, "mutch\n"); // change 4 (inverse of 3) and 5
buffer.commit_undo_group(); buffer.commit_undo_group();
buffer.erase({2, 1}, {2, 5}); // change 5 buffer.erase({2, 1}, {2, 5}); // change 6
buffer.commit_undo_group(); buffer.commit_undo_group();
buffer.undo(2); buffer.undo(2);
buffer.redo(2); buffer.redo(2);
buffer.undo(); buffer.undo();
buffer.replace(2_line, buffer.end_coord(), "foo"); // change 6 buffer.replace(2_line, buffer.end_coord(), "foo"); // change 7 (inverse of 6) and 8
buffer.commit_undo_group(); buffer.commit_undo_group();
kak_assert((int)buffer.line_count() == 3); kak_assert(buffer.history().size() == 9);
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");
buffer.move_to((Buffer::HistoryId)3); auto check_content = [&](const Vector<StringView>& lines) {
kak_assert((int)buffer.line_count() == 5); kak_assert(buffer.line_count() == lines.size());
kak_assert(buffer[0_line] == "allo ?\n"); for (size_t i = 0; i < lines.size(); ++i)
kak_assert(buffer[1_line] == "mais que fais la police\n"); kak_assert(buffer[i] == lines[i] + "\n");
kak_assert(buffer[2_line] == "tchou\n"); };
kak_assert(buffer[3_line] == " hein ?\n");
kak_assert(buffer[4_line] == " youpi\n");
buffer.move_to((Buffer::HistoryId)4); kak_assert(not buffer.redo(1));
kak_assert((int)buffer.line_count() == 5); check_content({ "allo ?", "mais que fais la police", "foo" , });
kak_assert(buffer[0_line] == "allo ?\n"); kak_assert(buffer.undo(1));
kak_assert(buffer[1_line] == "mais que fais la police\n"); check_content({ "allo ?", "mais que fais la police", "mutch" , " hein ?", " youpi", });
kak_assert(buffer[2_line] == "mutch\n"); kak_assert(buffer.undo(1));
kak_assert(buffer[3_line] == " hein ?\n"); check_content({ "allo ?", "mais que fais la police", "m" , " hein ?", " youpi", });
kak_assert(buffer[4_line] == " youpi\n"); 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::First); kak_assert(buffer.redo(1000));
kak_assert((int)buffer.line_count() == 4); check_content({ "allo ?", "mais que fais la police", "foo" , });
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());
buffer.move_to((Buffer::HistoryId)5); kak_assert(buffer.undo(1000));
kak_assert(not buffer.redo()); check_content({ "allo ?", "mais que fais la police", " hein ?", " youpi" , });
buffer.move_to((Buffer::HistoryId)6);
kak_assert(not buffer.redo());
}}; }};
} }

23
src/buffer.hh
View File

@ -123,8 +123,6 @@ public:
}; };
friend constexpr bool with_bit_ops(Meta::Type<Flags>) { return true; } friend constexpr bool with_bit_ops(Meta::Type<Flags>) { return true; }
enum class HistoryId : size_t { First = 0, Invalid = (size_t)-1 };
Buffer(String name, Flags flags, BufferLines lines, Buffer(String name, Flags flags, BufferLines lines,
ByteOrderMark bom = ByteOrderMark::None, ByteOrderMark bom = ByteOrderMark::None,
EolFormat eolformat = EolFormat::Lf, EolFormat eolformat = EolFormat::Lf,
@ -150,9 +148,7 @@ public:
void commit_undo_group(); void commit_undo_group();
bool undo(size_t count = 1); bool undo(size_t count = 1);
bool redo(size_t count = 1); bool redo(size_t count = 1);
bool move_to(HistoryId id); size_t current_history_id() const noexcept { return m_history_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; String string(BufferCoord begin, BufferCoord end) const;
StringView substr(BufferCoord begin, BufferCoord end) const; StringView substr(BufferCoord begin, BufferCoord end) const;
@ -245,14 +241,14 @@ public:
struct HistoryNode : UseMemoryDomain<MemoryDomain::BufferMeta> struct HistoryNode : UseMemoryDomain<MemoryDomain::BufferMeta>
{ {
HistoryNode(HistoryId parent); HistoryNode();
HistoryId parent;
HistoryId redo_child = HistoryId::Invalid;
TimePoint committed; TimePoint committed;
UndoGroup undo_group; UndoGroup undo_group;
}; };
static constexpr size_t InvalidHistoryId = (size_t)-1;
const Vector<HistoryNode>& history() const { return m_history; } const Vector<HistoryNode>& history() const { return m_history; }
const UndoGroup& current_undo_group() const { return m_current_undo_group; } const UndoGroup& current_undo_group() const { return m_current_undo_group; }
@ -263,7 +259,6 @@ private:
BufferCoord do_erase(BufferCoord begin, BufferCoord end); BufferCoord do_erase(BufferCoord begin, BufferCoord end);
void apply_modification(const Modification& modification); void apply_modification(const Modification& modification);
void revert_modification(const Modification& modification);
struct LineList : BufferLines struct LineList : BufferLines
{ {
@ -289,14 +284,12 @@ private:
Flags m_flags; Flags m_flags;
Vector<HistoryNode> m_history; Vector<HistoryNode> m_history;
HistoryId m_history_id = HistoryId::Invalid; size_t m_history_id = InvalidHistoryId;
HistoryId m_last_save_history_id = HistoryId::Invalid; size_t m_last_save_history_id = InvalidHistoryId;
UndoGroup m_current_undo_group; UndoGroup m_current_undo_group;
HistoryNode& history_node(HistoryId id) { return m_history[(size_t)id]; } HistoryNode& current_history_node() { return m_history[m_history_id]; }
const HistoryNode& history_node(HistoryId id) const { return m_history[(size_t)id]; } const HistoryNode& current_history_node() const { return m_history[m_history_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<Change, MemoryDomain::BufferMeta> m_changes; Vector<Change, MemoryDomain::BufferMeta> m_changes;

10
src/buffer_utils.cc
View File

@ -301,14 +301,6 @@ 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<size_t>(id));
}
static String modification_as_string(const Buffer::Modification& modification) static String modification_as_string(const Buffer::Modification& modification)
{ {
return format("{}{}.{}|{}", return format("{}{}.{}|{}",
@ -323,9 +315,7 @@ Vector<String> history_as_strings(const Vector<Buffer::HistoryNode>& history)
for (auto& node : history) for (auto& node : history)
{ {
auto seconds = std::chrono::duration_cast<std::chrono::seconds>(node.committed.time_since_epoch()); auto seconds = std::chrono::duration_cast<std::chrono::seconds>(node.committed.time_since_epoch());
res.push_back(to_string(node.parent));
res.push_back(to_string(seconds.count())); res.push_back(to_string(seconds.count()));
res.push_back(to_string(node.redo_child));
for (auto& modification : node.undo_group) for (auto& modification : node.undo_group)
res.push_back(modification_as_string(modification)); res.push_back(modification_as_string(modification));
}; };

25
src/normal.cc
View File

@ -2009,29 +2009,6 @@ void redo(Context& context, NormalParams params)
throw runtime_error("nothing left to redo"); throw runtime_error("nothing left to redo");
} }
template<Direction direction>
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<Direction direction> template<Direction direction>
void undo_selection_change(Context& context, NormalParams params) void undo_selection_change(Context& context, NormalParams params)
{ {
@ -2362,8 +2339,6 @@ static constexpr HashMap<Key, NormalCmd, MemoryDomain::Undefined, KeymapBackend>
{ {'u'}, {"undo", undo} }, { {'u'}, {"undo", undo} },
{ {'U'}, {"redo", redo} }, { {'U'}, {"redo", redo} },
{ {alt('u')}, {"move backward in history", move_in_history<Direction::Backward>} },
{ {alt('U')}, {"move forward in history", move_in_history<Direction::Forward>} },
{ {ctrl('h')}, {"undo selection change", undo_selection_change<Backward>} }, { {ctrl('h')}, {"undo selection change", undo_selection_change<Backward>} },
{ {ctrl('k')}, {"redo selection change", undo_selection_change<Forward>} }, { {ctrl('k')}, {"redo selection change", undo_selection_change<Forward>} },

2
test/compose/history/kak_quoted_history
View File

@ -1 +1 @@
'-' '$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' '$timestamp' '$timestamp' '+0.5|m' '+0.6|i' '+0.7|d' '+0.8|d' '+0.9|l' '+0.10|e' '-0.8|dle'

2
test/compose/history/rc
View File

@ -1,6 +1,6 @@
# Make our expansion have a predictable timestamp # Make our expansion have a predictable timestamp
hook global ClientClose .* %{ hook global ClientClose .* %{
nop %sh{ nop %sh{
kak -f 'ghf<space>lwc$timestamp<esc>3f<space>lwc$timestamp<esc>' kak_quoted_history kak -f 'ghlwc$timestamp<esc>1f<space>lwc$timestamp<esc>' kak_quoted_history
} }
} }