From 16d16a4ddc10710b47cb45ae54ddc86de7b18633 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Thu, 21 Aug 2025 10:36:58 -0700 Subject: [PATCH] refactor: move slash command handling into chatwidget (#2536) no functional change, just moving the code that handles /foo into chatwidget, since most commands just do things with chatwidget. --- codex-rs/tui/src/app.rs | 121 ++---------------- codex-rs/tui/src/app_event.rs | 8 +- codex-rs/tui/src/bottom_pane/chat_composer.rs | 53 +++----- codex-rs/tui/src/chatwidget.rs | 101 +++++++++++++++ codex-rs/tui/src/session_log.rs | 6 +- 5 files changed, 137 insertions(+), 152 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 2585c6c2..971d1d16 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -2,15 +2,11 @@ use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::chatwidget::ChatWidget; use crate::file_search::FileSearchManager; -use crate::get_git_diff::get_git_diff; -use crate::slash_command::SlashCommand; use crate::transcript_app::run_transcript_app; use crate::tui; use crate::tui::TuiEvent; use codex_core::ConversationManager; use codex_core::config::Config; -use codex_core::protocol::Event; -use codex_core::protocol::Op; use codex_core::protocol::TokenUsage; use color_eyre::eyre::Result; use crossterm::event::KeyCode; @@ -140,6 +136,18 @@ impl App { fn handle_event(&mut self, tui: &mut tui::Tui, event: AppEvent) -> Result { match event { + AppEvent::NewSession => { + self.chat_widget = ChatWidget::new( + self.config.clone(), + self.server.clone(), + tui.frame_requester(), + self.app_event_tx.clone(), + None, + Vec::new(), + self.enhanced_keys_supported, + ); + tui.frame_requester().schedule_frame(); + } AppEvent::InsertHistoryLines(lines) => { self.transcript_lines.extend(lines.clone()); tui.insert_history_lines(lines); @@ -183,111 +191,6 @@ impl App { AppEvent::DiffResult(text) => { self.chat_widget.add_diff_output(text); } - AppEvent::DispatchCommand(command) => match command { - SlashCommand::New => { - // User accepted – switch to chat view. - let new_widget = ChatWidget::new( - self.config.clone(), - self.server.clone(), - tui.frame_requester(), - self.app_event_tx.clone(), - None, - Vec::new(), - self.enhanced_keys_supported, - ); - self.chat_widget = new_widget; - tui.frame_requester().schedule_frame(); - } - SlashCommand::Init => { - // Guard: do not run if a task is active. - const INIT_PROMPT: &str = include_str!("../prompt_for_init_command.md"); - self.chat_widget - .submit_text_message(INIT_PROMPT.to_string()); - } - SlashCommand::Compact => { - self.chat_widget.clear_token_usage(); - self.app_event_tx.send(AppEvent::CodexOp(Op::Compact)); - } - SlashCommand::Model => { - self.chat_widget.open_model_popup(); - } - SlashCommand::Approvals => { - self.chat_widget.open_approvals_popup(); - } - SlashCommand::Quit => { - return Ok(false); - } - SlashCommand::Logout => { - if let Err(e) = codex_login::logout(&self.config.codex_home) { - tracing::error!("failed to logout: {e}"); - } - return Ok(false); - } - SlashCommand::Diff => { - self.chat_widget.add_diff_in_progress(); - let tx = self.app_event_tx.clone(); - tokio::spawn(async move { - let text = match get_git_diff().await { - Ok((is_git_repo, diff_text)) => { - if is_git_repo { - diff_text - } else { - "`/diff` — _not inside a git repository_".to_string() - } - } - Err(e) => format!("Failed to compute diff: {e}"), - }; - tx.send(AppEvent::DiffResult(text)); - }); - } - SlashCommand::Mention => { - self.chat_widget.insert_str("@"); - } - SlashCommand::Status => { - self.chat_widget.add_status_output(); - } - SlashCommand::Mcp => { - self.chat_widget.add_mcp_output(); - } - #[cfg(debug_assertions)] - SlashCommand::TestApproval => { - use codex_core::protocol::EventMsg; - use std::collections::HashMap; - - use codex_core::protocol::ApplyPatchApprovalRequestEvent; - use codex_core::protocol::FileChange; - - self.app_event_tx.send(AppEvent::CodexEvent(Event { - id: "1".to_string(), - // msg: EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { - // call_id: "1".to_string(), - // command: vec!["git".into(), "apply".into()], - // cwd: self.config.cwd.clone(), - // reason: Some("test".to_string()), - // }), - msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { - call_id: "1".to_string(), - changes: HashMap::from([ - ( - PathBuf::from("/tmp/test.txt"), - FileChange::Add { - content: "test".to_string(), - }, - ), - ( - PathBuf::from("/tmp/test2.txt"), - FileChange::Update { - unified_diff: "+test\n-test2".to_string(), - move_path: None, - }, - ), - ]), - reason: None, - grant_root: Some(PathBuf::from("/tmp")), - }), - })); - } - }, AppEvent::StartFileSearch(query) => { if !query.is_empty() { self.file_search.on_user_query(query); diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 31ac032b..a961f19e 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -4,7 +4,6 @@ use ratatui::text::Line; use crate::history_cell::HistoryCell; -use crate::slash_command::SlashCommand; use codex_core::protocol::AskForApproval; use codex_core::protocol::SandboxPolicy; use codex_core::protocol_config_types::ReasoningEffort; @@ -14,6 +13,9 @@ use codex_core::protocol_config_types::ReasoningEffort; pub(crate) enum AppEvent { CodexEvent(Event), + /// Start a new session. + NewSession, + /// Request to exit the application gracefully. ExitRequest, @@ -21,10 +23,6 @@ pub(crate) enum AppEvent { /// bubbling channels through layers of widgets. CodexOp(codex_core::protocol::Op), - /// Dispatch a recognized slash command from the UI (composer) to the app - /// layer so it can be handled centrally. - DispatchCommand(SlashCommand), - /// Kick off an asynchronous file search for the given query (text after /// the `@`). Previous searches may be cancelled by the app layer so there /// is at most one in-flight search. diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 675d2292..0cc34542 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -23,6 +23,7 @@ use ratatui::widgets::WidgetRef; use super::chat_composer_history::ChatComposerHistory; use super::command_popup::CommandPopup; use super::file_search_popup::FileSearchPopup; +use crate::slash_command::SlashCommand; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; @@ -38,6 +39,7 @@ const LARGE_PASTE_CHAR_THRESHOLD: usize = 1000; /// Result returned when the user interacts with the text area. pub enum InputResult { Submitted(String), + Command(SlashCommand), None, } @@ -289,15 +291,15 @@ impl ChatComposer { .. } => { if let Some(cmd) = popup.selected_command() { - // Send command to the app layer. - self.app_event_tx.send(AppEvent::DispatchCommand(*cmd)); - // Clear textarea so no residual text remains. self.textarea.set_text(""); + let result = (InputResult::Command(*cmd), true); + // Hide popup since the command has been dispatched. self.active_popup = ActivePopup::None; - return (InputResult::None, true); + + return result; } // Fallback to default newline handling if no command selected. self.handle_key_event_without_popup(key_event) @@ -1039,9 +1041,8 @@ mod tests { use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; - use tokio::sync::mpsc::error::TryRecvError; - let (tx, mut rx) = unbounded_channel::(); + let (tx, _rx) = unbounded_channel::(); let sender = AppEventSender::new(tx); let mut composer = ChatComposer::new(true, sender, false, "Ask Codex to do anything".to_string()); @@ -1057,25 +1058,18 @@ mod tests { let (result, _needs_redraw) = composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - // When a slash command is dispatched, the composer should not submit - // literal text and should clear its textarea. + // When a slash command is dispatched, the composer should return a + // Command result (not submit literal text) and clear its textarea. match result { - InputResult::None => {} + InputResult::Command(cmd) => { + assert_eq!(cmd.command(), "init"); + } InputResult::Submitted(text) => { panic!("expected command dispatch, but composer submitted literal text: {text}") } + InputResult::None => panic!("expected Command result for '/init'"), } assert!(composer.textarea.is_empty(), "composer should be cleared"); - - // Verify a DispatchCommand event for the "init" command was sent. - match rx.try_recv() { - Ok(AppEvent::DispatchCommand(cmd)) => { - assert_eq!(cmd.command(), "init"); - } - Ok(_other) => panic!("unexpected app event"), - Err(TryRecvError::Empty) => panic!("expected a DispatchCommand event for '/init'"), - Err(TryRecvError::Disconnected) => panic!("app event channel disconnected"), - } } #[test] @@ -1105,9 +1099,8 @@ mod tests { use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; - use tokio::sync::mpsc::error::TryRecvError; - let (tx, mut rx) = unbounded_channel::(); + let (tx, _rx) = unbounded_channel::(); let sender = AppEventSender::new(tx); let mut composer = ChatComposer::new(true, sender, false, "Ask Codex to do anything".to_string()); @@ -1120,24 +1113,16 @@ mod tests { composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); match result { - InputResult::None => {} + InputResult::Command(cmd) => { + assert_eq!(cmd.command(), "mention"); + } InputResult::Submitted(text) => { panic!("expected command dispatch, but composer submitted literal text: {text}") } + InputResult::None => panic!("expected Command result for '/mention'"), } assert!(composer.textarea.is_empty(), "composer should be cleared"); - - match rx.try_recv() { - Ok(AppEvent::DispatchCommand(cmd)) => { - assert_eq!(cmd.command(), "mention"); - composer.insert_str("@"); - } - Ok(_other) => panic!("unexpected app event"), - Err(TryRecvError::Empty) => panic!("expected a DispatchCommand event for '/mention'"), - Err(TryRecvError::Disconnected) => { - panic!("app event channel disconnected") - } - } + composer.insert_str("@"); assert_eq!(composer.textarea.text(), "@"); } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 3386133f..fa18342b 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -48,11 +48,13 @@ use crate::bottom_pane::CancellationEvent; use crate::bottom_pane::InputResult; use crate::bottom_pane::SelectionAction; use crate::bottom_pane::SelectionItem; +use crate::get_git_diff::get_git_diff; use crate::history_cell; use crate::history_cell::CommandOutput; use crate::history_cell::ExecCell; use crate::history_cell::HistoryCell; use crate::history_cell::PatchEventType; +use crate::slash_command::SlashCommand; use crate::tui::FrameRequester; // streaming internals are provided by crate::streaming and crate::markdown_stream use crate::user_approval_widget::ApprovalRequest; @@ -583,10 +585,109 @@ impl ChatWidget { InputResult::Submitted(text) => { self.submit_user_message(text.into()); } + InputResult::Command(cmd) => { + self.dispatch_command(cmd); + } InputResult::None => {} } } + fn dispatch_command(&mut self, cmd: SlashCommand) { + match cmd { + SlashCommand::New => { + self.app_event_tx.send(AppEvent::NewSession); + } + SlashCommand::Init => { + // Guard: do not run if a task is active. + const INIT_PROMPT: &str = include_str!("../prompt_for_init_command.md"); + self.submit_text_message(INIT_PROMPT.to_string()); + } + SlashCommand::Compact => { + self.clear_token_usage(); + self.app_event_tx.send(AppEvent::CodexOp(Op::Compact)); + } + SlashCommand::Model => { + self.open_model_popup(); + } + SlashCommand::Approvals => { + self.open_approvals_popup(); + } + SlashCommand::Quit => { + self.app_event_tx.send(AppEvent::ExitRequest); + } + SlashCommand::Logout => { + if let Err(e) = codex_login::logout(&self.config.codex_home) { + tracing::error!("failed to logout: {e}"); + } + self.app_event_tx.send(AppEvent::ExitRequest); + } + SlashCommand::Diff => { + self.add_diff_in_progress(); + let tx = self.app_event_tx.clone(); + tokio::spawn(async move { + let text = match get_git_diff().await { + Ok((is_git_repo, diff_text)) => { + if is_git_repo { + diff_text + } else { + "`/diff` — _not inside a git repository_".to_string() + } + } + Err(e) => format!("Failed to compute diff: {e}"), + }; + tx.send(AppEvent::DiffResult(text)); + }); + } + SlashCommand::Mention => { + self.insert_str("@"); + } + SlashCommand::Status => { + self.add_status_output(); + } + SlashCommand::Mcp => { + self.add_mcp_output(); + } + #[cfg(debug_assertions)] + SlashCommand::TestApproval => { + use codex_core::protocol::EventMsg; + use std::collections::HashMap; + + use codex_core::protocol::ApplyPatchApprovalRequestEvent; + use codex_core::protocol::FileChange; + + self.app_event_tx.send(AppEvent::CodexEvent(Event { + id: "1".to_string(), + // msg: EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { + // call_id: "1".to_string(), + // command: vec!["git".into(), "apply".into()], + // cwd: self.config.cwd.clone(), + // reason: Some("test".to_string()), + // }), + msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { + call_id: "1".to_string(), + changes: HashMap::from([ + ( + PathBuf::from("/tmp/test.txt"), + FileChange::Add { + content: "test".to_string(), + }, + ), + ( + PathBuf::from("/tmp/test2.txt"), + FileChange::Update { + unified_diff: "+test\n-test2".to_string(), + move_path: None, + }, + ), + ]), + reason: None, + grant_root: Some(PathBuf::from("/tmp")), + }), + })); + } + } + } + pub(crate) fn handle_paste(&mut self, text: String) { self.bottom_pane.handle_paste(text); } diff --git a/codex-rs/tui/src/session_log.rs b/codex-rs/tui/src/session_log.rs index d1dc4532..4937be8b 100644 --- a/codex-rs/tui/src/session_log.rs +++ b/codex-rs/tui/src/session_log.rs @@ -132,13 +132,11 @@ pub(crate) fn log_inbound_app_event(event: &AppEvent) { AppEvent::CodexEvent(ev) => { write_record("to_tui", "codex_event", ev); } - - AppEvent::DispatchCommand(cmd) => { + AppEvent::NewSession => { let value = json!({ "ts": now_ts(), "dir": "to_tui", - "kind": "slash_command", - "command": format!("{:?}", cmd), + "kind": "new_session", }); LOGGER.write_json_line(value); }