Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to undo and redo selection changes #4725

Merged
merged 5 commits into from
Nov 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions doc/pages/keys.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,12 @@ Yanking (copying) and pasting use the *"* register by default (See <<registers#,
*<a-U>*::
move forward in history

*<c-h>*::
undo last selection change

*<c-k>*::
redo last selection change

*&*::
align selections, align the cursor of each selection by inserting spaces
before the first character of each selection
Expand Down
14 changes: 11 additions & 3 deletions src/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ DisplayLine Client::generate_mode_line() const
return modeline;
}

void Client::change_buffer(Buffer& buffer)
void Client::change_buffer(Buffer& buffer, Optional<FunctionRef<void()>> set_selections)
{
if (m_buffer_reload_dialog_opened)
close_buffer_reload_dialog();
Expand All @@ -181,12 +181,20 @@ void Client::change_buffer(Buffer& buffer)
m_window->options().unregister_watcher(*this);
m_window->set_client(nullptr);
client_manager.add_free_window(std::move(m_window),
std::move(context().selections()));
context().selections());

m_window = std::move(ws.window);
m_window->set_client(this);
m_window->options().register_watcher(*this);
context().selections_write_only() = std::move(ws.selections);

if (set_selections)
(*set_selections)();
else
{
ScopedSelectionEdition selection_edition{context()};
context().selections_write_only() = std::move(ws.selections);
}

context().set_window(*m_window);

m_window->set_dimensions(m_ui->dimensions());
Expand Down
2 changes: 1 addition & 1 deletion src/client.hh
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public:
InputHandler& input_handler() { return m_input_handler; }
const InputHandler& input_handler() const { return m_input_handler; }

void change_buffer(Buffer& buffer);
void change_buffer(Buffer& buffer, Optional<FunctionRef<void()>> set_selection);

StringView get_env_var(StringView name) const;

Expand Down
2 changes: 2 additions & 0 deletions src/commands.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2009,6 +2009,7 @@ void context_wrap(const ParametersParser& parser, Context& context, StringView d

ScopedSetBool disable_history(c.history_disabled());
ScopedEdition edition{c};
ScopedSelectionEdition selection_edition{c};

if (parser.get_switch("itersel"))
{
Expand Down Expand Up @@ -2523,6 +2524,7 @@ const CommandDesc select_cmd = {
else if (parser.get_switch("display-column"))
column_type = ColumnType::DisplayColumn;
ColumnCount tabstop = context.options()["tabstop"].get<int>();
ScopedSelectionEdition selection_edition{context};
context.selections_write_only() = selection_list_from_strings(buffer, column_type, parser.positionals_from(0), timestamp, 0, tabstop);
}
};
Expand Down
199 changes: 176 additions & 23 deletions src/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@ Context::Context(InputHandler& input_handler, SelectionList selections,
Flags flags, String name)
: m_flags(flags),
m_input_handler{&input_handler},
m_selections{std::move(selections)},
m_selection_history{*this, std::move(selections)},
m_name(std::move(name))
{}

Context::Context(EmptyContextFlag) {}
Context::Context(EmptyContextFlag) : m_selection_history{*this} {}

Buffer& Context::buffer() const
{
if (not has_buffer())
throw runtime_error("no buffer in context");
return const_cast<Buffer&>((*m_selections).buffer());
return const_cast<Buffer&>(selections(false).buffer());
}

Window& Context::window() const
Expand Down Expand Up @@ -164,7 +164,147 @@ void JumpList::forget_buffer(Buffer& buffer)
}
}

void Context::change_buffer(Buffer& buffer)
Context::SelectionHistory::SelectionHistory(Context& context) : m_context(context) {}

Context::SelectionHistory::SelectionHistory(Context& context, SelectionList selections)
: m_context(context),
m_history{HistoryNode{std::move(selections), HistoryId::Invalid}},
m_history_id(HistoryId::First) {}

void Context::SelectionHistory::initialize(SelectionList selections)
{
kak_assert(empty());
m_history = {HistoryNode{std::move(selections), HistoryId::Invalid}};
m_history_id = HistoryId::First;
}

SelectionList& Context::SelectionHistory::selections(bool update)
{
if (empty())
throw runtime_error("no selections in context");
auto& sels = m_staging ? m_staging->selections : current_history_node().selections;
if (update)
sels.update();
return sels;
}

void Context::SelectionHistory::begin_edition()
{
if (not in_edition())
m_staging = HistoryNode{selections(), m_history_id};
m_in_edition.set();
}

void Context::SelectionHistory::end_edition()
{
kak_assert(in_edition());
m_in_edition.unset();
if (in_edition())
return;

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());
}
else
{
m_history_id = next_history_id();
m_history.push_back(std::move(*m_staging));
}
m_staging.reset();
}

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 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();
else
m_context.change_buffer(history_node(parent).selections.buffer(), { std::move(select_parent) });
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there an issue with letting change_buffer move the saved selections and overwrite them here ? Its unclear to me why we need to pass that lambda to change_buffer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context::change_buffer calls Client::change_buffer, which creates a new selection in the destination buffer and then runs the WinDisplay hooks.
The original problem was that it would crash inside some WinDisplay hook because of broken invariants in the selection undo-stack.
I think those problems only occurred in a previous version, as I can't reproduce a problem anymore. Will try some more later.
Still, there is a behavior difference that is probably correct:

If we overwrite selections later, then the WinDisplay hooks will be run with whatever selection is returned from client_manager.get_free_window(). By passing the lambda we make sure the selection inside the hook is the same that the user will see. Though the lambda is awkward, would be nice to simplify this somehow.

// });
}

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;
m_staging = current_history_node();
};
if (&history_node(child).selections.buffer() == &m_context.buffer())
select_child();
else
m_context.change_buffer(history_node(child).selections.buffer(), { std::move(select_child) });
}

void Context::SelectionHistory::forget_buffer(Buffer& buffer)
{
Vector<HistoryId, MemoryDomain::Selections> new_ids;
size_t bias = 0;
for (size_t i = 0; i < m_history.size(); ++i)
{
auto& node = history_node((HistoryId)i);
HistoryId id;
if (&node.selections.buffer() == &buffer)
{
id = HistoryId::Invalid;
++bias;
}
else
id = (HistoryId)(i - bias);
new_ids.push_back(id);
}
auto new_id = [&new_ids](HistoryId old_id) -> HistoryId {
return old_id == HistoryId::Invalid ? HistoryId::Invalid : new_ids[(size_t)old_id];
};

m_history.erase(remove_if(m_history, [&buffer](const auto& node) {
return &node.selections.buffer() == &buffer;
}), m_history.end());

for (auto& node : m_history)
{
node.parent = new_id(node.parent);
node.redo_child = new_id(node.redo_child);
}
m_history_id = new_id(m_history_id);
if (m_staging)
{
m_staging->parent = new_id(m_staging->parent);
kak_assert(m_staging->redo_child == HistoryId::Invalid);
}
kak_assert(m_history_id != HistoryId::Invalid or m_staging);
}

void Context::change_buffer(Buffer& buffer, Optional<FunctionRef<void()>> set_selections)
{
if (has_buffer() and &buffer == &this->buffer())
return;
Expand All @@ -176,12 +316,18 @@ void Context::change_buffer(Buffer& buffer)
{
client().info_hide();
client().menu_hide();
client().change_buffer(buffer);
client().change_buffer(buffer, std::move(set_selections));
}
else
{
m_window.reset();
m_selections = SelectionList{buffer, Selection{}};
if (m_selection_history.empty())
m_selection_history.initialize(SelectionList{buffer, Selection{}});
else
{
ScopedSelectionEdition selection_edition{*this};
selections_write_only() = SelectionList{buffer, Selection{}};
}
}

if (has_input_handler())
Expand All @@ -192,14 +338,16 @@ void Context::forget_buffer(Buffer& buffer)
{
m_jump_list.forget_buffer(buffer);

if (&this->buffer() != &buffer)
return;
if (&this->buffer() == &buffer)
{
if (is_editing() && has_input_handler())
input_handler().reset_normal_mode();

if (is_editing() && has_input_handler())
input_handler().reset_normal_mode();
auto last_buffer = this->last_buffer();
change_buffer(last_buffer ? *last_buffer : BufferManager::instance().get_first_buffer());
}

auto last_buffer = this->last_buffer();
change_buffer(last_buffer ? *last_buffer : BufferManager::instance().get_first_buffer());
m_selection_history.forget_buffer(buffer);
}

Buffer* Context::last_buffer() const
Expand All @@ -223,24 +371,29 @@ Buffer* Context::last_buffer() const
return previous_buffer != jump_list.rend() ? &previous_buffer->buffer() : nullptr;
}

SelectionList& Context::selections()
SelectionList& Context::selections(bool update)
{
if (not m_selections)
throw runtime_error("no selections in context");
(*m_selections).update();
return *m_selections;
return m_selection_history.selections(update);
}

void Context::undo_selection_change()
{
m_selection_history.undo();
}

void Context::redo_selection_change()
{
m_selection_history.redo();
}

SelectionList& Context::selections_write_only()
{
if (not m_selections)
throw runtime_error("no selections in context");
return *m_selections;
return selections(false);
}

const SelectionList& Context::selections() const
const SelectionList& Context::selections(bool update) const
{
return const_cast<Context&>(*this).selections();
return const_cast<Context&>(*this).selections(update);
}

Vector<String> Context::selections_content() const
Expand Down Expand Up @@ -277,7 +430,7 @@ void Context::end_edition()

StringView Context::main_sel_register_value(StringView reg) const
{
size_t index = m_selections ? (*m_selections).main_index() : 0;
size_t index = has_buffer() ? selections(false).main_index() : 0;
return RegisterManager::instance()[reg].get_main(*this, index);
}

Expand Down
Loading