diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 03f13903c..6085a67d8 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -3880,11 +3880,11 @@ static void test_undo() { do_test(!line.undo()); // nothing to undo do_test(line.text().empty()); do_test(line.position() == 0); - line.push_edit(edit_t(0, 0, L"a b c")); + line.push_edit(edit_t(0, 0, L"a b c"), true); do_test(line.text() == L"a b c"); do_test(line.position() == 5); line.set_position(2); - line.push_edit(edit_t(2, 1, L"B")); // replacement right of cursor + line.push_edit(edit_t(2, 1, L"B"), true); // replacement right of cursor do_test(line.text() == L"a B c"); line.undo(); do_test(line.text() == L"a b c"); @@ -3895,7 +3895,7 @@ static void test_undo() { do_test(!line.redo()); // nothing to redo - line.push_edit(edit_t(0, 2, L"")); // deletion left of cursor + line.push_edit(edit_t(0, 2, L""), true); // deletion left of cursor do_test(line.text() == L"B c"); do_test(line.position() == 1); line.undo(); @@ -3905,27 +3905,23 @@ static void test_undo() { do_test(line.text() == L"B c"); do_test(line.position() == 1); - line.push_edit(edit_t(0, line.size(), L"a b c")); // replacement left and right of cursor + line.push_edit(edit_t(0, line.size(), L"a b c"), true); // replacement left and right of cursor do_test(line.text() == L"a b c"); do_test(line.position() == 5); say(L"Testing undoing coalesced edits."); line.clear(); - line.push_edit(edit_t(line.position(), 0, L"a")); - line.insert_coalesce(L"b"); - line.insert_coalesce(L"c"); - do_test(line.undo_history.edits.size() == 1); - line.push_edit(edit_t(line.position(), 0, L" ")); - do_test(line.undo_history.edits.size() == 2); + line.push_edit(edit_t(line.position(), 0, L"a"), true); + line.push_edit(edit_t(line.position(), 0, L"b"), true); + line.push_edit(edit_t(line.position(), 0, L"c"), true); + line.push_edit(edit_t(line.position(), 0, L" "), true); line.undo(); line.undo(); line.redo(); do_test(line.text() == L"abc"); - do_test(line.undo_history.edits.size() == 2); // This removes the space insertion from the history, but does not coalesce with the first edit. - line.push_edit(edit_t(line.position(), 0, L"d")); - do_test(line.undo_history.edits.size() == 2); - line.insert_coalesce(L"e"); + line.push_edit(edit_t(line.position(), 0, L"d"), true); + line.push_edit(edit_t(line.position(), 0, L"e"), true); do_test(line.text() == L"abcde"); line.undo(); do_test(line.text() == L"abc"); diff --git a/src/reader.cpp b/src/reader.cpp index 87c6ebd3e..8d62dd57a 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -223,34 +223,17 @@ static size_t cursor_position_after_edit(const edit_t &edit) { return cursor > removed ? cursor - removed : 0; } -/// Whether we want to append this string to the previous edit. -static bool want_to_coalesce_insertion_of(const editable_line_t &el, const wcstring &str) { - // The previous edit must support coalescing. - if (!el.undo_history.may_coalesce) return false; - // Only consolidate single character inserts. - if (str.size() != 1) return false; - // Make an undo group after every space. - 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. - if (last_edit.length != 0) return false; - // Must not have moved the cursor! - if (cursor_position_after_edit(last_edit) != el.position()) return false; - return true; -} - bool editable_line_t::undo() { 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); + 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--; + 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; @@ -260,18 +243,31 @@ bool editable_line_t::undo() { } end_edit_group(); - undo_history.may_coalesce = false; + undo_history_.may_coalesce = false; return did_undo; } void editable_line_t::clear() { - undo_history.clear(); + undo_history_.clear(); if (empty()) return; set_text_bypassing_undo_history(L""); set_position(0); } -void editable_line_t::push_edit(edit_t &&edit) { +void editable_line_t::push_edit(edit_t &&edit, bool allow_coalesce) { + bool is_insertion = edit.length == 0; + /// Coalescing insertion does not create a new undo entry but adds to the last insertion. + if (allow_coalesce && is_insertion && want_to_coalesce_insertion_of(edit.replacement)) { + assert(edit.offset == position()); + edit_t &last_edit = undo_history_.edits.back(); + last_edit.replacement.append(edit.replacement); + apply_edit(&text_, edit); + set_position(position() + edit.replacement.size()); + + assert(undo_history_.may_coalesce); + return; + } + // 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_; @@ -279,42 +275,37 @@ void editable_line_t::push_edit(edit_t &&edit) { bool edit_does_nothing = edit.length == 0 && edit.replacement.empty(); if (edit_does_nothing) return; - if (undo_history.edits_applied != undo_history.edits.size()) { + if (undo_history_.edits_applied != undo_history_.edits.size()) { // After undoing some edits, the user is making a new edit; // we are about to create a new edit branch. // Discard all edits that were undone because we only support // linear undo/redo, they will be unreachable. - undo_history.edits.erase(undo_history.edits.begin() + undo_history.edits_applied, - undo_history.edits.end()); + undo_history_.edits.erase(undo_history_.edits.begin() + undo_history_.edits_applied, + undo_history_.edits.end()); } edit.cursor_position_before_edit = position(); edit.old = text_.substr(edit.offset, edit.length); apply_edit(&text_, edit); set_position(cursor_position_after_edit(edit)); - assert(undo_history.edits_applied == undo_history.edits.size()); - undo_history.edits_applied++; - undo_history.edits.emplace_back(edit); -} - -void editable_line_t::insert_coalesce(const wcstring &str) { - edit_t &edit = undo_history.edits.back(); - edit.replacement.append(str); - apply_edit(&text_, edit_t(position(), 0, str)); - set_position(position() + str.size()); + assert(undo_history_.edits_applied == undo_history_.edits.size()); + undo_history_.may_coalesce = + is_insertion && (undo_history_.try_coalesce || edit.replacement.size() == 1); + undo_history_.edits_applied++; + undo_history_.edits.emplace_back(std::move(edit)); } bool editable_line_t::redo() { 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); + 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++; + undo_history_.edits_applied++; apply_edit(&text_, edit); set_position(cursor_position_after_edit(edit)); did_redo = true; @@ -327,9 +318,9 @@ bool editable_line_t::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; + undo_history_.may_coalesce = false; // Indicate that future changes should be coalesced into the same edit if possible. - undo_history.try_coalesce = true; + undo_history_.try_coalesce = true; // Assign a logical edit group id to future edits in this group edit_group_id_ += 1; } @@ -343,11 +334,28 @@ void editable_line_t::end_edit_group() { } if (--edit_group_level_ == -1) { - undo_history.try_coalesce = false; - undo_history.may_coalesce = false; + undo_history_.try_coalesce = false; + undo_history_.may_coalesce = false; } } +/// Whether we want to append this string to the previous edit. +bool editable_line_t::want_to_coalesce_insertion_of(const wcstring &str) const { + // The previous edit must support coalescing. + if (!undo_history_.may_coalesce) return false; + // Only consolidate single character inserts. + if (str.size() != 1) return false; + // Make an undo group after every space. + if (str.at(0) == L' ' && !undo_history_.try_coalesce) return false; + assert(!undo_history_.edits.empty()); + const edit_t &last_edit = undo_history_.edits.back(); + // Don't add to the last edit if it deleted something. + if (last_edit.length != 0) return false; + // Must not have moved the cursor! + if (cursor_position_after_edit(last_edit) != position()) return false; + return true; +} + // Make the search case-insensitive unless we have an uppercase character. static history_search_flags_t smartcase_flags(const wcstring &query) { return query == wcstolower(query) ? history_search_ignore_case : 0; @@ -1696,13 +1704,7 @@ void reader_data_t::delete_char(bool backward) { /// Returns true if the string changed. void reader_data_t::insert_string(editable_line_t *el, const wcstring &str) { if (!str.empty()) { - if (!history_search.active() && want_to_coalesce_insertion_of(*el, str)) { - el->insert_coalesce(str); - assert(el->undo_history.may_coalesce); - } else { - el->push_edit(edit_t(el->position(), 0, str)); - el->undo_history.may_coalesce = el->undo_history.try_coalesce || (str.size() == 1); - } + el->push_edit(edit_t(el->position(), 0, str), !history_search.active()); } if (el == &command_line) { @@ -1713,8 +1715,7 @@ void reader_data_t::insert_string(editable_line_t *el, const wcstring &str) { } void reader_data_t::push_edit(editable_line_t *el, edit_t &&edit) { - el->push_edit(std::move(edit)); - el->undo_history.may_coalesce = false; + el->push_edit(std::move(edit), false); maybe_refilter_pager(el); } @@ -3048,7 +3049,7 @@ void reader_data_t::compute_and_apply_completions(readline_cmd_t c, readline_loo rls.complete_did_insert = false; size_t tok_off = static_cast(token_begin - buff); size_t tok_len = static_cast(token_end - token_begin); - el->push_edit(edit_t{tok_off, tok_len, std::move(wc_expanded)}); + push_edit(el, edit_t{tok_off, tok_len, std::move(wc_expanded)}); return; } diff --git a/src/reader.h b/src/reader.h index 6306e02b2..989fff6a2 100644 --- a/src/reader.h +++ b/src/reader.h @@ -80,8 +80,6 @@ struct undo_history_t { /// Helper class for storing a command line. class editable_line_t { public: - undo_history_t undo_history; - const wcstring &text() const { return text_; } /// Set the text directly without maintaining undo invariants. Use with caution. void set_text_bypassing_undo_history(wcstring &&text) { text_ = text; } @@ -100,12 +98,7 @@ class editable_line_t { /// Modify the commandline according to @edit. Most modifications to the /// text should pass through this function. - void push_edit(edit_t &&edit); - - /// Modify the commandline by inserting a string at the cursor. - /// Does not create a new undo point, but adds to the last edit which - /// must be an insertion, too. - void insert_coalesce(const wcstring &str); + void push_edit(edit_t &&edit, bool allow_coalesce); /// Undo the most recent edit that was not yet undone. Returns true on success. bool undo(); @@ -119,11 +112,16 @@ class editable_line_t { void end_edit_group(); private: + /// Whether we want to append this string to the previous edit. + bool want_to_coalesce_insertion_of(const wcstring &str) const; + /// The command line. wcstring text_; /// The current position of the cursor in the command line. size_t position_ = 0; + /// The history of all edits. + undo_history_t undo_history_; /// 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;