Quellcode durchsuchen

fix(minibuffer): resolve TUI backspace, GTK rendering, and Popover issues

Fixed TUI Backspace by matching key string case. Fixed GTK minibuffer rendering artifacts by clearing dedicated surface. Constrained GTK Popover height and enabled keyboard scroll-to-selection.
Bernardo Magri vor 1 Monat
Ursprung
Commit
c64fab6e6d
5 geänderte Dateien mit 51 neuen und 14 gelöschten Zeilen
  1. 2 2
      documentation/PLAN.md
  2. 2 1
      src/gtk_completion_popup.cpp
  3. 6 4
      src/gtk_renderer.cpp
  4. 2 2
      src/minibuffer_manager.cpp
  5. 39 5
      src/tui_editor.cpp

+ 2 - 2
documentation/PLAN.md

@@ -138,7 +138,7 @@ Lumacs/
 - ✅ **GTK Enhancements**: (Fully completed - Phase A).
 - ✅ **Minibuffer & Command System**: Minibuffer core logic, history management, and completion are fully centralized and integrated with the Command System (Phases Z and C completed).
     - **Advanced Completion UI**: Completed (Implemented popup completion window with descriptions and better visual feedback).
-    - **ISearch & TUI Polish**: ✅ Fixed. Exposed ISearch direction control, fixed TUI rendering overlap, and implemented `y_or_n_p` input handling.
+    - **ISearch & TUI Polish**: ✅ Fixed. Exposed ISearch direction control, fixed TUI rendering overlap, fixed TUI Backspace, GTK rendering artifacts, and GTK Popover sizing/scrolling. Implemented `y_or_n_p` input handling.
 - ✅ **Theme System Refactoring**:
     - [x] Implemented `editor:create_and_register_theme` Lua API to allow theme definition from Lua.
     - [x] Factored all hardcoded C++ themes (`default`, `everforest-dark`, `dracula`, `solarized-dark`, `nord`, `gruvbox-light`) into individual Lua files (`lua/themes/*.lua`).
@@ -176,7 +176,7 @@ Lumacs/
 - **Build System**: CMake-based with proper dependency management
 - **Rendering Performance**: ✅ Fixed. Implemented line-based rendering cache in GtkRenderer to optimize drawing of unchanged text lines, especially during scrolling and minor edits. Dynamic elements like cursor and selection are composited on top.
 - **Focus Stability**: GTK frontend caches active_window_ during redraw cycles to prevent race conditions in multi-window async rendering
-- **GTK Popup**: Uses `Gtk::Popover` but needs refinement for positioning and styling.
+- **GTK Popup**: Refined with max height and scroll-to-selection. Uses `Gtk::Popover`.
 - **TUI ISearch**: ISearch highlighting temporarily disabled in TUI.
 - **Backspace Bug**: ✅ Fixed. Was a logical error in Lua's `lumacs_backward_delete_char` function regarding position calculation for `erase_char` and cursor update.
 - **`EditorCoreTest.MoveCursorRight` Disabled**: ✅ Fixed. Re-enabled and passing.

+ 2 - 1
src/gtk_completion_popup.cpp

@@ -45,6 +45,7 @@ GtkCompletionPopup::GtkCompletionPopup()
     list_scrolled_window_.set_policy(Gtk::PolicyType::AUTOMATIC, Gtk::PolicyType::AUTOMATIC);
     list_scrolled_window_.set_propagate_natural_width(true);
     list_scrolled_window_.set_propagate_natural_height(true);
+    list_scrolled_window_.set_max_content_height(300); // Limit height
 
     list_box_.set_selection_mode(Gtk::SelectionMode::SINGLE);
     list_box_.set_activate_on_single_click(true); // Activate on single click
@@ -131,7 +132,7 @@ void GtkCompletionPopup::set_active_row(size_t index) {
         Gtk::ListBoxRow* row = list_box_.get_row_at_index(static_cast<int>(index));
         if (row) {
             list_box_.select_row(*row);
-            // row->scroll_to_row(); // Ensure the selected row is visible (TODO: Fix for GTK4)
+            row->grab_focus(); // Ensure the selected row is visible and focused
             active_index_ = index;
         }
     }

+ 6 - 4
src/gtk_renderer.cpp

@@ -458,15 +458,17 @@ void GtkRenderer::render_minibuffer(const Cairo::RefPtr<Cairo::Context>& cr, int
     Color fg = theme->get_fg_color(ThemeElement::Normal);
     
     // Draw minibuffer background (slightly different shade)
+    // Clear the ENTIRE surface first to avoid artifacts
     cr->set_source_rgb(bg.r / 255.0 * 0.9, bg.g / 255.0 * 0.9, bg.b / 255.0 * 0.9);
-    cr->rectangle(0, minibuffer_y - 2, width, line_height_ + 4);
+    cr->rectangle(0, 0, width, height); // Clear full dedicated area
     cr->fill();
     
-    // Draw separator line above minibuffer
+    // Draw separator line above minibuffer (optional if dedicated area)
+    // We can draw it at the very top 0
     cr->set_source_rgb(fg.r / 255.0 * 0.5, fg.g / 255.0 * 0.5, fg.b / 255.0 * 0.5);
     cr->set_line_width(1.0);
-    cr->move_to(0, minibuffer_y - 2);
-    cr->line_to(width, minibuffer_y - 2);
+    cr->move_to(0, 0);
+    cr->line_to(width, 0);
     cr->stroke();
     
     // Prepare minibuffer text

+ 2 - 2
src/minibuffer_manager.cpp

@@ -94,7 +94,7 @@ bool MinibufferManager::handle_key_event(const std::string& key_name) {
             isearch_direction_forward_ = false;
             previous_isearch_match();
             return true;
-        } else if (key_name == "BackSpace") {
+        } else if (key_name == "Backspace") {
             if (cursor_position_ > 0) {
                 input_buffer_.erase(cursor_position_ - 1, 1);
                 cursor_position_--;
@@ -148,7 +148,7 @@ bool MinibufferManager::handle_key_event(const std::string& key_name) {
         }
         deactivate_minibuffer();
         return true;
-    } else if (key_name == "BackSpace") {
+    } else if (key_name == "Backspace") {
         if (cursor_position_ > 0) {
             input_buffer_.erase(cursor_position_ - 1, 1);
             cursor_position_--;

+ 39 - 5
src/tui_editor.cpp

@@ -298,14 +298,47 @@ std::string TuiEditor::resolve_key(int ch) {
 }
 
 bool TuiEditor::handle_input(int ch) {
-    // Resolve key name
-    std::string key_name = resolve_key(ch);
+    std::string key_name;
+
+    // Check for Meta key sequence (Escape + Key)
+    if (ch == 27) {
+        // Set non-blocking read to check for immediate next key
+        timeout(0);
+        int next_ch = getch();
+        timeout(50); // Restore timeout
+
+        if (next_ch != ERR) {
+            // Ensure next_ch is a valid printable char or special key we can map
+            // For now, handle simple M-char sequences
+            std::string next_key = resolve_key(next_ch);
+            if (!next_key.empty() && next_key.length() == 1) { // Simple char
+                 key_name = "M-" + next_key;
+            } else if (!next_key.empty()) {
+                 // Special key with Meta, e.g. M-ArrowUp? 
+                 // resolve_key returns "ArrowUp", so we get "M-ArrowUp". This is valid.
+                 key_name = "M-" + next_key;
+            } else {
+                // Couldn't resolve next key, just treat as Escape then ignore next?
+                // Or treat as Escape sequence.
+                key_name = "Escape";
+                // We effectively consumed next_ch and ignored it. 
+                // Better might be to ungetch, but ncurses ungetch is tricky.
+                // Let's assume if resolve_key fails it was garbage.
+            }
+        } else {
+            key_name = "Escape";
+        }
+    } else {
+        key_name = resolve_key(ch);
+    }
     
     if (key_name.empty()) {
         debug_log << "Empty key name, ignoring input" << std::endl;
         return false;
     }
 
+    debug_log << "Resolved key: " << key_name << std::endl;
+
     // Handle Minibuffer Input Logic first
     if (core_->minibuffer_manager().is_active()) {
         return core_->minibuffer_manager().handle_key_event(key_name);
@@ -323,10 +356,11 @@ bool TuiEditor::handle_input(int ch) {
         // Check if key is a single character and not a control sequence
         // The resolve_key function returns "C-x", "M-x", "Esc", "Return", etc.
         // Printable characters are returned as single chars "a", "1", etc.
-        if (key_name.length() == 1) {
+        bool has_ctrl = key_name.find("C-") != std::string::npos;
+        bool has_meta = key_name.find("M-") != std::string::npos;
+
+        if (!has_ctrl && !has_meta && key_name.length() == 1) {
             // We can assume it's printable if length is 1 and it's not a special key (which resolve_key handles)
-            // Actually, resolve_key might return special single chars? No, most are named.
-            // But let's be safe.
             core_->command_system().execute("self-insert-command", {key_name});
             
             // --- Macro Recording Logic for Self-Insert ---