From 27691ffcb94306b15b24a47c9043aec33420e2b3 Mon Sep 17 00:00:00 2001 From: Shen Bowen Date: Tue, 19 May 2026 10:41:00 +0800 Subject: [PATCH 1/3] refactor: extract file-tree key handling into key_actions module --- crates/tui/src/tui/key_actions.rs | 56 +++++++++++++++++++++++++++++++ crates/tui/src/tui/mod.rs | 1 + crates/tui/src/tui/ui.rs | 46 +++---------------------- 3 files changed, 62 insertions(+), 41 deletions(-) create mode 100644 crates/tui/src/tui/key_actions.rs diff --git a/crates/tui/src/tui/key_actions.rs b/crates/tui/src/tui/key_actions.rs new file mode 100644 index 000000000..995c7f603 --- /dev/null +++ b/crates/tui/src/tui/key_actions.rs @@ -0,0 +1,56 @@ +//! Keyboard event action handlers extracted from `ui.rs`. +//! +//! Each function handles a focused subset of keyboard input so the +//! main event loop stays lean. Functions that need to signal a +//! control-flow change (shutdown, return to caller) communicate via +//! [`KeyActionResult`]. + +use crossterm::event::{KeyCode, KeyEvent}; + +use super::app::App; + +// ── File-tree key handling ─────────────────────────────────────── + +/// Handle keyboard input when the file-tree pane is visible. +/// +/// Returns `true` when the key was consumed (caller should `continue`). +pub fn handle_file_tree_key(app: &mut App, key: &KeyEvent) -> bool { + if app.file_tree.is_none() { + return false; + } + match key.code { + KeyCode::Up => { + if let Some(state) = app.file_tree.as_mut() { + state.cursor_up(); + } + app.needs_redraw = true; + true + } + KeyCode::Down => { + if let Some(state) = app.file_tree.as_mut() { + state.cursor_down(); + } + app.needs_redraw = true; + true + } + KeyCode::Enter => { + if let Some(state) = app.file_tree.as_mut() { + if let Some(rel_path) = state.activate() { + let path_str = rel_path.to_string_lossy().to_string(); + app.status_message = Some(format!("Attached @{path_str}")); + app.insert_str(&format!("@{} ", path_str)); + } else { + app.needs_redraw = true; + } + } + true + } + KeyCode::Esc => { + app.file_tree = None; + app.status_message = Some("File tree closed".to_string()); + app.needs_redraw = true; + true + } + _ => false, + } +} diff --git a/crates/tui/src/tui/mod.rs b/crates/tui/src/tui/mod.rs index 34b70ee27..bfb2e477f 100644 --- a/crates/tui/src/tui/mod.rs +++ b/crates/tui/src/tui/mod.rs @@ -35,6 +35,7 @@ pub mod footer_ui; pub mod format_helpers; pub mod frame_rate_limiter; pub mod history; +pub mod key_actions; pub mod key_shortcuts; pub mod keybindings; pub mod live_transcript; diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 1444f1341..b3857875d 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -103,6 +103,8 @@ use crate::tui::views::subagent_view_agents; use crate::tui::vim_mode; use crate::tui::workspace_context; +use super::key_actions; + use super::app::{ App, AppAction, AppMode, OnboardingState, QueuedMessage, ReasoningEffort, SidebarFocus, StatusToastLevel, SubmitDisposition, TaskPanelEntry, TuiOptions, @@ -2676,47 +2678,9 @@ async fn run_event_loop( continue; } - // File-tree navigation: intercept keys when the file-tree pane is - // visible so Up/Down/Enter/Esc operate on the tree rather than - // falling through to composer or modal handlers. - if app.file_tree_visible { - match key.code { - KeyCode::Up => { - if let Some(state) = app.file_tree.as_mut() { - state.cursor_up(); - } - app.needs_redraw = true; - continue; - } - KeyCode::Down => { - if let Some(state) = app.file_tree.as_mut() { - state.cursor_down(); - } - app.needs_redraw = true; - continue; - } - KeyCode::Enter => { - if let Some(state) = app.file_tree.as_mut() { - if let Some(rel_path) = state.activate() { - // Insert @path into the composer. - let path_str = rel_path.to_string_lossy().to_string(); - app.status_message = Some(format!("Attached @{path_str}")); - app.insert_str(&format!("@{path_str} ")); - } else { - // Directory was expanded/collapsed; rebuild. - app.needs_redraw = true; - } - } - continue; - } - KeyCode::Esc => { - app.file_tree = None; - app.status_message = Some("File tree closed".to_string()); - app.needs_redraw = true; - continue; - } - _ => {} - } + // File-tree navigation: delegated to key_actions module. + if key_actions::handle_file_tree_key(app, &key) { + continue; } if app.is_history_search_active() { From 41ed4e0461a22f1a5dde00ecd95303896c37fdc0 Mon Sep 17 00:00:00 2001 From: Shen Bowen Date: Tue, 19 May 2026 11:30:23 +0800 Subject: [PATCH 2/3] refactor: simplify file_tree_key with early Esc return and let-else binding --- crates/tui/src/tui/key_actions.rs | 41 ++++++++++++++----------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/crates/tui/src/tui/key_actions.rs b/crates/tui/src/tui/key_actions.rs index 995c7f603..2d212bdbc 100644 --- a/crates/tui/src/tui/key_actions.rs +++ b/crates/tui/src/tui/key_actions.rs @@ -15,42 +15,39 @@ use super::app::App; /// /// Returns `true` when the key was consumed (caller should `continue`). pub fn handle_file_tree_key(app: &mut App, key: &KeyEvent) -> bool { - if app.file_tree.is_none() { - return false; + // Esc closes the tree even when entries are still loading. + if key.code == KeyCode::Esc && app.file_tree.is_some() { + app.file_tree = None; + app.status_message = Some("File tree closed".to_string()); + app.needs_redraw = true; + return true; } + + let Some(file_tree) = app.file_tree.as_mut() else { + return false; + }; + match key.code { KeyCode::Up => { - if let Some(state) = app.file_tree.as_mut() { - state.cursor_up(); - } + file_tree.cursor_up(); app.needs_redraw = true; true } KeyCode::Down => { - if let Some(state) = app.file_tree.as_mut() { - state.cursor_down(); - } + file_tree.cursor_down(); app.needs_redraw = true; true } KeyCode::Enter => { - if let Some(state) = app.file_tree.as_mut() { - if let Some(rel_path) = state.activate() { - let path_str = rel_path.to_string_lossy().to_string(); - app.status_message = Some(format!("Attached @{path_str}")); - app.insert_str(&format!("@{} ", path_str)); - } else { - app.needs_redraw = true; - } + if let Some(rel_path) = file_tree.activate() { + let path_str = rel_path.to_string_lossy().to_string(); + app.status_message = Some(format!("Attached @{path_str}")); + app.insert_str(&format!("@{} ", path_str)); + } else { + app.needs_redraw = true; } true } - KeyCode::Esc => { - app.file_tree = None; - app.status_message = Some("File tree closed".to_string()); - app.needs_redraw = true; - true - } _ => false, } } From c76eca38c57309bdb9fcb61a651ea68519735ac0 Mon Sep 17 00:00:00 2001 From: Shen Bowen Date: Mon, 25 May 2026 09:38:43 +0800 Subject: [PATCH 3/3] fix: address review comments on file-tree key_actions refactor Restore file_tree_visible guard, remove stale KeyActionResult docs, unify format string style. --- crates/tui/src/tui/key_actions.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/tui/src/tui/key_actions.rs b/crates/tui/src/tui/key_actions.rs index 2d212bdbc..ad815031c 100644 --- a/crates/tui/src/tui/key_actions.rs +++ b/crates/tui/src/tui/key_actions.rs @@ -1,9 +1,7 @@ //! Keyboard event action handlers extracted from `ui.rs`. //! //! Each function handles a focused subset of keyboard input so the -//! main event loop stays lean. Functions that need to signal a -//! control-flow change (shutdown, return to caller) communicate via -//! [`KeyActionResult`]. +//! main event loop stays lean. use crossterm::event::{KeyCode, KeyEvent}; @@ -15,6 +13,11 @@ use super::app::App; /// /// Returns `true` when the key was consumed (caller should `continue`). pub fn handle_file_tree_key(app: &mut App, key: &KeyEvent) -> bool { + // Guard: do not intercept keys when the file-tree pane is not visible. + if !app.file_tree_visible { + return false; + } + // Esc closes the tree even when entries are still loading. if key.code == KeyCode::Esc && app.file_tree.is_some() { app.file_tree = None; @@ -42,7 +45,7 @@ pub fn handle_file_tree_key(app: &mut App, key: &KeyEvent) -> bool { if let Some(rel_path) = file_tree.activate() { let path_str = rel_path.to_string_lossy().to_string(); app.status_message = Some(format!("Attached @{path_str}")); - app.insert_str(&format!("@{} ", path_str)); + app.insert_str(&format!("@{path_str} ")); } else { app.needs_redraw = true; }