From 1adc9c94d6dde78d80cfaf07cd06108e0cbac8c7 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Fri, 22 Aug 2025 21:26:51 +0100 Subject: [PATCH 1/5] refactor: Add section markers to enhanced_tui.rs for better organization - Added 29 section markers to group related functionality - No code changes, only organizational comments - Preparation for future refactoring to break down the 9800+ line file - Sections include: Navigation, Column Operations, Yank, Rendering, etc. --- sql-cli/src/ui/enhanced_tui.rs | 412 ++++++++++++++++++--------------- 1 file changed, 226 insertions(+), 186 deletions(-) diff --git a/sql-cli/src/ui/enhanced_tui.rs b/sql-cli/src/ui/enhanced_tui.rs index e127339a..6d95ac30 100644 --- a/sql-cli/src/ui/enhanced_tui.rs +++ b/sql-cli/src/ui/enhanced_tui.rs @@ -161,6 +161,8 @@ pub struct EnhancedTuiApp { } impl EnhancedTuiApp { + // ========== COLUMN OPERATIONS ========== + // --- Column Visibility Management --- /// Hide the currently selected column @@ -376,6 +378,8 @@ impl EnhancedTuiApp { } } + // ========== JUMP TO ROW ========== + /// Get jump-to-row input text fn get_jump_to_row_input(&self) -> String { self.state_container.jump_to_row().input.clone() @@ -413,6 +417,8 @@ impl EnhancedTuiApp { log_state_clear!(self, "jump_to_row_input", "clear_jump_to_row_input"); } + // ========== BUFFER MANAGEMENT ========== + /// Get current buffer if available (for reading) fn current_buffer(&self) -> Option<&dyn buffer::BufferAPI> { self.buffer_manager @@ -435,6 +441,8 @@ impl EnhancedTuiApp { .expect("No buffer available - this should not happen") } + // ========== ACTION CONTEXT ========== + /// Build action context from current state fn build_action_context(&self) -> ActionContext { let buffer = self.buffer(); @@ -454,6 +462,8 @@ impl EnhancedTuiApp { } } + // ========== ACTION HANDLERS ========== + /// Try to handle an action using the new action system fn try_handle_action( &mut self, @@ -1283,6 +1293,8 @@ impl EnhancedTuiApp { } } + // ========== DATA PROVIDER ACCESS ========== + /// Get a DataProvider view of the current buffer /// This allows using the new trait-based data access pattern fn get_data_provider(&self) -> Option> { @@ -1299,6 +1311,8 @@ impl EnhancedTuiApp { // Note: edit_mode methods removed - use buffer directly + // ========== INPUT MANAGEMENT ========== + // Helper to get input text from buffer or fallback to direct input fn get_input_text(&self) -> String { // For special modes that use the input field for their own purposes @@ -1447,6 +1461,7 @@ impl EnhancedTuiApp { } } } + // ========== CURSOR AND SELECTION ========== // Helper to get visual cursor position (for rendering) fn get_visual_cursor(&self) -> (usize, usize) { @@ -1476,6 +1491,7 @@ impl EnhancedTuiApp { fn get_selection_mode(&self) -> SelectionMode { self.state_container.get_selection_mode() + // ========== UTILITY FUNCTIONS ========== } fn sanitize_table_name(name: &str) -> String { @@ -1595,6 +1611,7 @@ impl EnhancedTuiApp { key_chord_handler: KeyChordHandler::new(), key_dispatcher: KeyDispatcher::new(), key_mapper: KeyMapper::new(), + // ========== INITIALIZATION ========== last_yanked: None, // CSV fields now in Buffer buffer_manager, @@ -2430,6 +2447,7 @@ impl EnhancedTuiApp { } else { format!( "History: {}", + // ========== MAIN RUN LOOP ========== if text.len() > 50 { format!("{}...", &text[..50]) } else { @@ -2765,192 +2783,194 @@ impl EnhancedTuiApp { // Remove the old dispatcher-based handling entirely // The action system above should handle everything /* - if let Some(action) = self.key_dispatcher.get_results_action(&normalized_key) { - debug!( - "Dispatcher returned action '{}' for key {:?}", - action, normalized_key - ); - match action { - "quit" => return Ok(true), - "exit_results_mode" => { - // If vim search is active, just exit search mode but stay in Results - if self.vim_search_manager.borrow().is_active() { - self.vim_search_manager.borrow_mut().exit_navigation(); - self.buffer_mut() - .set_status_message("Search mode exited".to_string()); - return Ok(false); - } - - // Otherwise, switch to Command mode as usual - // Save current position before switching to Command mode - if let Some(selected) = self.state_container.get_table_selected_row() { - self.buffer_mut().set_last_results_row(Some(selected)); - let scroll_offset = self.buffer().get_scroll_offset(); - self.buffer_mut().set_last_scroll_offset(scroll_offset); - } - - // Restore the last executed query to input_text for editing - let last_query = self.buffer().get_last_query(); - let current_input = self.buffer().get_input_text(); - debug!(target: "mode", "Exiting Results mode: current input_text='{}', last_query='{}'", current_input, last_query); - - if !last_query.is_empty() { - debug!(target: "buffer", "Restoring last_query to input_text: '{}'", last_query); - // Use the helper method to sync all three input states - self.set_input_text(last_query.clone()); - } else if !current_input.is_empty() { - debug!(target: "buffer", "No last_query but input_text has content, keeping: '{}'", current_input); - } else { - debug!(target: "buffer", "No last_query to restore when exiting Results mode"); - } - - debug!(target: "mode", "Switching from Results to Command mode"); - self.buffer_mut().set_mode(AppMode::Command); - self.state_container.set_table_selected_row(None); - } - "next_row" => self.next_row(), - "previous_row" => self.previous_row(), - "move_column_left" => self.move_column_left(), - "move_column_right" => self.move_column_right(), - "goto_first_row" => self.goto_first_row(), - "goto_last_row" => { - debug!("Executing goto_last_row action"); - self.goto_last_row(); - } - "goto_viewport_top" => self.goto_viewport_top(), - "goto_viewport_middle" => self.goto_viewport_middle(), - "goto_viewport_bottom" => self.goto_viewport_bottom(), - "goto_first_column" => self.goto_first_column(), - "goto_last_column" => self.goto_last_column(), - "page_up" => self.page_up(), - "page_down" => self.page_down(), - "start_search" => { - self.enter_search_mode(SearchMode::Search); - } - "start_column_search" => { - self.enter_search_mode(SearchMode::ColumnSearch); - } - "start_filter" => { - self.enter_search_mode(SearchMode::Filter); - } - "start_fuzzy_filter" => { - self.enter_search_mode(SearchMode::FuzzyFilter); - } - "sort_by_column" => { - // Use the DataView's toggle_sort for proper 3-state cycling - self.toggle_sort_current_column(); - return Ok(false); // Event handled, continue running - } - "show_column_stats" => self.calculate_column_statistics(), - "next_search_match" => self.next_search_match(), - "previous_search_match" => self.previous_search_match(), - "toggle_compact_mode" => { - let current_mode = self.buffer().is_compact_mode(); - self.buffer_mut().set_compact_mode(!current_mode); - let message = if !current_mode { - "Compact mode: ON (reduced padding, more columns visible)".to_string() - } else { - "Compact mode: OFF (normal padding)".to_string() - }; - self.buffer_mut().set_status_message(message); - } - "toggle_row_numbers" => { - let current_mode = self.buffer().is_show_row_numbers(); - self.buffer_mut().set_show_row_numbers(!current_mode); - let message = if !current_mode { - "Row numbers: ON".to_string() - } else { - "Row numbers: OFF".to_string() - }; - self.buffer_mut().set_status_message(message); - } - "jump_to_row" => { - self.buffer_mut().set_mode(AppMode::JumpToRow); - self.clear_jump_to_row_input(); - - // Set jump-to-row state as active - let container_ptr = - Arc::as_ptr(&self.state_container) as *mut AppStateContainer; - unsafe { - (*container_ptr).jump_to_row_mut().is_active = true; - } - - self.buffer_mut() - .set_status_message("Enter row number:".to_string()); - } - "pin_column" => self.toggle_column_pin(), - "clear_pins" => self.clear_all_pinned_columns(), - "toggle_selection_mode" => { - self.state_container.toggle_selection_mode(); - let new_mode = self.state_container.get_selection_mode(); - let msg = match new_mode { - SelectionMode::Cell => "Cell mode - Navigate to select individual cells", - SelectionMode::Row => "Row mode - Navigate to select rows", - SelectionMode::Column => "Column mode - Navigate to select columns", - }; - self.buffer_mut().set_status_message(msg.to_string()); - return Ok(false); // Return to prevent duplicate handling - } - "export_to_csv" => self.export_to_csv(), - "export_to_json" => self.export_to_json(), - "toggle_help" => { - if self.buffer().get_mode() == AppMode::Help { - self.buffer_mut().set_mode(AppMode::Results); - self.state_container.set_help_visible(false); - } else { - self.buffer_mut().set_mode(AppMode::Help); - self.state_container.set_help_visible(true); - } - } - "toggle_debug" => { - // Use the unified debug handler - self.toggle_debug_mode(); - } - "toggle_case_insensitive" => { - // Toggle case-insensitive string comparisons - let current = self.buffer().is_case_insensitive(); - self.buffer_mut().set_case_insensitive(!current); - self.buffer_mut().set_status_message(format!( - "Case-insensitive string comparisons: {}", - if !current { "ON" } else { "OFF" } - )); - } - "start_history_search" => { - // Switch to Command mode first - let last_query = self.buffer().get_last_query(); - - if !last_query.is_empty() { - // Use helper to sync all states - self.set_input_text(last_query.clone()); - } - - self.buffer_mut().set_mode(AppMode::Command); - self.state_container.set_table_selected_row(None); - - // Start history search - let current_input = self.get_input_text(); - - // Start history search - self.state_container.start_history_search(current_input); - - // Initialize with schema context - self.update_history_matches_in_container(); - - // Get match count - let match_count = self.state_container.history_search().matches.len(); - - self.buffer_mut() - .set_status_message(format!("History search: {} matches", match_count)); - - // Switch to History mode to show the search interface - self.buffer_mut().set_mode(AppMode::History); - } - _ => { - // Action not recognized, continue to handle key directly - } - } - } - */ + if let Some(action) = self.key_dispatcher.get_results_action(&normalized_key) { + debug!( + "Dispatcher returned action '{}' for key {:?}", + action, normalized_key + ); + match action { + "quit" => return Ok(true), + "exit_results_mode" => { + // If vim search is active, just exit search mode but stay in Results + if self.vim_search_manager.borrow().is_active() { + self.vim_search_manager.borrow_mut().exit_navigation(); + self.buffer_mut() + .set_status_message("Search mode exited".to_string()); + return Ok(false); + } + + // Otherwise, switch to Command mode as usual + // Save current position before switching to Command mode + if let Some(selected) = self.state_container.get_table_selected_row() { + self.buffer_mut().set_last_results_row(Some(selected)); + let scroll_offset = self.buffer().get_scroll_offset(); + self.buffer_mut().set_last_scroll_offset(scroll_offset); + } + + // Restore the last executed query to input_text for editing + let last_query = self.buffer().get_last_query(); + let current_input = self.buffer().get_input_text(); + debug!(target: "mode", "Exiting Results mode: current input_text='{}', last_query='{}'", current_input, last_query); + + if !last_query.is_empty() { + debug!(target: "buffer", "Restoring last_query to input_text: '{}'", last_query); + // Use the helper method to sync all three input states + self.set_input_text(last_query.clone()); + } else if !current_input.is_empty() { + debug!(target: "buffer", "No last_query but input_text has content, keeping: '{}'", current_input); + } else { + debug!(target: "buffer", "No last_query to restore when exiting Results mode"); + } + + debug!(target: "mode", "Switching from Results to Command mode"); + self.buffer_mut().set_mode(AppMode::Command); + self.state_container.set_table_selected_row(None); + } + "next_row" => self.next_row(), + "previous_row" => self.previous_row(), + "move_column_left" => self.move_column_left(), + "move_column_right" => self.move_column_right(), + "goto_first_row" => self.goto_first_row(), + "goto_last_row" => { + debug!("Executing goto_last_row action"); + self.goto_last_row(); + } + "goto_viewport_top" => self.goto_viewport_top(), + "goto_viewport_middle" => self.goto_viewport_middle(), + "goto_viewport_bottom" => self.goto_viewport_bottom(), + "goto_first_column" => self.goto_first_column(), + "goto_last_column" => self.goto_last_column(), + "page_up" => self.page_up(), + "page_down" => self.page_down(), + "start_search" => { + self.enter_search_mode(SearchMode::Search); + } + "start_column_search" => { + self.enter_search_mode(SearchMode::ColumnSearch); + } + "start_filter" => { + self.enter_search_mode(SearchMode::Filter); + } + "start_fuzzy_filter" => { + self.enter_search_mode(SearchMode::FuzzyFilter); + } + "sort_by_column" => { + // Use the DataView's toggle_sort for proper 3-state cycling + self.toggle_sort_current_column(); + return Ok(false); // Event handled, continue running + } + "show_column_stats" => self.calculate_column_statistics(), + "next_search_match" => self.next_search_match(), + "previous_search_match" => self.previous_search_match(), + "toggle_compact_mode" => { + let current_mode = self.buffer().is_compact_mode(); + self.buffer_mut().set_compact_mode(!current_mode); + let message = if !current_mode { + "Compact mode: ON (reduced padding, more columns visible)".to_string() + } else { + "Compact mode: OFF (normal padding)".to_string() + }; + self.buffer_mut().set_status_message(message); + } + "toggle_row_numbers" => { + let current_mode = self.buffer().is_show_row_numbers(); + self.buffer_mut().set_show_row_numbers(!current_mode); + let message = if !current_mode { + "Row numbers: ON".to_string() + } else { + "Row numbers: OFF".to_string() + }; + self.buffer_mut().set_status_message(message); + } + "jump_to_row" => { + self.buffer_mut().set_mode(AppMode::JumpToRow); + self.clear_jump_to_row_input(); + + // Set jump-to-row state as active + let container_ptr = + Arc::as_ptr(&self.state_container) as *mut AppStateContainer; + unsafe { + (*container_ptr).jump_to_row_mut().is_active = true; + } + + self.buffer_mut() + .set_status_message("Enter row number:".to_string()); + } + "pin_column" => self.toggle_column_pin(), + "clear_pins" => self.clear_all_pinned_columns(), + "toggle_selection_mode" => { + self.state_container.toggle_selection_mode(); + let new_mode = self.state_container.get_selection_mode(); + let msg = match new_mode { + SelectionMode::Cell => "Cell mode - Navigate to select individual cells", + SelectionMode::Row => "Row mode - Navigate to select rows", + SelectionMode::Column => "Column mode - Navigate to select columns", + }; + self.buffer_mut().set_status_message(msg.to_string()); + return Ok(false); // Return to prevent duplicate handling + } + "export_to_csv" => self.export_to_csv(), + "export_to_json" => self.export_to_json(), + "toggle_help" => { + if self.buffer().get_mode() == AppMode::Help { + self.buffer_mut().set_mode(AppMode::Results); + self.state_container.set_help_visible(false); + } else { + self.buffer_mut().set_mode(AppMode::Help); + self.state_container.set_help_visible(true); + } + } + "toggle_debug" => { + // Use the unified debug handler + self.toggle_debug_mode(); + } + "toggle_case_insensitive" => { + // Toggle case-insensitive string comparisons + let current = self.buffer().is_case_insensitive(); + self.buffer_mut().set_case_insensitive(!current); + self.buffer_mut().set_status_message(format!( + "Case-insensitive string comparisons: {}", + if !current { "ON" } else { "OFF" } + )); + } + "start_history_search" => { + // Switch to Command mode first + let last_query = self.buffer().get_last_query(); + + if !last_query.is_empty() { + // Use helper to sync all states + self.set_input_text(last_query.clone()); + } + // ========== MODE HANDLERS ========== + + + self.buffer_mut().set_mode(AppMode::Command); + self.state_container.set_table_selected_row(None); + + // Start history search + let current_input = self.get_input_text(); + + // Start history search + self.state_container.start_history_search(current_input); + + // Initialize with schema context + self.update_history_matches_in_container(); + + // Get match count + let match_count = self.state_container.history_search().matches.len(); + + self.buffer_mut() + .set_status_message(format!("History search: {} matches", match_count)); + + // Switch to History mode to show the search interface + self.buffer_mut().set_mode(AppMode::History); + } + _ => { + // Action not recognized, continue to handle key directly + } + } + } + */ // Fall back to direct key handling for special cases not in dispatcher match normalized_key.code { @@ -3028,6 +3048,7 @@ impl EnhancedTuiApp { } Ok(false) } + // ========== SEARCH OPERATIONS ========== fn execute_search_action(&mut self, mode: SearchMode, pattern: String) { debug!(target: "search", "execute_search_action called: mode={:?}, pattern='{}', current_app_mode={:?}, thread={:?}", @@ -3478,6 +3499,8 @@ impl EnhancedTuiApp { SearchModesAction::PassThrough => {} } + // ========== FILTER OPERATIONS ========== + Ok(false) } @@ -3636,6 +3659,7 @@ impl EnhancedTuiApp { _ => {} } Ok(false) + // ========== COLUMN SEARCH ========== } fn handle_column_search_input(&mut self, key: crossterm::event::KeyEvent) -> Result { @@ -3785,6 +3809,7 @@ impl EnhancedTuiApp { AppMode::Command }; self.buffer_mut().set_mode(mode); + // ========== HELP NAVIGATION ========== } fn scroll_help_down(&mut self) { @@ -3858,6 +3883,7 @@ impl EnhancedTuiApp { _ => {} } Ok(false) + // ========== HISTORY MANAGEMENT ========== } /// Update history matches in the AppStateContainer with schema context @@ -3909,6 +3935,7 @@ impl EnhancedTuiApp { self.buffer_mut().set_mode(AppMode::Command); } Ok(false) + // ========== QUERY OPERATIONS ========== } fn handle_pretty_query_input(&mut self, key: crossterm::event::KeyEvent) -> Result { @@ -4238,6 +4265,8 @@ impl EnhancedTuiApp { } } + // ========== COLUMN INFO ========== + // Helper to get estimated visible rows based on terminal size fn get_column_count(&self) -> usize { @@ -4317,6 +4346,7 @@ impl EnhancedTuiApp { // Extract the sorted indices Some(indexed_values.into_iter().map(|(_, idx)| idx).collect()) } + // ========== NAVIGATION METHODS ========== // Navigation functions fn next_row(&mut self) { @@ -4768,6 +4798,7 @@ impl EnhancedTuiApp { self.state_container.set_table_selected_row(Some(new_row)); self.buffer_mut().set_status_message(status_msg); } + // ========== COLUMN PIN/HIDE ========== } fn toggle_column_pin(&mut self) { @@ -4968,6 +4999,7 @@ impl EnhancedTuiApp { } None + // ========== VIEWPORT MANAGEMENT ========== } fn update_viewport_size(&mut self) { @@ -5141,6 +5173,7 @@ impl EnhancedTuiApp { debug!(target: "navigation", "Page up via ViewportManager to row {}", result.row_position + 1); } + // ========== SEARCH EXECUTION ========== // Search and filter functions fn perform_search(&mut self) { @@ -5679,6 +5712,7 @@ impl EnhancedTuiApp { )); } } + // ========== FILTER EXECUTION ========== } fn apply_filter(&mut self, pattern: &str) { @@ -6275,6 +6309,7 @@ impl EnhancedTuiApp { buffer.set_current_column(0); buffer.set_last_results_row(None); // Reset saved position for new results buffer.set_last_scroll_offset((0, 0)); // Reset saved scroll offset for new results + // ========== SORT OPERATIONS ========== } // Reset ViewportManager if it exists @@ -6516,6 +6551,7 @@ impl EnhancedTuiApp { Ok(message) => self.set_status_message(message), Err(e) => self.set_error_status("Export failed", e), } + // ========== YANK OPERATIONS ========== } fn yank_cell(&mut self) { @@ -7129,6 +7165,7 @@ impl EnhancedTuiApp { // Render mode-specific status line self.render_status_line(f, chunks[2]); + // ========== RENDERING ========== } fn render_status_line(&self, f: &mut Frame, area: Rect) { @@ -8647,6 +8684,8 @@ impl EnhancedTuiApp { } // === Editor Widget Helper Methods === + // ========== QUERY EXECUTION ========== + // These methods handle the actions returned by the editor widget fn handle_execute_query(&mut self) -> Result { @@ -9606,6 +9645,7 @@ impl EnhancedTuiApp { } else { None } + // ========== DEBUG OPERATIONS ========== } fn show_pretty_query(&mut self) { From 070a906619433538794445afbeb79e3240529d81 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Fri, 22 Aug 2025 21:33:45 +0100 Subject: [PATCH 2/5] refactor: Extract pure function sanitize_table_name to helpers module - Created enhanced_tui_helpers.rs for pure functions - Moved sanitize_table_name (1 of 7 pure functions) - Reduced enhanced_tui.rs by 33 lines - This is the first step in reducing coupling in the TUI --- sql-cli/src/ui/enhanced_tui.rs | 39 ++------------------------ sql-cli/src/ui/enhanced_tui_helpers.rs | 39 ++++++++++++++++++++++++++ sql-cli/src/ui/mod.rs | 1 + 3 files changed, 43 insertions(+), 36 deletions(-) create mode 100644 sql-cli/src/ui/enhanced_tui_helpers.rs diff --git a/sql-cli/src/ui/enhanced_tui.rs b/sql-cli/src/ui/enhanced_tui.rs index 6d95ac30..c3059cc1 100644 --- a/sql-cli/src/ui/enhanced_tui.rs +++ b/sql-cli/src/ui/enhanced_tui.rs @@ -21,6 +21,7 @@ use crate::sql_highlighter::SqlHighlighter; use crate::text_navigation::TextNavigator; use crate::ui::actions::{Action, ActionContext, ActionResult}; use crate::ui::cell_renderer::CellRenderer; +use crate::ui::enhanced_tui_helpers; use crate::ui::key_chord_handler::{ChordResult, KeyChordHandler}; use crate::ui::key_dispatcher::KeyDispatcher; use crate::ui::key_indicator::{format_key_for_display, KeyPressIndicator}; @@ -1494,40 +1495,6 @@ impl EnhancedTuiApp { // ========== UTILITY FUNCTIONS ========== } - fn sanitize_table_name(name: &str) -> String { - // Replace spaces and other problematic characters with underscores - // to create SQL-friendly table names - // Examples: "Business Crime Borough Level" -> "Business_Crime_Borough_Level" - let sanitized: String = name - .trim() - .chars() - .map(|c| { - if c.is_alphanumeric() || c == '_' { - c - } else { - '_' - } - }) - .collect(); - - // If the sanitized name is too complex (too long or has too many underscores), - // fall back to a simple default name - const MAX_LENGTH: usize = 30; - const MAX_UNDERSCORES: usize = 5; - - let underscore_count = sanitized.chars().filter(|&c| c == '_').count(); - - if sanitized.len() > MAX_LENGTH || underscore_count > MAX_UNDERSCORES { - // Use a simple fallback name - "data".to_string() - } else if sanitized.is_empty() || sanitized.chars().all(|c| c == '_') { - // If the name is empty or all underscores after sanitization - "data".to_string() - } else { - sanitized - } - } - pub fn new(api_url: &str) -> Self { // Load configuration let config = Config::load().unwrap_or_else(|_e| { @@ -1666,7 +1633,7 @@ impl EnhancedTuiApp { .to_string(); // Sanitize the table name to be SQL-friendly - let table_name = Self::sanitize_table_name(&raw_name); + let table_name = enhanced_tui_helpers::sanitize_table_name(&raw_name); // Direct DataTable loading let (file_type_str, memory_before, memory_after) = match file_type { @@ -9717,7 +9684,7 @@ pub fn run_enhanced_tui_multi(api_url: &str, data_files: Vec<&str>) -> Result<() .and_then(|s| s.to_str()) .unwrap_or("data") .to_string(); - let table_name = EnhancedTuiApp::sanitize_table_name(&raw_name); + let table_name = enhanced_tui_helpers::sanitize_table_name(&raw_name); // Load the data if extension.to_lowercase() == "csv" { diff --git a/sql-cli/src/ui/enhanced_tui_helpers.rs b/sql-cli/src/ui/enhanced_tui_helpers.rs new file mode 100644 index 00000000..cfbd54f1 --- /dev/null +++ b/sql-cli/src/ui/enhanced_tui_helpers.rs @@ -0,0 +1,39 @@ +// Helper functions extracted from enhanced_tui.rs +// These are pure functions with no dependencies on self + +use anyhow::Result; + +/// Sanitize table name by removing special characters and limiting length +pub fn sanitize_table_name(name: &str) -> String { + // Replace spaces and other problematic characters with underscores + // to create SQL-friendly table names + // Examples: "Business Crime Borough Level" -> "Business_Crime_Borough_Level" + let sanitized: String = name + .trim() + .chars() + .map(|c| { + if c.is_alphanumeric() || c == '_' { + c + } else { + '_' + } + }) + .collect(); + + // If the sanitized name is too complex (too long or has too many underscores), + // fall back to a simple default name + const MAX_LENGTH: usize = 30; + const MAX_UNDERSCORES: usize = 5; + + let underscore_count = sanitized.chars().filter(|&c| c == '_').count(); + + if sanitized.len() > MAX_LENGTH || underscore_count > MAX_UNDERSCORES { + // Use a simple fallback name + "data".to_string() + } else if sanitized.is_empty() || sanitized.chars().all(|c| c == '_') { + // If the name is empty or all underscores after sanitization + "data".to_string() + } else { + sanitized + } +} diff --git a/sql-cli/src/ui/mod.rs b/sql-cli/src/ui/mod.rs index 59d34a3e..c4cd2779 100644 --- a/sql-cli/src/ui/mod.rs +++ b/sql-cli/src/ui/mod.rs @@ -8,6 +8,7 @@ pub mod column_utils; pub mod enhanced_tui; pub mod enhanced_tui_debug; pub mod enhanced_tui_debug_integration; +pub mod enhanced_tui_helpers; pub mod key_chord_handler; pub mod key_dispatcher; pub mod key_indicator; From ae4070ae2799b09340942292ba668a4f27bffa4b Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Fri, 22 Aug 2025 21:42:05 +0100 Subject: [PATCH 3/5] refactor: Simplify navigation methods to reduce coupling - Added apply_row_navigation_result() helper to consolidate state updates - Simplified next_row() from 29 lines to 11 lines - This removes the duplicated state updates that were in every navigation method - Navigation now has a single place where state is synchronized This is the first step in reducing coupling in navigation. The pattern: 1. Call ViewportManager to get navigation result 2. Apply result using helper method 3. No more updating 3-4 different places manually --- sql-cli/src/ui/enhanced_tui.rs | 51 +++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/sql-cli/src/ui/enhanced_tui.rs b/sql-cli/src/ui/enhanced_tui.rs index c3059cc1..d4233919 100644 --- a/sql-cli/src/ui/enhanced_tui.rs +++ b/sql-cli/src/ui/enhanced_tui.rs @@ -27,7 +27,9 @@ use crate::ui::key_dispatcher::KeyDispatcher; use crate::ui::key_indicator::{format_key_for_display, KeyPressIndicator}; use crate::ui::key_mapper::KeyMapper; use crate::ui::key_sequence_renderer::KeySequenceRenderer; -use crate::ui::viewport_manager::{ColumnPackingMode, ViewportEfficiency, ViewportManager}; +use crate::ui::viewport_manager::{ + ColumnPackingMode, RowNavigationResult, ViewportEfficiency, ViewportManager, +}; use crate::utils::logging::LogRingBuffer; use crate::widget_traits::DebugInfoProvider; use crate::widgets::debug_widget::DebugWidget; @@ -4315,34 +4317,37 @@ impl EnhancedTuiApp { } // ========== NAVIGATION METHODS ========== + /// Helper to apply row navigation result to all state locations + /// This consolidates the duplicated state updates that were in every navigation method + fn apply_row_navigation_result(&mut self, result: RowNavigationResult) { + // Update Buffer's selected row + self.buffer_mut() + .set_selected_row(Some(result.row_position)); + + // Update scroll offset if viewport changed + if result.viewport_changed { + let mut offset = self.buffer().get_scroll_offset(); + offset.0 = result.row_scroll_offset; + self.buffer_mut().set_scroll_offset(offset); + } + + // Update AppStateContainer for consistency + self.state_container.navigation_mut().selected_row = result.row_position; + if result.viewport_changed { + self.state_container.navigation_mut().scroll_offset.0 = result.row_scroll_offset; + } + } + // Navigation functions fn next_row(&mut self) { // Use ViewportManager to navigate (it manages the crosshair) - let nav_result = if let Some(ref mut viewport_manager) = *self.viewport_manager.borrow_mut() - { - Some(viewport_manager.navigate_row_down()) - } else { - None + let nav_result = { + let mut viewport_borrow = self.viewport_manager.borrow_mut(); + viewport_borrow.as_mut().map(|vm| vm.navigate_row_down()) }; if let Some(nav_result) = nav_result { - // Update Buffer with the new row position - self.buffer_mut() - .set_selected_row(Some(nav_result.row_position)); - - // Update viewport if changed - if nav_result.viewport_changed { - let mut offset = self.buffer().get_scroll_offset(); - offset.0 = nav_result.row_scroll_offset; - self.buffer_mut().set_scroll_offset(offset); - } - - // Also update AppStateContainer for consistency - self.state_container.navigation_mut().selected_row = nav_result.row_position; - if nav_result.viewport_changed { - self.state_container.navigation_mut().scroll_offset.0 = - nav_result.row_scroll_offset; - } + self.apply_row_navigation_result(nav_result); } } From 0e8d517c214d7d3b73544f174e6501d95750a70b Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Fri, 22 Aug 2025 21:44:53 +0100 Subject: [PATCH 4/5] refactor: Simplify previous_row navigation method - Applied same pattern as next_row - Reduced from 29 lines to 11 lines - Uses apply_row_navigation_result helper to consolidate state updates - Both up/down navigation now follow the same clean pattern Ready to apply this pattern to remaining 11 navigation methods --- sql-cli/src/ui/enhanced_tui.rs | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/sql-cli/src/ui/enhanced_tui.rs b/sql-cli/src/ui/enhanced_tui.rs index d4233919..e6bb704f 100644 --- a/sql-cli/src/ui/enhanced_tui.rs +++ b/sql-cli/src/ui/enhanced_tui.rs @@ -4353,31 +4353,13 @@ impl EnhancedTuiApp { fn previous_row(&mut self) { // Use ViewportManager to navigate (it manages the crosshair) - let nav_result = if let Some(ref mut viewport_manager) = *self.viewport_manager.borrow_mut() - { - Some(viewport_manager.navigate_row_up()) - } else { - None + let nav_result = { + let mut viewport_borrow = self.viewport_manager.borrow_mut(); + viewport_borrow.as_mut().map(|vm| vm.navigate_row_up()) }; if let Some(nav_result) = nav_result { - // Update Buffer with the new row position - self.buffer_mut() - .set_selected_row(Some(nav_result.row_position)); - - // Update viewport if changed - if nav_result.viewport_changed { - let mut offset = self.buffer().get_scroll_offset(); - offset.0 = nav_result.row_scroll_offset; - self.buffer_mut().set_scroll_offset(offset); - } - - // Also update AppStateContainer for consistency - self.state_container.navigation_mut().selected_row = nav_result.row_position; - if nav_result.viewport_changed { - self.state_container.navigation_mut().scroll_offset.0 = - nav_result.row_scroll_offset; - } + self.apply_row_navigation_result(nav_result); } } From bae64c0851899274805c46d11d75e1b94cb481ea Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Fri, 22 Aug 2025 21:52:04 +0100 Subject: [PATCH 5/5] refactor: Simplify column navigation methods - Added apply_column_navigation_result() helper - Simplified move_column_left from 51 to 13 lines (74% reduction) - Simplified move_column_right from 62 to 13 lines (79% reduction) - Follows same pattern as row navigation helpers Column navigation now uses single helper for state updates instead of duplicating the same updates in every method. Ready for testing before applying to remaining navigation methods. --- sql-cli/src/ui/enhanced_tui.rs | 160 +++++++++++++-------------------- 1 file changed, 60 insertions(+), 100 deletions(-) diff --git a/sql-cli/src/ui/enhanced_tui.rs b/sql-cli/src/ui/enhanced_tui.rs index e6bb704f..bc203ad4 100644 --- a/sql-cli/src/ui/enhanced_tui.rs +++ b/sql-cli/src/ui/enhanced_tui.rs @@ -28,7 +28,7 @@ use crate::ui::key_indicator::{format_key_for_display, KeyPressIndicator}; use crate::ui::key_mapper::KeyMapper; use crate::ui::key_sequence_renderer::KeySequenceRenderer; use crate::ui::viewport_manager::{ - ColumnPackingMode, RowNavigationResult, ViewportEfficiency, ViewportManager, + ColumnPackingMode, NavigationResult, RowNavigationResult, ViewportEfficiency, ViewportManager, }; use crate::utils::logging::LogRingBuffer; use crate::widget_traits::DebugInfoProvider; @@ -4338,6 +4338,53 @@ impl EnhancedTuiApp { } } + /// Helper to apply column navigation result to all state locations + fn apply_column_navigation_result(&mut self, result: NavigationResult, direction: &str) { + // Update cursor_manager for table navigation + match direction { + "left" => { + self.cursor_manager.move_table_left(); + } + "right" => { + let max_columns = if let Some(provider) = self.get_data_provider() { + provider.get_column_count() + } else { + 0 + }; + self.cursor_manager.move_table_right(max_columns); + } + _ => {} + } + + // Get the visual position from ViewportManager after navigation + let visual_position = { + let viewport_borrow = self.viewport_manager.borrow(); + viewport_borrow + .as_ref() + .map(|vm| vm.get_crosshair_col()) + .unwrap_or(0) + }; + + // Update Buffer's current column + self.buffer_mut().set_current_column(visual_position); + + // Update AppStateContainer's navigation state + self.state_container.navigation_mut().selected_column = visual_position; + + // Update scroll offset if viewport changed + if result.viewport_changed { + let mut offset = self.buffer().get_scroll_offset(); + offset.1 = result.scroll_offset; + self.buffer_mut().set_scroll_offset(offset); + self.state_container.navigation_mut().scroll_offset = offset; + } + + // Set status message + let column_num = visual_position + 1; + self.buffer_mut() + .set_status_message(format!("Column {} selected", column_num)); + } + // Navigation functions fn next_row(&mut self) { // Use ViewportManager to navigate (it manages the crosshair) @@ -4364,118 +4411,31 @@ impl EnhancedTuiApp { } fn move_column_left(&mut self) { - // Get navigation result from ViewportManager, then drop the borrow + // Get navigation result from ViewportManager let nav_result = { - let mut viewport_manager_borrow = self.viewport_manager.borrow_mut(); - let viewport_manager = viewport_manager_borrow + let mut viewport_borrow = self.viewport_manager.borrow_mut(); + let vm = viewport_borrow .as_mut() .expect("ViewportManager must exist for navigation"); - - // ViewportManager knows its current crosshair position - // Just tell it to move left - let current_visual = viewport_manager.get_crosshair_col(); - debug!(target: "navigation", - "move_column_left: current visual position from crosshair={}", - current_visual); - - let result = viewport_manager.navigate_column_left(current_visual); - debug!(target: "navigation", "move_column_left: ViewportManager result: {:?}", result); - result - }; // viewport_manager borrow dropped here - - // Update cursor_manager for table navigation (incremental step) - self.cursor_manager.move_table_left(); - - // Get the visual position from ViewportManager after navigation - let visual_position = { - let viewport_manager_borrow = self.viewport_manager.borrow(); - viewport_manager_borrow - .as_ref() - .map(|vm| vm.get_crosshair_col()) - .unwrap_or(0) + let current_visual = vm.get_crosshair_col(); + vm.navigate_column_left(current_visual) }; - // Apply navigation result to TUI state (using visual position) - self.buffer_mut().set_current_column(visual_position); - - // Sync with navigation state in AppStateContainer (using visual position) - self.state_container.navigation_mut().selected_column = visual_position; - - // Update scroll offset if viewport changed - if nav_result.viewport_changed { - let mut offset = self.buffer().get_scroll_offset(); - offset.1 = nav_result.scroll_offset; - self.buffer_mut().set_scroll_offset(offset); - self.state_container.navigation_mut().scroll_offset = offset; - } - - // Set status message - let column_num = visual_position + 1; - self.buffer_mut() - .set_status_message(format!("Column {} selected", column_num)); + self.apply_column_navigation_result(nav_result, "left"); } fn move_column_right(&mut self) { - // Get navigation result from ViewportManager, then drop the borrow + // Get navigation result from ViewportManager let nav_result = { - let mut viewport_manager_borrow = self.viewport_manager.borrow_mut(); - let viewport_manager = viewport_manager_borrow + let mut viewport_borrow = self.viewport_manager.borrow_mut(); + let vm = viewport_borrow .as_mut() .expect("ViewportManager must exist for navigation"); - - // ViewportManager knows its current crosshair position - // Just tell it to move right - let current_visual = viewport_manager.get_crosshair_col(); - debug!(target: "navigation", - "move_column_right: current visual position from crosshair={}", - current_visual); - let result = viewport_manager.navigate_column_right(current_visual); - - debug!(target: "navigation", "move_column_right: ViewportManager returned: {:?}", result); - result - }; // viewport_manager borrow dropped here - - // Get max columns for cursor_manager - let max_columns = if let Some(provider) = self.get_data_provider() { - provider.get_column_count() - } else { - 0 - }; - - // Update cursor_manager for table navigation (incremental step) - self.cursor_manager.move_table_right(max_columns); - - // Get the visual position from ViewportManager after navigation - let visual_position = { - let viewport_manager_borrow = self.viewport_manager.borrow(); - viewport_manager_borrow - .as_ref() - .map(|vm| vm.get_crosshair_col()) - .unwrap_or(0) + let current_visual = vm.get_crosshair_col(); + vm.navigate_column_right(current_visual) }; - debug!(target: "navigation", - "move_column_right END: storing visual_pos={} in Buffer", - visual_position); - - // Apply navigation result to TUI state (using visual position) - self.buffer_mut().set_current_column(visual_position); - - // Sync with navigation state in AppStateContainer (using visual position) - self.state_container.navigation_mut().selected_column = visual_position; - - // Update scroll offset if viewport changed - if nav_result.viewport_changed { - let mut offset = self.buffer().get_scroll_offset(); - offset.1 = nav_result.scroll_offset; - self.buffer_mut().set_scroll_offset(offset); - self.state_container.navigation_mut().scroll_offset = offset; - } - - // Set status message - let column_num = visual_position + 1; - self.buffer_mut() - .set_status_message(format!("Column {} selected", column_num)); + self.apply_column_navigation_result(nav_result, "right"); } fn goto_first_column(&mut self) {