From 8a990b5401ea9bb6557d136d46420cd1f130b4dc Mon Sep 17 00:00:00 2001 From: Gabriel Peal Date: Wed, 6 Aug 2025 19:39:07 -0700 Subject: [PATCH 1/3] Migrate GitWarning to OnboardingScreen (#1915) This paves the way to do per-directory approval settings (https://github.com/openai/codex/pull/1912). This also lets us pass in a Config/ChatWidgetArgs into onboarding which can then mutate it and emit the ChatWidgetArgs it wants at the end which may be modified by the said approval settings. CleanShot 2025-08-06 at 19 30 55 --- codex-rs/core/src/util.rs | 7 +- codex-rs/exec/src/lib.rs | 2 +- codex-rs/tui/src/app.rs | 164 ++++++------------ codex-rs/tui/src/app_event.rs | 2 + codex-rs/tui/src/app_event_sender.rs | 2 +- codex-rs/tui/src/git_warning_screen.rs | 122 ------------- codex-rs/tui/src/lib.rs | 14 +- codex-rs/tui/src/onboarding/auth.rs | 121 +++++++++---- .../tui/src/onboarding/continue_to_chat.rs | 30 ++++ codex-rs/tui/src/onboarding/git_warning.rs | 126 ++++++++++++++ codex-rs/tui/src/onboarding/mod.rs | 2 + .../tui/src/onboarding/onboarding_screen.rs | 152 ++++++++++++---- codex-rs/tui/src/onboarding/welcome.rs | 17 +- 13 files changed, 443 insertions(+), 318 deletions(-) delete mode 100644 codex-rs/tui/src/git_warning_screen.rs create mode 100644 codex-rs/tui/src/onboarding/continue_to_chat.rs create mode 100644 codex-rs/tui/src/onboarding/git_warning.rs diff --git a/codex-rs/core/src/util.rs b/codex-rs/core/src/util.rs index a7c1485273..5ba1e25666 100644 --- a/codex-rs/core/src/util.rs +++ b/codex-rs/core/src/util.rs @@ -1,3 +1,4 @@ +use std::path::Path; use std::sync::Arc; use std::time::Duration; @@ -5,8 +6,6 @@ use rand::Rng; use tokio::sync::Notify; use tracing::debug; -use crate::config::Config; - const INITIAL_DELAY_MS: u64 = 200; const BACKOFF_FACTOR: f64 = 1.3; @@ -47,8 +46,8 @@ pub(crate) fn backoff(attempt: u64) -> Duration { /// `git worktree add` where the checkout lives outside the main repository /// directory. If you need Codex to work from such a checkout simply pass the /// `--allow-no-git-exec` CLI flag that disables the repo requirement. -pub fn is_inside_git_repo(config: &Config) -> bool { - let mut dir = config.cwd.to_path_buf(); +pub fn is_inside_git_repo(base_dir: &Path) -> bool { + let mut dir = base_dir.to_path_buf(); loop { if dir.join(".git").exists() { diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 06df2aebca..5d7f1281ee 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -180,7 +180,7 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any // is using. event_processor.print_config_summary(&config, &prompt); - if !skip_git_repo_check && !is_inside_git_repo(&config) { + if !skip_git_repo_check && !is_inside_git_repo(&config.cwd.to_path_buf()) { eprintln!("Not inside a Git repo and --skip-git-repo-check was not specified."); std::process::exit(1); } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 23e12be38f..ad3b4f3372 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -3,11 +3,9 @@ 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::git_warning_screen::GitWarningOutcome; -use crate::git_warning_screen::GitWarningScreen; -use crate::onboarding::onboarding_screen::KeyEventResult; use crate::onboarding::onboarding_screen::KeyboardHandler; use crate::onboarding::onboarding_screen::OnboardingScreen; +use crate::onboarding::onboarding_screen::OnboardingScreenArgs; use crate::should_show_login_screen; use crate::slash_command::SlashCommand; use crate::tui; @@ -15,6 +13,7 @@ use codex_core::config::Config; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::Op; +use codex_core::util::is_inside_git_repo; use color_eyre::eyre::Result; use crossterm::SynchronizedUpdate; use crossterm::event::KeyCode; @@ -48,10 +47,6 @@ enum AppState<'a> { /// `AppState`. widget: Box>, }, - /// The start-up warning that recommends running codex inside a Git repo. - GitWarning { - screen: GitWarningScreen, - }, } pub(crate) struct App<'a> { @@ -69,17 +64,13 @@ pub(crate) struct App<'a> { pending_history_lines: Vec>, - /// Stored parameters needed to instantiate the ChatWidget later, e.g., - /// after dismissing the Git-repo warning. - chat_args: Option, - enhanced_keys_supported: bool, } /// Aggregate parameters needed to create a `ChatWidget`, as creation may be /// deferred until after the Git warning screen is dismissed. -#[derive(Clone)] -struct ChatWidgetArgs { +#[derive(Clone, Debug)] +pub(crate) struct ChatWidgetArgs { config: Config, initial_prompt: Option, initial_images: Vec, @@ -90,7 +81,7 @@ impl App<'_> { pub(crate) fn new( config: Config, initial_prompt: Option, - show_git_warning: bool, + skip_git_repo_check: bool, initial_images: Vec, ) -> Self { let (app_event_tx, app_event_rx) = channel(); @@ -143,30 +134,25 @@ impl App<'_> { } let show_login_screen = should_show_login_screen(&config); - let (app_state, chat_args) = if show_login_screen { - ( - AppState::Onboarding { - screen: OnboardingScreen::new(app_event_tx.clone(), config.codex_home.clone()), - }, - Some(ChatWidgetArgs { - config: config.clone(), - initial_prompt, - initial_images, - enhanced_keys_supported, + let show_git_warning = + !skip_git_repo_check && !is_inside_git_repo(&config.cwd.to_path_buf()); + let app_state = if show_login_screen || show_git_warning { + let chat_widget_args = ChatWidgetArgs { + config: config.clone(), + initial_prompt, + initial_images, + enhanced_keys_supported, + }; + AppState::Onboarding { + screen: OnboardingScreen::new(OnboardingScreenArgs { + event_tx: app_event_tx.clone(), + codex_home: config.codex_home.clone(), + cwd: config.cwd.clone(), + show_login_screen, + show_git_warning, + chat_widget_args, }), - ) - } else if show_git_warning { - ( - AppState::GitWarning { - screen: GitWarningScreen::new(), - }, - Some(ChatWidgetArgs { - config: config.clone(), - initial_prompt, - initial_images, - enhanced_keys_supported, - }), - ) + } } else { let chat_widget = ChatWidget::new( config.clone(), @@ -175,12 +161,9 @@ impl App<'_> { initial_images, enhanced_keys_supported, ); - ( - AppState::Chat { - widget: Box::new(chat_widget), - }, - None, - ) + AppState::Chat { + widget: Box::new(chat_widget), + } }; let file_search = FileSearchManager::new(config.cwd.clone(), app_event_tx.clone()); @@ -192,7 +175,6 @@ impl App<'_> { config, file_search, pending_redraw, - chat_args, enhanced_keys_supported, } } @@ -249,20 +231,14 @@ impl App<'_> { modifiers: crossterm::event::KeyModifiers::CONTROL, kind: KeyEventKind::Press, .. - } => { - match &mut self.app_state { - AppState::Chat { widget } => { - widget.on_ctrl_c(); - } - AppState::Onboarding { .. } => { - self.app_event_tx.send(AppEvent::ExitRequest); - } - AppState::GitWarning { .. } => { - // Allow exiting the app with Ctrl+C from the warning screen. - self.app_event_tx.send(AppEvent::ExitRequest); - } + } => match &mut self.app_state { + AppState::Chat { widget } => { + widget.on_ctrl_c(); } - } + AppState::Onboarding { .. } => { + self.app_event_tx.send(AppEvent::ExitRequest); + } + }, KeyEvent { code: KeyCode::Char('z'), modifiers: crossterm::event::KeyModifiers::CONTROL, @@ -293,9 +269,6 @@ impl App<'_> { AppState::Onboarding { .. } => { self.app_event_tx.send(AppEvent::ExitRequest); } - AppState::GitWarning { .. } => { - self.app_event_tx.send(AppEvent::ExitRequest); - } } } KeyEvent { @@ -321,15 +294,14 @@ impl App<'_> { AppEvent::CodexOp(op) => match &mut self.app_state { AppState::Chat { widget } => widget.submit_op(op), AppState::Onboarding { .. } => {} - AppState::GitWarning { .. } => {} }, AppEvent::LatestLog(line) => match &mut self.app_state { AppState::Chat { widget } => widget.update_latest_log(line), AppState::Onboarding { .. } => {} - AppState::GitWarning { .. } => {} }, AppEvent::DispatchCommand(command) => match command { SlashCommand::New => { + // User accepted – switch to chat view. let new_widget = Box::new(ChatWidget::new( self.config.clone(), self.app_event_tx.clone(), @@ -424,8 +396,23 @@ impl App<'_> { }, AppEvent::OnboardingAuthComplete(result) => { if let AppState::Onboarding { screen } = &mut self.app_state { - // Let the onboarding screen handle success/failure and emit follow-up events. - let _ = screen.on_auth_complete(result); + screen.on_auth_complete(result); + } + } + AppEvent::OnboardingComplete(ChatWidgetArgs { + config, + enhanced_keys_supported, + initial_images, + initial_prompt, + }) => { + self.app_state = AppState::Chat { + widget: Box::new(ChatWidget::new( + config, + app_event_tx.clone(), + initial_prompt, + initial_images, + enhanced_keys_supported, + )), } } AppEvent::StartFileSearch(query) => { @@ -447,7 +434,6 @@ impl App<'_> { match &self.app_state { AppState::Chat { widget } => widget.token_usage().clone(), AppState::Onboarding { .. } => codex_core::protocol::TokenUsage::default(), - AppState::GitWarning { .. } => codex_core::protocol::TokenUsage::default(), } } @@ -476,7 +462,6 @@ impl App<'_> { let desired_height = match &self.app_state { AppState::Chat { widget } => widget.desired_height(size.width), AppState::Onboarding { .. } => size.height, - AppState::GitWarning { .. } => size.height, }; let mut area = terminal.viewport_area; @@ -507,7 +492,6 @@ impl App<'_> { frame.render_widget_ref(&**widget, frame.area()) } AppState::Onboarding { screen } => frame.render_widget_ref(&*screen, frame.area()), - AppState::GitWarning { screen } => frame.render_widget_ref(&*screen, frame.area()), })?; Ok(()) } @@ -519,49 +503,11 @@ impl App<'_> { AppState::Chat { widget } => { widget.handle_key_event(key_event); } - AppState::Onboarding { screen } => match screen.handle_key_event(key_event) { - KeyEventResult::Continue => { - self.app_state = AppState::Chat { - widget: Box::new(ChatWidget::new( - self.config.clone(), - self.app_event_tx.clone(), - None, - Vec::new(), - self.enhanced_keys_supported, - )), - }; - } - KeyEventResult::Quit => { + AppState::Onboarding { screen } => match key_event.code { + KeyCode::Char('q') => { self.app_event_tx.send(AppEvent::ExitRequest); } - KeyEventResult::None => { - // do nothing - } - }, - AppState::GitWarning { screen } => match screen.handle_key_event(key_event) { - GitWarningOutcome::Continue => { - // User accepted – switch to chat view. - let args = match self.chat_args.take() { - Some(args) => args, - None => panic!("ChatWidgetArgs already consumed"), - }; - - let widget = Box::new(ChatWidget::new( - args.config, - self.app_event_tx.clone(), - args.initial_prompt, - args.initial_images, - args.enhanced_keys_supported, - )); - self.app_state = AppState::Chat { widget }; - self.app_event_tx.send(AppEvent::RequestRedraw); - } - GitWarningOutcome::Quit => { - self.app_event_tx.send(AppEvent::ExitRequest); - } - GitWarningOutcome::None => { - // do nothing - } + _ => screen.handle_key_event(key_event), }, } } @@ -570,7 +516,6 @@ impl App<'_> { match &mut self.app_state { AppState::Chat { widget } => widget.handle_paste(pasted), AppState::Onboarding { .. } => {} - AppState::GitWarning { .. } => {} } } @@ -578,7 +523,6 @@ impl App<'_> { match &mut self.app_state { AppState::Chat { widget } => widget.handle_codex_event(event), AppState::Onboarding { .. } => {} - AppState::GitWarning { .. } => {} } } } diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 7df7761ac5..7f96fe1e47 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -3,6 +3,7 @@ use codex_file_search::FileMatch; use crossterm::event::KeyEvent; use ratatui::text::Line; +use crate::app::ChatWidgetArgs; use crate::slash_command::SlashCommand; #[allow(clippy::large_enum_variant)] @@ -51,4 +52,5 @@ pub(crate) enum AppEvent { /// Onboarding: result of login_with_chatgpt. OnboardingAuthComplete(Result<(), String>), + OnboardingComplete(ChatWidgetArgs), } diff --git a/codex-rs/tui/src/app_event_sender.rs b/codex-rs/tui/src/app_event_sender.rs index 9d838273ef..f6c8c18c98 100644 --- a/codex-rs/tui/src/app_event_sender.rs +++ b/codex-rs/tui/src/app_event_sender.rs @@ -4,7 +4,7 @@ use crate::app_event::AppEvent; #[derive(Clone, Debug)] pub(crate) struct AppEventSender { - app_event_tx: Sender, + pub app_event_tx: Sender, } impl AppEventSender { diff --git a/codex-rs/tui/src/git_warning_screen.rs b/codex-rs/tui/src/git_warning_screen.rs deleted file mode 100644 index 3a7ea21159..0000000000 --- a/codex-rs/tui/src/git_warning_screen.rs +++ /dev/null @@ -1,122 +0,0 @@ -//! Full‑screen warning displayed when Codex is started outside a Git -//! repository (unless the user passed `--allow-no-git-exec`). The screen -//! blocks all input until the user explicitly decides whether to continue or -//! quit. - -use crossterm::event::KeyCode; -use crossterm::event::KeyEvent; -use ratatui::buffer::Buffer; -use ratatui::layout::Alignment; -use ratatui::layout::Constraint; -use ratatui::layout::Direction; -use ratatui::layout::Layout; -use ratatui::layout::Rect; -use ratatui::style::Color; -use ratatui::style::Modifier; -use ratatui::style::Style; -use ratatui::text::Span; -use ratatui::widgets::Block; -use ratatui::widgets::BorderType; -use ratatui::widgets::Borders; -use ratatui::widgets::Paragraph; -use ratatui::widgets::Widget; -use ratatui::widgets::WidgetRef; -use ratatui::widgets::Wrap; - -const NO_GIT_ERROR: &str = "We recommend running codex inside a git repository. \ -This helps ensure that changes can be tracked and easily rolled back if necessary. \ -Do you wish to proceed?"; - -/// Result of handling a key event while the warning screen is active. -pub(crate) enum GitWarningOutcome { - /// User chose to proceed – switch to the main Chat UI. - Continue, - /// User opted to quit the application. - Quit, - /// No actionable key was pressed – stay on the warning screen. - None, -} - -pub(crate) struct GitWarningScreen; - -impl GitWarningScreen { - pub(crate) fn new() -> Self { - Self - } - - /// Handle a key event, returning an outcome indicating whether the user - /// chose to continue, quit, or neither. - pub(crate) fn handle_key_event(&self, key_event: KeyEvent) -> GitWarningOutcome { - match key_event.code { - KeyCode::Char('y') | KeyCode::Char('Y') => GitWarningOutcome::Continue, - KeyCode::Char('n') | KeyCode::Char('q') | KeyCode::Esc => GitWarningOutcome::Quit, - _ => GitWarningOutcome::None, - } - } -} - -impl WidgetRef for &GitWarningScreen { - fn render_ref(&self, area: Rect, buf: &mut Buffer) { - const MIN_WIDTH: u16 = 35; - const MIN_HEIGHT: u16 = 15; - // Check if the available area is too small for our popup. - if area.width < MIN_WIDTH || area.height < MIN_HEIGHT { - // Fallback rendering: a simple abbreviated message that fits the available area. - let fallback_message = Paragraph::new(NO_GIT_ERROR) - .wrap(Wrap { trim: true }) - .alignment(Alignment::Center); - fallback_message.render(area, buf); - return; - } - - // Determine the popup (modal) size – aim for 60 % width, 30 % height - // but keep a sensible minimum so the content is always readable. - let popup_width = std::cmp::max(MIN_WIDTH, (area.width as f32 * 0.6) as u16); - let popup_height = std::cmp::max(MIN_HEIGHT, (area.height as f32 * 0.3) as u16); - - // Center the popup in the available area. - let popup_x = area.x + (area.width.saturating_sub(popup_width)) / 2; - let popup_y = area.y + (area.height.saturating_sub(popup_height)) / 2; - let popup_area = Rect::new(popup_x, popup_y, popup_width, popup_height); - - // The modal block that contains everything. - let popup_block = Block::default() - .borders(Borders::ALL) - .border_type(BorderType::Plain) - .title(Span::styled( - "Warning: Not a Git repository", // bold warning title - Style::default().add_modifier(Modifier::BOLD).fg(Color::Red), - )); - - // Obtain the inner area before rendering (render consumes the block). - let inner = popup_block.inner(popup_area); - popup_block.render(popup_area, buf); - - // Split the inner area vertically into two boxes: one for the warning - // explanation, one for the user action instructions. - let chunks = Layout::default() - .direction(Direction::Vertical) - .constraints([Constraint::Min(3), Constraint::Length(3)]) - .split(inner); - - // ----- First box: detailed warning text -------------------------------- - let text_block = Block::default().borders(Borders::ALL); - let text_inner = text_block.inner(chunks[0]); - text_block.render(chunks[0], buf); - - let warning_paragraph = Paragraph::new(NO_GIT_ERROR) - .wrap(Wrap { trim: true }) - .alignment(Alignment::Left); - warning_paragraph.render(text_inner, buf); - - // ----- Second box: "proceed? y/n" instructions -------------------------- - let action_block = Block::default().borders(Borders::ALL); - let action_inner = action_block.inner(chunks[1]); - action_block.render(chunks[1], buf); - - let action_text = Paragraph::new("press 'y' to continue, 'n' to quit") - .alignment(Alignment::Center) - .style(Style::default().add_modifier(Modifier::BOLD)); - action_text.render(action_inner, buf); - } -} diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index e65083bfe1..0e809afdbe 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -8,7 +8,6 @@ use codex_core::config::Config; use codex_core::config::ConfigOverrides; use codex_core::config_types::SandboxMode; use codex_core::protocol::AskForApproval; -use codex_core::util::is_inside_git_repo; use codex_login::load_auth; use codex_ollama::DEFAULT_OSS_MODEL; use log_layer::TuiLogLayer; @@ -31,7 +30,6 @@ pub mod custom_terminal; mod exec_command; mod file_search; mod get_git_diff; -mod git_warning_screen; mod history_cell; pub mod insert_history; pub mod live_wrap; @@ -206,20 +204,12 @@ pub async fn run_main( eprintln!(""); } - // Determine whether we need to display the "not a git repo" warning - // modal. The flag is shown when the current working directory is *not* - // inside a Git repository **and** the user did *not* pass the - // `--allow-no-git-exec` flag. - let show_git_warning = !cli.skip_git_repo_check && !is_inside_git_repo(&config); - - run_ratatui_app(cli, config, show_git_warning, log_rx) - .map_err(|err| std::io::Error::other(err.to_string())) + run_ratatui_app(cli, config, log_rx).map_err(|err| std::io::Error::other(err.to_string())) } fn run_ratatui_app( cli: Cli, config: Config, - show_git_warning: bool, mut log_rx: tokio::sync::mpsc::UnboundedReceiver, ) -> color_eyre::Result { color_eyre::install()?; @@ -237,7 +227,7 @@ fn run_ratatui_app( terminal.clear()?; let Cli { prompt, images, .. } = cli; - let mut app = App::new(config.clone(), prompt, show_git_warning, images); + let mut app = App::new(config.clone(), prompt, cli.skip_git_repo_check, images); // Bridge log receiver into the AppEvent channel so latest log lines update the UI. { diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 834c36819e..b91bf4a0d9 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -18,18 +18,23 @@ use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::colors::LIGHT_BLUE; use crate::colors::SUCCESS_GREEN; -use crate::onboarding::onboarding_screen::KeyEventResult; use crate::onboarding::onboarding_screen::KeyboardHandler; +use crate::onboarding::onboarding_screen::StepStateProvider; use crate::shimmer::FrameTicker; use crate::shimmer::shimmer_spans; use std::path::PathBuf; + +use super::onboarding_screen::StepState; // no additional imports #[derive(Debug)] pub(crate) enum SignInState { PickMode, ChatGptContinueInBrowser(#[allow(dead_code)] ContinueInBrowserState), + ChatGptSuccessMessage, ChatGptSuccess, + EnvVarMissing, + EnvVarFound, } #[derive(Debug)] @@ -38,7 +43,6 @@ pub(crate) struct ContinueInBrowserState { _login_child: Option, _frame_ticker: Option, } - impl Drop for ContinueInBrowserState { fn drop(&mut self) { if let Some(child) = &self._login_child { @@ -52,54 +56,45 @@ impl Drop for ContinueInBrowserState { } impl KeyboardHandler for AuthModeWidget { - fn handle_key_event(&mut self, key_event: KeyEvent) -> KeyEventResult { + fn handle_key_event(&mut self, key_event: KeyEvent) { match key_event.code { KeyCode::Up | KeyCode::Char('k') => { - self.mode = AuthMode::ChatGPT; - KeyEventResult::None + self.highlighted_mode = AuthMode::ChatGPT; } KeyCode::Down | KeyCode::Char('j') => { - self.mode = AuthMode::ApiKey; - KeyEventResult::None + self.highlighted_mode = AuthMode::ApiKey; } KeyCode::Char('1') => { - self.mode = AuthMode::ChatGPT; self.start_chatgpt_login(); - KeyEventResult::None } - KeyCode::Char('2') => { - self.mode = AuthMode::ApiKey; - self.verify_api_key() - } - KeyCode::Enter => match self.mode { - AuthMode::ChatGPT => match &self.sign_in_state { - SignInState::PickMode => self.start_chatgpt_login(), - SignInState::ChatGptContinueInBrowser(_) => KeyEventResult::None, - SignInState::ChatGptSuccess => KeyEventResult::Continue, + KeyCode::Char('2') => self.verify_api_key(), + KeyCode::Enter => match self.sign_in_state { + SignInState::PickMode => match self.highlighted_mode { + AuthMode::ChatGPT => self.start_chatgpt_login(), + AuthMode::ApiKey => self.verify_api_key(), }, - AuthMode::ApiKey => self.verify_api_key(), + SignInState::EnvVarMissing => self.sign_in_state = SignInState::PickMode, + SignInState::ChatGptSuccessMessage => { + self.sign_in_state = SignInState::ChatGptSuccess + } + _ => {} }, KeyCode::Esc => { if matches!(self.sign_in_state, SignInState::ChatGptContinueInBrowser(_)) { self.sign_in_state = SignInState::PickMode; - self.event_tx.send(AppEvent::RequestRedraw); - KeyEventResult::None - } else { - KeyEventResult::Quit } } - KeyCode::Char('q') => KeyEventResult::Quit, - _ => KeyEventResult::None, + _ => {} } } } #[derive(Debug)] pub(crate) struct AuthModeWidget { - pub mode: AuthMode, + pub event_tx: AppEventSender, + pub highlighted_mode: AuthMode, pub error: Option, pub sign_in_state: SignInState, - pub event_tx: AppEventSender, pub codex_home: PathBuf, } @@ -121,7 +116,7 @@ impl AuthModeWidget { text: &str, description: &str| -> Vec> { - let is_selected = self.mode == selected_mode; + let is_selected = self.highlighted_mode == selected_mode; let caret = if is_selected { ">" } else { " " }; let line1 = if is_selected { @@ -192,7 +187,7 @@ impl AuthModeWidget { .render(area, buf); } - fn render_chatgpt_success(&self, area: Rect, buf: &mut Buffer) { + fn render_chatgpt_success_message(&self, area: Rect, buf: &mut Buffer) { let lines = vec![ Line::from("✓ Signed in with your ChatGPT account") .style(Style::default().fg(SUCCESS_GREEN)), @@ -219,7 +214,40 @@ impl AuthModeWidget { .render(area, buf); } - fn start_chatgpt_login(&mut self) -> KeyEventResult { + fn render_chatgpt_success(&self, area: Rect, buf: &mut Buffer) { + let lines = vec![ + Line::from("✓ Signed in with your ChatGPT account") + .style(Style::default().fg(SUCCESS_GREEN)), + ]; + + Paragraph::new(lines) + .wrap(Wrap { trim: false }) + .render(area, buf); + } + + fn render_env_var_found(&self, area: Rect, buf: &mut Buffer) { + let lines = + vec![Line::from("✓ Using OPENAI_API_KEY").style(Style::default().fg(SUCCESS_GREEN))]; + + Paragraph::new(lines) + .wrap(Wrap { trim: false }) + .render(area, buf); + } + + fn render_env_var_missing(&self, area: Rect, buf: &mut Buffer) { + let lines = vec![ + Line::from("✘ OPENAI_API_KEY not found").style(Style::default().fg(Color::Red)), + Line::from(""), + Line::from(" Press Enter to return") + .style(Style::default().add_modifier(Modifier::DIM)), + ]; + + Paragraph::new(lines) + .wrap(Wrap { trim: false }) + .render(area, buf); + } + + fn start_chatgpt_login(&mut self) { self.error = None; match codex_login::spawn_login_with_chatgpt(&self.codex_home) { Ok(child) => { @@ -230,27 +258,23 @@ impl AuthModeWidget { _frame_ticker: Some(FrameTicker::new(self.event_tx.clone())), }); self.event_tx.send(AppEvent::RequestRedraw); - KeyEventResult::None } Err(e) => { self.sign_in_state = SignInState::PickMode; self.error = Some(e.to_string()); self.event_tx.send(AppEvent::RequestRedraw); - KeyEventResult::None } } } /// TODO: Read/write from the correct hierarchy config overrides + auth json + OPENAI_API_KEY. - fn verify_api_key(&mut self) -> KeyEventResult { + fn verify_api_key(&mut self) { if std::env::var("OPENAI_API_KEY").is_err() { - self.error = - Some("Set OPENAI_API_KEY in your environment. Learn more: https://platform.openai.com/docs/libraries".to_string()); - self.event_tx.send(AppEvent::RequestRedraw); - KeyEventResult::None + self.sign_in_state = SignInState::EnvVarMissing; } else { - KeyEventResult::Continue + self.sign_in_state = SignInState::EnvVarFound; } + self.event_tx.send(AppEvent::RequestRedraw); } fn spawn_completion_poller(&self, child: codex_login::SpawnedLogin) { @@ -299,6 +323,18 @@ impl AuthModeWidget { } } +impl StepStateProvider for AuthModeWidget { + fn get_step_state(&self) -> StepState { + match &self.sign_in_state { + SignInState::PickMode + | SignInState::EnvVarMissing + | SignInState::ChatGptContinueInBrowser(_) + | SignInState::ChatGptSuccessMessage => StepState::InProgress, + SignInState::ChatGptSuccess | SignInState::EnvVarFound => StepState::Complete, + } + } +} + impl WidgetRef for AuthModeWidget { fn render_ref(&self, area: Rect, buf: &mut Buffer) { match self.sign_in_state { @@ -308,9 +344,18 @@ impl WidgetRef for AuthModeWidget { SignInState::ChatGptContinueInBrowser(_) => { self.render_continue_in_browser(area, buf); } + SignInState::ChatGptSuccessMessage => { + self.render_chatgpt_success_message(area, buf); + } SignInState::ChatGptSuccess => { self.render_chatgpt_success(area, buf); } + SignInState::EnvVarMissing => { + self.render_env_var_missing(area, buf); + } + SignInState::EnvVarFound => { + self.render_env_var_found(area, buf); + } } } } diff --git a/codex-rs/tui/src/onboarding/continue_to_chat.rs b/codex-rs/tui/src/onboarding/continue_to_chat.rs new file mode 100644 index 0000000000..071d0851da --- /dev/null +++ b/codex-rs/tui/src/onboarding/continue_to_chat.rs @@ -0,0 +1,30 @@ +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use ratatui::widgets::WidgetRef; + +use crate::app::ChatWidgetArgs; +use crate::app_event::AppEvent; +use crate::app_event_sender::AppEventSender; +use crate::onboarding::onboarding_screen::StepStateProvider; + +use super::onboarding_screen::StepState; + +/// This doesn't render anything explicitly but serves as a signal that we made it to the end and +/// we should continue to the chat. +pub(crate) struct ContinueToChatWidget { + pub event_tx: AppEventSender, + pub chat_widget_args: ChatWidgetArgs, +} + +impl StepStateProvider for ContinueToChatWidget { + fn get_step_state(&self) -> StepState { + StepState::Complete + } +} + +impl WidgetRef for &ContinueToChatWidget { + fn render_ref(&self, _area: Rect, _buf: &mut Buffer) { + self.event_tx + .send(AppEvent::OnboardingComplete(self.chat_widget_args.clone())); + } +} diff --git a/codex-rs/tui/src/onboarding/git_warning.rs b/codex-rs/tui/src/onboarding/git_warning.rs new file mode 100644 index 0000000000..e4e5747404 --- /dev/null +++ b/codex-rs/tui/src/onboarding/git_warning.rs @@ -0,0 +1,126 @@ +use std::path::PathBuf; + +use codex_core::util::is_inside_git_repo; +use crossterm::event::KeyCode; +use crossterm::event::KeyEvent; +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use ratatui::prelude::Widget; +use ratatui::style::Modifier; +use ratatui::style::Style; +use ratatui::style::Stylize; +use ratatui::text::Line; +use ratatui::text::Span; +use ratatui::widgets::Paragraph; +use ratatui::widgets::WidgetRef; +use ratatui::widgets::Wrap; + +use crate::app_event::AppEvent; +use crate::app_event_sender::AppEventSender; +use crate::colors::LIGHT_BLUE; + +use crate::onboarding::onboarding_screen::KeyboardHandler; +use crate::onboarding::onboarding_screen::StepStateProvider; + +use super::onboarding_screen::StepState; + +pub(crate) struct GitWarningWidget { + pub event_tx: AppEventSender, + pub cwd: PathBuf, + pub selection: Option, + pub highlighted: GitWarningSelection, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum GitWarningSelection { + Continue, + Exit, +} + +impl WidgetRef for &GitWarningWidget { + fn render_ref(&self, area: Rect, buf: &mut Buffer) { + let mut lines: Vec = vec![ + Line::from(vec![ + Span::raw("> "), + Span::raw("You are running Codex in "), + Span::styled( + self.cwd.to_string_lossy().to_string(), + Style::default().add_modifier(Modifier::BOLD), + ), + Span::raw(". This folder is not version controlled."), + ]), + Line::from(""), + Line::from(" Do you want to continue?"), + Line::from(""), + ]; + + let create_option = + |idx: usize, option: GitWarningSelection, text: &str| -> Line<'static> { + let is_selected = self.highlighted == option; + if is_selected { + Line::from(vec![ + Span::styled( + format!("> {}. ", idx + 1), + Style::default().fg(LIGHT_BLUE).add_modifier(Modifier::DIM), + ), + Span::styled(text.to_owned(), Style::default().fg(LIGHT_BLUE)), + ]) + } else { + Line::from(format!(" {}. {}", idx + 1, text)) + } + }; + + lines.push(create_option(0, GitWarningSelection::Continue, "Yes")); + lines.push(create_option(1, GitWarningSelection::Exit, "No")); + lines.push(Line::from("")); + lines.push(Line::from(" Press Enter to continue").add_modifier(Modifier::DIM)); + + Paragraph::new(lines) + .wrap(Wrap { trim: false }) + .render(area, buf); + } +} + +impl KeyboardHandler for GitWarningWidget { + fn handle_key_event(&mut self, key_event: KeyEvent) { + match key_event.code { + KeyCode::Up | KeyCode::Char('k') => { + self.highlighted = GitWarningSelection::Continue; + } + KeyCode::Down | KeyCode::Char('j') => { + self.highlighted = GitWarningSelection::Exit; + } + KeyCode::Char('1') => self.handle_continue(), + KeyCode::Char('2') => self.handle_quit(), + KeyCode::Enter => match self.highlighted { + GitWarningSelection::Continue => self.handle_continue(), + GitWarningSelection::Exit => self.handle_quit(), + }, + _ => {} + } + } +} + +impl StepStateProvider for GitWarningWidget { + fn get_step_state(&self) -> StepState { + let is_git_repo = is_inside_git_repo(&self.cwd); + match is_git_repo { + true => StepState::Hidden, + false => match self.selection { + Some(_) => StepState::Complete, + None => StepState::InProgress, + }, + } + } +} + +impl GitWarningWidget { + fn handle_continue(&mut self) { + self.selection = Some(GitWarningSelection::Continue); + } + + fn handle_quit(&mut self) { + self.highlighted = GitWarningSelection::Exit; + self.event_tx.send(AppEvent::ExitRequest); + } +} diff --git a/codex-rs/tui/src/onboarding/mod.rs b/codex-rs/tui/src/onboarding/mod.rs index 42d3ac8187..645cda22d9 100644 --- a/codex-rs/tui/src/onboarding/mod.rs +++ b/codex-rs/tui/src/onboarding/mod.rs @@ -1,3 +1,5 @@ mod auth; +mod continue_to_chat; +mod git_warning; pub mod onboarding_screen; mod welcome; diff --git a/codex-rs/tui/src/onboarding/onboarding_screen.rs b/codex-rs/tui/src/onboarding/onboarding_screen.rs index e2548bacb3..7ce7d16c47 100644 --- a/codex-rs/tui/src/onboarding/onboarding_screen.rs +++ b/codex-rs/tui/src/onboarding/onboarding_screen.rs @@ -5,26 +5,37 @@ use ratatui::widgets::WidgetRef; use codex_login::AuthMode; +use crate::app::ChatWidgetArgs; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::onboarding::auth::AuthModeWidget; use crate::onboarding::auth::SignInState; +use crate::onboarding::continue_to_chat::ContinueToChatWidget; +use crate::onboarding::git_warning::GitWarningSelection; +use crate::onboarding::git_warning::GitWarningWidget; use crate::onboarding::welcome::WelcomeWidget; use std::path::PathBuf; +#[allow(clippy::large_enum_variant)] enum Step { Welcome(WelcomeWidget), Auth(AuthModeWidget), + GitWarning(GitWarningWidget), + ContinueToChat(ContinueToChatWidget), } pub(crate) trait KeyboardHandler { - fn handle_key_event(&mut self, key_event: KeyEvent) -> KeyEventResult; + fn handle_key_event(&mut self, key_event: KeyEvent); } -pub(crate) enum KeyEventResult { - Continue, - Quit, - None, +pub(crate) enum StepState { + Hidden, + InProgress, + Complete, +} + +pub(crate) trait StepStateProvider { + fn get_step_state(&self) -> StepState; } pub(crate) struct OnboardingScreen { @@ -32,50 +43,113 @@ pub(crate) struct OnboardingScreen { steps: Vec, } +pub(crate) struct OnboardingScreenArgs { + pub event_tx: AppEventSender, + pub chat_widget_args: ChatWidgetArgs, + pub codex_home: PathBuf, + pub cwd: PathBuf, + pub show_login_screen: bool, + pub show_git_warning: bool, +} + impl OnboardingScreen { - pub(crate) fn new(event_tx: AppEventSender, codex_home: PathBuf) -> Self { - let steps: Vec = vec![ - Step::Welcome(WelcomeWidget {}), - Step::Auth(AuthModeWidget { + pub(crate) fn new(args: OnboardingScreenArgs) -> Self { + let OnboardingScreenArgs { + event_tx, + chat_widget_args, + codex_home, + cwd, + show_login_screen, + show_git_warning, + } = args; + let mut steps: Vec = vec![Step::Welcome(WelcomeWidget { + is_logged_in: !show_login_screen, + })]; + if show_login_screen { + steps.push(Step::Auth(AuthModeWidget { event_tx: event_tx.clone(), - mode: AuthMode::ChatGPT, + highlighted_mode: AuthMode::ChatGPT, error: None, sign_in_state: SignInState::PickMode, codex_home, - }), - ]; + })) + } + if show_git_warning { + steps.push(Step::GitWarning(GitWarningWidget { + event_tx: event_tx.clone(), + cwd, + selection: None, + highlighted: GitWarningSelection::Continue, + })) + } + steps.push(Step::ContinueToChat(ContinueToChatWidget { + event_tx: event_tx.clone(), + chat_widget_args, + })); + // TODO: add git warning. Self { event_tx, steps } } - pub(crate) fn on_auth_complete(&mut self, result: Result<(), String>) -> KeyEventResult { - if let Some(Step::Auth(state)) = self.steps.last_mut() { + pub(crate) fn on_auth_complete(&mut self, result: Result<(), String>) { + let current_step = self.current_step_mut(); + if let Some(Step::Auth(state)) = current_step { match result { Ok(()) => { - state.sign_in_state = SignInState::ChatGptSuccess; + state.sign_in_state = SignInState::ChatGptSuccessMessage; self.event_tx.send(AppEvent::RequestRedraw); - KeyEventResult::None } Err(e) => { state.sign_in_state = SignInState::PickMode; state.error = Some(e); self.event_tx.send(AppEvent::RequestRedraw); - KeyEventResult::None } } - } else { - KeyEventResult::None } } + + fn current_steps_mut(&mut self) -> Vec<&mut Step> { + let mut out: Vec<&mut Step> = Vec::new(); + for step in self.steps.iter_mut() { + match step.get_step_state() { + StepState::Hidden => continue, + StepState::Complete => out.push(step), + StepState::InProgress => { + out.push(step); + break; + } + } + } + out + } + + fn current_steps(&self) -> Vec<&Step> { + let mut out: Vec<&Step> = Vec::new(); + for step in self.steps.iter() { + match step.get_step_state() { + StepState::Hidden => continue, + StepState::Complete => out.push(step), + StepState::InProgress => { + out.push(step); + break; + } + } + } + out + } + + fn current_step_mut(&mut self) -> Option<&mut Step> { + self.steps + .iter_mut() + .find(|step| matches!(step.get_step_state(), StepState::InProgress)) + } } impl KeyboardHandler for OnboardingScreen { - fn handle_key_event(&mut self, key_event: KeyEvent) -> KeyEventResult { - if let Some(last_step) = self.steps.last_mut() { - self.event_tx.send(AppEvent::RequestRedraw); - last_step.handle_key_event(key_event) - } else { - KeyEventResult::None + fn handle_key_event(&mut self, key_event: KeyEvent) { + if let Some(active_step) = self.current_steps_mut().into_iter().last() { + active_step.handle_key_event(key_event); } + self.event_tx.send(AppEvent::RequestRedraw); } } @@ -109,8 +183,10 @@ impl WidgetRef for &OnboardingScreen { } let mut i = 0usize; - while i < self.steps.len() && y < bottom { - let step = &self.steps[i]; + let current_steps = self.current_steps(); + + while i < current_steps.len() && y < bottom { + let step = ¤t_steps[i]; let max_h = bottom.saturating_sub(y); if max_h == 0 || width == 0 { break; @@ -135,10 +211,22 @@ impl WidgetRef for &OnboardingScreen { } impl KeyboardHandler for Step { - fn handle_key_event(&mut self, key_event: KeyEvent) -> KeyEventResult { + fn handle_key_event(&mut self, key_event: KeyEvent) { match self { - Step::Welcome(_) => KeyEventResult::None, + Step::Welcome(_) | Step::ContinueToChat(_) => (), Step::Auth(widget) => widget.handle_key_event(key_event), + Step::GitWarning(widget) => widget.handle_key_event(key_event), + } + } +} + +impl StepStateProvider for Step { + fn get_step_state(&self) -> StepState { + match self { + Step::Welcome(w) => w.get_step_state(), + Step::Auth(w) => w.get_step_state(), + Step::GitWarning(w) => w.get_step_state(), + Step::ContinueToChat(w) => w.get_step_state(), } } } @@ -152,6 +240,12 @@ impl WidgetRef for Step { Step::Auth(widget) => { widget.render_ref(area, buf); } + Step::GitWarning(widget) => { + widget.render_ref(area, buf); + } + Step::ContinueToChat(widget) => { + widget.render_ref(area, buf); + } } } } diff --git a/codex-rs/tui/src/onboarding/welcome.rs b/codex-rs/tui/src/onboarding/welcome.rs index e00e3004b6..a35f6528ab 100644 --- a/codex-rs/tui/src/onboarding/welcome.rs +++ b/codex-rs/tui/src/onboarding/welcome.rs @@ -7,7 +7,13 @@ use ratatui::text::Line; use ratatui::text::Span; use ratatui::widgets::WidgetRef; -pub(crate) struct WelcomeWidget {} +use crate::onboarding::onboarding_screen::StepStateProvider; + +use super::onboarding_screen::StepState; + +pub(crate) struct WelcomeWidget { + pub is_logged_in: bool, +} impl WidgetRef for &WelcomeWidget { fn render_ref(&self, area: Rect, buf: &mut Buffer) { @@ -21,3 +27,12 @@ impl WidgetRef for &WelcomeWidget { line.render(area, buf); } } + +impl StepStateProvider for WelcomeWidget { + fn get_step_state(&self) -> StepState { + match self.is_logged_in { + true => StepState::Hidden, + false => StepState::Complete, + } + } +} From 4971d54ca7b193dfb66b634c8f9dc9185a1dbb53 Mon Sep 17 00:00:00 2001 From: aibrahim-oai Date: Wed, 6 Aug 2025 21:20:09 -0700 Subject: [PATCH 2/3] Show timing and token counts in status indicator (#1909) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - track start time and cumulative tokens in status indicator - display dim "(Ns • N tokens • Ctrl z to interrupt)" text after animated Working header - propagate token usage updates to status indicator views https://github.com/user-attachments/assets/b73210c1-1533-40b5-b6c2-3c640029fd54 ## Testing - `just fmt` - `just fix` *(fails: let expressions in this position are unstable)* - `cargo test --all-features` *(fails: let expressions in this position are unstable)* ------ https://chatgpt.com/codex/tasks/task_i_6893ec0d74a883218b94005172d7bc4c --- codex-rs/tui/src/status_indicator_widget.rs | 56 +++++++++++++++------ 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index fad7e41a39..8cca0fb04d 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -7,6 +7,7 @@ use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; use std::thread; use std::time::Duration; +use std::time::Instant; use ratatui::buffer::Buffer; use ratatui::layout::Rect; @@ -42,6 +43,7 @@ pub(crate) struct StatusIndicatorWidget { frame_idx: Arc, running: Arc, + start_time: Instant, // Keep one sender alive to prevent the channel from closing while the // animation thread is still running. The field itself is currently not // accessed anywhere, therefore the leading underscore silences the @@ -78,6 +80,7 @@ impl StatusIndicatorWidget { reveal_len_at_base: 0, frame_idx, running, + start_time: Instant::now(), _app_event_tx: app_event_tx, } @@ -167,11 +170,13 @@ impl WidgetRef for StatusIndicatorWidget { return; } - // Build animated gradient header for the word "Working". let idx = self.frame_idx.load(std::sync::atomic::Ordering::Relaxed); - let header_text = "Working"; - let header_chars: Vec = header_text.chars().collect(); - let padding = 4usize; // virtual padding around the word for smoother loop + let elapsed = self.start_time.elapsed().as_secs(); + let shown_now = self.current_shown_len(idx); + let status_prefix: String = self.text.chars().take(shown_now).collect(); + let animated_text = "Working"; + let header_chars: Vec = animated_text.chars().collect(); + let padding = 4usize; // virtual padding around the animated segment for smoother loop let period = header_chars.len() + padding * 2; let pos = idx % period; let has_true_color = supports_color::on_cached(supports_color::Stream::Stdout) @@ -179,7 +184,7 @@ impl WidgetRef for StatusIndicatorWidget { .unwrap_or(false); let band_half_width = 2.0; // width of the bright band in characters - let mut header_spans: Vec> = Vec::new(); + let mut animated_spans: Vec> = Vec::new(); for (i, ch) in header_chars.iter().enumerate() { let i_pos = i as isize + padding as isize; let pos = pos as isize; @@ -199,28 +204,49 @@ impl WidgetRef for StatusIndicatorWidget { .fg(Color::Rgb(level, level, level)) .add_modifier(Modifier::BOLD) } else { - // Bold makes dark gray and gray look the same, so don't use it when true color is not supported. Style::default().fg(color_for_level(level)) }; - header_spans.push(Span::styled(ch.to_string(), style)); + animated_spans.push(Span::styled(ch.to_string(), style)); } // Plain rendering: no borders or padding so the live cell is visually indistinguishable from terminal scrollback. let inner_width = area.width as usize; - // Compose a single status line like: "▌ Working [•] waiting for model" + // Compose a single status line like: "▌ Working (Xs • Ctrl z to interrupt) " let mut spans: Vec> = Vec::new(); spans.push(Span::styled("▌ ", Style::default().fg(Color::Cyan))); - // Gradient header - spans.extend(header_spans); - // Space after header + // Animated header after the left bar + spans.extend(animated_spans); + // Space between header and bracket block + spans.push(Span::raw(" ")); + // Non-animated, dim bracket content, with only "Ctrl z" bold + let bracket_prefix = format!("({elapsed}s • "); spans.push(Span::styled( - " ", - Style::default() - .fg(Color::White) - .add_modifier(Modifier::BOLD), + bracket_prefix, + Style::default().fg(Color::Gray).add_modifier(Modifier::DIM), )); + spans.push(Span::styled( + "Ctrl z", + Style::default() + .fg(Color::Gray) + .add_modifier(Modifier::DIM | Modifier::BOLD), + )); + spans.push(Span::styled( + " to interrupt)", + Style::default().fg(Color::Gray).add_modifier(Modifier::DIM), + )); + // Add a space and then the log text (not animated by the gradient) + if !status_prefix.is_empty() { + spans.push(Span::styled( + " ", + Style::default().fg(Color::Gray).add_modifier(Modifier::DIM), + )); + spans.push(Span::styled( + status_prefix, + Style::default().fg(Color::Gray).add_modifier(Modifier::DIM), + )); + } // Truncate spans to fit the width. let mut acc: Vec> = Vec::new(); From 2098b4036994878fffb221459bebd96648375285 Mon Sep 17 00:00:00 2001 From: easong-openai Date: Wed, 6 Aug 2025 21:23:09 -0700 Subject: [PATCH 3/3] Scrollable slash commands (#1830) Scrollable slash commands. Part 1 of the multi PR. --- codex-rs/common/src/fuzzy_match.rs | 177 ++++++++++++++++++ codex-rs/common/src/lib.rs | 2 + codex-rs/tui/src/bottom_pane/command_popup.rs | 173 ++++++----------- .../tui/src/bottom_pane/file_search_popup.rs | 129 ++++--------- codex-rs/tui/src/bottom_pane/mod.rs | 3 + codex-rs/tui/src/bottom_pane/popup_consts.rs | 5 + codex-rs/tui/src/bottom_pane/scroll_state.rs | 115 ++++++++++++ .../src/bottom_pane/selection_popup_common.rs | 126 +++++++++++++ 8 files changed, 523 insertions(+), 207 deletions(-) create mode 100644 codex-rs/common/src/fuzzy_match.rs create mode 100644 codex-rs/tui/src/bottom_pane/popup_consts.rs create mode 100644 codex-rs/tui/src/bottom_pane/scroll_state.rs create mode 100644 codex-rs/tui/src/bottom_pane/selection_popup_common.rs diff --git a/codex-rs/common/src/fuzzy_match.rs b/codex-rs/common/src/fuzzy_match.rs new file mode 100644 index 0000000000..836848d6a4 --- /dev/null +++ b/codex-rs/common/src/fuzzy_match.rs @@ -0,0 +1,177 @@ +/// Simple case-insensitive subsequence matcher used for fuzzy filtering. +/// +/// Returns the indices (character positions) of the matched characters in the +/// ORIGINAL `haystack` string and a score where smaller is better. +/// +/// Unicode correctness: we perform the match on a lowercased copy of the +/// haystack and needle but maintain a mapping from each character in the +/// lowercased haystack back to the original character index in `haystack`. +/// This ensures the returned indices can be safely used with +/// `str::chars().enumerate()` consumers for highlighting, even when +/// lowercasing expands certain characters (e.g., ß → ss, İ → i̇). +pub fn fuzzy_match(haystack: &str, needle: &str) -> Option<(Vec, i32)> { + if needle.is_empty() { + return Some((Vec::new(), i32::MAX)); + } + + let mut lowered_chars: Vec = Vec::new(); + let mut lowered_to_orig_char_idx: Vec = Vec::new(); + for (orig_idx, ch) in haystack.chars().enumerate() { + for lc in ch.to_lowercase() { + lowered_chars.push(lc); + lowered_to_orig_char_idx.push(orig_idx); + } + } + + let lowered_needle: Vec = needle.to_lowercase().chars().collect(); + + let mut result_orig_indices: Vec = Vec::with_capacity(lowered_needle.len()); + let mut last_lower_pos: Option = None; + let mut cur = 0usize; + for &nc in lowered_needle.iter() { + let mut found_at: Option = None; + while cur < lowered_chars.len() { + if lowered_chars[cur] == nc { + found_at = Some(cur); + cur += 1; + break; + } + cur += 1; + } + let pos = found_at?; + result_orig_indices.push(lowered_to_orig_char_idx[pos]); + last_lower_pos = Some(pos); + } + + let first_lower_pos = if result_orig_indices.is_empty() { + 0usize + } else { + let target_orig = result_orig_indices[0]; + lowered_to_orig_char_idx + .iter() + .position(|&oi| oi == target_orig) + .unwrap_or(0) + }; + // last defaults to first for single-hit; score = extra span between first/last hit + // minus needle len (≥0). + // Strongly reward prefix matches by subtracting 100 when the first hit is at index 0. + let last_lower_pos = last_lower_pos.unwrap_or(first_lower_pos); + let window = + (last_lower_pos as i32 - first_lower_pos as i32 + 1) - (lowered_needle.len() as i32); + let mut score = window.max(0); + if first_lower_pos == 0 { + score -= 100; + } + + result_orig_indices.sort_unstable(); + result_orig_indices.dedup(); + Some((result_orig_indices, score)) +} + +/// Convenience wrapper to get only the indices for a fuzzy match. +pub fn fuzzy_indices(haystack: &str, needle: &str) -> Option> { + fuzzy_match(haystack, needle).map(|(mut idx, _)| { + idx.sort_unstable(); + idx.dedup(); + idx + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ascii_basic_indices() { + let (idx, score) = match fuzzy_match("hello", "hl") { + Some(v) => v, + None => panic!("expected a match"), + }; + assert_eq!(idx, vec![0, 2]); + // 'h' at 0, 'l' at 2 -> window 1; start-of-string bonus applies (-100) + assert_eq!(score, -99); + } + + #[test] + fn unicode_dotted_i_istanbul_highlighting() { + let (idx, score) = match fuzzy_match("İstanbul", "is") { + Some(v) => v, + None => panic!("expected a match"), + }; + assert_eq!(idx, vec![0, 1]); + // Matches at lowered positions 0 and 2 -> window 1; start-of-string bonus applies + assert_eq!(score, -99); + } + + #[test] + fn unicode_german_sharp_s_casefold() { + assert!(fuzzy_match("straße", "strasse").is_none()); + } + + #[test] + fn prefer_contiguous_match_over_spread() { + let (_idx_a, score_a) = match fuzzy_match("abc", "abc") { + Some(v) => v, + None => panic!("expected a match"), + }; + let (_idx_b, score_b) = match fuzzy_match("a-b-c", "abc") { + Some(v) => v, + None => panic!("expected a match"), + }; + // Contiguous window -> 0; start-of-string bonus -> -100 + assert_eq!(score_a, -100); + // Spread over 5 chars for 3-letter needle -> window 2; with bonus -> -98 + assert_eq!(score_b, -98); + assert!(score_a < score_b); + } + + #[test] + fn start_of_string_bonus_applies() { + let (_idx_a, score_a) = match fuzzy_match("file_name", "file") { + Some(v) => v, + None => panic!("expected a match"), + }; + let (_idx_b, score_b) = match fuzzy_match("my_file_name", "file") { + Some(v) => v, + None => panic!("expected a match"), + }; + // Start-of-string contiguous -> window 0; bonus -> -100 + assert_eq!(score_a, -100); + // Non-prefix contiguous -> window 0; no bonus -> 0 + assert_eq!(score_b, 0); + assert!(score_a < score_b); + } + + #[test] + fn empty_needle_matches_with_max_score_and_no_indices() { + let (idx, score) = match fuzzy_match("anything", "") { + Some(v) => v, + None => panic!("empty needle should match"), + }; + assert!(idx.is_empty()); + assert_eq!(score, i32::MAX); + } + + #[test] + fn case_insensitive_matching_basic() { + let (idx, score) = match fuzzy_match("FooBar", "foO") { + Some(v) => v, + None => panic!("expected a match"), + }; + assert_eq!(idx, vec![0, 1, 2]); + // Contiguous prefix match (case-insensitive) -> window 0 with bonus + assert_eq!(score, -100); + } + + #[test] + fn indices_are_deduped_for_multichar_lowercase_expansion() { + let needle = "\u{0069}\u{0307}"; // "i" + combining dot above + let (idx, score) = match fuzzy_match("İ", needle) { + Some(v) => v, + None => panic!("expected a match"), + }; + assert_eq!(idx, vec![0]); + // Lowercasing 'İ' expands to two chars; contiguous prefix -> window 0 with bonus + assert_eq!(score, -100); + } +} diff --git a/codex-rs/common/src/lib.rs b/codex-rs/common/src/lib.rs index 38f3832bfd..8595262cc0 100644 --- a/codex-rs/common/src/lib.rs +++ b/codex-rs/common/src/lib.rs @@ -27,3 +27,5 @@ pub use sandbox_summary::summarize_sandbox_policy; mod config_summary; pub use config_summary::create_config_summary_entries; +// Shared fuzzy matcher (used by TUI selection popups and other UI filtering) +pub mod fuzzy_match; diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index 1027df1a67..b7a203e9fd 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -1,30 +1,19 @@ use ratatui::buffer::Buffer; use ratatui::layout::Rect; -use ratatui::style::Color; -use ratatui::style::Style; -use ratatui::style::Stylize; -use ratatui::symbols::border::QUADRANT_LEFT_HALF; -use ratatui::text::Line; -use ratatui::text::Span; -use ratatui::widgets::Cell; -use ratatui::widgets::Row; -use ratatui::widgets::Table; -use ratatui::widgets::Widget; use ratatui::widgets::WidgetRef; +use super::popup_consts::MAX_POPUP_ROWS; +use super::scroll_state::ScrollState; +use super::selection_popup_common::GenericDisplayRow; +use super::selection_popup_common::render_rows; use crate::slash_command::SlashCommand; use crate::slash_command::built_in_slash_commands; - -const MAX_POPUP_ROWS: usize = 5; -/// Ideally this is enough to show the longest command name. -const FIRST_COLUMN_WIDTH: u16 = 20; - -use ratatui::style::Modifier; +use codex_common::fuzzy_match::fuzzy_match; pub(crate) struct CommandPopup { command_filter: String, all_commands: Vec<(&'static str, SlashCommand)>, - selected_idx: Option, + state: ScrollState, } impl CommandPopup { @@ -32,7 +21,7 @@ impl CommandPopup { Self { command_filter: String::new(), all_commands: built_in_slash_commands(), - selected_idx: None, + state: ScrollState::new(), } } @@ -62,130 +51,84 @@ impl CommandPopup { // Reset or clamp selected index based on new filtered list. let matches_len = self.filtered_commands().len(); - self.selected_idx = match matches_len { - 0 => None, - _ => Some(self.selected_idx.unwrap_or(0).min(matches_len - 1)), - }; + self.state.clamp_selection(matches_len); + self.state + .ensure_visible(matches_len, MAX_POPUP_ROWS.min(matches_len)); } /// Determine the preferred height of the popup. This is the number of - /// rows required to show **at most** `MAX_POPUP_ROWS` commands plus the - /// table/border overhead (one line at the top and one at the bottom). + /// rows required to show at most MAX_POPUP_ROWS commands. pub(crate) fn calculate_required_height(&self) -> u16 { self.filtered_commands().len().clamp(1, MAX_POPUP_ROWS) as u16 } - /// Return the list of commands that match the current filter. Matching is - /// performed using a *prefix* comparison on the command name. - fn filtered_commands(&self) -> Vec<&SlashCommand> { - self.all_commands - .iter() - .filter_map(|(_name, cmd)| { - if self.command_filter.is_empty() - || cmd - .command() - .starts_with(&self.command_filter.to_ascii_lowercase()) - { - Some(cmd) - } else { - None + /// Compute fuzzy-filtered matches paired with optional highlight indices and score. + /// Sorted by ascending score, then by command name for stability. + fn filtered(&self) -> Vec<(&SlashCommand, Option>, i32)> { + let filter = self.command_filter.trim(); + let mut out: Vec<(&SlashCommand, Option>, i32)> = Vec::new(); + if filter.is_empty() { + for (_, cmd) in self.all_commands.iter() { + out.push((cmd, None, 0)); + } + } else { + for (_, cmd) in self.all_commands.iter() { + if let Some((indices, score)) = fuzzy_match(cmd.command(), filter) { + out.push((cmd, Some(indices), score)); } - }) - .collect::>() + } + } + out.sort_by(|a, b| a.2.cmp(&b.2).then_with(|| a.0.command().cmp(b.0.command()))); + out + } + + fn filtered_commands(&self) -> Vec<&SlashCommand> { + self.filtered().into_iter().map(|(c, _, _)| c).collect() } /// Move the selection cursor one step up. pub(crate) fn move_up(&mut self) { - if let Some(len) = self.filtered_commands().len().checked_sub(1) { - if len == usize::MAX { - return; - } - } - - if let Some(idx) = self.selected_idx { - if idx > 0 { - self.selected_idx = Some(idx - 1); - } - } else if !self.filtered_commands().is_empty() { - self.selected_idx = Some(0); - } + let matches = self.filtered_commands(); + let len = matches.len(); + self.state.move_up_wrap(len); + self.state.ensure_visible(len, MAX_POPUP_ROWS.min(len)); } /// Move the selection cursor one step down. pub(crate) fn move_down(&mut self) { - let matches_len = self.filtered_commands().len(); - if matches_len == 0 { - self.selected_idx = None; - return; - } - - match self.selected_idx { - Some(idx) if idx + 1 < matches_len => { - self.selected_idx = Some(idx + 1); - } - None => { - self.selected_idx = Some(0); - } - _ => {} - } + let matches = self.filtered_commands(); + let matches_len = matches.len(); + self.state.move_down_wrap(matches_len); + self.state + .ensure_visible(matches_len, MAX_POPUP_ROWS.min(matches_len)); } /// Return currently selected command, if any. pub(crate) fn selected_command(&self) -> Option<&SlashCommand> { let matches = self.filtered_commands(); - self.selected_idx.and_then(|idx| matches.get(idx).copied()) + self.state + .selected_idx + .and_then(|idx| matches.get(idx).copied()) } } impl WidgetRef for CommandPopup { fn render_ref(&self, area: Rect, buf: &mut Buffer) { - let matches = self.filtered_commands(); - - let mut rows: Vec = Vec::new(); - let visible_matches: Vec<&SlashCommand> = - matches.into_iter().take(MAX_POPUP_ROWS).collect(); - - if visible_matches.is_empty() { - rows.push(Row::new(vec![ - Cell::from(""), - Cell::from("No matching commands").add_modifier(Modifier::ITALIC), - ])); + let matches = self.filtered(); + let rows_all: Vec = if matches.is_empty() { + Vec::new() } else { - let default_style = Style::default(); - let command_style = Style::default().fg(Color::LightBlue); - for (idx, cmd) in visible_matches.iter().enumerate() { - rows.push(Row::new(vec![ - Cell::from(Line::from(vec![ - if Some(idx) == self.selected_idx { - Span::styled( - "›", - Style::default().bg(Color::DarkGray).fg(Color::LightCyan), - ) - } else { - Span::styled(QUADRANT_LEFT_HALF, Style::default().fg(Color::DarkGray)) - }, - Span::styled(format!("/{}", cmd.command()), command_style), - ])), - Cell::from(cmd.description().to_string()).style(default_style), - ])); - } - } - - use ratatui::layout::Constraint; - - let table = Table::new( - rows, - [Constraint::Length(FIRST_COLUMN_WIDTH), Constraint::Min(10)], - ) - .column_spacing(0); - // .block( - // Block::default() - // .borders(Borders::LEFT) - // .border_type(BorderType::QuadrantOutside) - // .border_style(Style::default().fg(Color::DarkGray)), - // ); - - table.render(area, buf); + matches + .into_iter() + .map(|(cmd, indices, _)| GenericDisplayRow { + name: format!("/{}", cmd.command()), + match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()), + is_current: false, + description: Some(cmd.description().to_string()), + }) + .collect() + }; + render_rows(area, buf, &rows_all, &self.state, MAX_POPUP_ROWS); } } diff --git a/codex-rs/tui/src/bottom_pane/file_search_popup.rs b/codex-rs/tui/src/bottom_pane/file_search_popup.rs index ac6c91cf47..c30a24f984 100644 --- a/codex-rs/tui/src/bottom_pane/file_search_popup.rs +++ b/codex-rs/tui/src/bottom_pane/file_search_popup.rs @@ -1,23 +1,12 @@ use codex_file_search::FileMatch; use ratatui::buffer::Buffer; use ratatui::layout::Rect; -use ratatui::prelude::Constraint; -use ratatui::style::Color; -use ratatui::style::Modifier; -use ratatui::style::Style; -use ratatui::text::Line; -use ratatui::text::Span; -use ratatui::widgets::Block; -use ratatui::widgets::BorderType; -use ratatui::widgets::Borders; -use ratatui::widgets::Cell; -use ratatui::widgets::Row; -use ratatui::widgets::Table; -use ratatui::widgets::Widget; use ratatui::widgets::WidgetRef; -/// Maximum number of suggestions shown in the popup. -const MAX_RESULTS: usize = 8; +use super::popup_consts::MAX_POPUP_ROWS; +use super::scroll_state::ScrollState; +use super::selection_popup_common::GenericDisplayRow; +use super::selection_popup_common::render_rows; /// Visual state for the file-search popup. pub(crate) struct FileSearchPopup { @@ -30,8 +19,8 @@ pub(crate) struct FileSearchPopup { waiting: bool, /// Cached matches; paths relative to the search dir. matches: Vec, - /// Currently selected index inside `matches` (if any). - selected_idx: Option, + /// Shared selection/scroll state. + state: ScrollState, } impl FileSearchPopup { @@ -41,7 +30,7 @@ impl FileSearchPopup { pending_query: String::new(), waiting: true, matches: Vec::new(), - selected_idx: None, + state: ScrollState::new(), } } @@ -61,7 +50,7 @@ impl FileSearchPopup { if !keep_existing { self.matches.clear(); - self.selected_idx = None; + self.state.reset(); } } @@ -75,40 +64,32 @@ impl FileSearchPopup { self.display_query = query.to_string(); self.matches = matches; self.waiting = false; - self.selected_idx = if self.matches.is_empty() { - None - } else { - Some(0) - }; + let len = self.matches.len(); + self.state.clamp_selection(len); + self.state.ensure_visible(len, len.min(MAX_POPUP_ROWS)); } /// Move selection cursor up. pub(crate) fn move_up(&mut self) { - if let Some(idx) = self.selected_idx { - if idx > 0 { - self.selected_idx = Some(idx - 1); - } - } + let len = self.matches.len(); + self.state.move_up_wrap(len); + self.state.ensure_visible(len, len.min(MAX_POPUP_ROWS)); } /// Move selection cursor down. pub(crate) fn move_down(&mut self) { - if let Some(idx) = self.selected_idx { - if idx + 1 < self.matches.len() { - self.selected_idx = Some(idx + 1); - } - } else if !self.matches.is_empty() { - self.selected_idx = Some(0); - } + let len = self.matches.len(); + self.state.move_down_wrap(len); + self.state.ensure_visible(len, len.min(MAX_POPUP_ROWS)); } pub(crate) fn selected_match(&self) -> Option<&str> { - self.selected_idx + self.state + .selected_idx .and_then(|idx| self.matches.get(idx)) .map(|file_match| file_match.path.as_str()) } - /// Preferred height (rows) including border. pub(crate) fn calculate_required_height(&self) -> u16 { // Row count depends on whether we already have matches. If no matches // yet (e.g. initial search or query with no results) reserve a single @@ -116,71 +97,35 @@ impl FileSearchPopup { // up to MAX_RESULTS regardless of the waiting flag so the list // remains stable while a newer search is in-flight. - self.matches.len().clamp(1, MAX_RESULTS) as u16 + self.matches.len().clamp(1, MAX_POPUP_ROWS) as u16 } } impl WidgetRef for &FileSearchPopup { fn render_ref(&self, area: Rect, buf: &mut Buffer) { - // Prepare rows. - let rows: Vec = if self.matches.is_empty() { - vec![Row::new(vec![ - Cell::from(if self.waiting { - "(searching …)" - } else { - "no matches" - }) - .style(Style::new().add_modifier(Modifier::ITALIC | Modifier::DIM)), - ])] + // Convert matches to GenericDisplayRow, translating indices to usize at the UI boundary. + let rows_all: Vec = if self.matches.is_empty() { + Vec::new() } else { self.matches .iter() - .take(MAX_RESULTS) - .enumerate() - .map(|(i, file_match)| { - let FileMatch { path, indices, .. } = file_match; - let path = path.as_str(); - #[allow(clippy::expect_used)] - let indices = indices.as_ref().expect("indices should be present"); - - // Build spans with bold on matching indices. - let mut idx_iter = indices.iter().peekable(); - let mut spans: Vec = Vec::with_capacity(path.len()); - - for (char_idx, ch) in path.chars().enumerate() { - let mut style = Style::default(); - if idx_iter - .peek() - .is_some_and(|next| **next == char_idx as u32) - { - idx_iter.next(); - style = style.add_modifier(Modifier::BOLD); - } - spans.push(Span::styled(ch.to_string(), style)); - } - - // Create cell from the spans. - let mut cell = Cell::from(Line::from(spans)); - - // If selected, also paint yellow. - if Some(i) == self.selected_idx { - cell = cell.style(Style::default().fg(Color::Yellow)); - } - - Row::new(vec![cell]) + .map(|m| GenericDisplayRow { + name: m.path.clone(), + match_indices: m + .indices + .as_ref() + .map(|v| v.iter().map(|&i| i as usize).collect()), + is_current: false, + description: None, }) .collect() }; - let table = Table::new(rows, vec![Constraint::Percentage(100)]) - .block( - Block::default() - .borders(Borders::LEFT) - .border_type(BorderType::QuadrantOutside) - .border_style(Style::default().fg(Color::DarkGray)), - ) - .widths([Constraint::Percentage(100)]); - - table.render(area, buf); + if self.waiting && rows_all.is_empty() { + // Render a minimal waiting stub using the shared renderer (no rows -> "no matches"). + render_rows(area, buf, &[], &self.state, MAX_POPUP_ROWS); + } else { + render_rows(area, buf, &rows_all, &self.state, MAX_POPUP_ROWS); + } } } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index cdb01ba06a..ff3cf2f2c4 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -19,6 +19,9 @@ mod chat_composer_history; mod command_popup; mod file_search_popup; mod live_ring_widget; +mod popup_consts; +mod scroll_state; +mod selection_popup_common; mod status_indicator_view; mod textarea; diff --git a/codex-rs/tui/src/bottom_pane/popup_consts.rs b/codex-rs/tui/src/bottom_pane/popup_consts.rs new file mode 100644 index 0000000000..5f447d735c --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/popup_consts.rs @@ -0,0 +1,5 @@ +//! Shared popup-related constants for bottom pane widgets. + +/// Maximum number of rows any popup should attempt to display. +/// Keep this consistent across all popups for a uniform feel. +pub(crate) const MAX_POPUP_ROWS: usize = 8; diff --git a/codex-rs/tui/src/bottom_pane/scroll_state.rs b/codex-rs/tui/src/bottom_pane/scroll_state.rs new file mode 100644 index 0000000000..a9728d1a0d --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/scroll_state.rs @@ -0,0 +1,115 @@ +/// Generic scroll/selection state for a vertical list menu. +/// +/// Encapsulates the common behavior of a selectable list that supports: +/// - Optional selection (None when list is empty) +/// - Wrap-around navigation on Up/Down +/// - Maintaining a scroll window (`scroll_top`) so the selected row stays visible +#[derive(Debug, Default, Clone, Copy)] +pub(crate) struct ScrollState { + pub selected_idx: Option, + pub scroll_top: usize, +} + +impl ScrollState { + pub fn new() -> Self { + Self { + selected_idx: None, + scroll_top: 0, + } + } + + /// Reset selection and scroll. + pub fn reset(&mut self) { + self.selected_idx = None; + self.scroll_top = 0; + } + + /// Clamp selection to be within the [0, len-1] range, or None when empty. + pub fn clamp_selection(&mut self, len: usize) { + self.selected_idx = match len { + 0 => None, + _ => Some(self.selected_idx.unwrap_or(0).min(len - 1)), + }; + if len == 0 { + self.scroll_top = 0; + } + } + + /// Move selection up by one, wrapping to the bottom when necessary. + pub fn move_up_wrap(&mut self, len: usize) { + if len == 0 { + self.selected_idx = None; + self.scroll_top = 0; + return; + } + self.selected_idx = Some(match self.selected_idx { + Some(idx) if idx > 0 => idx - 1, + Some(_) => len - 1, + None => 0, + }); + } + + /// Move selection down by one, wrapping to the top when necessary. + pub fn move_down_wrap(&mut self, len: usize) { + if len == 0 { + self.selected_idx = None; + self.scroll_top = 0; + return; + } + self.selected_idx = Some(match self.selected_idx { + Some(idx) if idx + 1 < len => idx + 1, + _ => 0, + }); + } + + /// Adjust `scroll_top` so that the current `selected_idx` is visible within + /// the window of `visible_rows`. + pub fn ensure_visible(&mut self, len: usize, visible_rows: usize) { + if len == 0 || visible_rows == 0 { + self.scroll_top = 0; + return; + } + if let Some(sel) = self.selected_idx { + if sel < self.scroll_top { + self.scroll_top = sel; + } else { + let bottom = self.scroll_top + visible_rows - 1; + if sel > bottom { + self.scroll_top = sel + 1 - visible_rows; + } + } + } else { + self.scroll_top = 0; + } + } +} + +#[cfg(test)] +mod tests { + use super::ScrollState; + + #[test] + fn wrap_navigation_and_visibility() { + let mut s = ScrollState::new(); + let len = 10; + let vis = 5; + + s.clamp_selection(len); + assert_eq!(s.selected_idx, Some(0)); + s.ensure_visible(len, vis); + assert_eq!(s.scroll_top, 0); + + s.move_up_wrap(len); + s.ensure_visible(len, vis); + assert_eq!(s.selected_idx, Some(len - 1)); + match s.selected_idx { + Some(sel) => assert!(s.scroll_top <= sel), + None => panic!("expected Some(selected_idx) after wrap"), + } + + s.move_down_wrap(len); + s.ensure_visible(len, vis); + assert_eq!(s.selected_idx, Some(0)); + assert_eq!(s.scroll_top, 0); + } +} diff --git a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs new file mode 100644 index 0000000000..1a31115d77 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs @@ -0,0 +1,126 @@ +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use ratatui::prelude::Constraint; +use ratatui::style::Color; +use ratatui::style::Modifier; +use ratatui::style::Style; +use ratatui::text::Line; +use ratatui::text::Span; +use ratatui::widgets::Block; +use ratatui::widgets::BorderType; +use ratatui::widgets::Borders; +use ratatui::widgets::Cell; +use ratatui::widgets::Row; +use ratatui::widgets::Table; +use ratatui::widgets::Widget; + +use super::scroll_state::ScrollState; + +/// A generic representation of a display row for selection popups. +pub(crate) struct GenericDisplayRow { + pub name: String, + pub match_indices: Option>, // indices to bold (char positions) + pub is_current: bool, + pub description: Option, // optional grey text after the name +} + +impl GenericDisplayRow {} + +/// Render a list of rows using the provided ScrollState, with shared styling +/// and behavior for selection popups. +pub(crate) fn render_rows( + area: Rect, + buf: &mut Buffer, + rows_all: &[GenericDisplayRow], + state: &ScrollState, + max_results: usize, +) { + let mut rows: Vec = Vec::new(); + if rows_all.is_empty() { + rows.push(Row::new(vec![Cell::from(Line::from(Span::styled( + "no matches", + Style::default().add_modifier(Modifier::ITALIC | Modifier::DIM), + )))])); + } else { + let max_rows_from_area = area.height as usize; + let visible_rows = max_results + .min(rows_all.len()) + .min(max_rows_from_area.max(1)); + + // Compute starting index based on scroll state and selection. + let mut start_idx = state.scroll_top.min(rows_all.len().saturating_sub(1)); + if let Some(sel) = state.selected_idx { + if sel < start_idx { + start_idx = sel; + } else if visible_rows > 0 { + let bottom = start_idx + visible_rows - 1; + if sel > bottom { + start_idx = sel + 1 - visible_rows; + } + } + } + + for (i, row) in rows_all + .iter() + .enumerate() + .skip(start_idx) + .take(visible_rows) + { + let GenericDisplayRow { + name, + match_indices, + is_current, + description, + } = row; + + // Highlight fuzzy indices when present. + let mut spans: Vec = Vec::with_capacity(name.len()); + if let Some(idxs) = match_indices.as_ref() { + let mut idx_iter = idxs.iter().peekable(); + for (char_idx, ch) in name.chars().enumerate() { + let mut style = Style::default(); + if idx_iter.peek().is_some_and(|next| **next == char_idx) { + idx_iter.next(); + style = style.add_modifier(Modifier::BOLD); + } + spans.push(Span::styled(ch.to_string(), style)); + } + } else { + spans.push(Span::raw(name.clone())); + } + + if let Some(desc) = description.as_ref() { + spans.push(Span::raw(" ")); + spans.push(Span::styled( + desc.clone(), + Style::default() + .fg(Color::DarkGray) + .add_modifier(Modifier::DIM), + )); + } + + let mut cell = Cell::from(Line::from(spans)); + if Some(i) == state.selected_idx { + cell = cell.style( + Style::default() + .fg(Color::Yellow) + .add_modifier(Modifier::BOLD), + ); + } else if *is_current { + cell = cell.style(Style::default().fg(Color::Cyan)); + } + rows.push(Row::new(vec![cell])); + } + } + + let table = Table::new(rows, vec![Constraint::Percentage(100)]) + .block( + Block::default() + .borders(Borders::LEFT) + .border_type(BorderType::QuadrantOutside) + .border_style(Style::default().fg(Color::DarkGray)), + ) + .widths([Constraint::Percentage(100)]); + + table.render(area, buf); +}