From 200a4a00ddb816aebb1954179022ecfa82194f1c Mon Sep 17 00:00:00 2001 From: Xuekun Li Date: Thu, 18 May 2023 16:43:46 +0800 Subject: [PATCH] LibGUI: Fix wrong cursor position after undoing `RemoveTextCommand` When you undo some forward delete shortcuts like or , the cursor will be put at the end of the text deleted, while the right position should be the start of those text. --- Userland/Libraries/LibGUI/TextDocument.cpp | 7 +++--- Userland/Libraries/LibGUI/TextDocument.h | 3 ++- Userland/Libraries/LibGUI/TextEditor.cpp | 26 +++++++++++----------- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/Userland/Libraries/LibGUI/TextDocument.cpp b/Userland/Libraries/LibGUI/TextDocument.cpp index 17aa14605d..182004504d 100644 --- a/Userland/Libraries/LibGUI/TextDocument.cpp +++ b/Userland/Libraries/LibGUI/TextDocument.cpp @@ -963,10 +963,11 @@ void InsertTextCommand::undo() m_document.set_all_cursors(m_range.start()); } -RemoveTextCommand::RemoveTextCommand(TextDocument& document, DeprecatedString const& text, TextRange const& range) +RemoveTextCommand::RemoveTextCommand(TextDocument& document, DeprecatedString const& text, TextRange const& range, TextPosition const& original_cursor_position) : TextDocumentUndoCommand(document) , m_text(text) , m_range(range) + , m_original_cursor_position(original_cursor_position) { } @@ -1006,8 +1007,8 @@ void RemoveTextCommand::redo() void RemoveTextCommand::undo() { - auto new_cursor = m_document.insert_at(m_range.start(), m_text); - m_document.set_all_cursors(new_cursor); + m_document.insert_at(m_range.start(), m_text); + m_document.set_all_cursors(m_original_cursor_position); } InsertLineCommand::InsertLineCommand(TextDocument& document, TextPosition cursor, DeprecatedString&& text, InsertPosition pos) diff --git a/Userland/Libraries/LibGUI/TextDocument.h b/Userland/Libraries/LibGUI/TextDocument.h index b005014f14..c381347501 100644 --- a/Userland/Libraries/LibGUI/TextDocument.h +++ b/Userland/Libraries/LibGUI/TextDocument.h @@ -254,7 +254,7 @@ private: class RemoveTextCommand : public TextDocumentUndoCommand { public: - RemoveTextCommand(TextDocument&, DeprecatedString const&, TextRange const&); + RemoveTextCommand(TextDocument&, DeprecatedString const&, TextRange const&, TextPosition const&); virtual ~RemoveTextCommand() = default; virtual void undo() override; virtual void redo() override; @@ -265,6 +265,7 @@ public: private: DeprecatedString m_text; TextRange m_range; + TextPosition m_original_cursor_position; }; class InsertLineCommand : public TextDocumentUndoCommand { diff --git a/Userland/Libraries/LibGUI/TextEditor.cpp b/Userland/Libraries/LibGUI/TextEditor.cpp index 8082310433..41443a1478 100644 --- a/Userland/Libraries/LibGUI/TextEditor.cpp +++ b/Userland/Libraries/LibGUI/TextEditor.cpp @@ -1126,7 +1126,7 @@ void TextEditor::keydown_event(KeyEvent& event) erase_count = grapheme_break_position - m_cursor.column(); } TextRange erased_range(m_cursor, { m_cursor.line(), m_cursor.column() + erase_count }); - execute(document().text_in_range(erased_range), erased_range); + execute(document().text_in_range(erased_range), erased_range, erased_range.start()); return; } if (m_cursor.column() == current_line().length() && m_cursor.line() != line_count() - 1) { @@ -1136,7 +1136,7 @@ void TextEditor::keydown_event(KeyEvent& event) erase_count = document().first_word_break_after({ m_cursor.line() + 1, 0 }).column(); } TextRange erased_range(m_cursor, { m_cursor.line() + 1, erase_count }); - execute(document().text_in_range(erased_range), erased_range); + execute(document().text_in_range(erased_range), erased_range, erased_range.end()); return; } return; @@ -1172,14 +1172,14 @@ void TextEditor::keydown_event(KeyEvent& event) // Backspace within line TextRange erased_range({ m_cursor.line(), m_cursor.column() - erase_count }, m_cursor); auto erased_text = document().text_in_range(erased_range); - execute(erased_text, erased_range); + execute(erased_text, erased_range, erased_range.end()); return; } if (m_cursor.column() == 0 && m_cursor.line() != 0) { // Backspace at column 0; merge with previous line size_t previous_length = line(m_cursor.line() - 1).length(); TextRange erased_range({ m_cursor.line() - 1, previous_length }, m_cursor); - execute("\n", erased_range); + execute("\n", erased_range, erased_range.end()); return; } return; @@ -1278,7 +1278,7 @@ void TextEditor::unindent_line() void TextEditor::delete_previous_word() { TextRange to_erase(document().first_word_before(m_cursor, true), m_cursor); - execute(document().text_in_range(to_erase), to_erase); + execute(document().text_in_range(to_erase), to_erase, to_erase.end()); } void TextEditor::delete_current_line() @@ -1300,7 +1300,7 @@ void TextEditor::delete_current_line() } TextRange erased_range(start, end); - execute(document().text_in_range(erased_range), erased_range); + execute(document().text_in_range(erased_range), erased_range, m_cursor); } void TextEditor::delete_previous_char() @@ -1317,14 +1317,14 @@ void TextEditor::delete_previous_char() to_erase.set_start({ m_cursor.line() - 1, prev_line_len }); } - execute(document().text_in_range(to_erase), to_erase); + execute(document().text_in_range(to_erase), to_erase, to_erase.end()); } void TextEditor::delete_from_line_start_to_cursor() { TextPosition start(m_cursor.line(), current_line().first_non_whitespace_column()); TextRange to_erase(start, m_cursor); - execute(document().text_in_range(to_erase), to_erase); + execute(document().text_in_range(to_erase), to_erase, m_cursor); } void TextEditor::do_delete() @@ -1338,13 +1338,13 @@ void TextEditor::do_delete() if (m_cursor.column() < current_line().length()) { // Delete within line TextRange erased_range(m_cursor, { m_cursor.line(), m_cursor.column() + 1 }); - execute(document().text_in_range(erased_range), erased_range); + execute(document().text_in_range(erased_range), erased_range, erased_range.start()); return; } if (m_cursor.column() == current_line().length() && m_cursor.line() != line_count() - 1) { // Delete at end of line; merge with next line TextRange erased_range(m_cursor, { m_cursor.line() + 1, 0 }); - execute(document().text_in_range(erased_range), erased_range); + execute(document().text_in_range(erased_range), erased_range, erased_range.start()); return; } } @@ -1755,7 +1755,7 @@ void TextEditor::delete_selection() auto selection = normalized_selection(); auto selected = selected_text(); m_selection.clear(); - execute(selected, selection); + execute(selected, selection, selection.end()); did_update_selection(); did_change(); set_cursor(selection.start()); @@ -1765,7 +1765,7 @@ void TextEditor::delete_selection() void TextEditor::delete_text_range(TextRange range) { auto normalized_range = range.normalized(); - execute(document().text_in_range(normalized_range), normalized_range); + execute(document().text_in_range(normalized_range), normalized_range, normalized_range.end()); did_change(); set_cursor(normalized_range.start()); update(); @@ -1791,7 +1791,7 @@ void TextEditor::insert_at_cursor_or_replace_selection(StringView text) TextPosition start(original_cursor_position.line() - 1, 0); TextPosition end(original_cursor_position.line() - 1, clear_length); TextRange erased_range(start, end); - execute(document().text_in_range(erased_range), erased_range); + execute(document().text_in_range(erased_range), erased_range, erased_range.end()); set_cursor(original_cursor_position); } }