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

@@ -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)
-    : GTextDocumentUndoCommand(document)
-    , m_character(ch)
-    , m_text_position(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);
 }
 
-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 InsertTextCommand::undo()
 {
+    m_document.remove(m_range);
+    m_document.set_all_cursors(m_range.start());
 }
 
-CreateLineCommand::CreateLineCommand(GTextDocument& document, Vector<char> line_content, GTextPosition text_position)
+RemoveTextCommand::RemoveTextCommand(GTextDocument& document, const String& text, const GTextRange& range)
     : GTextDocumentUndoCommand(document)
-    , m_line_content(move(line_content))
-    , m_text_position(text_position)
-{
-}
-
-void InsertCharacterCommand::undo()
+    , m_text(text)
+    , m_range(range)
 {
-    m_document.lines()[m_text_position.line()].remove(m_document, (m_text_position.column() - 1));
-    m_document.notify_did_change();
 }
 
-void InsertCharacterCommand::redo()
+void RemoveTextCommand::redo()
 {
-    m_document.lines()[m_text_position.line()].insert(m_document, m_text_position.column() - 1, m_character);
+    m_document.remove(m_range);
+    m_document.set_all_cursors(m_range.start());
 }
 
-void RemoveCharacterCommand::undo()
+void RemoveTextCommand::undo()
 {
-    m_document.lines()[m_text_position.line()].insert(m_document, m_text_position.column(), m_character);
+    auto new_cursor = m_document.insert_at(m_range.start(), m_text);
+    m_document.set_all_cursors(new_cursor);
 }
 
-void RemoveCharacterCommand::redo()
+void GTextDocument::update_undo_timer()
 {
-    m_document.lines()[m_text_position.line()].remove(m_document, (m_text_position.column()));
-    m_document.notify_did_change();
+    m_undo_stack.finalize_current_combo();
 }
 
-void RemoveLineCommand::undo()
+GTextPosition GTextDocument::insert_at(const GTextPosition& position, const StringView& text)
 {
-    // Insert back the line
-    m_document.insert_line(m_text_position.line(), make<GTextDocumentLine>(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);
+    GTextPosition cursor = position;
+    for (int i = 0; i < text.length(); ++i) {
+        cursor = insert_at(cursor, text[i]);
     }
-}
-
-void RemoveLineCommand::redo()
-{
-    // Remove the created line
-    m_document.remove_line(m_text_position.line());
+    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);
+            }
 
-    // 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]);
+            int row = position.line();
+            Vector<char> 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<GTextDocumentLine>(*this, new_line_contents));
+            notify_did_change();
+            return { position.line() + 1, line(position.line() + 1).length() };
+        }
+        auto new_line = make<GTextDocumentLine>(*this);
+        new_line->append(*this, line(position.line()).characters() + position.column(), line(position.line()).length() - position.column());
+
+        Vector<char> 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 CreateLineCommand::undo()
+void GTextDocument::remove(const GTextRange& unnormalized_range)
 {
-    // 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() });
+    if (!unnormalized_range.is_valid())
+        return;
 
-    // Remove the created line
-    m_document.remove_line(m_text_position.line() + 1);
-}
+    auto range = unnormalized_range.normalized();
 
-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);
+    // 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);
+    }
 
-    m_document.notify_did_change();
+    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();
 
-    // Then we want to add BACK the created line
-    m_document.insert_line(m_text_position.line() + 1, make<GTextDocumentLine>(m_document, ""));
+        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());
+    }
 
-    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]);
-}
+    if (lines().is_empty()) {
+        append_line(make<GTextDocumentLine>(*this));
+    }
 
-void GTextDocument::update_undo_timer()
-{
-    m_undo_stack.finalize_current_combo();
+    notify_did_change();
 }

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<char> line_content, GTextPosition);
-    virtual void undo() override;
-    virtual void redo() override;
-
-private:
-    Vector<char> m_line_content;
-    GTextPosition m_text_position;
+    String m_text;
+    GTextRange m_range;
 };
 
 class GTextDocument : public RefCounted<GTextDocument> {
@@ -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);
 

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<RemoveCharacterCommand>(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<RemoveTextCommand>(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<RemoveLineCommand>(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<RemoveTextCommand>("\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<char> 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<CreateLineCommand>(document(), line_content, GTextPosition(row, column)));
-
-            document().insert_line(m_cursor.line() + (at_tail ? 1 : 0), make<GTextDocumentLine>(document(), new_line_contents));
-            update();
-            did_change();
-            set_cursor(m_cursor.line() + 1, lines()[m_cursor.line() + 1].length());
-            return;
-        }
-        auto new_line = make<GTextDocumentLine>(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<char> line_content;
-        for (int i = 0; i < new_line->length(); i++)
-            line_content.append(new_line->characters()[i]);
-        document().add_to_undo_stack(make<CreateLineCommand>(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<InsertCharacterCommand>(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<RemoveLineCommand>(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<RemoveCharacterCommand>(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<RemoveCharacterCommand>(document(), document().line(row).characters()[column], GTextPosition(row, column)));
-        }
-
-        document().add_to_undo_stack(make<RemoveLineCommand>(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<InsertCharacterCommand>(document(), first_line.characters()[i], GTextPosition(selection.start().line(), i + 1)));
-    }
-
-    if (lines().is_empty()) {
-        document().append_line(make<GTextDocumentLine>(document()));
-    }
-
+    execute<RemoveTextCommand>(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<InsertTextCommand>(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

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<class T, class... Args>
+    inline void execute(Args&&... args)
+    {
+        auto command = make<T>(*m_document, forward<Args>(args)...);
+        command->redo();
+        m_document->add_to_undo_stack(move(command));
+    }
+
     Type m_type { MultiLine };
 
     GTextPosition m_cursor;