From 7669e8e4977103da9225403824b7f7f03f767779 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 5 Jan 2021 15:40:09 -0600 Subject: [PATCH] Add concept of edit groups This allows for multiple edits to be undone/redone in one go, as if they were one edit. Useful when a function is editing the commandline buffer via scripted changes or via a keybinding so the internal changes to the buffer can be abstracted away. (Having extreme difficulty getting pexpect to play nice with the concept of undo/redo...) --- share/functions/fish_clipboard_paste.fish | 3 + src/builtin_commandline.cpp | 17 +++- src/input.cpp | 2 + src/input_common.h | 2 + src/reader.cpp | 106 ++++++++++++++++++---- src/reader.h | 20 ++++ 6 files changed, 129 insertions(+), 21 deletions(-) diff --git a/share/functions/fish_clipboard_paste.fish b/share/functions/fish_clipboard_paste.fish index 9e766d667..2dcc823db 100644 --- a/share/functions/fish_clipboard_paste.fish +++ b/share/functions/fish_clipboard_paste.fish @@ -38,7 +38,10 @@ function fish_clipboard_paste # so we don't trigger ignoring history. set data[1] (string trim -l -- $data[1]) end + if test -n "$data" + begin-undo-group commandline -i -- $data + end-undo-group end end diff --git a/src/builtin_commandline.cpp b/src/builtin_commandline.cpp index d53c54a63..da4c4afdc 100644 --- a/src/builtin_commandline.cpp +++ b/src/builtin_commandline.cpp @@ -36,6 +36,9 @@ enum { APPEND_MODE // insert at end of current token/command/buffer }; +/// Handle a single readline_cmd_t command out-of-band. +void reader_handle_command(readline_cmd_t cmd); + /// Replace/append/insert the selection with/at/after the specified string. /// /// \param begin beginning of selection @@ -302,8 +305,18 @@ maybe_t builtin_commandline(parser_t &parser, io_streams_t &streams, wchar_ if (mc == rl::repaint_mode || mc == rl::force_repaint || mc == rl::repaint) { if (ld.is_repaint) continue; } - // Inserts the readline function at the back of the queue. - reader_queue_ch(*mc); + + // HACK: Execute these right here and now so they can affect any insertions/changes + // made via bindings. The correct solution is to change all `commandline` + // insert/replace operations into readline functions with associated data, so that + // all queued `commandline` operations - including buffer modifications - are + // executed in order + if (mc == rl::begin_undo_group || mc == rl::end_undo_group) { + reader_handle_command(*mc); + } else { + // Inserts the readline function at the back of the queue. + reader_queue_ch(*mc); + } } else { streams.err.append_format(_(L"%ls: Unknown input function '%ls'"), cmd, argv[i]); builtin_print_error_trailer(parser, streams.err, cmd); diff --git a/src/input.cpp b/src/input.cpp index cd3c22eba..ffaf6558f 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -157,6 +157,8 @@ static const input_function_metadata_t input_function_metadata[] = { {readline_cmd_t::cancel, L"cancel"}, {readline_cmd_t::undo, L"undo"}, {readline_cmd_t::redo, L"redo"}, + {readline_cmd_t::begin_undo_group, L"begin-undo-group"}, + {readline_cmd_t::end_undo_group, L"end-undo-group"}, }; static_assert(sizeof(input_function_metadata) / sizeof(input_function_metadata[0]) == diff --git a/src/input_common.h b/src/input_common.h index 09fe9dba6..35cf30387 100644 --- a/src/input_common.h +++ b/src/input_common.h @@ -80,6 +80,8 @@ enum class readline_cmd_t { cancel, undo, redo, + begin_undo_group, + end_undo_group, repeat_jump, // NOTE: This one has to be last. reverse_repeat_jump diff --git a/src/reader.cpp b/src/reader.cpp index 42e907d9a..2c4b8e5b3 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -200,7 +200,7 @@ static bool want_to_coalesce_insertion_of(const editable_line_t &el, const wcstr // Only consolidate single character inserts. if (str.size() != 1) return false; // Make an undo group after every space. - if (str.at(0) == L' ') return false; + if (str.at(0) == L' ' && !el.undo_history.try_coalesce) return false; assert(!el.undo_history.edits.empty()); const edit_t &last_edit = el.undo_history.edits.back(); // Don't add to the last edit if it deleted something. @@ -211,19 +211,35 @@ static bool want_to_coalesce_insertion_of(const editable_line_t &el, const wcstr } bool editable_line_t::undo() { - if (undo_history.edits_applied == 0) return false; // nothing to undo - const edit_t &edit = undo_history.edits.at(undo_history.edits_applied - 1); - undo_history.edits_applied--; - edit_t inverse = edit_t(edit.offset, edit.replacement.size(), L""); - inverse.replacement = edit.old; - size_t old_position = edit.cursor_position_before_edit; - apply_edit(&text_, inverse); - set_position(old_position); + bool did_undo = false; + maybe_t last_group_id{-1}; + while (undo_history.edits_applied != 0) { + const edit_t &edit = undo_history.edits.at(undo_history.edits_applied - 1); + if (did_undo && (!edit.group_id.has_value() || edit.group_id != last_group_id)) { + // We've restored all the edits in this logical undo group + break; + } + last_group_id = edit.group_id; + undo_history.edits_applied--; + edit_t inverse = edit_t(edit.offset, edit.replacement.size(), L""); + inverse.replacement = edit.old; + size_t old_position = edit.cursor_position_before_edit; + apply_edit(&text_, inverse); + set_position(old_position); + did_undo = true; + } + + end_edit_group(); undo_history.may_coalesce = false; - return true; + return did_undo; } void editable_line_t::push_edit(edit_t &&edit) { + // Assign a new group id or propagate the old one if we're in a logical grouping of edits + if (edit_group_level_ != -1) { + edit.group_id = edit_group_id_; + } + bool edit_does_nothing = edit.length == 0 && edit.replacement.empty(); if (edit_does_nothing) return; if (undo_history.edits_applied != undo_history.edits.size()) { @@ -251,13 +267,48 @@ void editable_line_t::insert_coalesce(const wcstring &str) { } bool editable_line_t::redo() { - if (undo_history.edits_applied >= undo_history.edits.size()) return false; // nothing to redo - const edit_t &edit = undo_history.edits.at(undo_history.edits_applied); - undo_history.edits_applied++; - apply_edit(&text_, edit); - set_position(cursor_position_after_edit(edit)); - undo_history.may_coalesce = false; // Make a new undo group here. - return true; + bool did_redo = false; + + maybe_t last_group_id{-1}; + while (undo_history.edits_applied < undo_history.edits.size()) { + const edit_t &edit = undo_history.edits.at(undo_history.edits_applied); + if (did_redo && (!edit.group_id.has_value() || edit.group_id != last_group_id)) { + // We've restored all the edits in this logical undo group + break; + } + last_group_id = edit.group_id; + undo_history.edits_applied++; + apply_edit(&text_, edit); + set_position(cursor_position_after_edit(edit)); + did_redo = true; + } + + end_edit_group(); + return did_redo; +} + +void editable_line_t::begin_edit_group() { + if (++edit_group_level_ == 0) { + // Indicate that the next change must trigger the creation of a new history item + undo_history.may_coalesce = false; + // Indicate that future changes should be coalesced into the same edit if possible. + undo_history.try_coalesce = true; + // Assign a logical edit group id to future edits in this group + edit_group_id_ += 1; + } +} + +void editable_line_t::end_edit_group() { + if (edit_group_level_ == -1) { + // Clamp the minimum value to -1 to prevent unbalanced end_edit_group() calls from breaking + // everything. + return; + } + + if (--edit_group_level_ == -1) { + undo_history.try_coalesce = false; + undo_history.may_coalesce = false; + } } namespace { @@ -1415,7 +1466,7 @@ void reader_data_t::insert_string(editable_line_t *el, const wcstring &str) { assert(el->undo_history.may_coalesce); } else { el->push_edit(edit_t(el->position(), 0, str)); - el->undo_history.may_coalesce = (str.size() == 1); + el->undo_history.may_coalesce = el->undo_history.try_coalesce || (str.size() == 1); } if (el == &command_line) suppress_autosuggestion = false; @@ -3710,7 +3761,17 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat } break; } - // Some commands should have been handled internally by inputter_t::readch(). + case rl::begin_undo_group: { + editable_line_t *el = active_edit_line(); + el->begin_edit_group(); + break; + } + case rl::end_undo_group: { + editable_line_t *el = active_edit_line(); + el->end_edit_group(); + break; + } + // Some commands should have been handled internally by inputter_t::readch(). case rl::self_insert: case rl::self_insert_notfirst: case rl::func_or: @@ -3981,6 +4042,13 @@ void reader_schedule_prompt_repaint() { } } +void reader_handle_command(readline_cmd_t cmd) { + if (reader_data_t *data = current_data_or_null()) { + readline_loop_state_t rls{}; + data->handle_readline_command(cmd, rls); + } +} + void reader_queue_ch(const char_event_t &ch) { if (reader_data_t *data = current_data_or_null()) { data->inputter.queue_ch(ch); diff --git a/src/reader.h b/src/reader.h index 18fbadd35..7acb76c3a 100644 --- a/src/reader.h +++ b/src/reader.h @@ -31,6 +31,10 @@ struct edit_t { /// The strings that are removed and added by this edit, respectively. wcstring old, replacement; + /// edit_t is only for contiguous changes, so to restore a group of arbitrary changes to the + /// command line we need to have a group id as forcibly coalescing changes is not enough. + maybe_t group_id; + explicit edit_t(size_t offset, size_t length, wcstring replacement) : offset(offset), length(length), replacement(std::move(replacement)) {} @@ -61,6 +65,11 @@ struct undo_history_t { /// last one. bool may_coalesce = false; + /// Whether to be more aggressive in coalescing edits. Ideally, it would be "force coalesce" + /// with guaranteed atomicity but as `edit_t` is strictly for contiguous changes, that guarantee + /// can't be made at this time. + bool try_coalesce = false; + /// Empty the history. void clear(); }; @@ -72,6 +81,12 @@ class editable_line_t { /// The current position of the cursor in the command line. size_t position_ = 0; + /// The nesting level for atomic edits, so that recursive invocations of start_edit_group() + /// are not ended by one end_edit_group() call. + int32_t edit_group_level_ = -1; + /// Monotonically increasing edit group, ignored when edit_group_level_ is -1. Allowed to wrap. + uint32_t edit_group_id_ = -1; + public: undo_history_t undo_history; @@ -110,6 +125,11 @@ class editable_line_t { /// Redo the most recent undo. Returns true on success. bool redo(); + + /// Start a logical grouping of command line edits that should be undone/redone together. + void begin_edit_group(); + /// End a logical grouping of command line edits that should be undone/redone together. + void end_edit_group(); }; /// Read commands from \c fd until encountering EOF.