feat: Add StateCoordinator for centralized state management #16
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…mode cleanup - Added cancel_search and cancel_search_with_refs methods to StateCoordinator - These methods properly synchronize all state when exiting search mode - Updated search cancellation in TUI to use StateCoordinator - Ensures AppStateContainer, Buffer, and ShadowState stay synchronized - Fixes issue where Escape during search didn't properly restore state This continues the StateCoordinator migration, centralizing complex state synchronization logic and reducing coupling in the TUI. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
…lures - Implemented Default trait for AppStateContainer for testing purposes - Uses CommandHistory::default() which handles file system errors gracefully - Updated test_proxy_with_no_buffer to use AppStateContainer::default() - This avoids file system access issues in test environments - All 3 tests in test_buffer_state_refactor now pass 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Two critical fixes for vim search mode synchronization: 1. When search is applied (Enter pressed), use sync_mode instead of direct set_mode to ensure all state containers stay synchronized 2. When Escape is pressed during vim search navigation: - Properly detect if vim search is navigating - Exit vim navigation mode - Clear search patterns - Use StateCoordinator to sync mode back to Results This fixes the issue where after search + navigation (/, text, Enter, n, n, Escape), the system would be in a confused state with Command mode active but Results mode displayed, causing keypresses to append to SQL instead of navigate. The root cause was that mode changes weren't going through StateCoordinator's sync_mode method, causing AppStateContainer, Buffer, and ShadowState to become desynchronized. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Major improvements to search state centralization: 1. Enhanced cancel_search_with_refs to handle ALL search cleanup: - Clears vim search adapter state - Clears search patterns - Syncs mode properly - Single point of control for search cancellation 2. Added should_handle_search_navigation method: - Centralizes logic for determining if n/N keys should navigate search - Prevents n/N from navigating when search is cleared - Fixes issue where N wouldn't toggle line numbers after search cancel 3. Updated TUI to delegate to StateCoordinator: - Search cancellation now fully handled by StateCoordinator - n/N key behavior determined by StateCoordinator - Reduces coupling and improves consistency This fixes the long-standing issue where after canceling search with Escape, the N key wouldn't revert to toggling line numbers. The problem was that search state wasn't being fully cleared and checks were scattered across multiple components. Future work: Refactor StateCoordinator to own all state rather than using static delegation methods. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
- Fix Escape key not properly clearing search state after search completion - VimSearchAdapter was intercepting Escape and not clearing AppStateContainer - Now Escape falls through to handle_results_input for proper StateCoordinator handling - Added complete_search_with_refs() to keep matches active for n/N after Enter - Added cancel_search_with_refs() to completely clear search on Escape - Split should_handle_next_match/previous_match for separate n/N behavior - Enhanced logging to track Escape handling and state transitions After search (Enter): n/N navigate matches After Escape: search fully cleared, N toggles line numbers 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Key Changes
New StateCoordinator struct (
src/ui/state_coordinator.rs)Fixed Escape key handling
Search state management
complete_search_with_refs()- keeps matches active for n/N navigation after Entercancel_search_with_refs()- completely clears search on EscapeTesting
Verified the following flow works correctly:
/to start vim searchnnavigates to next match,Nnavigates to previous matchndoes nothing,Ntoggles line numbers (correct behavior)Next Steps
This PR sets up the foundation for further refactoring:
🤖 Generated with Claude Code