EditingEngine: Split selection from movement functions

This patch moves selection updates outside movement functions in
EditingEngine.  Previously, movement functions would automatically
update the selection based on whether the Shift key was pressed down
during movement.  However, not all EditingEngine subclasses want that;
VimEditingEngine being a good example (because all selection is handled
in visual mode).

Therefore, this patch moves all selection updating to
EditingEngine::on_key().  Subclasses wishing to provide custom movement
and selection semantics should override it (and VimEditingEngine already
does).

Userland/Libraries/LibGUI/EditingEngine.cpp

@@ -39,14 +39,16 @@ bool EditingEngine::on_key(const KeyEvent& event)
             }
         }
         if (event.ctrl()) {
-            move_to_previous_span(event);
+            m_editor->update_selection(event.shift());
+            move_to_previous_span();
             if (event.shift() && m_editor->selection()->start().is_valid()) {
                 m_editor->selection()->set_end(m_editor->cursor());
                 m_editor->did_update_selection();
             }
             return true;
         }
-        move_one_left(event);
+        m_editor->update_selection(event.shift());
+        move_one_left();
         if (event.shift() && m_editor->selection()->start().is_valid()) {
             m_editor->selection()->set_end(m_editor->cursor());
             m_editor->did_update_selection();
@@ -68,7 +70,8 @@ bool EditingEngine::on_key(const KeyEvent& event)
             move_to_next_span(event);
             return true;
         }
-        move_one_right(event);
+        m_editor->update_selection(event.shift());
+        move_one_right();
         if (event.shift() && m_editor->selection()->start().is_valid()) {
             m_editor->selection()->set_end(m_editor->cursor());
             m_editor->did_update_selection();
@@ -77,6 +80,9 @@ bool EditingEngine::on_key(const KeyEvent& event)
     }
 
     if (event.key() == KeyCode::Key_Up) {
+        if (m_editor->cursor().line() > 0 || m_editor->is_wrapping_enabled()) {
+            m_editor->update_selection(event.shift());
+        }
         move_one_up(event);
         if (event.shift() && m_editor->selection()->start().is_valid()) {
             m_editor->selection()->set_end(m_editor->cursor());
@@ -86,6 +92,9 @@ bool EditingEngine::on_key(const KeyEvent& event)
     }
 
     if (event.key() == KeyCode::Key_Down) {
+        if (m_editor->cursor().line() < (m_editor->line_count() - 1) || m_editor->is_wrapping_enabled()) {
+            m_editor->update_selection(event.shift());
+        }
         move_one_down(event);
         if (event.shift() && m_editor->selection()->start().is_valid()) {
             m_editor->selection()->set_end(m_editor->cursor());
@@ -96,14 +105,14 @@ bool EditingEngine::on_key(const KeyEvent& event)
 
     if (event.key() == KeyCode::Key_Home) {
         if (event.ctrl()) {
-            m_editor->toggle_selection_if_needed_for_event(event.shift());
             move_to_first_line();
             if (event.shift() && m_editor->selection()->start().is_valid()) {
                 m_editor->selection()->set_end(m_editor->cursor());
                 m_editor->did_update_selection();
             }
         } else {
-            move_to_line_beginning(event);
+            m_editor->update_selection(event.shift());
+            move_to_line_beginning();
             if (event.shift() && m_editor->selection()->start().is_valid()) {
                 m_editor->selection()->set_end(m_editor->cursor());
                 m_editor->did_update_selection();
@@ -114,14 +123,14 @@ bool EditingEngine::on_key(const KeyEvent& event)
 
     if (event.key() == KeyCode::Key_End) {
         if (event.ctrl()) {
-            m_editor->toggle_selection_if_needed_for_event(event.shift());
             move_to_last_line();
             if (event.shift() && m_editor->selection()->start().is_valid()) {
                 m_editor->selection()->set_end(m_editor->cursor());
                 m_editor->did_update_selection();
             }
         } else {
-            move_to_line_end(event);
+            m_editor->update_selection(event.shift());
+            move_to_line_end();
             if (event.shift() && m_editor->selection()->start().is_valid()) {
                 m_editor->selection()->set_end(m_editor->cursor());
                 m_editor->did_update_selection();
@@ -131,7 +140,10 @@ bool EditingEngine::on_key(const KeyEvent& event)
     }
 
     if (event.key() == KeyCode::Key_PageUp) {
-        move_page_up(event);
+        if (m_editor->cursor().line() > 0 || m_editor->is_wrapping_enabled()) {
+            m_editor->update_selection(event.shift());
+        }
+        move_page_up();
         if (event.shift() && m_editor->selection()->start().is_valid()) {
             m_editor->selection()->set_end(m_editor->cursor());
             m_editor->did_update_selection();
@@ -140,7 +152,10 @@ bool EditingEngine::on_key(const KeyEvent& event)
     }
 
     if (event.key() == KeyCode::Key_PageDown) {
-        move_page_down(event);
+        if (m_editor->cursor().line() < (m_editor->line_count() - 1) || m_editor->is_wrapping_enabled()) {
+            m_editor->update_selection(event.shift());
+        }
+        move_page_down();
         if (event.shift() && m_editor->selection()->start().is_valid()) {
             m_editor->selection()->set_end(m_editor->cursor());
             m_editor->did_update_selection();
@@ -151,21 +166,19 @@ bool EditingEngine::on_key(const KeyEvent& event)
     return false;
 }
 
-void EditingEngine::move_one_left(const KeyEvent& event)
+void EditingEngine::move_one_left()
 {
     if (m_editor->cursor().column() > 0) {
         int new_column = m_editor->cursor().column() - 1;
-        m_editor->toggle_selection_if_needed_for_event(event.shift());
         m_editor->set_cursor(m_editor->cursor().line(), new_column);
     } else if (m_editor->cursor().line() > 0) {
         int new_line = m_editor->cursor().line() - 1;
         int new_column = m_editor->lines()[new_line].length();
-        m_editor->toggle_selection_if_needed_for_event(event.shift());
         m_editor->set_cursor(new_line, new_column);
     }
 }
 
-void EditingEngine::move_one_right(const KeyEvent& event)
+void EditingEngine::move_one_right()
 {
     int new_line = m_editor->cursor().line();
     int new_column = m_editor->cursor().column();
@@ -176,11 +189,10 @@ void EditingEngine::move_one_right(const KeyEvent& event)
         new_line = m_editor->cursor().line() + 1;
         new_column = 0;
     }
-    m_editor->toggle_selection_if_needed_for_event(event.shift());
     m_editor->set_cursor(new_line, new_column);
 }
 
-void EditingEngine::move_to_previous_span(const KeyEvent& event)
+void EditingEngine::move_to_previous_span()
 {
     TextPosition new_cursor;
     if (m_editor->document().has_spans()) {
@@ -194,7 +206,6 @@ void EditingEngine::move_to_previous_span(const KeyEvent& event)
     } else {
         new_cursor = m_editor->document().first_word_break_before(m_editor->cursor(), true);
     }
-    m_editor->toggle_selection_if_needed_for_event(event.shift());
     m_editor->set_cursor(new_cursor);
 }
 
@@ -212,7 +223,6 @@ void EditingEngine::move_to_next_span(const KeyEvent& event)
     } else {
         new_cursor = m_editor->document().first_word_break_after(m_editor->cursor());
     }
-    m_editor->toggle_selection_if_needed_for_event(event.shift());
     m_editor->set_cursor(new_cursor);
     if (event.shift() && m_editor->selection()->start().is_valid()) {
         m_editor->selection()->set_end(m_editor->cursor());
@@ -220,10 +230,9 @@ void EditingEngine::move_to_next_span(const KeyEvent& event)
     }
 }
 
-void EditingEngine::move_to_line_beginning(const KeyEvent& event)
+void EditingEngine::move_to_line_beginning()
 {
     TextPosition new_cursor;
-    m_editor->toggle_selection_if_needed_for_event(event.shift());
     if (m_editor->is_wrapping_enabled()) {
         // FIXME: Replicate the first_nonspace_column behavior in wrapping mode.
         auto home_position = m_editor->cursor_content_rect().location().translated(-m_editor->width(), 0);
@@ -239,7 +248,7 @@ void EditingEngine::move_to_line_beginning(const KeyEvent& event)
     m_editor->set_cursor(new_cursor);
 }
 
-void EditingEngine::move_to_line_end(const KeyEvent& event)
+void EditingEngine::move_to_line_end()
 {
     TextPosition new_cursor;
     if (m_editor->is_wrapping_enabled()) {
@@ -248,7 +257,6 @@ void EditingEngine::move_to_line_end(const KeyEvent& event)
     } else {
         new_cursor = { m_editor->cursor().line(), m_editor->current_line().length() };
     }
-    m_editor->toggle_selection_if_needed_for_event(event.shift());
     m_editor->set_cursor(new_cursor);
 }
 
@@ -268,7 +276,6 @@ void EditingEngine::move_one_up(const KeyEvent& event)
             size_t new_column = min(m_editor->cursor().column(), m_editor->line(new_line).length());
             new_cursor = { new_line, new_column };
         }
-        m_editor->toggle_selection_if_needed_for_event(event.shift());
         m_editor->set_cursor(new_cursor);
     }
 };
@@ -289,12 +296,11 @@ void EditingEngine::move_one_down(const KeyEvent& event)
             size_t new_column = min(m_editor->cursor().column(), m_editor->line(new_line).length());
             new_cursor = { new_line, new_column };
         }
-        m_editor->toggle_selection_if_needed_for_event(event.shift());
         m_editor->set_cursor(new_cursor);
     }
 };
 
-void EditingEngine::move_up(const KeyEvent& event, double page_height_factor)
+void EditingEngine::move_up(double page_height_factor)
 {
     if (m_editor->cursor().line() > 0 || m_editor->is_wrapping_enabled()) {
         int pixels = (int)(m_editor->visible_content_rect().height() * page_height_factor);
@@ -309,12 +315,11 @@ void EditingEngine::move_up(const KeyEvent& event, double page_height_factor)
             size_t new_column = min(m_editor->cursor().column(), m_editor->line(new_line).length());
             new_cursor = { new_line, new_column };
         }
-        m_editor->toggle_selection_if_needed_for_event(event.shift());
         m_editor->set_cursor(new_cursor);
     }
 };
 
-void EditingEngine::move_down(const KeyEvent& event, double page_height_factor)
+void EditingEngine::move_down(double page_height_factor)
 {
     if (m_editor->cursor().line() < (m_editor->line_count() - 1) || m_editor->is_wrapping_enabled()) {
         int pixels = (int)(m_editor->visible_content_rect().height() * page_height_factor);
@@ -327,19 +332,18 @@ void EditingEngine::move_down(const KeyEvent& event, double page_height_factor)
             size_t new_column = min(m_editor->cursor().column(), m_editor->lines()[new_line].length());
             new_cursor = { new_line, new_column };
         }
-        m_editor->toggle_selection_if_needed_for_event(event.shift());
         m_editor->set_cursor(new_cursor);
     };
 }
 
-void EditingEngine::move_page_up(const KeyEvent& event)
+void EditingEngine::move_page_up()
 {
-    move_up(event, 1);
+    move_up(1);
 };
 
-void EditingEngine::move_page_down(const KeyEvent& event)
+void EditingEngine::move_page_down()
 {
-    move_down(event, 1);
+    move_down(1);
 };
 
 void EditingEngine::move_to_first_line()

Userland/Libraries/LibGUI/EditingEngine.h

@@ -42,16 +42,16 @@ protected:
 
     WeakPtr<TextEditor> m_editor;
 
-    void move_one_left(const KeyEvent& event);
-    void move_one_right(const KeyEvent& event);
+    void move_one_left();
+    void move_one_right();
     void move_one_up(const KeyEvent& event);
     void move_one_down(const KeyEvent& event);
-    void move_to_previous_span(const KeyEvent& event);
+    void move_to_previous_span();
     void move_to_next_span(const KeyEvent& event);
-    void move_to_line_beginning(const KeyEvent& event);
-    void move_to_line_end(const KeyEvent& event);
-    void move_page_up(const KeyEvent& event);
-    void move_page_down(const KeyEvent& event);
+    void move_to_line_beginning();
+    void move_to_line_end();
+    void move_page_up();
+    void move_page_down();
     void move_to_first_line();
     void move_to_last_line();
     TextPosition find_beginning_of_next_word();
@@ -63,8 +63,8 @@ protected:
     TextPosition find_beginning_of_previous_word();
     void move_to_beginning_of_previous_word();
 
-    void move_up(const KeyEvent& event, double page_height_factor);
-    void move_down(const KeyEvent& event, double page_height_factor);
+    void move_up(double page_height_factor);
+    void move_down(double page_height_factor);
 
     void get_selection_line_boundaries(size_t& first_line, size_t& last_line);
 

Userland/Libraries/LibGUI/TextEditor.cpp

@@ -920,7 +920,7 @@ void TextEditor::reset_cursor_blink()
     start_timer(500);
 }
 
-void TextEditor::toggle_selection_if_needed_for_event(bool is_selecting)
+void TextEditor::update_selection(bool is_selecting)
 {
     if (is_selecting && !selection()->is_valid()) {
         selection()->set(cursor(), {});

Userland/Libraries/LibGUI/TextEditor.h

@@ -178,7 +178,7 @@ public:
 
     void add_code_point(u32 code_point);
     void reset_cursor_blink();
-    void toggle_selection_if_needed_for_event(bool is_selecting);
+    void update_selection(bool is_selecting);
 
     int number_of_visible_lines() const;
     Gfx::IntRect cursor_content_rect() const;

Userland/Libraries/LibGUI/VimEditingEngine.cpp

@@ -753,7 +753,7 @@ bool VimEditingEngine::on_key_in_insert_mode(const KeyEvent& event)
 {
     if (event.key() == KeyCode::Key_Escape || (event.ctrl() && event.key() == KeyCode::Key_LeftBracket) || (event.ctrl() && event.key() == KeyCode::Key_C)) {
         if (m_editor->cursor().column() > 0)
-            move_one_left(event);
+            move_one_left();
         switch_to_normal_mode();
         return true;
     }
@@ -824,10 +824,10 @@ bool VimEditingEngine::on_key_in_normal_mode(const KeyEvent& event)
             delete_line();
             if (was_second_last_line || (m_editor->cursor().line() != 0 && m_editor->cursor().line() != m_editor->line_count() - 1)) {
                 move_one_up(event);
-                move_to_line_end(event);
+                move_to_line_end();
                 m_editor->add_code_point(0x0A);
             } else if (m_editor->cursor().line() == 0) {
-                move_to_line_beginning(event);
+                move_to_line_beginning();
                 m_editor->add_code_point(0x0A);
                 move_one_up(event);
             } else if (m_editor->cursor().line() == m_editor->line_count() - 1) {
@@ -886,15 +886,15 @@ bool VimEditingEngine::on_key_in_normal_mode(const KeyEvent& event)
         if (event.shift() && !event.ctrl() && !event.alt()) {
             switch (event.key()) {
             case (KeyCode::Key_A):
-                move_to_line_end(event);
+                move_to_line_end();
                 switch_to_insert_mode();
                 return true;
             case (KeyCode::Key_I):
-                move_to_line_beginning(event);
+                move_to_line_beginning();
                 switch_to_insert_mode();
                 return true;
             case (KeyCode::Key_O):
-                move_to_line_beginning(event);
+                move_to_line_beginning();
                 m_editor->add_code_point(0x0A);
                 move_one_up(event);
                 switch_to_insert_mode();
@@ -915,13 +915,13 @@ bool VimEditingEngine::on_key_in_normal_mode(const KeyEvent& event)
         if (event.ctrl() && !event.shift() && !event.alt()) {
             switch (event.key()) {
             case (KeyCode::Key_D):
-                move_half_page_down(event);
+                move_half_page_down();
                 return true;
             case (KeyCode::Key_R):
                 m_editor->redo();
                 return true;
             case (KeyCode::Key_U):
-                move_half_page_up(event);
+                move_half_page_up();
                 return true;
             default:
                 break;
@@ -935,7 +935,7 @@ bool VimEditingEngine::on_key_in_normal_mode(const KeyEvent& event)
         if (!event.ctrl() && !event.shift() && !event.alt()) {
             switch (event.key()) {
             case (KeyCode::Key_A):
-                move_one_right(event);
+                move_one_right();
                 switch_to_insert_mode();
                 return true;
             case (KeyCode::Key_C):
@@ -948,7 +948,7 @@ bool VimEditingEngine::on_key_in_normal_mode(const KeyEvent& event)
                 switch_to_insert_mode();
                 return true;
             case (KeyCode::Key_O):
-                move_to_line_end(event);
+                move_to_line_end();
                 m_editor->add_code_point(0x0A);
                 switch_to_insert_mode();
                 return true;
@@ -966,7 +966,7 @@ bool VimEditingEngine::on_key_in_normal_mode(const KeyEvent& event)
                 m_previous_key = event.key();
                 return true;
             case (KeyCode::Key_P):
-                put(event);
+                put();
                 return true;
             default:
                 break;
@@ -1027,11 +1027,11 @@ bool VimEditingEngine::on_key_in_visual_mode(const KeyEvent& event)
     if (event.shift() && !event.ctrl() && !event.alt()) {
         switch (event.key()) {
         case (KeyCode::Key_A):
-            move_to_line_end(event);
+            move_to_line_end();
             switch_to_insert_mode();
             return true;
         case (KeyCode::Key_I):
-            move_to_line_beginning(event);
+            move_to_line_beginning();
             switch_to_insert_mode();
             return true;
         default:
@@ -1043,11 +1043,11 @@ bool VimEditingEngine::on_key_in_visual_mode(const KeyEvent& event)
     if (event.ctrl() && !event.shift() && !event.alt()) {
         switch (event.key()) {
         case (KeyCode::Key_D):
-            move_half_page_down(event);
+            move_half_page_down();
             update_selection_on_cursor_move();
             return true;
         case (KeyCode::Key_U):
-            move_half_page_up(event);
+            move_half_page_up();
             update_selection_on_cursor_move();
             return true;
         default:
@@ -1151,14 +1151,14 @@ void VimEditingEngine::clear_visual_mode_data()
     }
 }
 
-void VimEditingEngine::move_half_page_up(const KeyEvent& event)
+void VimEditingEngine::move_half_page_up()
 {
-    move_up(event, 0.5);
+    move_up(0.5);
 };
 
-void VimEditingEngine::move_half_page_down(const KeyEvent& event)
+void VimEditingEngine::move_half_page_down()
 {
-    move_down(event, 0.5);
+    move_down(0.5);
 };
 
 void VimEditingEngine::yank(YankType type)
@@ -1183,10 +1183,10 @@ void VimEditingEngine::yank(TextRange range)
     m_yank_buffer = m_editor->document().text_in_range(range);
 }
 
-void VimEditingEngine::put(const GUI::KeyEvent& event)
+void VimEditingEngine::put()
 {
     if (m_yank_type == YankType::Line) {
-        move_to_line_end(event);
+        move_to_line_end();
         StringBuilder sb = StringBuilder(m_yank_buffer.length() + 1);
         sb.append_code_point(0x0A);
         sb.append(m_yank_buffer);
@@ -1195,9 +1195,9 @@ void VimEditingEngine::put(const GUI::KeyEvent& event)
     } else {
         // FIXME: If attempting to put on the last column a line,
         // the buffer will bne placed on the next line due to the move_one_left/right behaviour.
-        move_one_right(event);
+        move_one_right();
         m_editor->insert_at_cursor_or_replace_selection(m_yank_buffer);
-        move_one_left(event);
+        move_one_left();
     }
 }
 

Userland/Libraries/LibGUI/VimEditingEngine.h

@@ -166,7 +166,7 @@ private:
     String m_yank_buffer {};
     void yank(YankType);
     void yank(TextRange);
-    void put(const GUI::KeyEvent&);
+    void put();
 
     void update_selection_on_cursor_move();
     void clear_visual_mode_data();
@@ -175,8 +175,8 @@ private:
     void switch_to_normal_mode();
     void switch_to_insert_mode();
     void switch_to_visual_mode();
-    void move_half_page_up(const KeyEvent& event);
-    void move_half_page_down(const KeyEvent& event);
+    void move_half_page_up();
+    void move_half_page_down();
     void move_to_previous_empty_lines_block();
     void move_to_next_empty_lines_block();