From 00a91bb02c00a0670b15357afe3df3218eef4472 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 30 Nov 2019 16:50:24 +0100 Subject: [PATCH] LibGUI: Consolidate and simplify commands used for insertion/removal This patch adds InsertTextCommand and RemoveTextCommand. These two commands are used to ... insert and remove text :^) The bulk of the logic is moved into GTextDocument, and we now use the command's redo() virtual to perform the action. Or in other words, when you type into the text editor, we create an InsertTextCommand, push it onto the undo stack, and call redo() on it immediately. That's how the text gets inserted. This makes it quite easy to implement more commands, as there is no distinction between a redo() and the initial application. --- Libraries/LibGUI/GTextDocument.cpp | 232 ++++++++++++++++++----------- Libraries/LibGUI/GTextDocument.h | 43 ++---- Libraries/LibGUI/GTextEditor.cpp | 182 ++-------------------- Libraries/LibGUI/GTextEditor.h | 10 +- 4 files changed, 179 insertions(+), 288 deletions(-) diff --git a/Libraries/LibGUI/GTextDocument.cpp b/Libraries/LibGUI/GTextDocument.cpp index 95123f22b0..2cedd2c1bf 100644 --- a/Libraries/LibGUI/GTextDocument.cpp +++ b/Libraries/LibGUI/GTextDocument.cpp @@ -392,110 +392,164 @@ GTextDocumentUndoCommand::~GTextDocumentUndoCommand() { } -InsertCharacterCommand::InsertCharacterCommand(GTextDocument& document, char ch, GTextPosition text_position) +InsertTextCommand::InsertTextCommand(GTextDocument& document, const String& text, const GTextPosition& position) : GTextDocumentUndoCommand(document) - , m_character(ch) - , m_text_position(text_position) + , m_text(text) + , m_range({ position, position }) { } -RemoveCharacterCommand::RemoveCharacterCommand(GTextDocument& document, char ch, GTextPosition text_position) +void InsertTextCommand::redo() +{ + auto new_cursor = m_document.insert_at(m_range.start(), m_text); + // NOTE: We don't know where the range ends until after doing redo(). + // This is okay since we always do redo() after adding this to the undo stack. + m_range.set_end(new_cursor); + m_document.set_all_cursors(new_cursor); +} + +void InsertTextCommand::undo() +{ + m_document.remove(m_range); + m_document.set_all_cursors(m_range.start()); +} + +RemoveTextCommand::RemoveTextCommand(GTextDocument& document, const String& text, const GTextRange& range) : GTextDocumentUndoCommand(document) - , m_character(ch) - , m_text_position(text_position) + , m_text(text) + , m_range(range) { } -RemoveLineCommand::RemoveLineCommand(GTextDocument& document, String line_content, GTextPosition text_position, bool has_merged_content) - : GTextDocumentUndoCommand(document) - , m_line_content(move(line_content)) - , m_text_position(text_position) - , m_has_merged_content(has_merged_content) +void RemoveTextCommand::redo() { + m_document.remove(m_range); + m_document.set_all_cursors(m_range.start()); } -CreateLineCommand::CreateLineCommand(GTextDocument& document, Vector line_content, GTextPosition text_position) - : GTextDocumentUndoCommand(document) - , m_line_content(move(line_content)) - , m_text_position(text_position) +void RemoveTextCommand::undo() { -} - -void InsertCharacterCommand::undo() -{ - m_document.lines()[m_text_position.line()].remove(m_document, (m_text_position.column() - 1)); - m_document.notify_did_change(); -} - -void InsertCharacterCommand::redo() -{ - m_document.lines()[m_text_position.line()].insert(m_document, m_text_position.column() - 1, m_character); -} - -void RemoveCharacterCommand::undo() -{ - m_document.lines()[m_text_position.line()].insert(m_document, m_text_position.column(), m_character); -} - -void RemoveCharacterCommand::redo() -{ - m_document.lines()[m_text_position.line()].remove(m_document, (m_text_position.column())); - m_document.notify_did_change(); -} - -void RemoveLineCommand::undo() -{ - // Insert back the line - m_document.insert_line(m_text_position.line(), make(m_document, m_line_content)); - - // Remove the merged line contents - if (m_has_merged_content) { - for (int i = m_line_content.length() - 1; i >= 0; i--) - m_document.lines()[m_text_position.line() - 1].remove(m_document, (m_text_position.column()) + i); - } -} - -void RemoveLineCommand::redo() -{ - // Remove the created line - m_document.remove_line(m_text_position.line()); - - // Add back the line contents - if (m_has_merged_content) { - for (int i = 0; i < m_line_content.length(); i++) - m_document.lines()[m_text_position.line() - 1].insert(m_document, (m_text_position.column()) + i, m_line_content[i]); - } -} - -void CreateLineCommand::undo() -{ - // Insert back the created line portion - for (int i = 0; i < m_line_content.size(); i++) - m_document.lines()[m_text_position.line()].insert(m_document, (m_text_position.column() - 1) + i, m_line_content[i]); - - // Move the cursor up a row back before the split. - m_document.set_all_cursors({ m_text_position.line(), m_document.lines()[m_text_position.line()].length() }); - - // Remove the created line - m_document.remove_line(m_text_position.line() + 1); -} - -void CreateLineCommand::redo() -{ - // Remove the characters that we're inserted back - for (int i = m_line_content.size() - 1; i >= 0; i--) - m_document.lines()[m_text_position.line()].remove(m_document, (m_text_position.column()) + i); - - m_document.notify_did_change(); - - // Then we want to add BACK the created line - m_document.insert_line(m_text_position.line() + 1, make(m_document, "")); - - for (int i = 0; i < m_line_content.size(); i++) - m_document.lines()[m_text_position.line() + 1].insert(m_document, i, m_line_content[i]); + auto new_cursor = m_document.insert_at(m_range.start(), m_text); + m_document.set_all_cursors(new_cursor); } void GTextDocument::update_undo_timer() { m_undo_stack.finalize_current_combo(); } + +GTextPosition GTextDocument::insert_at(const GTextPosition& position, const StringView& text) +{ + GTextPosition cursor = position; + for (int i = 0; i < text.length(); ++i) { + cursor = insert_at(cursor, text[i]); + } + return cursor; +} + +GTextPosition GTextDocument::insert_at(const GTextPosition& position, char ch) +{ + // FIXME: We need these from GTextEditor! + bool m_automatic_indentation_enabled = true; + int m_soft_tab_width = 4; + + bool at_head = position.column() == 0; + bool at_tail = position.column() == line(position.line()).length(); + if (ch == '\n') { + if (at_tail || at_head) { + String new_line_contents; + if (m_automatic_indentation_enabled && at_tail) { + int leading_spaces = 0; + auto& old_line = lines()[position.line()]; + for (int i = 0; i < old_line.length(); ++i) { + if (old_line.characters()[i] == ' ') + ++leading_spaces; + else + break; + } + if (leading_spaces) + new_line_contents = String::repeated(' ', leading_spaces); + } + + int row = position.line(); + Vector line_content; + for (int i = position.column(); i < line(row).length(); i++) + line_content.append(line(row).characters()[i]); + insert_line(position.line() + (at_tail ? 1 : 0), make(*this, new_line_contents)); + notify_did_change(); + return { position.line() + 1, line(position.line() + 1).length() }; + } + auto new_line = make(*this); + new_line->append(*this, line(position.line()).characters() + position.column(), line(position.line()).length() - position.column()); + + Vector line_content; + for (int i = 0; i < new_line->length(); i++) + line_content.append(new_line->characters()[i]); + line(position.line()).truncate(*this, position.column()); + insert_line(position.line() + 1, move(new_line)); + notify_did_change(); + return { position.line() + 1, 0 }; + } + if (ch == '\t') { + int next_soft_tab_stop = ((position.column() + m_soft_tab_width) / m_soft_tab_width) * m_soft_tab_width; + int spaces_to_insert = next_soft_tab_stop - position.column(); + for (int i = 0; i < spaces_to_insert; ++i) { + line(position.line()).insert(*this, position.column(), ' '); + } + notify_did_change(); + return { position.line(), next_soft_tab_stop }; + } + line(position.line()).insert(*this, position.column(), ch); + notify_did_change(); + return { position.line(), position.column() + 1 }; +} + +void GTextDocument::remove(const GTextRange& unnormalized_range) +{ + if (!unnormalized_range.is_valid()) + return; + + auto range = unnormalized_range.normalized(); + + // First delete all the lines in between the first and last one. + for (int i = range.start().line() + 1; i < range.end().line();) { + remove_line(i); + range.end().set_line(range.end().line() - 1); + } + + if (range.start().line() == range.end().line()) { + // Delete within same line. + auto& line = lines()[range.start().line()]; + bool whole_line_is_selected = range.start().column() == 0 && range.end().column() == line.length(); + + if (whole_line_is_selected) { + line.clear(*this); + } else { + auto before_selection = String(line.characters(), line.length()).substring(0, range.start().column()); + auto after_selection = String(line.characters(), line.length()).substring(range.end().column(), line.length() - range.end().column()); + StringBuilder builder(before_selection.length() + after_selection.length()); + builder.append(before_selection); + builder.append(after_selection); + line.set_text(*this, builder.to_string()); + } + } else { + // Delete across a newline, merging lines. + ASSERT(range.start().line() == range.end().line() - 1); + auto& first_line = lines()[range.start().line()]; + auto& second_line = lines()[range.end().line()]; + auto before_selection = String(first_line.characters(), first_line.length()).substring(0, range.start().column()); + auto after_selection = String(second_line.characters(), second_line.length()).substring(range.end().column(), second_line.length() - range.end().column()); + StringBuilder builder(before_selection.length() + after_selection.length()); + builder.append(before_selection); + builder.append(after_selection); + + first_line.set_text(*this, builder.to_string()); + remove_line(range.end().line()); + } + + if (lines().is_empty()) { + append_line(make(*this)); + } + + notify_did_change(); +} diff --git a/Libraries/LibGUI/GTextDocument.h b/Libraries/LibGUI/GTextDocument.h index 7b527559dd..9e14b4aaaf 100644 --- a/Libraries/LibGUI/GTextDocument.h +++ b/Libraries/LibGUI/GTextDocument.h @@ -33,49 +33,26 @@ protected: GTextDocument& m_document; }; -class InsertCharacterCommand : public GTextDocumentUndoCommand { +class InsertTextCommand : public GTextDocumentUndoCommand { public: - InsertCharacterCommand(GTextDocument&, char, GTextPosition); + InsertTextCommand(GTextDocument&, const String&, const GTextPosition&); virtual void undo() override; virtual void redo() override; private: - char m_character; - GTextPosition m_text_position; + String m_text; + GTextRange m_range; }; -class RemoveCharacterCommand : public GTextDocumentUndoCommand { +class RemoveTextCommand : public GTextDocumentUndoCommand { public: - RemoveCharacterCommand(GTextDocument&, char, GTextPosition); + RemoveTextCommand(GTextDocument&, const String&, const GTextRange&); virtual void undo() override; virtual void redo() override; private: - char m_character; - GTextPosition m_text_position; -}; - -class RemoveLineCommand : public GTextDocumentUndoCommand { -public: - RemoveLineCommand(GTextDocument&, String, GTextPosition, bool has_merged_content); - virtual void undo() override; - virtual void redo() override; - -private: - String m_line_content; - GTextPosition m_text_position; - bool m_has_merged_content; -}; - -class CreateLineCommand : public GTextDocumentUndoCommand { -public: - CreateLineCommand(GTextDocument&, Vector line_content, GTextPosition); - virtual void undo() override; - virtual void redo() override; - -private: - Vector m_line_content; - GTextPosition m_text_position; + String m_text; + GTextRange m_range; }; class GTextDocument : public RefCounted { @@ -152,6 +129,10 @@ public: void notify_did_change(); void set_all_cursors(const GTextPosition&); + GTextPosition insert_at(const GTextPosition&, char); + GTextPosition insert_at(const GTextPosition&, const StringView&); + void remove(const GTextRange&); + private: explicit GTextDocument(Client* client); diff --git a/Libraries/LibGUI/GTextEditor.cpp b/Libraries/LibGUI/GTextEditor.cpp index 7e4b1ce92c..6515e723ea 100644 --- a/Libraries/LibGUI/GTextEditor.cpp +++ b/Libraries/LibGUI/GTextEditor.cpp @@ -537,7 +537,7 @@ void GTextEditor::sort_selected_lines() if (!has_selection()) return; - + int first_line; int last_line; get_selection_line_boundaries(first_line, last_line); @@ -766,32 +766,16 @@ void GTextEditor::keydown_event(GKeyEvent& event) } // Backspace within line - for (int i = 0; i < erase_count; ++i) { - int row = m_cursor.line(); - int column = m_cursor.column() - 1 - i; - document().add_to_undo_stack(make(document(), document().line(row).characters()[column], GTextPosition(row, column))); - current_line().remove(document(), m_cursor.column() - 1 - i); - } - update_content_size(); - set_cursor(m_cursor.line(), m_cursor.column() - erase_count); - did_change(); + GTextRange 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); return; } if (m_cursor.column() == 0 && m_cursor.line() != 0) { // Backspace at column 0; merge with previous line - auto& previous_line = lines()[m_cursor.line() - 1]; - int previous_length = previous_line.length(); - - int row = m_cursor.line(); - int column = previous_length; - document().add_to_undo_stack(make(document(), String(lines()[m_cursor.line()].view()), GTextPosition(row, column), true)); - - previous_line.append(document(), current_line().characters(), current_line().length()); - document().remove_line(m_cursor.line()); - update_content_size(); - update(); - set_cursor(m_cursor.line() - 1, previous_length); - did_change(); + int previous_length = line(m_cursor.line() - 1).length(); + GTextRange erased_range({ m_cursor.line() - 1, previous_length }, m_cursor); + execute("\n", erased_range); return; } return; @@ -857,81 +841,6 @@ void GTextEditor::do_delete() } } -void GTextEditor::insert_at_cursor(const StringView& text) -{ - // FIXME: This should obviously not be implemented this way. - for (int i = 0; i < text.length(); ++i) { - insert_at_cursor(text[i]); - } -} - -void GTextEditor::insert_at_cursor(char ch) -{ - bool at_head = m_cursor.column() == 0; - bool at_tail = m_cursor.column() == current_line().length(); - if (ch == '\n') { - if (at_tail || at_head) { - String new_line_contents; - if (m_automatic_indentation_enabled && at_tail) { - int leading_spaces = 0; - auto& old_line = lines()[m_cursor.line()]; - for (int i = 0; i < old_line.length(); ++i) { - if (old_line.characters()[i] == ' ') - ++leading_spaces; - else - break; - } - if (leading_spaces) - new_line_contents = String::repeated(' ', leading_spaces); - } - - int row = m_cursor.line(); - int column = m_cursor.column() + 1; - Vector line_content; - for (int i = m_cursor.column(); i < document().lines()[row].length(); i++) - line_content.append(document().lines()[row].characters()[i]); - document().add_to_undo_stack(make(document(), line_content, GTextPosition(row, column))); - - document().insert_line(m_cursor.line() + (at_tail ? 1 : 0), make(document(), new_line_contents)); - update(); - did_change(); - set_cursor(m_cursor.line() + 1, lines()[m_cursor.line() + 1].length()); - return; - } - auto new_line = make(document()); - new_line->append(document(), current_line().characters() + m_cursor.column(), current_line().length() - m_cursor.column()); - - int row = m_cursor.line(); - int column = m_cursor.column() + 1; - Vector line_content; - for (int i = 0; i < new_line->length(); i++) - line_content.append(new_line->characters()[i]); - document().add_to_undo_stack(make(document(), line_content, GTextPosition(row, column))); - - current_line().truncate(document(), m_cursor.column()); - document().insert_line(m_cursor.line() + 1, move(new_line)); - update(); - did_change(); - set_cursor(m_cursor.line() + 1, 0); - return; - } - if (ch == '\t') { - int next_soft_tab_stop = ((m_cursor.column() + m_soft_tab_width) / m_soft_tab_width) * m_soft_tab_width; - int spaces_to_insert = next_soft_tab_stop - m_cursor.column(); - for (int i = 0; i < spaces_to_insert; ++i) { - current_line().insert(document(), m_cursor.column(), ' '); - } - did_change(); - set_cursor(m_cursor.line(), next_soft_tab_stop); - return; - } - current_line().insert(document(), m_cursor.column(), ch); - did_change(); - set_cursor(m_cursor.line(), m_cursor.column() + 1); - - document().add_to_undo_stack(make(document(), ch, m_cursor)); -} - int GTextEditor::content_x_for_position(const GTextPosition& position) const { auto& line = lines()[position.line()]; @@ -1171,72 +1080,8 @@ String GTextEditor::selected_text() const void GTextEditor::delete_selection() { - if (!has_selection()) - return; - auto selection = normalized_selection(); - - // First delete all the lines in between the first and last one. - for (int i = selection.start().line() + 1; i < selection.end().line();) { - int row = i; - int column = lines()[i].length(); - document().add_to_undo_stack(make(document(), String(lines()[i].view()), GTextPosition(row, column), false)); - - document().remove_line(i); - selection.end().set_line(selection.end().line() - 1); - } - - if (selection.start().line() == selection.end().line()) { - // Delete within same line. - auto& line = lines()[selection.start().line()]; - bool whole_line_is_selected = selection.start().column() == 0 && selection.end().column() == line.length(); - - for (int i = selection.end().column() - 1; i >= selection.start().column(); i--) { - int row = selection.start().line(); - int column = i; - document().add_to_undo_stack(make(document(), document().line(row).characters()[column], GTextPosition(row, column))); - } - - if (whole_line_is_selected) { - line.clear(document()); - } else { - auto before_selection = String(line.characters(), line.length()).substring(0, selection.start().column()); - auto after_selection = String(line.characters(), line.length()).substring(selection.end().column(), line.length() - selection.end().column()); - StringBuilder builder(before_selection.length() + after_selection.length()); - builder.append(before_selection); - builder.append(after_selection); - line.set_text(document(), builder.to_string()); - } - } else { - // Delete across a newline, merging lines. - ASSERT(selection.start().line() == selection.end().line() - 1); - auto& first_line = lines()[selection.start().line()]; - auto& second_line = lines()[selection.end().line()]; - auto before_selection = String(first_line.characters(), first_line.length()).substring(0, selection.start().column()); - auto after_selection = String(second_line.characters(), second_line.length()).substring(selection.end().column(), second_line.length() - selection.end().column()); - StringBuilder builder(before_selection.length() + after_selection.length()); - builder.append(before_selection); - builder.append(after_selection); - - for (int i = first_line.length() - 1; i > selection.start().column() - 1; i--) { - int row = selection.start().line(); - int column = i; - document().add_to_undo_stack(make(document(), document().line(row).characters()[column], GTextPosition(row, column))); - } - - document().add_to_undo_stack(make(document(), String(second_line.view()), selection.end(), false)); - - first_line.set_text(document(), builder.to_string()); - document().remove_line(selection.end().line()); - - for (int i = (first_line.length()) - after_selection.length(); i < first_line.length(); i++) - document().add_to_undo_stack(make(document(), first_line.characters()[i], GTextPosition(selection.start().line(), i + 1))); - } - - if (lines().is_empty()) { - document().append_line(make(document())); - } - + execute(selected_text(), selection); m_selection.clear(); did_update_selection(); did_change(); @@ -1249,7 +1094,7 @@ void GTextEditor::insert_at_cursor_or_replace_selection(const StringView& text) ASSERT(!is_readonly()); if (has_selection()) delete_selection(); - insert_at_cursor(text); + execute(text, m_cursor); } void GTextEditor::cut() @@ -1391,8 +1236,13 @@ void GTextEditor::recompute_all_visual_lines() void GTextEditor::ensure_cursor_is_valid() { - if (cursor().column() > lines()[cursor().line()].length()) - set_cursor(cursor().line(), cursor().column() - (lines()[cursor().line()].length() - cursor().column())); + auto new_cursor = m_cursor; + if (new_cursor.line() >= lines().size()) + new_cursor.set_line(lines().size() - 1); + if (new_cursor.column() > lines()[new_cursor.line()].length()) + new_cursor.set_column(lines()[new_cursor.line()].length()); + if (m_cursor != new_cursor) + set_cursor(new_cursor); } int GTextEditor::visual_line_containing(int line_index, int column) const diff --git a/Libraries/LibGUI/GTextEditor.h b/Libraries/LibGUI/GTextEditor.h index a5be13a415..2b5f658eb0 100644 --- a/Libraries/LibGUI/GTextEditor.h +++ b/Libraries/LibGUI/GTextEditor.h @@ -152,8 +152,6 @@ private: const GTextDocumentLine& line(int index) const { return document().line(index); } GTextDocumentLine& current_line() { return line(m_cursor.line()); } const GTextDocumentLine& current_line() const { return line(m_cursor.line()); } - void insert_at_cursor(char); - void insert_at_cursor(const StringView&); int ruler_width() const; Rect ruler_content_rect(int line) const; void toggle_selection_if_needed_for_event(const GKeyEvent&); @@ -174,6 +172,14 @@ private: int visual_line_containing(int line_index, int column) const; void recompute_visual_lines(int line_index); + template + inline void execute(Args&&... args) + { + auto command = make(*m_document, forward(args)...); + command->redo(); + m_document->add_to_undo_stack(move(command)); + } + Type m_type { MultiLine }; GTextPosition m_cursor;