diff --git a/sql-cli/CLAUDE.md b/sql-cli/CLAUDE.md index 558e1e9d..286f2ca9 100644 --- a/sql-cli/CLAUDE.md +++ b/sql-cli/CLAUDE.md @@ -38,6 +38,9 @@ cargo clippy - use the rust build fixer to fix any compilation issues - use the unit test fixer to correct unit test breaks +## docs +- place all docs in the docs folder + ## Project Structure - `src/ui/enhanced_tui.rs` - Main TUI interface (key handling to be migrated) - `src/app_state_container.rs` - Central state management diff --git a/sql-cli/ARCHITECTURE_PROBLEM.md b/sql-cli/docs/ARCHITECTURE_PROBLEM.md similarity index 100% rename from sql-cli/ARCHITECTURE_PROBLEM.md rename to sql-cli/docs/ARCHITECTURE_PROBLEM.md diff --git a/sql-cli/docs/BUFFER_STATE_REFACTOR_PLAN.md b/sql-cli/docs/BUFFER_STATE_REFACTOR_PLAN.md new file mode 100644 index 00000000..1a06e1cd --- /dev/null +++ b/sql-cli/docs/BUFFER_STATE_REFACTOR_PLAN.md @@ -0,0 +1,186 @@ +# Buffer State Refactor Plan + +## Problem Statement +We have view state (crosshair position, scroll offset, selection) duplicated across: +- ViewportManager (in TUI) +- NavigationState (in AppStateContainer) +- SelectionState (in AppStateContainer) +- Buffer (partial storage) + +This causes synchronization bugs and makes buffer switching unreliable. + +## Goal +Make Buffer the single source of truth for all its view state, with everything else being proxies or caches. + +## Phase 1: Proxy Pattern for AppStateContainer + +### Current (Broken) +```rust +AppStateContainer { + navigation: RefCell, // Duplicate state! + selection: RefCell, // Duplicate state! + buffers: BufferManager, +} +``` + +### Target (Proxy Pattern) +```rust +AppStateContainer { + buffers: BufferManager, + // No navigation or selection fields! +} + +impl AppStateContainer { + pub fn navigation(&self) -> NavigationProxy { + NavigationProxy::new(self.buffers.current()) + } + + pub fn selection(&self) -> SelectionProxy { + SelectionProxy::new(self.buffers.current()) + } +} +``` + +### Implementation Steps + +1. **Create ViewState struct in Buffer** +```rust +// In buffer.rs +pub struct ViewState { + // Position + pub crosshair_row: usize, + pub crosshair_col: usize, + pub scroll_offset: (usize, usize), + + // Selection + pub selection_mode: SelectionMode, + pub selected_cells: Vec<(usize, usize)>, + pub selection_anchor: Option<(usize, usize)>, + + // Viewport config + pub viewport_lock: bool, + pub cursor_lock: bool, +} + +impl Buffer { + pub view_state: ViewState, // Replaces individual fields +} +``` + +2. **Create Proxy structs** +```rust +// In app_state_container.rs +pub struct NavigationProxy<'a> { + buffer: Option<&'a Buffer>, +} + +impl NavigationProxy<'_> { + pub fn selected_row(&self) -> usize { + self.buffer + .map(|b| b.view_state.crosshair_row) + .unwrap_or(0) + } + + pub fn selected_column(&self) -> usize { + self.buffer + .map(|b| b.view_state.crosshair_col) + .unwrap_or(0) + } + + pub fn scroll_offset(&self) -> (usize, usize) { + self.buffer + .map(|b| b.view_state.scroll_offset) + .unwrap_or((0, 0)) + } +} + +pub struct NavigationProxyMut<'a> { + buffer: Option<&'a mut Buffer>, +} + +impl NavigationProxyMut<'_> { + pub fn set_selected_row(&mut self, row: usize) { + if let Some(buffer) = &mut self.buffer { + buffer.view_state.crosshair_row = row; + } + } + // etc... +} +``` + +3. **Update all callers** +- Change `state.navigation().selected_row` to work with proxy +- Change `state.navigation_mut().selected_row = x` to use proxy setter + +## Phase 2: ViewportManager Integration + +### Option A: Single ViewportManager (Current approach) +- On buffer switch: Load buffer's ViewState into ViewportManager +- On any change: Save ViewportManager state back to buffer's ViewState +- Pro: Simple, less memory +- Con: Need save/restore logic + +### Option B: ViewportManager per Buffer +- Each Buffer owns its ViewportManager +- On buffer switch: Just change which ViewportManager is active +- Pro: No save/restore needed +- Con: More memory, bigger refactor + +### Recommendation: Start with Option A +We already have save/restore working. We can migrate to Option B later if needed. + +## Phase 3: Cleanup + +1. **Remove duplicate fields from Buffer** + - `current_column` -> use `view_state.crosshair_col` + - `scroll_offset` -> use `view_state.scroll_offset` + - `table_state.selected()` -> use `view_state.crosshair_row` + +2. **Remove NavigationState and SelectionState structs entirely** + - They become just proxy types + +3. **Ensure ViewportManager syncs with Buffer's ViewState** + - On every navigation operation + - On buffer switch + +## Benefits + +1. **Single source of truth** - Buffer owns all its view state +2. **Reliable buffer switching** - State travels with buffer +3. **No synchronization bugs** - Can't get out of sync if there's only one copy +4. **Cleaner architecture** - Clear ownership and data flow + +## Migration Strategy + +1. Start with Phase 1 (proxy pattern) - Less invasive +2. Test thoroughly with buffer switching +3. Move to Phase 2 once stable +4. Clean up in Phase 3 + +## Testing Plan + +1. Create test with 3 buffers +2. Navigate to different positions in each +3. Switch between them rapidly +4. Verify position is preserved +5. Test with different selection modes +6. Test with hidden/pinned columns + +## Risk Assessment + +- **High Risk**: Breaking navigation during migration + - Mitigation: Keep old code paths until new ones proven + +- **Medium Risk**: Performance impact from proxy indirection + - Mitigation: Measure and optimize hot paths + +- **Low Risk**: Memory increase from ViewState struct + - Mitigation: Negligible per buffer + +## Success Criteria + +1. Buffer switching preserves exact position +2. No duplicate state fields +3. All navigation/selection operations work +4. Performance unchanged or better +5. Code is simpler and more maintainable \ No newline at end of file diff --git a/sql-cli/MIGRATION_PLAN.md b/sql-cli/docs/MIGRATION_PLAN.md similarity index 100% rename from sql-cli/MIGRATION_PLAN.md rename to sql-cli/docs/MIGRATION_PLAN.md diff --git a/sql-cli/STATE_DUPLICATION_ANALYSIS.md b/sql-cli/docs/STATE_DUPLICATION_ANALYSIS.md similarity index 100% rename from sql-cli/STATE_DUPLICATION_ANALYSIS.md rename to sql-cli/docs/STATE_DUPLICATION_ANALYSIS.md diff --git a/sql-cli/TUI_MIGRATION_STRATEGY.md b/sql-cli/docs/TUI_MIGRATION_STRATEGY.md similarity index 100% rename from sql-cli/TUI_MIGRATION_STRATEGY.md rename to sql-cli/docs/TUI_MIGRATION_STRATEGY.md diff --git a/sql-cli/VIEWPORT_STATE_REFACTOR.md b/sql-cli/docs/VIEWPORT_STATE_REFACTOR.md similarity index 100% rename from sql-cli/VIEWPORT_STATE_REFACTOR.md rename to sql-cli/docs/VIEWPORT_STATE_REFACTOR.md diff --git a/sql-cli/VIM_SEARCH_COMPLEXITY_ANALYSIS.md b/sql-cli/docs/VIM_SEARCH_COMPLEXITY_ANALYSIS.md similarity index 100% rename from sql-cli/VIM_SEARCH_COMPLEXITY_ANALYSIS.md rename to sql-cli/docs/VIM_SEARCH_COMPLEXITY_ANALYSIS.md diff --git a/sql-cli/VIM_SEARCH_COUPLING_ANALYSIS.md b/sql-cli/docs/VIM_SEARCH_COUPLING_ANALYSIS.md similarity index 100% rename from sql-cli/VIM_SEARCH_COUPLING_ANALYSIS.md rename to sql-cli/docs/VIM_SEARCH_COUPLING_ANALYSIS.md diff --git a/sql-cli/VIM_SEARCH_DELEGATION_PLAN.md b/sql-cli/docs/VIM_SEARCH_DELEGATION_PLAN.md similarity index 100% rename from sql-cli/VIM_SEARCH_DELEGATION_PLAN.md rename to sql-cli/docs/VIM_SEARCH_DELEGATION_PLAN.md diff --git a/sql-cli/VIM_SEARCH_LESSONS_LEARNED.md b/sql-cli/docs/VIM_SEARCH_LESSONS_LEARNED.md similarity index 100% rename from sql-cli/VIM_SEARCH_LESSONS_LEARNED.md rename to sql-cli/docs/VIM_SEARCH_LESSONS_LEARNED.md diff --git a/sql-cli/VIM_SEARCH_REFACTOR_PLAN.md b/sql-cli/docs/VIM_SEARCH_REFACTOR_PLAN.md similarity index 100% rename from sql-cli/VIM_SEARCH_REFACTOR_PLAN.md rename to sql-cli/docs/VIM_SEARCH_REFACTOR_PLAN.md diff --git a/sql-cli/docs/pin_columns_analysis.md b/sql-cli/docs/pin_columns_analysis.md new file mode 100644 index 00000000..7cd7f241 --- /dev/null +++ b/sql-cli/docs/pin_columns_analysis.md @@ -0,0 +1,105 @@ +# Pin Columns Feature Analysis + +## Current State (After Buffer State Refactor) + +### ✅ What's Already Implemented + +1. **DataView Layer** - WORKING + - `pin_column()` - pins a column by display index + - `unpin_column()` - unpins a column + - `rebuild_visible_columns()` - maintains pinned columns first, then unpinned + - `pinned_columns: Vec` - tracks pinned column indices + - Proper column ordering logic + +2. **Key Bindings** - CONFIGURED + - 'p' key mapped to `pin_column` action in Results mode + - 'P' (Shift+P) mapped to `clear_pins` action + - Actions are properly routed through ActionHandler + +3. **Action Handler** - IMPLEMENTED + - `handle_column("pin_column")` calls `toggle_column_pin()` + - `handle_column("clear_pins")` calls `clear_all_pinned_columns()` + +4. **EnhancedTUI** - PARTIALLY WORKING + - `toggle_column_pin_impl()` - toggles pin state for current column + - `clear_all_pinned_columns_impl()` - clears all pins + - Uses ViewportManager to get current column position + +### ⚠️ Potential Issues to Investigate + +1. **ViewportManager Integration** + - Does `calculate_visible_column_indices()` properly handle pinned columns? + - The viewport needs to understand that pinned columns are always visible on the left + - Scrolling logic must account for pinned columns staying fixed + +2. **Rendering Pipeline** + - TableWidgetManager needs to render pinned columns differently + - Visual separator between pinned and scrollable columns + - Column headers must align with data when pinned + +3. **State Synchronization** + - With our refactor, state duplication is reduced but we need to verify: + - ViewportManager respects DataView's pinned columns + - Buffer switching preserves pin state + - Crosshair navigation works correctly with pinned columns + +## Testing Strategy + +### Basic Functionality Tests +1. Pin a single column (press 'p' on column) +2. Verify it appears on the left +3. Scroll horizontally - pinned column should stay visible +4. Unpin the column (press 'p' again) +5. Pin multiple columns +6. Clear all pins (press 'P') + +### Edge Cases to Test +1. Pin the last column +2. Pin all columns +3. Navigate with arrow keys across pinned boundary +4. Buffer switching with pinned columns +5. Sorting with pinned columns +6. Hiding columns that are pinned + +## Architecture Assessment + +### Strengths After Refactor +- Single source of truth in DataView for column organization +- No duplicate state for pinned columns +- Proper separation of concerns + +### Remaining Challenges +1. **Rendering Complexity**: The actual rendering of pinned columns with proper scrolling is complex +2. **Coordinate Translation**: Visual column index vs actual data column index with pins +3. **Width Calculations**: Available width for scrollable area after pinned columns + +## Implementation Readiness + +**Readiness Score: 7/10** + +The foundation is solid after our refactor: +- ✅ Data model is correct +- ✅ Actions are wired up +- ✅ Key bindings work +- ⚠️ Rendering needs verification +- ⚠️ Viewport calculations may need adjustment + +## Next Steps + +1. **Enable debug logging** for pin operations: + ```bash + RUST_LOG=sql_cli::data::data_view=debug,sql_cli::ui::viewport_manager=debug + ``` + +2. **Create minimal test case** with 3-5 columns to isolate issues + +3. **Fix rendering pipeline** if needed: + - Update ViewportManager's column calculation + - Ensure TableWidgetManager respects pinned columns + - Add visual separator for pinned area + +4. **Add integration tests** for pin functionality + +## Conclusion + +The pin columns feature is **much closer to working** after our buffer state refactor. The main remaining work is in the rendering/viewport layer rather than state management. With focused debugging on the viewport calculations, this feature should be achievable. \ No newline at end of file diff --git a/sql-cli/quick_test.csv b/sql-cli/quick_test.csv deleted file mode 100644 index 375034d8..00000000 --- a/sql-cli/quick_test.csv +++ /dev/null @@ -1,4 +0,0 @@ -id,name,type -1,Alice,derivatives -2,Bob,futures -3,Charlie,derivatives \ No newline at end of file diff --git a/sql-cli/src/app_state_container.rs b/sql-cli/src/app_state_container.rs index 3783a838..d8f666a8 100644 --- a/sql-cli/src/app_state_container.rs +++ b/sql-cli/src/app_state_container.rs @@ -1607,6 +1607,166 @@ impl SelectionState { } } +// ========== Proxy Structures for Buffer ViewState ========== +// These proxies allow AppStateContainer to access navigation and selection state +// directly from the current buffer's ViewState, eliminating state duplication + +/// Read-only proxy for navigation state +pub struct NavigationProxy<'a> { + buffer: Option<&'a crate::buffer::Buffer>, +} + +impl<'a> NavigationProxy<'a> { + pub fn new(buffer: Option<&'a crate::buffer::Buffer>) -> Self { + Self { buffer } + } + + pub fn selected_row(&self) -> usize { + self.buffer.map(|b| b.view_state.crosshair_row).unwrap_or(0) + } + + pub fn selected_column(&self) -> usize { + self.buffer.map(|b| b.view_state.crosshair_col).unwrap_or(0) + } + + pub fn scroll_offset(&self) -> (usize, usize) { + self.buffer + .map(|b| b.view_state.scroll_offset) + .unwrap_or((0, 0)) + } + + pub fn viewport_lock(&self) -> bool { + self.buffer + .map(|b| b.view_state.viewport_lock) + .unwrap_or(false) + } + + pub fn cursor_lock(&self) -> bool { + self.buffer + .map(|b| b.view_state.cursor_lock) + .unwrap_or(false) + } + + pub fn total_rows(&self) -> usize { + self.buffer.map(|b| b.view_state.total_rows).unwrap_or(0) + } + + pub fn total_columns(&self) -> usize { + self.buffer.map(|b| b.view_state.total_columns).unwrap_or(0) + } +} + +/// Mutable proxy for navigation state +pub struct NavigationProxyMut<'a> { + buffer: Option<&'a mut crate::buffer::Buffer>, +} + +impl<'a> NavigationProxyMut<'a> { + pub fn new(buffer: Option<&'a mut crate::buffer::Buffer>) -> Self { + Self { buffer } + } + + pub fn set_selected_row(&mut self, row: usize) { + if let Some(buffer) = &mut self.buffer { + buffer.view_state.crosshair_row = row; + } + } + + pub fn set_selected_column(&mut self, col: usize) { + if let Some(buffer) = &mut self.buffer { + buffer.view_state.crosshair_col = col; + } + } + + pub fn set_scroll_offset(&mut self, offset: (usize, usize)) { + if let Some(buffer) = &mut self.buffer { + buffer.view_state.scroll_offset = offset; + } + } + + pub fn set_viewport_lock(&mut self, locked: bool) { + if let Some(buffer) = &mut self.buffer { + buffer.view_state.viewport_lock = locked; + } + } + + pub fn set_cursor_lock(&mut self, locked: bool) { + if let Some(buffer) = &mut self.buffer { + buffer.view_state.cursor_lock = locked; + } + } + + pub fn update_totals(&mut self, rows: usize, columns: usize) { + if let Some(buffer) = &mut self.buffer { + buffer.view_state.total_rows = rows; + buffer.view_state.total_columns = columns; + } + } +} + +/// Read-only proxy for selection state +pub struct SelectionProxy<'a> { + buffer: Option<&'a crate::buffer::Buffer>, +} + +impl<'a> SelectionProxy<'a> { + pub fn new(buffer: Option<&'a crate::buffer::Buffer>) -> Self { + Self { buffer } + } + + pub fn mode(&self) -> crate::buffer::SelectionMode { + self.buffer + .map(|b| b.view_state.selection_mode.clone()) + .unwrap_or(crate::buffer::SelectionMode::Row) + } + + pub fn selected_cells(&self) -> Vec<(usize, usize)> { + self.buffer + .map(|b| b.view_state.selected_cells.clone()) + .unwrap_or_default() + } + + pub fn selection_anchor(&self) -> Option<(usize, usize)> { + self.buffer.and_then(|b| b.view_state.selection_anchor) + } +} + +/// Mutable proxy for selection state +pub struct SelectionProxyMut<'a> { + buffer: Option<&'a mut crate::buffer::Buffer>, +} + +impl<'a> SelectionProxyMut<'a> { + pub fn new(buffer: Option<&'a mut crate::buffer::Buffer>) -> Self { + Self { buffer } + } + + pub fn set_mode(&mut self, mode: crate::buffer::SelectionMode) { + if let Some(buffer) = &mut self.buffer { + buffer.view_state.selection_mode = mode; + } + } + + pub fn add_selected_cell(&mut self, cell: (usize, usize)) { + if let Some(buffer) = &mut self.buffer { + buffer.view_state.selected_cells.push(cell); + } + } + + pub fn clear_selections(&mut self) { + if let Some(buffer) = &mut self.buffer { + buffer.view_state.selected_cells.clear(); + buffer.view_state.selection_anchor = None; + } + } + + pub fn set_selection_anchor(&mut self, anchor: Option<(usize, usize)>) { + if let Some(buffer) = &mut self.buffer { + buffer.view_state.selection_anchor = anchor; + } + } +} + /// History search state (for Ctrl+R functionality) #[derive(Debug, Clone)] pub struct HistorySearchState { @@ -3010,16 +3170,25 @@ impl AppStateContainer { .advance_sort_state(column_index, column_name, new_order); } - /// Get current selection state (read-only) + /// Get current selection state (read-only) - DEPRECATED: Will be removed after migration pub fn selection(&self) -> std::cell::Ref { self.selection.borrow() } - /// Get current selection state (mutable) + /// Get current selection state (mutable) - DEPRECATED: Will be removed after migration pub fn selection_mut(&self) -> std::cell::RefMut { self.selection.borrow_mut() } + /// NEW: Proxy-based selection access (single source of truth) + pub fn selection_proxy(&self) -> SelectionProxy { + SelectionProxy::new(self.buffers.current()) + } + + pub fn selection_proxy_mut(&mut self) -> SelectionProxyMut { + SelectionProxyMut::new(self.buffers.current_mut()) + } + /// Set selection mode pub fn set_selection_mode(&self, mode: SelectionMode) { let mut selection = self.selection.borrow_mut(); @@ -3789,7 +3958,7 @@ impl AppStateContainer { self.navigation.borrow().cursor_lock } - // Navigation state access + // Navigation state access - DEPRECATED: Will be removed after migration pub fn navigation(&self) -> std::cell::Ref<'_, NavigationState> { self.navigation.borrow() } @@ -3798,6 +3967,15 @@ impl AppStateContainer { self.navigation.borrow_mut() } + // NEW: Proxy-based navigation access (single source of truth) + pub fn navigation_proxy(&self) -> NavigationProxy { + NavigationProxy::new(self.buffers.current()) + } + + pub fn navigation_proxy_mut(&mut self) -> NavigationProxyMut { + NavigationProxyMut::new(self.buffers.current_mut()) + } + // get_current_position is defined earlier in the file at line 2946 pub fn get_scroll_offset(&self) -> (usize, usize) { diff --git a/sql-cli/src/buffer.rs b/sql-cli/src/buffer.rs index 9867b118..94c36d17 100644 --- a/sql-cli/src/buffer.rs +++ b/sql-cli/src/buffer.rs @@ -130,6 +130,63 @@ impl Default for SearchState { // ColumnSearchState: MIGRATED to AppStateContainer +#[derive(Clone, Debug, PartialEq)] +pub enum SelectionMode { + Row, + Cell, + Column, +} + +/// ViewState consolidates all view-related state for a buffer +/// This is the single source of truth for navigation, selection, and viewport state +#[derive(Clone, Debug)] +pub struct ViewState { + // Position + pub crosshair_row: usize, + pub crosshair_col: usize, + pub scroll_offset: (usize, usize), + + // Selection + pub selection_mode: SelectionMode, + pub selected_cells: Vec<(usize, usize)>, + pub selection_anchor: Option<(usize, usize)>, + + // Viewport config + pub viewport_lock: bool, + pub cursor_lock: bool, + + // Navigation history + pub navigation_history: Vec<(usize, usize)>, + pub history_index: usize, + + // Viewport dimensions (cached from last render) + pub viewport_rows: usize, + pub viewport_columns: usize, + pub total_rows: usize, + pub total_columns: usize, +} + +impl Default for ViewState { + fn default() -> Self { + Self { + crosshair_row: 0, + crosshair_col: 0, + scroll_offset: (0, 0), + selection_mode: SelectionMode::Row, + selected_cells: Vec::new(), + selection_anchor: None, + viewport_lock: false, + cursor_lock: false, + navigation_history: Vec::new(), + history_index: 0, + viewport_rows: 0, + viewport_columns: 0, + total_rows: 0, + total_columns: 0, + } + } +} + #[derive(Clone, Debug)] pub enum ColumnType { String, @@ -371,13 +428,12 @@ pub struct Buffer { pub column_stats: Option, - // --- View State --- + // --- View State (Consolidated) --- + pub view_state: ViewState, + + // --- Display Options (not navigation-related) --- pub column_widths: Vec, - pub scroll_offset: (usize, usize), - pub current_column: usize, pub compact_mode: bool, - pub viewport_lock: bool, - pub viewport_lock_row: Option, pub show_row_numbers: bool, pub case_insensitive: bool, @@ -547,31 +603,37 @@ impl BufferAPI for Buffer { // --- Table Navigation --- fn get_selected_row(&self) -> Option { + // For backward compatibility, check if table_state has a selection + // This maintains the old API behavior where None means no selection self.table_state.selected() } fn set_selected_row(&mut self, row: Option) { if let Some(r) = row { + self.view_state.crosshair_row = r; + // Also update table_state for compatibility during migration self.table_state.select(Some(r)); } else { + // When setting to None, reset crosshair to 0 but clear table selection + self.view_state.crosshair_row = 0; self.table_state.select(None); } } fn get_current_column(&self) -> usize { - self.current_column + self.view_state.crosshair_col } fn set_current_column(&mut self, col: usize) { - self.current_column = col; + self.view_state.crosshair_col = col; } fn get_scroll_offset(&self) -> (usize, usize) { - self.scroll_offset + self.view_state.scroll_offset } fn set_scroll_offset(&mut self, offset: (usize, usize)) { - self.scroll_offset = offset; + self.view_state.scroll_offset = offset; } fn get_last_results_row(&self) -> Option { @@ -725,19 +787,28 @@ impl BufferAPI for Buffer { } fn is_viewport_lock(&self) -> bool { - self.viewport_lock + self.view_state.viewport_lock } fn set_viewport_lock(&mut self, locked: bool) { - self.viewport_lock = locked; + self.view_state.viewport_lock = locked; } fn get_viewport_lock_row(&self) -> Option { - self.viewport_lock_row + // Return current crosshair row when viewport is locked + if self.view_state.viewport_lock { + Some(self.view_state.crosshair_row) + } else { + None + } } fn set_viewport_lock_row(&mut self, row: Option) { - self.viewport_lock_row = row; + // When setting viewport lock row, update the crosshair position + if let Some(r) = row { + self.view_state.crosshair_row = r; + self.view_state.viewport_lock = true; + } } fn get_column_widths(&self) -> &Vec { @@ -972,8 +1043,14 @@ impl BufferAPI for Buffer { "Selected Row: {:?}\n", self.table_state.selected() )); - output.push_str(&format!("Current Column: {}\n", self.current_column)); - output.push_str(&format!("Scroll Offset: {:?}\n", self.scroll_offset)); + output.push_str(&format!( + "Current Column: {}\n", + self.view_state.crosshair_col + )); + output.push_str(&format!( + "Scroll Offset: {:?}\n", + self.view_state.scroll_offset + )); output.push_str("\n--- Filtering ---\n"); output.push_str(&format!("Filter Active: {}\n", self.filter_state.active)); output.push_str(&format!( @@ -1029,11 +1106,7 @@ impl BufferAPI for Buffer { output.push_str("Pinned Columns: []\n"); } output.push_str(&format!("Column Widths: {:?}\n", self.column_widths)); - output.push_str(&format!("Viewport Lock: {}\n", self.viewport_lock)); - output.push_str(&format!( - "Viewport Lock Row: {:?}\n", - self.viewport_lock_row - )); + output.push_str(&format!("ViewState: {:?}\n", self.view_state)); output.push_str("\n--- Data Source ---\n"); output.push_str("Legacy CSV/Cache fields removed - using DataTable/DataView\n"); output.push_str("\n--- Undo/Redo ---\n"); @@ -1149,7 +1222,7 @@ impl BufferAPI for Buffer { // DataView handles filtering self.table_state.select(None); self.last_results_row = None; - self.scroll_offset = (0, 0); + self.view_state.scroll_offset = (0, 0); self.last_scroll_offset = (0, 0); self.column_widths.clear(); self.status_message = "Results cleared".to_string(); @@ -1196,12 +1269,9 @@ impl Buffer { // column_search_state: MIGRATED to AppStateContainer column_stats: None, + view_state: ViewState::default(), column_widths: Vec::new(), - scroll_offset: (0, 0), - current_column: 0, compact_mode: false, - viewport_lock: false, - viewport_lock_row: None, show_row_numbers: false, case_insensitive: false, @@ -1547,13 +1617,10 @@ impl Clone for Buffer { fuzzy_filter_state: self.fuzzy_filter_state.clone(), search_state: self.search_state.clone(), // column_search_state: MIGRATED to AppStateContainer - column_widths: self.column_widths.clone(), - scroll_offset: self.scroll_offset, - current_column: self.current_column, column_stats: self.column_stats.clone(), + view_state: self.view_state.clone(), + column_widths: self.column_widths.clone(), compact_mode: self.compact_mode, - viewport_lock: self.viewport_lock, - viewport_lock_row: self.viewport_lock_row, show_row_numbers: self.show_row_numbers, case_insensitive: self.case_insensitive, undo_stack: self.undo_stack.clone(), diff --git a/sql-cli/src/config/config.rs b/sql-cli/src/config/config.rs index 843db1d9..18cd4221 100644 --- a/sql-cli/src/config/config.rs +++ b/sql-cli/src/config/config.rs @@ -68,6 +68,9 @@ pub struct BehaviorConfig { /// Case-insensitive by default pub case_insensitive_default: bool, + /// Start mode when loading files: "command" or "results" + pub start_mode: String, + /// Maximum rows to display without pagination warning pub max_display_rows: usize, @@ -203,6 +206,7 @@ impl Default for BehaviorConfig { Self { auto_execute_on_load: true, case_insensitive_default: true, // Default to case-insensitive for practical use + start_mode: "results".to_string(), // Default to results mode for immediate data view max_display_rows: 10000, cache_dir: None, enable_history: true, @@ -267,6 +271,10 @@ impl Config { " case_insensitive_default = {}\n", self.behavior.case_insensitive_default )); + info.push_str(&format!( + " start_mode = \"{}\"\n", + self.behavior.start_mode + )); info.push_str(&format!( " max_display_rows = {}\n", self.behavior.max_display_rows @@ -405,6 +413,11 @@ auto_execute_on_load = true # Use case-insensitive string comparisons by default (recommended for practical use) case_insensitive_default = true +# Start mode when loading files: "command" or "results" +# - "command": Start in command mode (focus on SQL input) +# - "results": Start in results mode (focus on data, press 'i' to edit query) +start_mode = "results" + # Maximum rows to display without warning max_display_rows = 10000 diff --git a/sql-cli/src/help_text.rs b/sql-cli/src/help_text.rs index f8f9366d..15d011f2 100644 --- a/sql-cli/src/help_text.rs +++ b/sql-cli/src/help_text.rs @@ -61,17 +61,18 @@ impl HelpText { Line::from(" Alt+[ - Jump to previous SQL token"), Line::from(" Alt+] - Jump to next SQL token"), Line::from(""), - Line::from("BUFFER MANAGEMENT").style( + Line::from("BUFFER MANAGEMENT (works in Command & Results modes)").style( Style::default() .fg(Color::Yellow) .add_modifier(Modifier::BOLD), ), Line::from(" F11/Ctrl+PgUp - Previous buffer"), Line::from(" F12/Ctrl+PgDn - Next buffer"), - Line::from(" Ctrl+6 - Quick switch"), + Line::from(" Ctrl+6 - Quick switch (toggle last two)"), Line::from(" Alt+N - New buffer"), Line::from(" Alt+W - Close buffer"), Line::from(" Alt+B - List buffers"), + Line::from(" Alt+1-9 - Switch to buffer N"), Line::from(""), Line::from("VIEW MODES").style( Style::default() diff --git a/sql-cli/src/ui/enhanced_tui.rs b/sql-cli/src/ui/enhanced_tui.rs index 45eaceba..2e1c3a7a 100644 --- a/sql-cli/src/ui/enhanced_tui.rs +++ b/sql-cli/src/ui/enhanced_tui.rs @@ -45,6 +45,7 @@ use crate::widgets::editor_widget::{BufferAction, EditorAction, EditorWidget}; use crate::widgets::help_widget::HelpWidget; use crate::widgets::search_modes_widget::{SearchMode, SearchModesAction, SearchModesWidget}; use crate::widgets::stats_widget::StatsWidget; +use crate::widgets::tab_bar_widget::TabBarWidget; use crate::{buffer, data_analyzer, dual_logging}; use anyhow::Result; use crossterm::{ @@ -352,6 +353,113 @@ impl EnhancedTuiApp { &mut self.state_container } + // ========== STATE SYNCHRONIZATION ========== + + /// Synchronize ViewportManager state with NavigationState + /// This ensures single source of truth for crosshair position + pub(crate) fn sync_viewport_with_navigation(&self) { + let nav = self.state_container.navigation(); + let mut viewport_borrow = self.viewport_manager.borrow_mut(); + + if let Some(ref mut viewport) = viewport_borrow.as_mut() { + // Update ViewportManager's crosshair from NavigationState + viewport.set_crosshair(nav.selected_row, nav.selected_column); + + // If scroll offsets don't match, update ViewportManager + let current_offset = viewport.get_scroll_offset(); + if current_offset != nav.scroll_offset { + viewport.set_scroll_offset(nav.scroll_offset.0, nav.scroll_offset.1); + } + } + } + + /// Synchronize NavigationState with ViewportManager + /// This is the reverse - update NavigationState from ViewportManager + pub(crate) fn sync_navigation_with_viewport(&self) { + let viewport_borrow = self.viewport_manager.borrow(); + + if let Some(ref viewport) = viewport_borrow.as_ref() { + let mut nav = self.state_container.navigation_mut(); + + // Update NavigationState from ViewportManager's authoritative position + nav.selected_row = viewport.get_selected_row(); + nav.selected_column = viewport.get_selected_column(); + nav.scroll_offset = viewport.get_scroll_offset(); + } + } + + /// Save current ViewportManager state to the current buffer + fn save_viewport_to_current_buffer(&mut self) { + let viewport_borrow = self.viewport_manager.borrow(); + + if let Some(ref viewport) = viewport_borrow.as_ref() { + if let Some(buffer) = self.state_container.buffers_mut().current_mut() { + // Save crosshair position + buffer.set_selected_row(Some(viewport.get_selected_row())); + buffer.set_current_column(viewport.get_selected_column()); + + // Save scroll offset + buffer.set_scroll_offset(viewport.get_scroll_offset()); + } + } + } + + /// Restore ViewportManager state from the current buffer + fn restore_viewport_from_current_buffer(&mut self) { + if let Some(buffer) = self.state_container.buffers().current() { + // Check if this buffer has a DataView + if let Some(dataview) = buffer.get_dataview() { + // Check if we need to create or update ViewportManager + let needs_new_viewport = { + let viewport_borrow = self.viewport_manager.borrow(); + viewport_borrow.is_none() + }; + + if needs_new_viewport { + // Create new ViewportManager for this buffer's DataView + self.viewport_manager = + RefCell::new(Some(ViewportManager::new(Arc::new(dataview.clone())))); + } else { + // Update existing ViewportManager with new DataView + let mut viewport_borrow = self.viewport_manager.borrow_mut(); + if let Some(ref mut viewport) = viewport_borrow.as_mut() { + viewport.set_dataview(Arc::new(dataview.clone())); + } + } + + // Now restore the position + let mut viewport_borrow = self.viewport_manager.borrow_mut(); + if let Some(ref mut viewport) = viewport_borrow.as_mut() { + // The data dimensions are already updated by set_dataview above + // Just restore the saved position + + // Restore crosshair position + let row = buffer.get_selected_row().unwrap_or(0); + let col = buffer.get_current_column(); + viewport.set_crosshair(row, col); + + // Restore scroll offset + let scroll_offset = buffer.get_scroll_offset(); + viewport.set_scroll_offset(scroll_offset.0, scroll_offset.1); + + // Update terminal size to trigger viewport recalculation + let term_width = viewport.get_terminal_width(); + let term_height = viewport.get_terminal_height() as u16; + viewport.update_terminal_size(term_width, term_height); + + // Also update NavigationState for consistency + drop(viewport_borrow); + self.sync_navigation_with_viewport(); + } + + // Also update TableWidgetManager with the new DataView + let mut table_manager = self.table_widget_manager.borrow_mut(); + table_manager.set_dataview(Arc::new(dataview.clone())); + table_manager.force_render(); // Force a re-render with new data + } + } + } + // ========== VIEWPORT CALCULATIONS ========== /// Calculate the number of data rows available for the table @@ -1276,6 +1384,18 @@ impl EnhancedTuiApp { }; self.state_container.set_status_message(display_msg); } + + // Set initial mode based on config + let initial_mode = match self.config.behavior.start_mode.to_lowercase().as_str() { + "results" => AppMode::Results, + "command" => AppMode::Command, + _ => AppMode::Results, // Default to results if invalid config + }; + + self.state_container.set_mode(initial_mode.clone()); + self.shadow_state + .borrow_mut() + .observe_mode_change(initial_mode, "initial_load_from_config"); } pub fn run(mut self) -> Result<()> { @@ -1414,6 +1534,11 @@ impl EnhancedTuiApp { } } + // SECOND: Try buffer operations (F11/F12, Ctrl-6, etc) - these should work in Results mode too + if let Some(result) = self.try_handle_buffer_operations(&key)? { + return Ok(result); + } + let chord_result = self.key_chord_handler.process_key(key); debug!("Chord handler returned: {:?}", chord_result); @@ -1910,24 +2035,42 @@ impl EnhancedTuiApp { match action { "quit" => return Ok(Some(true)), "next_buffer" => { + // Save viewport state before switching + self.save_viewport_to_current_buffer(); + let message = self .buffer_handler .next_buffer(self.state_container.buffers_mut()); debug!("{}", message); + + // Restore viewport state after switching + self.restore_viewport_from_current_buffer(); return Ok(Some(false)); } "previous_buffer" => { + // Save viewport state before switching + self.save_viewport_to_current_buffer(); + let message = self .buffer_handler .previous_buffer(self.state_container.buffers_mut()); debug!("{}", message); + + // Restore viewport state after switching + self.restore_viewport_from_current_buffer(); return Ok(Some(false)); } "quick_switch_buffer" => { + // Save viewport state before switching + self.save_viewport_to_current_buffer(); + let message = self .buffer_handler .quick_switch(self.state_container.buffers_mut()); debug!("{}", message); + + // Restore viewport state after switching + self.restore_viewport_from_current_buffer(); return Ok(Some(false)); } "new_buffer" => { @@ -1956,11 +2099,17 @@ impl EnhancedTuiApp { action if action.starts_with("switch_to_buffer_") => { if let Some(buffer_num_str) = action.strip_prefix("switch_to_buffer_") { if let Ok(buffer_num) = buffer_num_str.parse::() { + // Save viewport state before switching + self.save_viewport_to_current_buffer(); + let message = self.buffer_handler.switch_to_buffer( self.state_container.buffers_mut(), buffer_num - 1, ); debug!("{}", message); + + // Restore viewport state after switching + self.restore_viewport_from_current_buffer(); } } return Ok(Some(false)); @@ -4057,27 +4206,13 @@ impl EnhancedTuiApp { self.state_container.get_current_column(), self.state_container.selection().selected_column); - // CRITICAL: Also update navigation's selected_row to trigger proper rendering - self.state_container.navigation_mut().selected_row = search_match.row; - - // CRITICAL: Update navigation scroll offset to match the viewport! - // The viewport manager has scrolled, but we need to sync that back to navigation state - if let Some(ref viewport) = *self.viewport_manager.borrow() { - let viewport_rows = viewport.get_viewport_rows(); - let viewport_cols = viewport.viewport_cols(); - - info!(target: "search", - "Syncing navigation scroll to viewport: row_offset={}, col_offset={}", - viewport_rows.start, viewport_cols.start); - - // Update the navigation scroll offset to match viewport - self.state_container.navigation_mut().scroll_offset = - (viewport_rows.start, viewport_cols.start); + // CRITICAL: Sync NavigationState with ViewportManager after vim search navigation + // ViewportManager has the correct state after navigation, sync it back + self.sync_navigation_with_viewport(); - // Also update the buffer scroll offset - self.state_container - .set_scroll_offset((viewport_rows.start, viewport_cols.start)); - } + // Also update the buffer scroll offset + let scroll_offset = self.state_container.navigation().scroll_offset; + self.state_container.set_scroll_offset(scroll_offset); // CRITICAL: Update TableWidgetManager to trigger re-render info!(target: "search", "Updating TableWidgetManager for vim search navigation to ({}, {})", @@ -4093,33 +4228,9 @@ impl EnhancedTuiApp { self.state_container.get_current_column(), self.state_container.selection().selected_column); - // Update scroll offset if row changed significantly - let viewport_height = 79; // Typical viewport height - let (current_scroll_row, _) = self.state_container.get_scroll_offset(); - - // Get the column scroll offset from the viewport manager - // The viewport manager has already updated the column viewport - let new_col_scroll = if let Some(ref viewport) = *self.viewport_manager.borrow() { - viewport.viewport_cols().start - } else { - 0 - }; - - // If the match is outside current viewport, update scroll - if search_match.row < current_scroll_row - || search_match.row >= current_scroll_row + viewport_height - { - let new_scroll = search_match.row.saturating_sub(viewport_height / 2); - self.state_container - .set_scroll_offset((new_scroll, new_col_scroll)); - self.state_container.navigation_mut().scroll_offset = (new_scroll, new_col_scroll); - } else { - // Even if row didn't change, we need to update column scroll - self.state_container - .set_scroll_offset((current_scroll_row, new_col_scroll)); - self.state_container.navigation_mut().scroll_offset = - (current_scroll_row, new_col_scroll); - } + // The ViewportManager has already handled all scrolling logic + // Our sync_navigation_with_viewport() call above has updated NavigationState + // No need for additional manual scroll updates } // Update status without borrow conflicts @@ -4248,27 +4359,13 @@ impl EnhancedTuiApp { self.state_container.get_current_column(), self.state_container.selection().selected_column); - // CRITICAL: Also update navigation's selected_row to trigger proper rendering - self.state_container.navigation_mut().selected_row = search_match.row; - - // CRITICAL: Update navigation scroll offset to match the viewport! - // The viewport manager has scrolled, but we need to sync that back to navigation state - if let Some(ref viewport) = *self.viewport_manager.borrow() { - let viewport_rows = viewport.get_viewport_rows(); - let viewport_cols = viewport.viewport_cols(); - - info!(target: "search", - "Syncing navigation scroll to viewport: row_offset={}, col_offset={}", - viewport_rows.start, viewport_cols.start); + // CRITICAL: Sync NavigationState with ViewportManager after vim search navigation + // ViewportManager has the correct state after navigation, sync it back + self.sync_navigation_with_viewport(); - // Update the navigation scroll offset to match viewport - self.state_container.navigation_mut().scroll_offset = - (viewport_rows.start, viewport_cols.start); - - // Also update the buffer scroll offset - self.state_container - .set_scroll_offset((viewport_rows.start, viewport_cols.start)); - } + // Also update the buffer scroll offset + let scroll_offset = self.state_container.navigation().scroll_offset; + self.state_container.set_scroll_offset(scroll_offset); // CRITICAL: Update TableWidgetManager to trigger re-render info!(target: "search", "Updating TableWidgetManager for vim search navigation to ({}, {})", @@ -4284,33 +4381,9 @@ impl EnhancedTuiApp { self.state_container.get_current_column(), self.state_container.selection().selected_column); - // Update scroll offset if row changed significantly - let viewport_height = 79; // Typical viewport height - let (current_scroll_row, _) = self.state_container.get_scroll_offset(); - - // Get the column scroll offset from the viewport manager - // The viewport manager has already updated the column viewport - let new_col_scroll = if let Some(ref viewport) = *self.viewport_manager.borrow() { - viewport.viewport_cols().start - } else { - 0 - }; - - // If the match is outside current viewport, update scroll - if search_match.row < current_scroll_row - || search_match.row >= current_scroll_row + viewport_height - { - let new_scroll = search_match.row.saturating_sub(viewport_height / 2); - self.state_container - .set_scroll_offset((new_scroll, new_col_scroll)); - self.state_container.navigation_mut().scroll_offset = (new_scroll, new_col_scroll); - } else { - // Even if row didn't change, we need to update column scroll - self.state_container - .set_scroll_offset((current_scroll_row, new_col_scroll)); - self.state_container.navigation_mut().scroll_offset = - (current_scroll_row, new_col_scroll); - } + // The ViewportManager has already handled all scrolling logic + // Our sync_navigation_with_viewport() call above has updated NavigationState + // No need for additional manual scroll updates } // Update status without borrow conflicts @@ -5075,10 +5148,15 @@ impl EnhancedTuiApp { // Always use single-line mode input height let input_height = INPUT_AREA_HEIGHT; + // Always show tab bar for consistent layout + let buffer_count = self.state_container.buffers().all_buffers().len(); + let tab_bar_height = 2; // Always reserve space for tab bar + let chunks = Layout::default() .direction(Direction::Vertical) .constraints( [ + Constraint::Length(tab_bar_height), // Tab bar (always shown) Constraint::Length(input_height), // Command input area Constraint::Min(0), // Results Constraint::Length(STATUS_BAR_HEIGHT), // Status bar @@ -5087,8 +5165,28 @@ impl EnhancedTuiApp { ) .split(f.area()); + // Always render tab bar (even with single buffer) + if buffer_count > 0 { + let buffer_names: Vec = self + .state_container + .buffers() + .all_buffers() + .iter() + .map(|b| b.get_name()) + .collect(); + let current_index = self.state_container.buffers().current_index(); + + let tab_widget = TabBarWidget::new(current_index, buffer_names); + tab_widget.render(f, chunks[0]); + } + + // Fixed chunk indices since tab bar is always present + let input_chunk_idx = 1; + let results_chunk_idx = 2; + let status_chunk_idx = 3; + // Update horizontal scroll based on actual terminal width - self.update_horizontal_scroll(chunks[0].width); + self.update_horizontal_scroll(chunks[input_chunk_idx].width); // Command input area // Get the current input text length and cursor position for display @@ -5143,7 +5241,7 @@ impl EnhancedTuiApp { if use_search_widget { // Let the search modes widget render the input field with debounce indicator - self.search_modes_widget.render(f, chunks[0]); + self.search_modes_widget.render(f, chunks[input_chunk_idx]); } else { // Always get input text through the buffer API for consistency let input_text_string = self.get_input_text(); @@ -5222,9 +5320,9 @@ impl EnhancedTuiApp { }; // Render the input paragraph (single-line mode) - f.render_widget(input_paragraph, chunks[0]); + f.render_widget(input_paragraph, chunks[input_chunk_idx]); } - let results_area = chunks[1]; + let results_area = chunks[results_chunk_idx]; // Set cursor position for input modes (skip if search widget is handling it) if !use_search_widget { @@ -5232,7 +5330,7 @@ impl EnhancedTuiApp { AppMode::Command => { // Always use single-line cursor handling // Calculate cursor position with horizontal scrolling - let inner_width = chunks[0].width.saturating_sub(2) as usize; + let inner_width = chunks[input_chunk_idx].width.saturating_sub(2) as usize; let cursor_pos = self.get_visual_cursor().1; // Get column position for single-line let scroll_offset = self.get_horizontal_scroll_offset() as usize; @@ -5240,44 +5338,47 @@ impl EnhancedTuiApp { if cursor_pos >= scroll_offset && cursor_pos < scroll_offset + inner_width { let visible_pos = cursor_pos - scroll_offset; f.set_cursor_position(( - chunks[0].x + visible_pos as u16 + 1, - chunks[0].y + 1, + chunks[input_chunk_idx].x + visible_pos as u16 + 1, + chunks[input_chunk_idx].y + 1, )); } } AppMode::Search => { f.set_cursor_position(( - chunks[0].x + self.get_input_cursor() as u16 + 1, - chunks[0].y + 1, + chunks[input_chunk_idx].x + self.get_input_cursor() as u16 + 1, + chunks[input_chunk_idx].y + 1, )); } AppMode::Filter => { f.set_cursor_position(( - chunks[0].x + self.get_input_cursor() as u16 + 1, - chunks[0].y + 1, + chunks[input_chunk_idx].x + self.get_input_cursor() as u16 + 1, + chunks[input_chunk_idx].y + 1, )); } AppMode::FuzzyFilter => { f.set_cursor_position(( - chunks[0].x + self.get_input_cursor() as u16 + 1, - chunks[0].y + 1, + chunks[input_chunk_idx].x + self.get_input_cursor() as u16 + 1, + chunks[input_chunk_idx].y + 1, )); } AppMode::ColumnSearch => { f.set_cursor_position(( - chunks[0].x + self.get_input_cursor() as u16 + 1, - chunks[0].y + 1, + chunks[input_chunk_idx].x + self.get_input_cursor() as u16 + 1, + chunks[input_chunk_idx].y + 1, )); } AppMode::JumpToRow => { f.set_cursor_position(( - chunks[0].x + self.get_jump_to_row_input().len() as u16 + 1, - chunks[0].y + 1, + chunks[input_chunk_idx].x + self.get_jump_to_row_input().len() as u16 + 1, + chunks[input_chunk_idx].y + 1, )); } AppMode::History => { let query_len = self.state_container.history_search().query.len(); - f.set_cursor_position((chunks[0].x + query_len as u16 + 1, chunks[0].y + 1)); + f.set_cursor_position(( + chunks[input_chunk_idx].x + query_len as u16 + 1, + chunks[input_chunk_idx].y + 1, + )); } _ => {} } @@ -5308,7 +5409,7 @@ impl EnhancedTuiApp { } // Render mode-specific status line - self.render_status_line(f, chunks[2]); + self.render_status_line(f, chunks[status_chunk_idx]); // ========== RENDERING ========== } @@ -5356,25 +5457,8 @@ impl EnhancedTuiApp { /// Add data source display to status spans fn add_data_source_display(&self, spans: &mut Vec) { - if let Some(ref source) = self.data_source { - spans.push(Span::raw(" ")); - let source_display = if source.starts_with("http://") || source.starts_with("https://") - { - // For API endpoints, show a shortened version - format!("[API: {}]", source.split('/').nth(2).unwrap_or("unknown")) - } else { - // For files, show just the filename - let filename = std::path::Path::new(source) - .file_name() - .and_then(|s| s.to_str()) - .unwrap_or(source); - format!("[{}]", filename) - }; - spans.push(Span::styled( - source_display, - Style::default().fg(Color::Cyan), - )); - } + // Skip showing data source in status line since tab bar shows file names + // This avoids redundancy like "[trades.csv] [1/2] trades.csv" } /// Add buffer information to status spans @@ -5391,19 +5475,28 @@ impl EnhancedTuiApp { )); } - // Don't show buffer name if it's the same as the data source - // The data source is already displayed, so we avoid duplication + // Show table name from current query (simplified) + // Since tab bar shows file names, we just show the table being queried + if let Some(buffer) = self.state_container.buffers().current() { + let query = buffer.get_input_text(); + // Simple extraction of table name from "SELECT ... FROM table" pattern + if let Some(from_pos) = query.to_uppercase().find(" FROM ") { + let after_from = &query[from_pos + 6..]; + // Take the first word after FROM as the table name + if let Some(table_name) = after_from.split_whitespace().next() { + // Clean up the table name (remove quotes, etc.) + let clean_name = table_name + .trim_matches('"') + .trim_matches('\'') + .trim_matches('`'); - // Get buffer name from the current buffer - let buffer_name = self.state_container.get_buffer_name(); - if !buffer_name.is_empty() && buffer_name != "[Buffer 1]" { - spans.push(Span::raw(" ")); - spans.push(Span::styled( - buffer_name, - Style::default() - .fg(Color::Cyan) - .add_modifier(Modifier::BOLD), - )); + spans.push(Span::raw(" ")); + spans.push(Span::styled( + clean_name.to_string(), + Style::default().fg(Color::Cyan), + )); + } + } } } @@ -6953,16 +7046,17 @@ impl ActionHandlerContext for EnhancedTuiApp { }; if let Some(result) = result { + // ViewportManager has updated, sync NavigationState from it + self.sync_navigation_with_viewport(); + + // Update buffer's selected row self.state_container .set_selected_row(Some(result.row_position)); + + // Update buffer's scroll offset if result.viewport_changed { - let mut offset = self.state_container.get_scroll_offset(); - offset.0 = result.row_scroll_offset; - self.state_container.set_scroll_offset(offset); - } - 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; + let scroll_offset = self.state_container.navigation().scroll_offset; + self.state_container.set_scroll_offset(scroll_offset); } } } @@ -6978,16 +7072,17 @@ impl ActionHandlerContext for EnhancedTuiApp { }; if let Some(result) = result { + // ViewportManager has updated, sync NavigationState from it + self.sync_navigation_with_viewport(); + + // Update buffer's selected row self.state_container .set_selected_row(Some(result.row_position)); + + // Update buffer's scroll offset if result.viewport_changed { - let mut offset = self.state_container.get_scroll_offset(); - offset.0 = result.row_scroll_offset; - self.state_container.set_scroll_offset(offset); - } - 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; + let scroll_offset = self.state_container.navigation().scroll_offset; + self.state_container.set_scroll_offset(scroll_offset); } } } @@ -7003,16 +7098,17 @@ impl ActionHandlerContext for EnhancedTuiApp { }; if let Some(result) = result { + // ViewportManager has updated, sync NavigationState from it + self.sync_navigation_with_viewport(); + + // Update buffer's selected row self.state_container .set_selected_row(Some(result.row_position)); + + // Update buffer's scroll offset if result.viewport_changed { - let mut offset = self.state_container.get_scroll_offset(); - offset.0 = result.row_scroll_offset; - self.state_container.set_scroll_offset(offset); - } - 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; + let scroll_offset = self.state_container.navigation().scroll_offset; + self.state_container.set_scroll_offset(scroll_offset); } } } @@ -7284,18 +7380,45 @@ impl BufferManagementBehavior for EnhancedTuiApp { } fn next_buffer(&mut self) -> String { - self.buffer_handler - .next_buffer(self.state_container.buffers_mut()) + // Save current viewport state to current buffer before switching + self.save_viewport_to_current_buffer(); + + let result = self + .buffer_handler + .next_buffer(self.state_container.buffers_mut()); + + // Restore viewport state from new buffer after switching + self.restore_viewport_from_current_buffer(); + + result } fn previous_buffer(&mut self) -> String { - self.buffer_handler - .previous_buffer(self.state_container.buffers_mut()) + // Save current viewport state to current buffer before switching + self.save_viewport_to_current_buffer(); + + let result = self + .buffer_handler + .previous_buffer(self.state_container.buffers_mut()); + + // Restore viewport state from new buffer after switching + self.restore_viewport_from_current_buffer(); + + result } fn quick_switch_buffer(&mut self) -> String { - self.buffer_handler - .quick_switch(self.state_container.buffers_mut()) + // Save current viewport state to current buffer before switching + self.save_viewport_to_current_buffer(); + + let result = self + .buffer_handler + .quick_switch(self.state_container.buffers_mut()); + + // Restore viewport state from new buffer after switching + self.restore_viewport_from_current_buffer(); + + result } fn close_buffer(&mut self) -> (bool, String) { @@ -7304,8 +7427,18 @@ impl BufferManagementBehavior for EnhancedTuiApp { } fn switch_to_buffer(&mut self, index: usize) -> String { - self.buffer_handler - .switch_to_buffer(self.state_container.buffers_mut(), index) + // Save current viewport state to current buffer before switching + self.save_viewport_to_current_buffer(); + + // Switch buffer + let result = self + .buffer_handler + .switch_to_buffer(self.state_container.buffers_mut(), index); + + // Restore viewport state from new buffer after switching + self.restore_viewport_from_current_buffer(); + + result } fn buffer_count(&self) -> usize { @@ -7344,6 +7477,8 @@ pub fn run_enhanced_tui_multi(api_url: &str, data_files: Vec<&str>) -> Result<() // Switch back to the first buffer if we loaded multiple if data_files.len() > 1 { app.state_container.buffers_mut().switch_to(0); + // CRITICAL: Sync ViewportManager with the current buffer after switching + app.restore_viewport_from_current_buffer(); app.state_container.set_status_message(format!( "Loaded {} files into separate buffers. Use Alt+Tab to switch.", data_files.len() diff --git a/sql-cli/src/ui/viewport_manager.rs b/sql-cli/src/ui/viewport_manager.rs index b128a2ed..70b38671 100644 --- a/sql-cli/src/ui/viewport_manager.rs +++ b/sql-cli/src/ui/viewport_manager.rs @@ -233,6 +233,46 @@ impl ViewportManager { self.crosshair_row } + /// Get selected row (alias for crosshair_row for compatibility) + pub fn get_selected_row(&self) -> usize { + self.crosshair_row + } + + /// Get selected column (alias for crosshair_col for compatibility) + pub fn get_selected_column(&self) -> usize { + self.crosshair_col + } + + /// Get scroll offset as (row_offset, col_offset) + pub fn get_scroll_offset(&self) -> (usize, usize) { + (self.viewport_rows.start, self.viewport_cols.start) + } + + /// Set scroll offset and update viewport accordingly + pub fn set_scroll_offset(&mut self, row_offset: usize, col_offset: usize) { + let viewport_height = self.viewport_rows.end - self.viewport_rows.start; + let viewport_width = self.viewport_cols.end - self.viewport_cols.start; + + // Update viewport ranges based on new scroll offset + self.viewport_rows = row_offset..(row_offset + viewport_height); + self.viewport_cols = col_offset..(col_offset + viewport_width); + + // Ensure crosshair stays within new viewport + if self.crosshair_row < self.viewport_rows.start { + self.crosshair_row = self.viewport_rows.start; + } else if self.crosshair_row >= self.viewport_rows.end { + self.crosshair_row = self.viewport_rows.end.saturating_sub(1); + } + + if self.crosshair_col < self.viewport_cols.start { + self.crosshair_col = self.viewport_cols.start; + } else if self.crosshair_col >= self.viewport_cols.end { + self.crosshair_col = self.viewport_cols.end.saturating_sub(1); + } + + self.cache_dirty = true; + } + /// Get crosshair position relative to current viewport for rendering /// Returns (row_offset, col_offset) within the viewport, or None if outside pub fn get_crosshair_viewport_position(&self) -> Option<(usize, usize)> { diff --git a/sql-cli/src/widgets/mod.rs b/sql-cli/src/widgets/mod.rs index 3ec8ab0a..753727c5 100644 --- a/sql-cli/src/widgets/mod.rs +++ b/sql-cli/src/widgets/mod.rs @@ -11,3 +11,4 @@ pub mod help_widget; pub mod history_widget; pub mod search_modes_widget; pub mod stats_widget; +pub mod tab_bar_widget; diff --git a/sql-cli/src/widgets/tab_bar_widget.rs b/sql-cli/src/widgets/tab_bar_widget.rs new file mode 100644 index 00000000..82668bc0 --- /dev/null +++ b/sql-cli/src/widgets/tab_bar_widget.rs @@ -0,0 +1,99 @@ +use ratatui::{ + layout::Rect, + style::{Color, Modifier, Style}, + text::{Line, Span}, + widgets::{Block, Borders, Tabs}, + Frame, +}; + +/// Renders a tab bar showing all open buffers +pub struct TabBarWidget { + /// Current buffer index + current_index: usize, + /// List of buffer names + buffer_names: Vec, + /// Whether to show Alt+N shortcuts + show_shortcuts: bool, +} + +impl TabBarWidget { + pub fn new(current_index: usize, buffer_names: Vec) -> Self { + Self { + current_index, + buffer_names, + show_shortcuts: true, + } + } + + /// Set whether to show Alt+N shortcuts + pub fn with_shortcuts(mut self, show: bool) -> Self { + self.show_shortcuts = show; + self + } + + /// Render the tab bar + pub fn render(&self, f: &mut Frame, area: Rect) { + // Always render tab bar to maintain consistent layout + // Even with one buffer, showing the tab provides visual consistency + + // Create tab titles with optional shortcuts + let titles: Vec = self + .buffer_names + .iter() + .enumerate() + .map(|(i, name)| { + let mut spans = vec![]; + + // Add shortcut indicator if enabled and within Alt+1-9 range + if self.show_shortcuts && i < 9 { + spans.push(Span::styled( + format!("{}:", i + 1), + Style::default() + .fg(Color::DarkGray) + .add_modifier(Modifier::DIM), + )); + } + + // Truncate long buffer names + let display_name = if name.len() > 20 { + format!("{}...", &name[..17]) + } else { + name.clone() + }; + + spans.push(Span::raw(display_name)); + Line::from(spans) + }) + .collect(); + + let tabs = Tabs::new(titles) + .block( + Block::default() + .borders(Borders::BOTTOM) + .border_style(Style::default().fg(Color::DarkGray)), + ) + .select(self.current_index) + .style(Style::default().fg(Color::Gray)) + .highlight_style( + Style::default() + .fg(Color::White) + .add_modifier(Modifier::BOLD) + .bg(Color::DarkGray), + ) + .divider(Span::styled(" │ ", Style::default().fg(Color::DarkGray))); + + f.render_widget(tabs, area); + } + + /// Calculate the height needed for the tab bar + pub fn height(&self) -> u16 { + // Always reserve space for tab bar to maintain consistent layout + 2 // Tab bar with border + } +} + +/// Helper function to create and render a tab bar in one call +pub fn render_tab_bar(f: &mut Frame, area: Rect, current_index: usize, buffer_names: Vec) { + let widget = TabBarWidget::new(current_index, buffer_names); + widget.render(f, area); +} diff --git a/sql-cli/test_debug_final.sh b/sql-cli/test_debug_final.sh deleted file mode 100644 index 8fbc3f5c..00000000 --- a/sql-cli/test_debug_final.sh +++ /dev/null @@ -1,34 +0,0 @@ -#!/bin/bash -# Test with comprehensive debugging to see exact state values - -echo "🔍 COMPREHENSIVE DEBUG TEST" -echo "Testing with enhanced logging to see exact VimSearchAdapter state..." - -# Create test data -cat > test_debug_final.csv << EOF -name,age,city -Alice,30,New York -Bob,25,London -Charlie,35,Paris -EOF - -echo "" -echo "🎯 RUN THIS COMMAND:" -echo "RUST_LOG=info,sql_cli::ui::enhanced_tui=debug,sql_cli::ui::vim_search_adapter=debug,vim_search=debug ./target/release/sql-cli test_debug_final.csv -e \"select * from data\"" -echo "" - -echo "📝 EXPECTED LOGS AFTER PRESSING N (after search exit):" -echo "- 'Action system: key Char('N') -> action PreviousSearchMatch'" -echo "- '🎯 PreviousSearchMatch: is_active=?, has_pattern=?, pattern=?'" -echo "- Either 'N key -> vim search navigation' OR 'N key -> toggle row numbers'" -echo "- '🎯 ToggleActionHandler: Processing ToggleRowNumbers action' (if working)" -echo "- '🎯 TOGGLE_ROW_NUMBERS CALLED: current=?, will set to=?' (if working)" -echo "" - -echo "🐛 THE ISSUE:" -echo "If you see 'Searching for ...' logs after pressing N (post-search)," -echo "it means VimSearchAdapter still has a pattern stored even after clear!" -echo "" - -# Keep the test file -echo "Test file: test_debug_final.csv" \ No newline at end of file diff --git a/sql-cli/test_escape_fix.sh b/sql-cli/test_escape_fix.sh deleted file mode 100755 index 8c12f1ea..00000000 --- a/sql-cli/test_escape_fix.sh +++ /dev/null @@ -1,28 +0,0 @@ -#!/bin/bash - -echo "=== Testing Simple Escape Fix ===" -echo "" -echo "Creating test data..." - -cat > test_escape.csv << 'EOF' -id,name,type,status -1,Alice,derivatives,active -2,Bob,futures,pending -3,Charlie,derivatives,active -4,David,options,inactive -5,Eve,derivatives,pending -EOF - -echo "" -echo "TEST INSTRUCTIONS:" -echo "1. Press '/' to enter search mode" -echo "2. Type 'derivatives' and press Enter" -echo "3. Press 'n' to navigate forward (should work)" -echo "4. Press 'N' to navigate backward (should work)" -echo "5. Press Escape to clear search" -echo "6. Press 'N' - should toggle line numbers (NOT navigate)" -echo "" -echo "Starting application..." -echo "" - -./target/release/sql-cli test_escape.csv -e "select * from data" \ No newline at end of file diff --git a/sql-cli/test_f5_fix.exp b/sql-cli/test_f5_fix.exp deleted file mode 100755 index 8c903af1..00000000 --- a/sql-cli/test_f5_fix.exp +++ /dev/null @@ -1,33 +0,0 @@ -#!/usr/bin/expect -f - -set timeout 3 -spawn ./target/release/sql-cli test_f5.csv -e "select * from data" - -# Wait for the TUI to start -sleep 0.5 - -# Press F5 to enter debug mode -send "\033\[15~" -sleep 0.5 - -# Should enter debug mode without hanging -expect { - timeout { - puts "FAIL: Timeout waiting for debug mode" - exit 1 - } - eof { - puts "FAIL: Process exited unexpectedly" - exit 1 - } -} - -# Press F5 again to exit debug mode -send "\033\[15~" -sleep 0.5 - -# Press q to quit -send "q" -expect eof - -puts "SUCCESS: F5 debug mode works without hanging" \ No newline at end of file diff --git a/sql-cli/test_final_fix.sh b/sql-cli/test_final_fix.sh deleted file mode 100644 index e67080cb..00000000 --- a/sql-cli/test_final_fix.sh +++ /dev/null @@ -1,28 +0,0 @@ -#!/bin/bash - -echo "Testing final fix for N key toggle after search..." -echo "" -echo "Creating test data..." - -cat > test_final.csv << 'EOF' -id,name,type,status -1,Alice,derivatives,active -2,Bob,futures,pending -3,Charlie,derivatives,active -4,David,options,inactive -5,Eve,derivatives,pending -EOF - -echo "" -echo "=== TEST INSTRUCTIONS ===" -echo "1. App will start with test data" -echo "2. Press '/' to enter search mode" -echo "3. Type 'derivatives' and press Enter" -echo "4. Press 'n' to navigate forward (should work)" -echo "5. Press 'N' to navigate backward (should work)" -echo "6. Press Escape to clear search" -echo "7. Press 'N' - should toggle line numbers (NOT navigate)" -echo "" -echo "Starting app..." - -./target/release/sql-cli test_final.csv -e "select * from data" \ No newline at end of file diff --git a/sql-cli/test_mode_switching.sh b/sql-cli/test_mode_switching.sh deleted file mode 100755 index e1384e88..00000000 --- a/sql-cli/test_mode_switching.sh +++ /dev/null @@ -1,66 +0,0 @@ -#!/bin/bash - -# Test script for jump-to-row and help mode functionality - -echo "Testing jump-to-row and help mode fixes..." - -# Create test file with expect script to test modes -cat > test_modes.exp << 'EOF' -#!/usr/bin/expect -f - -set timeout 10 - -# Start the application -spawn timeout 30 ./target/release/sql-cli test_jump_to_row.csv -e "select * from data" - -# Wait for the application to load -expect "Results" - -# Test F1 help mode -send "F1" -expect "Help Mode" -sleep 1 -send "\033" # Escape key -expect "Results" -puts "✅ F1 help mode test: PASSED" - -# Test G jump-to-row mode -send "G" -expect "Jump to row:" -puts "✅ Jump to row mode entry: PASSED" - -# Test typing a row number -send "5" -expect "5" - -# Test Enter to jump -send "\r" -expect "Results" -puts "✅ Jump to row with Enter: PASSED" - -# Test G and Escape -send "G" -expect "Jump to row:" -send "\033" # Escape key -expect "Results" -puts "✅ Jump to row with Escape: PASSED" - -# Exit application -send "q" -expect eof -puts "✅ All mode switching tests PASSED!" -EOF - -# Make expect script executable -chmod +x test_modes.exp - -# Run the test -if ./test_modes.exp; then - echo "✅ Mode switching functionality working correctly!" - rm -f test_modes.exp - exit 0 -else - echo "❌ Mode switching tests failed" - rm -f test_modes.exp - exit 1 -fi \ No newline at end of file diff --git a/sql-cli/test_n_escape.exp b/sql-cli/test_n_escape.exp deleted file mode 100755 index fbffae54..00000000 --- a/sql-cli/test_n_escape.exp +++ /dev/null @@ -1,85 +0,0 @@ -#!/usr/bin/expect -f -set timeout 5 - -# Start the application -spawn ./target/release/sql-cli test_n_escape.csv -e "select * from data" - -# Wait for initial display -expect { - timeout { puts "TIMEOUT: Initial display"; exit 1 } - "rows*cols" { } -} - -# Enter search mode with / -send "/" -expect { - timeout { puts "TIMEOUT: Entering search mode"; exit 1 } - "Search:" { } -} - -# Type search pattern -send "derivatives" -expect { - timeout { puts "TIMEOUT: Typing search pattern"; exit 1 } - "derivatives" { } -} - -# Apply search with Enter -send "\r" -expect { - timeout { puts "TIMEOUT: Applying search"; exit 1 } - -re "Match.*of" { } -} - -# Navigate with n a few times -send "n" -expect { - timeout { puts "TIMEOUT: First n navigation"; exit 1 } - -re "Match.*of" { } -} - -send "n" -expect { - timeout { puts "TIMEOUT: Second n navigation"; exit 1 } - -re "Match.*of" { } -} - -# Press Escape to clear search -send "\033" -expect { - timeout { puts "TIMEOUT: Escape to clear search"; exit 1 } - "Search cleared" { puts "✓ Search cleared message appeared" } - -re ".*" { } -} - -# Brief pause -sleep 0.5 - -# Now test N key - should toggle line numbers, not navigate -send "N" -expect { - timeout { puts "TIMEOUT: N key after Escape"; exit 1 } - "Toggled line numbers" { puts "✓ SUCCESS: N key toggles line numbers after Escape!" } - -re "Match.*of" { puts "✗ FAIL: N key still navigating search!"; exit 1 } - -re ".*" { - # Check F5 debug to see state - send "\[24~" - expect { - timeout { puts "TIMEOUT: F5 debug"; exit 1 } - -re "pattern='(\[^']*)'.*active=(\[^ ]*)" { - set pattern $expect_out(1,string) - set active $expect_out(2,string) - if {$pattern != ""} { - puts "✗ FAIL: Search pattern still present: '$pattern', active=$active" - exit 1 - } else { - puts "✓ Pattern cleared, checking for line number toggle..." - } - } - } - } -} - -# Exit -send "q" -expect eof diff --git a/sql-cli/test_n_escape_manual.sh b/sql-cli/test_n_escape_manual.sh deleted file mode 100644 index 05e66d84..00000000 --- a/sql-cli/test_n_escape_manual.sh +++ /dev/null @@ -1,35 +0,0 @@ -#!/bin/bash - -# Manual test script for N key toggle fix -echo "=== Manual Test for N Key Toggle After Search ===" -echo "" -echo "Creating test data..." - -cat > test_n_manual.csv << 'EOF' -id,name,type,status -1,Alice,derivatives,active -2,Bob,futures,pending -3,Charlie,derivatives,active -4,David,options,inactive -5,Eve,derivatives,pending -EOF - -echo "Test data created: test_n_manual.csv" -echo "" -echo "INSTRUCTIONS:" -echo "1. Run: RUST_LOG=info ./target/release/sql-cli test_n_manual.csv -e \"select * from data\"" -echo "2. Press '/' to enter search mode" -echo "3. Type 'derivatives' and press Enter" -echo "4. Press 'n' a few times to navigate search results" -echo "5. Press Escape to clear search" -echo "6. Press 'N' - it should toggle line numbers, NOT navigate search" -echo "" -echo "Look for these log messages:" -echo "- 'VimSearchAdapter: ESCAPE pressed with active search - clearing'" -echo "- 'Buffer.set_search_pattern: 'derivatives' -> '''" -echo "- 'Search cleared'" -echo "" -echo "Starting application with logging..." -echo "" - -RUST_LOG=info ./target/release/sql-cli test_n_manual.csv -e "select * from data" 2>&1 | grep -E "VimSearchAdapter|set_search_pattern|Search cleared|Toggled line numbers|KeyPressed" \ No newline at end of file diff --git a/sql-cli/test_n_key_complete.sh b/sql-cli/test_n_key_complete.sh deleted file mode 100755 index 188ca9f1..00000000 --- a/sql-cli/test_n_key_complete.sh +++ /dev/null @@ -1,59 +0,0 @@ -#!/bin/bash -# Final test of the N key toggle fix with comprehensive logging - -set -e - -echo "🎯 FINAL N KEY TOGGLE TEST" - -# Create test data -cat > test_n_final.csv << EOF -name,age,city -Alice,30,New York -Bob,25,London -Charlie,35,Paris -David,40,Tokyo -Eve,28,Berlin -EOF - -echo "✅ Created test data" - -echo "" -echo "🔍 RUN THIS COMMAND TO TEST WITH FULL DEBUG LOGGING:" -echo "" -echo "RUST_LOG=info,sql_cli::ui::vim_search_adapter=debug,sql_cli::ui::enhanced_tui=debug ./target/release/sql-cli test_n_final.csv -e \"select * from data\"" -echo "" - -echo "📋 TEST SEQUENCE:" -echo "1. Press 'N' → should toggle line numbers ✅" -echo " Look for: 'N key -> toggle row numbers (search inactive)'" -echo "" -echo "2. Press '/' → enter search mode" -echo " Look for: 'VimSearchAdapter: Activating for vim search'" -echo " Look for: 'Manually notified VimSearchAdapter of SearchStarted event'" -echo "" -echo "3. Type 'Alice' and press Enter" -echo " Look for: search results appearing" -echo "" -echo "4. Press Escape → exit search mode" -echo " Look for: 'VimSearchAdapter: Search ended, clearing'" -echo " Look for: 'Manually notified VimSearchAdapter of SearchEnded event'" -echo "" -echo "5. Press 'N' → should toggle line numbers (NOT search navigation)" -echo " Look for: 'PreviousSearchMatch: is_active=false, has_pattern=false'" -echo " Look for: 'N key -> toggle row numbers (search inactive)'" -echo "" - -echo "🐛 WHAT THE DEBUG LOGS WILL TELL US:" -echo "- If StateDispatcher events are being sent correctly" -echo "- If VimSearchAdapter is receiving and processing the events" -echo "- If is_active and get_pattern are returning the expected values" -echo "- Which branch the PreviousSearchMatch action takes" -echo "" - -echo "✅ SUCCESS CRITERIA:" -echo "After step 5, you should see 'N key -> toggle row numbers (search inactive)'" -echo "NOT 'N key -> vim search navigation'" -echo "" - -# Don't clean up - keep file for testing -echo "Test file: test_n_final.csv (not cleaning up for manual testing)" \ No newline at end of file diff --git a/sql-cli/test_n_key_debug.sh b/sql-cli/test_n_key_debug.sh deleted file mode 100644 index 86660813..00000000 --- a/sql-cli/test_n_key_debug.sh +++ /dev/null @@ -1,45 +0,0 @@ -#!/bin/bash -# Test the N key toggle fix with detailed debug logging - -set -e - -echo "Testing N key toggle fix with debug logging..." - -# Create test data -cat > test_n_debug.csv << EOF -name,age,city -Alice,30,New York -Bob,25,London -Charlie,35,Paris -David,40,Tokyo -Eve,28,Berlin -Frank,33,Rome -Grace,27,Madrid -Henry,45,Dublin -EOF - -echo "✅ Created test data with 8 rows" - -# Build the project to ensure latest changes -cargo build --release - -echo "" -echo "🎯 TESTING WITH DEBUG LOGS:" -echo "Run this command to see detailed logs:" -echo "RUST_LOG=info,sql_cli::ui::vim_search_adapter=debug,sql_cli::state=debug ./target/release/sql-cli test_n_debug.csv -e \"select * from data\"" -echo "" -echo "Test sequence:" -echo "1. Press 'N' → should toggle line numbers" -echo "2. Press '/' → enter search mode" -echo "3. Type 'Alice'" -echo "4. Press Escape → exit search mode (look for StateDispatcher event in logs)" -echo "5. Press 'N' → should toggle line numbers (NOT navigate search)" -echo "" -echo "Look for these log entries:" -echo "- 'VimSearchAdapter: should_handle_key? mode=Results, pattern='', active=false'" -echo "- 'StateDispatcher dispatching event: SearchEnded'" -echo "- 'VimSearchAdapter: Search ended, clearing'" -echo "" - -# Don't clean up - keep file for testing -echo "Test file: test_n_debug.csv (not cleaning up for manual testing)" \ No newline at end of file diff --git a/sql-cli/test_n_key_escape_fix.sh b/sql-cli/test_n_key_escape_fix.sh deleted file mode 100755 index 7e5b5288..00000000 --- a/sql-cli/test_n_key_escape_fix.sh +++ /dev/null @@ -1,115 +0,0 @@ -#!/bin/bash - -# Test script to verify N key toggle fix after search -# Bug: After entering search mode (/) and exiting, N continues to navigate search instead of toggling line numbers - -echo "Testing N key toggle after search mode..." -echo "" - -# Create test data -cat > test_n_escape.csv << 'EOF' -id,name,type,status -1,Alice,derivatives,active -2,Bob,futures,pending -3,Charlie,derivatives,active -4,David,options,inactive -5,Eve,derivatives,pending -EOF - -# Create expect script to test the interaction -cat > test_n_escape.exp << 'EOF' -#!/usr/bin/expect -f -set timeout 5 - -# Start the application -spawn ./target/release/sql-cli test_n_escape.csv -e "select * from data" - -# Wait for initial display -expect { - timeout { puts "TIMEOUT: Initial display"; exit 1 } - "rows*cols" { } -} - -# Enter search mode with / -send "/" -expect { - timeout { puts "TIMEOUT: Entering search mode"; exit 1 } - "Search:" { } -} - -# Type search pattern -send "derivatives" -expect { - timeout { puts "TIMEOUT: Typing search pattern"; exit 1 } - "derivatives" { } -} - -# Apply search with Enter -send "\r" -expect { - timeout { puts "TIMEOUT: Applying search"; exit 1 } - -re "Match.*of" { } -} - -# Navigate with n a few times -send "n" -expect { - timeout { puts "TIMEOUT: First n navigation"; exit 1 } - -re "Match.*of" { } -} - -send "n" -expect { - timeout { puts "TIMEOUT: Second n navigation"; exit 1 } - -re "Match.*of" { } -} - -# Press Escape to clear search -send "\033" -expect { - timeout { puts "TIMEOUT: Escape to clear search"; exit 1 } - "Search cleared" { puts "✓ Search cleared message appeared" } - -re ".*" { } -} - -# Brief pause -sleep 0.5 - -# Now test N key - should toggle line numbers, not navigate -send "N" -expect { - timeout { puts "TIMEOUT: N key after Escape"; exit 1 } - "Toggled line numbers" { puts "✓ SUCCESS: N key toggles line numbers after Escape!" } - -re "Match.*of" { puts "✗ FAIL: N key still navigating search!"; exit 1 } - -re ".*" { - # Check F5 debug to see state - send "\[24~" - expect { - timeout { puts "TIMEOUT: F5 debug"; exit 1 } - -re "pattern='(\[^']*)'.*active=(\[^ ]*)" { - set pattern $expect_out(1,string) - set active $expect_out(2,string) - if {$pattern != ""} { - puts "✗ FAIL: Search pattern still present: '$pattern', active=$active" - exit 1 - } else { - puts "✓ Pattern cleared, checking for line number toggle..." - } - } - } - } -} - -# Exit -send "q" -expect eof -EOF - -chmod +x test_n_escape.exp - -# Run the test -echo "Running test..." -RUST_LOG=info ./test_n_escape.exp 2>&1 | grep -E "✓|✗|FAIL|SUCCESS|TIMEOUT|VimSearchAdapter.*ESCAPE|Buffer.set_search_pattern|Search cleared" - -echo "" -echo "Test complete. Check output above for results." \ No newline at end of file diff --git a/sql-cli/test_n_key_fix.sh b/sql-cli/test_n_key_fix.sh deleted file mode 100755 index 20b0902d..00000000 --- a/sql-cli/test_n_key_fix.sh +++ /dev/null @@ -1,50 +0,0 @@ -#!/bin/bash -# Test the N key toggle fix after search mode - -set -e - -echo "Testing N key toggle fix after search mode..." - -# Create test data -cat > test_n_key.csv << EOF -name,age,city -Alice,30,New York -Bob,25,London -Charlie,35,Paris -David,40,Tokyo -Eve,28,Berlin -EOF - -echo "✅ Created test data" - -# Build the project -echo "Building project..." -cargo build --release - -if [ $? -ne 0 ]; then - echo "❌ Build failed" - exit 1 -fi - -echo "✅ Build successful" - -echo "" -echo "🎯 MANUAL TEST STEPS:" -echo "1. Run: ./target/release/sql-cli test_n_key.csv -e \"select * from data\"" -echo "2. Press 'N' (should toggle line numbers) ✅" -echo "3. Press '/' to enter search mode" -echo "4. Type 'Alice' or any search term" -echo "5. Press Escape to exit search mode" -echo "6. Press 'N' again (should toggle line numbers, NOT search navigation) ✅" -echo "" -echo "If step 6 toggles line numbers instead of doing search navigation," -echo "then the fix is working correctly!" -echo "" -echo "🐛 Before fix: N would navigate to 'next search match' even after Escape" -echo "✅ After fix: N should toggle line numbers after Escape" -echo "" - -# Clean up -rm -f test_n_key.csv - -echo "Ready to test manually!" \ No newline at end of file diff --git a/sql-cli/test_new_architecture.sh b/sql-cli/test_new_architecture.sh deleted file mode 100755 index 7355adad..00000000 --- a/sql-cli/test_new_architecture.sh +++ /dev/null @@ -1,21 +0,0 @@ -#!/bin/bash - -# Test the new architecture with DataLoaderService - -echo "Testing new architecture..." - -# Create a test CSV file -cat > test_architecture.csv << EOF -id,name,value -1,Alice,100 -2,Bob,200 -3,Charlie,300 -EOF - -# Run the application -timeout 2 ./target/release/sql-cli test_architecture.csv -e "select * from data" --classic 2>&1 | head -20 - -# Clean up -rm -f test_architecture.csv - -echo "Test completed" \ No newline at end of file diff --git a/sql-cli/tests/test_buffer_state_refactor.rs b/sql-cli/tests/test_buffer_state_refactor.rs new file mode 100644 index 00000000..dfd442f3 --- /dev/null +++ b/sql-cli/tests/test_buffer_state_refactor.rs @@ -0,0 +1,200 @@ +/// Test for Phase 1 of Buffer State Refactor +/// Verifies that ViewState is preserved when switching between buffers +/// using the new proxy-based architecture +use sql_cli::app_state_container::AppStateContainer; +use sql_cli::buffer::{Buffer, BufferAPI, BufferManager, SelectionMode}; + +#[test] +fn test_buffer_state_preserved_on_switch() { + // Create a BufferManager with two buffers + let mut buffer_manager = BufferManager::new(); + + // Add first buffer + let mut buffer1 = Buffer::new(1); + buffer1.name = "Buffer 1".to_string(); + buffer_manager.add_buffer(buffer1); + + // Add second buffer + let mut buffer2 = Buffer::new(2); + buffer2.name = "Buffer 2".to_string(); + buffer_manager.add_buffer(buffer2); + + // Create AppStateContainer + let mut state = AppStateContainer::new(buffer_manager).unwrap(); + + // Switch to first buffer + state.buffers_mut().switch_to(0); + + // Set some state in buffer 1 using proxies + { + let mut nav_proxy = state.navigation_proxy_mut(); + nav_proxy.set_selected_row(10); + nav_proxy.set_selected_column(5); + nav_proxy.set_scroll_offset((20, 3)); + nav_proxy.set_viewport_lock(true); + } + + { + let mut sel_proxy = state.selection_proxy_mut(); + sel_proxy.set_mode(SelectionMode::Cell); + sel_proxy.add_selected_cell((10, 5)); + sel_proxy.set_selection_anchor(Some((10, 5))); + } + + // Switch to buffer 2 + state.buffers_mut().switch_to(1); + + // Set different state in buffer 2 + { + let mut nav_proxy = state.navigation_proxy_mut(); + nav_proxy.set_selected_row(25); + nav_proxy.set_selected_column(8); + nav_proxy.set_scroll_offset((50, 10)); + nav_proxy.set_viewport_lock(false); + } + + { + let mut sel_proxy = state.selection_proxy_mut(); + sel_proxy.set_mode(SelectionMode::Row); + sel_proxy.clear_selections(); + } + + // Switch back to buffer 1 + state.buffers_mut().switch_to(0); + + // Verify buffer 1's state was preserved + { + let nav_proxy = state.navigation_proxy(); + assert_eq!( + nav_proxy.selected_row(), + 10, + "Buffer 1 row should be preserved" + ); + assert_eq!( + nav_proxy.selected_column(), + 5, + "Buffer 1 column should be preserved" + ); + assert_eq!( + nav_proxy.scroll_offset(), + (20, 3), + "Buffer 1 scroll offset should be preserved" + ); + assert_eq!( + nav_proxy.viewport_lock(), + true, + "Buffer 1 viewport lock should be preserved" + ); + } + + { + let sel_proxy = state.selection_proxy(); + assert_eq!( + sel_proxy.mode(), + SelectionMode::Cell, + "Buffer 1 selection mode should be preserved" + ); + assert_eq!( + sel_proxy.selected_cells(), + vec![(10, 5)], + "Buffer 1 selected cells should be preserved" + ); + assert_eq!( + sel_proxy.selection_anchor(), + Some((10, 5)), + "Buffer 1 selection anchor should be preserved" + ); + } + + // Switch to buffer 2 again + state.buffers_mut().switch_to(1); + + // Verify buffer 2's state was preserved + { + let nav_proxy = state.navigation_proxy(); + assert_eq!( + nav_proxy.selected_row(), + 25, + "Buffer 2 row should be preserved" + ); + assert_eq!( + nav_proxy.selected_column(), + 8, + "Buffer 2 column should be preserved" + ); + assert_eq!( + nav_proxy.scroll_offset(), + (50, 10), + "Buffer 2 scroll offset should be preserved" + ); + assert_eq!( + nav_proxy.viewport_lock(), + false, + "Buffer 2 viewport lock should be preserved" + ); + } + + { + let sel_proxy = state.selection_proxy(); + assert_eq!( + sel_proxy.mode(), + SelectionMode::Row, + "Buffer 2 selection mode should be preserved" + ); + assert_eq!( + sel_proxy.selected_cells().len(), + 0, + "Buffer 2 should have no selected cells" + ); + assert_eq!( + sel_proxy.selection_anchor(), + None, + "Buffer 2 should have no selection anchor" + ); + } +} + +#[test] +fn test_proxy_with_no_buffer() { + // Create empty BufferManager + let buffer_manager = BufferManager::new(); + let state = AppStateContainer::new(buffer_manager).unwrap(); + + // Test that proxies return defaults when no buffer exists + let nav_proxy = state.navigation_proxy(); + assert_eq!(nav_proxy.selected_row(), 0); + assert_eq!(nav_proxy.selected_column(), 0); + assert_eq!(nav_proxy.scroll_offset(), (0, 0)); + assert_eq!(nav_proxy.viewport_lock(), false); + + let sel_proxy = state.selection_proxy(); + assert_eq!(sel_proxy.mode(), SelectionMode::Row); + assert_eq!(sel_proxy.selected_cells().len(), 0); + assert_eq!(sel_proxy.selection_anchor(), None); +} + +#[test] +fn test_direct_buffer_viewstate_access() { + // Test that we can also access ViewState directly from Buffer + let mut buffer = Buffer::new(1); + + // Modify ViewState directly + buffer.view_state.crosshair_row = 15; + buffer.view_state.crosshair_col = 7; + buffer.view_state.selection_mode = SelectionMode::Column; + buffer.view_state.viewport_lock = true; + + // Verify the changes + assert_eq!(buffer.view_state.crosshair_row, 15); + assert_eq!(buffer.view_state.crosshair_col, 7); + assert_eq!(buffer.view_state.selection_mode, SelectionMode::Column); + assert_eq!(buffer.view_state.viewport_lock, true); + + // Use the proper API method to set selected row (which syncs both ViewState and table_state) + buffer.set_selected_row(Some(15)); + + // Now verify through BufferAPI methods + assert_eq!(buffer.get_selected_row(), Some(15)); + assert_eq!(buffer.get_current_column(), 7); + assert_eq!(buffer.is_viewport_lock(), true); +} diff --git a/sql-cli/tests/test_viewport_buffer_sync.rs b/sql-cli/tests/test_viewport_buffer_sync.rs new file mode 100644 index 00000000..ccda25ef --- /dev/null +++ b/sql-cli/tests/test_viewport_buffer_sync.rs @@ -0,0 +1,146 @@ +/// Test that ViewportManager is correctly synced with the current buffer +/// when loading multiple files +use sql_cli::buffer::{Buffer, BufferAPI, BufferManager}; +use sql_cli::data::data_view::DataView; +use sql_cli::data::datatable::{DataColumn, DataRow, DataTable, DataValue}; +use std::sync::Arc; + +#[test] +fn test_viewport_sync_after_multi_file_load() { + // Create mock DataTables with distinct data + let mut datatable1 = DataTable::new("table1"); + datatable1.add_column(DataColumn::new("id")); + datatable1.add_column(DataColumn::new("name")); + datatable1 + .add_row(DataRow::new(vec![ + DataValue::String("1".to_string()), + DataValue::String("first_buffer".to_string()), + ])) + .unwrap(); + datatable1 + .add_row(DataRow::new(vec![ + DataValue::String("2".to_string()), + DataValue::String("first_buffer".to_string()), + ])) + .unwrap(); + let dataview1 = DataView::new(Arc::new(datatable1)); + + let mut datatable2 = DataTable::new("table2"); + datatable2.add_column(DataColumn::new("code")); + datatable2.add_column(DataColumn::new("desc")); + datatable2 + .add_row(DataRow::new(vec![ + DataValue::String("A".to_string()), + DataValue::String("second_buffer".to_string()), + ])) + .unwrap(); + datatable2 + .add_row(DataRow::new(vec![ + DataValue::String("B".to_string()), + DataValue::String("second_buffer".to_string()), + ])) + .unwrap(); + let dataview2 = DataView::new(Arc::new(datatable2)); + + // Create a BufferManager and add buffers + let mut buffer_manager = BufferManager::new(); + + // Create first buffer with dataview1 + let mut buffer1 = Buffer::new(1); + buffer1.set_dataview(Some(dataview1.clone())); + buffer1.set_name("file1.csv".to_string()); + buffer_manager.add_buffer(buffer1); + + // Create second buffer with dataview2 + let mut buffer2 = Buffer::new(2); + buffer2.set_dataview(Some(dataview2.clone())); + buffer2.set_name("file2.csv".to_string()); + buffer_manager.add_buffer(buffer2); + + // Switch to buffer 2 (simulating what happens when adding files) + buffer_manager.switch_to(1); + assert_eq!(buffer_manager.current_index(), 1, "Should be on buffer 2"); + + // Switch back to buffer 1 (simulating what run_enhanced_tui_multi does) + buffer_manager.switch_to(0); + assert_eq!( + buffer_manager.current_index(), + 0, + "Should be on buffer 1 after switch" + ); + + // Verify the current buffer has the correct dataview + let current_buffer = buffer_manager.current().unwrap(); + let buffer_dataview = current_buffer.get_dataview().unwrap(); + + assert_eq!( + buffer_dataview.column_names(), + vec!["id", "name"], + "Buffer 1 should have first dataview headers" + ); + + assert_eq!( + buffer_dataview.row_count(), + 2, + "Buffer 1 should have 2 rows" + ); +} + +#[test] +fn test_buffer_switching_preserves_dataview() { + // Create two DataTables with different data + let mut datatable1 = DataTable::new("table1"); + datatable1.add_column(DataColumn::new("col1")); + datatable1 + .add_row(DataRow::new(vec![DataValue::String( + "buffer1_data".to_string(), + )])) + .unwrap(); + let dataview1 = DataView::new(Arc::new(datatable1)); + + let mut datatable2 = DataTable::new("table2"); + datatable2.add_column(DataColumn::new("col2")); + datatable2 + .add_row(DataRow::new(vec![DataValue::String( + "buffer2_data".to_string(), + )])) + .unwrap(); + let dataview2 = DataView::new(Arc::new(datatable2)); + + // Create BufferManager + let mut buffer_manager = BufferManager::new(); + + // Add both buffers + let mut buffer1 = Buffer::new(1); + buffer1.set_dataview(Some(dataview1.clone())); + buffer1.set_name("test1.csv".to_string()); + buffer_manager.add_buffer(buffer1); + + let mut buffer2 = Buffer::new(2); + buffer2.set_dataview(Some(dataview2.clone())); + buffer2.set_name("test2.csv".to_string()); + buffer_manager.add_buffer(buffer2); + + // Switch between buffers multiple times and verify data is preserved + for _ in 0..3 { + // Switch to buffer 1 + buffer_manager.switch_to(0); + let current = buffer_manager.current().unwrap(); + let dataview = current.get_dataview().unwrap(); + assert_eq!( + dataview.column_names(), + vec!["col1"], + "Buffer 1 should have col1 header" + ); + + // Switch to buffer 2 + buffer_manager.switch_to(1); + let current = buffer_manager.current().unwrap(); + let dataview = current.get_dataview().unwrap(); + assert_eq!( + dataview.column_names(), + vec!["col2"], + "Buffer 2 should have col2 header" + ); + } +}