diff --git a/codex-rs/core/src/git_info.rs b/codex-rs/core/src/git_info.rs index ca62ad49..832b28f1 100644 --- a/codex-rs/core/src/git_info.rs +++ b/codex-rs/core/src/git_info.rs @@ -108,6 +108,61 @@ pub async fn collect_git_info(cwd: &Path) -> Option { Some(git_info) } +/// A minimal commit summary entry used for pickers (subject + timestamp + sha). +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct CommitLogEntry { + pub sha: String, + /// Unix timestamp (seconds since epoch) of the commit time (committer time). + pub timestamp: i64, + /// Single-line subject of the commit message. + pub subject: String, +} + +/// Return the last `limit` commits reachable from HEAD for the current branch. +/// Each entry contains the SHA, commit timestamp (seconds), and subject line. +/// Returns an empty vector if not in a git repo or on error/timeout. +pub async fn recent_commits(cwd: &Path, limit: usize) -> Vec { + // Ensure we're in a git repo first to avoid noisy errors. + let Some(out) = run_git_command_with_timeout(&["rev-parse", "--git-dir"], cwd).await else { + return Vec::new(); + }; + if !out.status.success() { + return Vec::new(); + } + + let fmt = "%H%x1f%ct%x1f%s"; // + let n = limit.max(1).to_string(); + let Some(log_out) = + run_git_command_with_timeout(&["log", "-n", &n, &format!("--pretty=format:{fmt}")], cwd) + .await + else { + return Vec::new(); + }; + if !log_out.status.success() { + return Vec::new(); + } + + let text = String::from_utf8_lossy(&log_out.stdout); + let mut entries: Vec = Vec::new(); + for line in text.lines() { + let mut parts = line.split('\u{001f}'); + let sha = parts.next().unwrap_or("").trim(); + let ts_s = parts.next().unwrap_or("").trim(); + let subject = parts.next().unwrap_or("").trim(); + if sha.is_empty() || ts_s.is_empty() { + continue; + } + let timestamp = ts_s.parse::().unwrap_or(0); + entries.push(CommitLogEntry { + sha: sha.to_string(), + timestamp, + subject: subject.to_string(), + }); + } + + entries +} + /// Returns the closest git sha to HEAD that is on a remote as well as the diff to that sha. pub async fn git_diff_to_remote(cwd: &Path) -> Option { get_git_repo_root(cwd)?; @@ -202,6 +257,11 @@ async fn get_default_branch(cwd: &Path) -> Option { } // No remote-derived default; try common local defaults if they exist + get_default_branch_local(cwd).await +} + +/// Attempt to determine the repository's default branch name from local branches. +async fn get_default_branch_local(cwd: &Path) -> Option { for candidate in ["main", "master"] { if let Some(verify) = run_git_command_with_timeout( &[ @@ -485,6 +545,46 @@ pub fn resolve_root_git_project_for_trust(cwd: &Path) -> Option { git_dir_path.parent().map(Path::to_path_buf) } +/// Returns a list of local git branches. +/// Includes the default branch at the beginning of the list, if it exists. +pub async fn local_git_branches(cwd: &Path) -> Vec { + let mut branches: Vec = if let Some(out) = + run_git_command_with_timeout(&["branch", "--format=%(refname:short)"], cwd).await + && out.status.success() + { + String::from_utf8_lossy(&out.stdout) + .lines() + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect() + } else { + Vec::new() + }; + + branches.sort_unstable(); + + if let Some(base) = get_default_branch_local(cwd).await + && let Some(pos) = branches.iter().position(|name| name == &base) + { + let base_branch = branches.remove(pos); + branches.insert(0, base_branch); + } + + branches +} + +/// Returns the current checked out branch name. +pub async fn current_branch_name(cwd: &Path) -> Option { + let out = run_git_command_with_timeout(&["branch", "--show-current"], cwd).await?; + if !out.status.success() { + return None; + } + String::from_utf8(out.stdout) + .ok() + .map(|s| s.trim().to_string()) + .filter(|name| !name.is_empty()) +} + #[cfg(test)] mod tests { use super::*; @@ -551,6 +651,80 @@ mod tests { repo_path } + #[tokio::test] + async fn test_recent_commits_non_git_directory_returns_empty() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let entries = recent_commits(temp_dir.path(), 10).await; + assert!(entries.is_empty(), "expected no commits outside a git repo"); + } + + #[tokio::test] + async fn test_recent_commits_orders_and_limits() { + use tokio::time::Duration; + use tokio::time::sleep; + + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let repo_path = create_test_git_repo(&temp_dir).await; + + // Make three distinct commits with small delays to ensure ordering by timestamp. + fs::write(repo_path.join("file.txt"), "one").unwrap(); + Command::new("git") + .args(["add", "file.txt"]) + .current_dir(&repo_path) + .output() + .await + .expect("git add"); + Command::new("git") + .args(["commit", "-m", "first change"]) + .current_dir(&repo_path) + .output() + .await + .expect("git commit 1"); + + sleep(Duration::from_millis(1100)).await; + + fs::write(repo_path.join("file.txt"), "two").unwrap(); + Command::new("git") + .args(["add", "file.txt"]) + .current_dir(&repo_path) + .output() + .await + .expect("git add 2"); + Command::new("git") + .args(["commit", "-m", "second change"]) + .current_dir(&repo_path) + .output() + .await + .expect("git commit 2"); + + sleep(Duration::from_millis(1100)).await; + + fs::write(repo_path.join("file.txt"), "three").unwrap(); + Command::new("git") + .args(["add", "file.txt"]) + .current_dir(&repo_path) + .output() + .await + .expect("git add 3"); + Command::new("git") + .args(["commit", "-m", "third change"]) + .current_dir(&repo_path) + .output() + .await + .expect("git commit 3"); + + // Request the latest 3 commits; should be our three changes in reverse time order. + let entries = recent_commits(&repo_path, 3).await; + assert_eq!(entries.len(), 3); + assert_eq!(entries[0].subject, "third change"); + assert_eq!(entries[1].subject, "second change"); + assert_eq!(entries[2].subject, "first change"); + // Basic sanity on SHA formatting + for e in entries { + assert!(e.sha.len() >= 7 && e.sha.chars().all(|c| c.is_ascii_hexdigit())); + } + } + async fn create_test_git_repo_with_remote(temp_dir: &TempDir) -> (PathBuf, String) { let repo_path = create_test_git_repo(temp_dir).await; let remote_path = temp_dir.path().join("remote.git"); diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index f6b735b1..138d65a0 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -354,6 +354,18 @@ impl App { AppEvent::UpdateSandboxPolicy(policy) => { self.chat_widget.set_sandbox_policy(policy); } + AppEvent::OpenReviewBranchPicker(cwd) => { + self.chat_widget.show_review_branch_picker(&cwd).await; + } + AppEvent::OpenReviewCommitPicker(cwd) => { + self.chat_widget.show_review_commit_picker(&cwd).await; + } + AppEvent::OpenReviewCustomPrompt => { + self.chat_widget.show_review_custom_prompt(); + } + AppEvent::OpenReviewPopup => { + self.chat_widget.open_review_popup(); + } } Ok(true) } diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 41992cdd..52e9393d 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -1,3 +1,5 @@ +use std::path::PathBuf; + use codex_core::protocol::ConversationPathResponseEvent; use codex_core::protocol::Event; use codex_file_search::FileMatch; @@ -65,4 +67,16 @@ pub(crate) enum AppEvent { /// Forwarded conversation history snapshot from the current conversation. ConversationHistory(ConversationPathResponseEvent), + + /// Open the branch picker option from the review popup. + OpenReviewBranchPicker(PathBuf), + + /// Open the commit picker option from the review popup. + OpenReviewCommitPicker(PathBuf), + + /// Open the custom prompt option from the review popup. + OpenReviewCustomPrompt, + + /// Open the top-level review presets popup. + OpenReviewPopup, } diff --git a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs index 794dd8c4..de1beaa2 100644 --- a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs +++ b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs @@ -28,6 +28,17 @@ pub(crate) trait BottomPaneView { /// Render the view: this will be displayed in place of the composer. fn render(&self, area: Rect, buf: &mut Buffer); + /// Optional paste handler. Return true if the view modified its state and + /// needs a redraw. + fn handle_paste(&mut self, _pane: &mut BottomPane, _pasted: String) -> bool { + false + } + + /// Cursor position when this view is active. + fn cursor_pos(&self, _area: Rect) -> Option<(u16, u16)> { + None + } + /// Try to handle approval request; return the original value if not /// consumed. fn try_consume_approval_request( diff --git a/codex-rs/tui/src/bottom_pane/custom_prompt_view.rs b/codex-rs/tui/src/bottom_pane/custom_prompt_view.rs new file mode 100644 index 00000000..4bad7077 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/custom_prompt_view.rs @@ -0,0 +1,254 @@ +use crossterm::event::KeyCode; +use crossterm::event::KeyEvent; +use crossterm::event::KeyModifiers; +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use ratatui::style::Stylize; +use ratatui::text::Line; +use ratatui::text::Span; +use ratatui::widgets::Clear; +use ratatui::widgets::Paragraph; +use ratatui::widgets::StatefulWidgetRef; +use ratatui::widgets::Widget; +use std::cell::RefCell; + +use super::popup_consts::STANDARD_POPUP_HINT_LINE; +use crate::app_event_sender::AppEventSender; +use crate::bottom_pane::SelectionAction; + +use super::CancellationEvent; +use super::bottom_pane_view::BottomPaneView; +use super::textarea::TextArea; +use super::textarea::TextAreaState; + +/// Callback invoked when the user submits a custom prompt. +pub(crate) type PromptSubmitted = Box; + +/// Minimal multi-line text input view to collect custom review instructions. +pub(crate) struct CustomPromptView { + title: String, + placeholder: String, + context_label: Option, + on_submit: PromptSubmitted, + app_event_tx: AppEventSender, + on_escape: Option, + + // UI state + textarea: TextArea, + textarea_state: RefCell, + complete: bool, +} + +impl CustomPromptView { + pub(crate) fn new( + title: String, + placeholder: String, + context_label: Option, + app_event_tx: AppEventSender, + on_escape: Option, + on_submit: PromptSubmitted, + ) -> Self { + Self { + title, + placeholder, + context_label, + on_submit, + app_event_tx, + on_escape, + textarea: TextArea::new(), + textarea_state: RefCell::new(TextAreaState::default()), + complete: false, + } + } +} + +impl BottomPaneView for CustomPromptView { + fn handle_key_event(&mut self, _pane: &mut super::BottomPane, key_event: KeyEvent) { + match key_event { + KeyEvent { + code: KeyCode::Esc, .. + } => { + self.on_ctrl_c(_pane); + } + KeyEvent { + code: KeyCode::Enter, + modifiers: KeyModifiers::NONE, + .. + } => { + let text = self.textarea.text().trim().to_string(); + if !text.is_empty() { + (self.on_submit)(text); + self.complete = true; + } + } + KeyEvent { + code: KeyCode::Enter, + .. + } => { + self.textarea.input(key_event); + } + other => { + self.textarea.input(other); + } + } + } + + fn on_ctrl_c(&mut self, _pane: &mut super::BottomPane) -> CancellationEvent { + self.complete = true; + if let Some(cb) = &self.on_escape { + cb(&self.app_event_tx); + } + CancellationEvent::Handled + } + + fn is_complete(&self) -> bool { + self.complete + } + + fn desired_height(&self, width: u16) -> u16 { + let extra_top: u16 = if self.context_label.is_some() { 1 } else { 0 }; + 1u16 + extra_top + self.input_height(width) + 3u16 + } + + fn render(&self, area: Rect, buf: &mut Buffer) { + if area.height == 0 || area.width == 0 { + return; + } + + let input_height = self.input_height(area.width); + + // Title line + let title_area = Rect { + x: area.x, + y: area.y, + width: area.width, + height: 1, + }; + let title_spans: Vec> = vec![gutter(), self.title.clone().bold()]; + Paragraph::new(Line::from(title_spans)).render(title_area, buf); + + // Optional context line + let mut input_y = area.y.saturating_add(1); + if let Some(context_label) = &self.context_label { + let context_area = Rect { + x: area.x, + y: input_y, + width: area.width, + height: 1, + }; + let spans: Vec> = vec![gutter(), context_label.clone().cyan()]; + Paragraph::new(Line::from(spans)).render(context_area, buf); + input_y = input_y.saturating_add(1); + } + + // Input line + let input_area = Rect { + x: area.x, + y: input_y, + width: area.width, + height: input_height, + }; + if input_area.width >= 2 { + for row in 0..input_area.height { + Paragraph::new(Line::from(vec![gutter()])).render( + Rect { + x: input_area.x, + y: input_area.y.saturating_add(row), + width: 2, + height: 1, + }, + buf, + ); + } + + let text_area_height = input_area.height.saturating_sub(1); + if text_area_height > 0 { + if input_area.width > 2 { + let blank_rect = Rect { + x: input_area.x.saturating_add(2), + y: input_area.y, + width: input_area.width.saturating_sub(2), + height: 1, + }; + Clear.render(blank_rect, buf); + } + let textarea_rect = Rect { + x: input_area.x.saturating_add(2), + y: input_area.y.saturating_add(1), + width: input_area.width.saturating_sub(2), + height: text_area_height, + }; + let mut state = self.textarea_state.borrow_mut(); + StatefulWidgetRef::render_ref(&(&self.textarea), textarea_rect, buf, &mut state); + if self.textarea.text().is_empty() { + Paragraph::new(Line::from(self.placeholder.clone().dim())) + .render(textarea_rect, buf); + } + } + } + + let hint_blank_y = input_area.y.saturating_add(input_height); + if hint_blank_y < area.y.saturating_add(area.height) { + let blank_area = Rect { + x: area.x, + y: hint_blank_y, + width: area.width, + height: 1, + }; + Clear.render(blank_area, buf); + } + + let hint_y = hint_blank_y.saturating_add(1); + if hint_y < area.y.saturating_add(area.height) { + Paragraph::new(STANDARD_POPUP_HINT_LINE).render( + Rect { + x: area.x, + y: hint_y, + width: area.width, + height: 1, + }, + buf, + ); + } + } + + fn handle_paste(&mut self, _pane: &mut super::BottomPane, pasted: String) -> bool { + if pasted.is_empty() { + return false; + } + self.textarea.insert_str(&pasted); + true + } + + fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { + if area.height < 2 || area.width <= 2 { + return None; + } + let text_area_height = self.input_height(area.width).saturating_sub(1); + if text_area_height == 0 { + return None; + } + let extra_offset: u16 = if self.context_label.is_some() { 1 } else { 0 }; + let top_line_count = 1u16 + extra_offset; + let textarea_rect = Rect { + x: area.x.saturating_add(2), + y: area.y.saturating_add(top_line_count).saturating_add(1), + width: area.width.saturating_sub(2), + height: text_area_height, + }; + let state = self.textarea_state.borrow(); + self.textarea.cursor_pos_with_state(textarea_rect, &state) + } +} + +impl CustomPromptView { + fn input_height(&self, width: u16) -> u16 { + let usable_width = width.saturating_sub(2); + let text_height = self.textarea.desired_height(usable_width).clamp(1, 8); + text_height.saturating_add(1).min(9) + } +} + +fn gutter() -> Span<'static> { + "▌ ".cyan() +} diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index 5d5dbf0f..82466bf7 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -28,6 +28,19 @@ pub(crate) struct SelectionItem { pub description: Option, pub is_current: bool, pub actions: Vec, + pub dismiss_on_select: bool, + pub search_value: Option, +} + +#[derive(Default)] +pub(crate) struct SelectionViewParams { + pub title: String, + pub subtitle: Option, + pub footer_hint: Option, + pub items: Vec, + pub is_searchable: bool, + pub search_placeholder: Option, + pub on_escape: Option, } pub(crate) struct ListSelectionView { @@ -38,6 +51,11 @@ pub(crate) struct ListSelectionView { state: ScrollState, complete: bool, app_event_tx: AppEventSender, + on_escape: Option, + is_searchable: bool, + search_query: String, + search_placeholder: Option, + filtered_indices: Vec, } impl ListSelectionView { @@ -49,49 +67,145 @@ impl ListSelectionView { let para = Paragraph::new(Line::from(Self::dim_prefix_span())); para.render(area, buf); } - pub fn new( - title: String, - subtitle: Option, - footer_hint: Option, - items: Vec, - app_event_tx: AppEventSender, - ) -> Self { + + pub fn new(params: SelectionViewParams, app_event_tx: AppEventSender) -> Self { let mut s = Self { - title, - subtitle, - footer_hint, - items, + title: params.title, + subtitle: params.subtitle, + footer_hint: params.footer_hint, + items: params.items, state: ScrollState::new(), complete: false, app_event_tx, + on_escape: params.on_escape, + is_searchable: params.is_searchable, + search_query: String::new(), + search_placeholder: if params.is_searchable { + params.search_placeholder + } else { + None + }, + filtered_indices: Vec::new(), }; - let len = s.items.len(); - if let Some(idx) = s.items.iter().position(|it| it.is_current) { - s.state.selected_idx = Some(idx); - } - s.state.clamp_selection(len); - s.state.ensure_visible(len, MAX_POPUP_ROWS.min(len)); + s.apply_filter(); s } + fn visible_len(&self) -> usize { + self.filtered_indices.len() + } + + fn max_visible_rows(len: usize) -> usize { + MAX_POPUP_ROWS.min(len.max(1)) + } + + fn apply_filter(&mut self) { + let previously_selected = self + .state + .selected_idx + .and_then(|visible_idx| self.filtered_indices.get(visible_idx).copied()) + .or_else(|| { + (!self.is_searchable) + .then(|| self.items.iter().position(|item| item.is_current)) + .flatten() + }); + + if self.is_searchable && !self.search_query.is_empty() { + let query_lower = self.search_query.to_lowercase(); + self.filtered_indices = self + .items + .iter() + .enumerate() + .filter_map(|(idx, item)| { + let matches = if let Some(search_value) = &item.search_value { + search_value.to_lowercase().contains(&query_lower) + } else { + let mut matches = item.name.to_lowercase().contains(&query_lower); + if !matches && let Some(desc) = &item.description { + matches = desc.to_lowercase().contains(&query_lower); + } + matches + }; + matches.then_some(idx) + }) + .collect(); + } else { + self.filtered_indices = (0..self.items.len()).collect(); + } + + let len = self.filtered_indices.len(); + self.state.selected_idx = self + .state + .selected_idx + .and_then(|visible_idx| { + self.filtered_indices + .get(visible_idx) + .and_then(|idx| self.filtered_indices.iter().position(|cur| cur == idx)) + }) + .or_else(|| { + previously_selected.and_then(|actual_idx| { + self.filtered_indices + .iter() + .position(|idx| *idx == actual_idx) + }) + }) + .or_else(|| (len > 0).then_some(0)); + + let visible = Self::max_visible_rows(len); + self.state.clamp_selection(len); + self.state.ensure_visible(len, visible); + } + + fn build_rows(&self) -> Vec { + self.filtered_indices + .iter() + .enumerate() + .filter_map(|(visible_idx, actual_idx)| { + self.items.get(*actual_idx).map(|item| { + let is_selected = self.state.selected_idx == Some(visible_idx); + let prefix = if is_selected { '>' } else { ' ' }; + let name = item.name.as_str(); + let name_with_marker = if item.is_current { + format!("{name} (current)") + } else { + item.name.clone() + }; + let n = visible_idx + 1; + let display_name = format!("{prefix} {n}. {name_with_marker}"); + GenericDisplayRow { + name: display_name, + match_indices: None, + is_current: item.is_current, + description: item.description.clone(), + } + }) + }) + .collect() + } + fn move_up(&mut self) { - let len = self.items.len(); + let len = self.visible_len(); self.state.move_up_wrap(len); - self.state.ensure_visible(len, MAX_POPUP_ROWS.min(len)); + let visible = Self::max_visible_rows(len); + self.state.ensure_visible(len, visible); } fn move_down(&mut self) { - let len = self.items.len(); + let len = self.visible_len(); self.state.move_down_wrap(len); - self.state.ensure_visible(len, MAX_POPUP_ROWS.min(len)); + let visible = Self::max_visible_rows(len); + self.state.ensure_visible(len, visible); } fn accept(&mut self) { - if let Some(idx) = self.state.selected_idx { - if let Some(item) = self.items.get(idx) { - for act in &item.actions { - act(&self.app_event_tx); - } + if let Some(idx) = self.state.selected_idx + && let Some(actual_idx) = self.filtered_indices.get(idx) + && let Some(item) = self.items.get(*actual_idx) + { + for act in &item.actions { + act(&self.app_event_tx); + } + if item.dismiss_on_select { self.complete = true; } } else { @@ -99,9 +213,10 @@ impl ListSelectionView { } } - fn cancel(&mut self) { - // Close the popup without performing any actions. - self.complete = true; + #[cfg(test)] + pub(crate) fn set_search_query(&mut self, query: String) { + self.search_query = query; + self.apply_filter(); } } @@ -115,9 +230,29 @@ impl BottomPaneView for ListSelectionView { code: KeyCode::Down, .. } => self.move_down(), + KeyEvent { + code: KeyCode::Backspace, + .. + } if self.is_searchable => { + self.search_query.pop(); + self.apply_filter(); + } KeyEvent { code: KeyCode::Esc, .. - } => self.cancel(), + } => { + self.on_ctrl_c(_pane); + } + KeyEvent { + code: KeyCode::Char(c), + modifiers, + .. + } if self.is_searchable + && !modifiers.contains(KeyModifiers::CONTROL) + && !modifiers.contains(KeyModifiers::ALT) => + { + self.search_query.push(c); + self.apply_filter(); + } KeyEvent { code: KeyCode::Enter, modifiers: KeyModifiers::NONE, @@ -133,39 +268,25 @@ impl BottomPaneView for ListSelectionView { fn on_ctrl_c(&mut self, _pane: &mut BottomPane) -> CancellationEvent { self.complete = true; + if let Some(cb) = &self.on_escape { + cb(&self.app_event_tx); + } CancellationEvent::Handled } fn desired_height(&self, width: u16) -> u16 { // Measure wrapped height for up to MAX_POPUP_ROWS items at the given width. // Build the same display rows used by the renderer so wrapping math matches. - let rows: Vec = self - .items - .iter() - .enumerate() - .map(|(i, it)| { - let is_selected = self.state.selected_idx == Some(i); - let prefix = if is_selected { '>' } else { ' ' }; - let name_with_marker = if it.is_current { - format!("{} (current)", it.name) - } else { - it.name.clone() - }; - let display_name = format!("{} {}. {}", prefix, i + 1, name_with_marker); - GenericDisplayRow { - name: display_name, - match_indices: None, - is_current: it.is_current, - description: it.description.clone(), - } - }) - .collect(); + let rows = self.build_rows(); let rows_height = measure_rows_height(&rows, &self.state, MAX_POPUP_ROWS, width); // +1 for the title row, +1 for a spacer line beneath the header, // +1 for optional subtitle, +1 for optional footer (2 lines incl. spacing) let mut height = rows_height + 2; + if self.is_searchable { + height = height.saturating_add(1); + } if self.subtitle.is_some() { // +1 for subtitle (the spacer is accounted for above) height = height.saturating_add(1); @@ -194,6 +315,25 @@ impl BottomPaneView for ListSelectionView { title_para.render(title_area, buf); let mut next_y = area.y.saturating_add(1); + if self.is_searchable { + let search_area = Rect { + x: area.x, + y: next_y, + width: area.width, + height: 1, + }; + let query_span: Span<'static> = if self.search_query.is_empty() { + self.search_placeholder + .as_ref() + .map(|placeholder| placeholder.clone().dim()) + .unwrap_or_else(|| "".into()) + } else { + self.search_query.clone().into() + }; + Paragraph::new(Line::from(vec![Self::dim_prefix_span(), query_span])) + .render(search_area, buf); + next_y = next_y.saturating_add(1); + } if let Some(sub) = &self.subtitle { let subtitle_area = Rect { x: area.x, @@ -228,27 +368,7 @@ impl BottomPaneView for ListSelectionView { .saturating_sub(footer_reserved), }; - let rows: Vec = self - .items - .iter() - .enumerate() - .map(|(i, it)| { - let is_selected = self.state.selected_idx == Some(i); - let prefix = if is_selected { '>' } else { ' ' }; - let name_with_marker = if it.is_current { - format!("{} (current)", it.name) - } else { - it.name.clone() - }; - let display_name = format!("{} {}. {}", prefix, i + 1, name_with_marker); - GenericDisplayRow { - name: display_name, - match_indices: None, - is_current: it.is_current, - description: it.description.clone(), - } - }) - .collect(); + let rows = self.build_rows(); if rows_area.height > 0 { render_rows( rows_area, @@ -279,6 +399,7 @@ mod tests { use super::BottomPaneView; use super::*; use crate::app_event::AppEvent; + use crate::bottom_pane::popup_consts::STANDARD_POPUP_HINT_LINE; use insta::assert_snapshot; use ratatui::layout::Rect; use tokio::sync::mpsc::unbounded_channel; @@ -292,19 +413,26 @@ mod tests { description: Some("Codex can read files".to_string()), is_current: true, actions: vec![], + dismiss_on_select: true, + search_value: None, }, SelectionItem { name: "Full Access".to_string(), description: Some("Codex can edit files".to_string()), is_current: false, actions: vec![], + dismiss_on_select: true, + search_value: None, }, ]; ListSelectionView::new( - "Select Approval Mode".to_string(), - subtitle.map(str::to_string), - Some("Press Enter to confirm or Esc to go back".to_string()), - items, + SelectionViewParams { + title: "Select Approval Mode".to_string(), + subtitle: subtitle.map(str::to_string), + footer_hint: Some(STANDARD_POPUP_HINT_LINE.to_string()), + items, + ..Default::default() + }, tx, ) } @@ -347,4 +475,33 @@ mod tests { let view = make_selection_view(Some("Switch between Codex approval presets")); assert_snapshot!("list_selection_spacing_with_subtitle", render_lines(&view)); } + + #[test] + fn renders_search_query_line_when_enabled() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let items = vec![SelectionItem { + name: "Read Only".to_string(), + description: Some("Codex can read files".to_string()), + is_current: false, + actions: vec![], + dismiss_on_select: true, + search_value: None, + }]; + let mut view = ListSelectionView::new( + SelectionViewParams { + title: "Select Approval Mode".to_string(), + footer_hint: Some(STANDARD_POPUP_HINT_LINE.to_string()), + items, + is_searchable: true, + search_placeholder: Some("Type to search branches".to_string()), + ..Default::default() + }, + tx, + ); + view.set_search_query("filters".to_string()); + + let lines = render_lines(&view); + assert!(lines.contains("▌ filters")); + } } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index f30bd418..06eff112 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -20,10 +20,12 @@ mod bottom_pane_view; mod chat_composer; mod chat_composer_history; mod command_popup; +pub mod custom_prompt_view; mod file_search_popup; mod list_selection_view; +pub(crate) use list_selection_view::SelectionViewParams; mod paste_burst; -mod popup_consts; +pub mod popup_consts; mod scroll_state; mod selection_popup_common; mod textarea; @@ -148,10 +150,10 @@ impl BottomPane { // status indicator shown while a task is running, or approval modal). // In these states the textarea is not interactable, so we should not // show its caret. - if self.active_view.is_some() { - None + let [_, content] = self.layout(area); + if let Some(view) = self.active_view.as_ref() { + view.cursor_pos(content) } else { - let [_, content] = self.layout(area); self.composer.cursor_pos(content) } } @@ -224,7 +226,17 @@ impl BottomPane { } pub fn handle_paste(&mut self, pasted: String) { - if self.active_view.is_none() { + if let Some(mut view) = self.active_view.take() { + let needs_redraw = view.handle_paste(self, pasted); + if view.is_complete() { + self.on_active_view_complete(); + } else { + self.active_view = Some(view); + } + if needs_redraw { + self.request_redraw(); + } + } else { let needs_redraw = self.composer.handle_paste(pasted); if needs_redraw { self.request_redraw(); @@ -318,22 +330,9 @@ impl BottomPane { } /// Show a generic list selection view with the provided items. - pub(crate) fn show_selection_view( - &mut self, - title: String, - subtitle: Option, - footer_hint: Option, - items: Vec, - ) { - let view = list_selection_view::ListSelectionView::new( - title, - subtitle, - footer_hint, - items, - self.app_event_tx.clone(), - ); + pub(crate) fn show_selection_view(&mut self, params: list_selection_view::SelectionViewParams) { + let view = list_selection_view::ListSelectionView::new(params, self.app_event_tx.clone()); self.active_view = Some(Box::new(view)); - self.request_redraw(); } /// Update the queued messages shown under the status header. @@ -373,6 +372,11 @@ impl BottomPane { self.request_redraw(); } + pub(crate) fn show_view(&mut self, view: Box) { + self.active_view = Some(view); + self.request_redraw(); + } + /// Called when the agent requests user approval. pub fn push_approval_request(&mut self, request: ApprovalRequest) { let request = if let Some(view) = self.active_view.as_mut() { diff --git a/codex-rs/tui/src/bottom_pane/popup_consts.rs b/codex-rs/tui/src/bottom_pane/popup_consts.rs index 5f447d73..5147b2ee 100644 --- a/codex-rs/tui/src/bottom_pane/popup_consts.rs +++ b/codex-rs/tui/src/bottom_pane/popup_consts.rs @@ -3,3 +3,6 @@ /// 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; + +/// Standard footer hint text used by popups. +pub(crate) const STANDARD_POPUP_HINT_LINE: &str = "Press Enter to confirm or Esc to go back"; diff --git a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs index 684924a4..38eaab74 100644 --- a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs +++ b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs @@ -13,6 +13,7 @@ use ratatui::widgets::BorderType; use ratatui::widgets::Borders; use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; +use unicode_width::UnicodeWidthChar; use super::scroll_state::ScrollState; use crate::ui_consts::LIVE_PREFIX_COLS; @@ -55,10 +56,24 @@ fn compute_desc_col( /// at `desc_col`. Applies fuzzy-match bolding when indices are present and /// dims the description. fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> { + // Enforce single-line name: allow at most desc_col - 2 cells for name, + // reserving two spaces before the description column. + let name_limit = desc_col.saturating_sub(2); + let mut name_spans: Vec = Vec::with_capacity(row.name.len()); + let mut used_width = 0usize; + let mut truncated = false; + if let Some(idxs) = row.match_indices.as_ref() { let mut idx_iter = idxs.iter().peekable(); for (char_idx, ch) in row.name.chars().enumerate() { + let ch_w = UnicodeWidthChar::width(ch).unwrap_or(0); + if used_width + ch_w > name_limit { + truncated = true; + break; + } + used_width += ch_w; + if idx_iter.peek().is_some_and(|next| **next == char_idx) { idx_iter.next(); name_spans.push(ch.to_string().bold()); @@ -67,7 +82,21 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> { } } } else { - name_spans.push(row.name.clone().into()); + for ch in row.name.chars() { + let ch_w = UnicodeWidthChar::width(ch).unwrap_or(0); + if used_width + ch_w > name_limit { + truncated = true; + break; + } + used_width += ch_w; + name_spans.push(ch.to_string().into()); + } + } + + if truncated { + // If there is at least one cell available, add an ellipsis. + // When name_limit is 0, we still show an ellipsis to indicate truncation. + name_spans.push("…".into()); } let this_name_width = Line::from(name_spans.clone()).width(); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index cb8acd04..b4204f3a 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -5,6 +5,8 @@ use std::sync::Arc; use codex_core::config::Config; use codex_core::config_types::Notifications; +use codex_core::git_info::current_branch_name; +use codex_core::git_info::local_git_branches; use codex_core::protocol::AgentMessageDeltaEvent; use codex_core::protocol::AgentMessageEvent; use codex_core::protocol::AgentReasoningDeltaEvent; @@ -63,6 +65,9 @@ use crate::bottom_pane::CancellationEvent; use crate::bottom_pane::InputResult; use crate::bottom_pane::SelectionAction; use crate::bottom_pane::SelectionItem; +use crate::bottom_pane::SelectionViewParams; +use crate::bottom_pane::custom_prompt_view::CustomPromptView; +use crate::bottom_pane::popup_consts::STANDARD_POPUP_HINT_LINE; use crate::clipboard_paste::paste_image_to_temp_png; use crate::diff_render::display_path_for; use crate::get_git_diff::get_git_diff; @@ -87,6 +92,7 @@ mod session_header; use self::session_header::SessionHeader; use crate::streaming::controller::AppEventHistorySink; use crate::streaming::controller::StreamController; +// use codex_common::approval_presets::ApprovalPreset; use codex_common::approval_presets::builtin_approval_presets; use codex_common::model_presets::ModelPreset; @@ -97,6 +103,7 @@ use codex_core::protocol::AskForApproval; use codex_core::protocol::SandboxPolicy; use codex_core::protocol_config_types::ReasoningEffort as ReasoningEffortConfig; use codex_file_search::FileMatch; +use std::path::Path; // Track information about an in-flight exec command. struct RunningCommand { @@ -941,13 +948,7 @@ impl ChatWidget { self.app_event_tx.send(AppEvent::CodexOp(Op::Compact)); } SlashCommand::Review => { - // Simplified flow: directly send a review op for current changes. - self.submit_op(Op::Review { - review_request: ReviewRequest { - prompt: "review current changes".to_string(), - user_facing_hint: "current changes".to_string(), - }, - }); + self.open_review_popup(); } SlashCommand::Model => { self.open_model_popup(); @@ -1417,15 +1418,20 @@ impl ChatWidget { description, is_current, actions, + dismiss_on_select: true, + search_value: None, }); } - self.bottom_pane.show_selection_view( - "Select model and reasoning level".to_string(), - Some("Switch between OpenAI models for this and future Codex CLI session".to_string()), - Some("Press Enter to confirm or Esc to go back".to_string()), + self.bottom_pane.show_selection_view(SelectionViewParams { + title: "Select model and reasoning level".to_string(), + subtitle: Some( + "Switch between OpenAI models for this and future Codex CLI session".to_string(), + ), + footer_hint: Some(STANDARD_POPUP_HINT_LINE.to_string()), items, - ); + ..Default::default() + }); } /// Open a popup to choose the approvals mode (ask for approval policy + sandbox policy). @@ -1458,15 +1464,17 @@ impl ChatWidget { description, is_current, actions, + dismiss_on_select: true, + search_value: None, }); } - self.bottom_pane.show_selection_view( - "Select Approval Mode".to_string(), - None, - Some("Press Enter to confirm or Esc to go back".to_string()), + self.bottom_pane.show_selection_view(SelectionViewParams { + title: "Select Approval Mode".to_string(), + footer_hint: Some(STANDARD_POPUP_HINT_LINE.to_string()), items, - ); + ..Default::default() + }); } /// Set the approval policy in the widget's config copy. @@ -1575,6 +1583,181 @@ impl ChatWidget { self.bottom_pane.set_custom_prompts(ev.custom_prompts); } + pub(crate) fn open_review_popup(&mut self) { + let mut items: Vec = Vec::new(); + + items.push(SelectionItem { + name: "Review uncommitted changes".to_string(), + description: None, + is_current: false, + actions: vec![Box::new( + move |tx: &AppEventSender| { + tx.send(AppEvent::CodexOp(Op::Review { + review_request: ReviewRequest { + prompt: "Review the current code changes (staged, unstaged, and untracked files) and provide prioritized findings.".to_string(), + user_facing_hint: "current changes".to_string(), + }, + })); + }, + )], + dismiss_on_select: true, + search_value: None, + }); + + // New: Review a specific commit (opens commit picker) + items.push(SelectionItem { + name: "Review a commit".to_string(), + description: None, + is_current: false, + actions: vec![Box::new({ + let cwd = self.config.cwd.clone(); + move |tx| { + tx.send(AppEvent::OpenReviewCommitPicker(cwd.clone())); + } + })], + dismiss_on_select: false, + search_value: None, + }); + + items.push(SelectionItem { + name: "Review against a base branch".to_string(), + description: None, + is_current: false, + actions: vec![Box::new({ + let cwd = self.config.cwd.clone(); + move |tx| { + tx.send(AppEvent::OpenReviewBranchPicker(cwd.clone())); + } + })], + dismiss_on_select: false, + search_value: None, + }); + + items.push(SelectionItem { + name: "Custom review instructions".to_string(), + description: None, + is_current: false, + actions: vec![Box::new(move |tx| { + tx.send(AppEvent::OpenReviewCustomPrompt); + })], + dismiss_on_select: false, + search_value: None, + }); + + self.bottom_pane.show_selection_view(SelectionViewParams { + title: "Select a review preset".into(), + footer_hint: Some(STANDARD_POPUP_HINT_LINE.to_string()), + items, + on_escape: None, + ..Default::default() + }); + } + + pub(crate) async fn show_review_branch_picker(&mut self, cwd: &Path) { + let branches = local_git_branches(cwd).await; + let current_branch = current_branch_name(cwd) + .await + .unwrap_or_else(|| "(detached HEAD)".to_string()); + let mut items: Vec = Vec::with_capacity(branches.len()); + + for option in branches { + let branch = option.clone(); + items.push(SelectionItem { + name: format!("{current_branch} -> {branch}"), + description: None, + is_current: false, + actions: vec![Box::new(move |tx3: &AppEventSender| { + tx3.send(AppEvent::CodexOp(Op::Review { + review_request: ReviewRequest { + prompt: format!( + "Review the code changes against the base branch '{branch}'. Start by finding the merge diff between the current branch and {branch} e.g. (git merge-base HEAD {branch}), then run `git diff` against that SHA to see what changes we would merge into the {branch} branch. Provide prioritized, actionable findings." + ), + user_facing_hint: format!("changes against '{branch}'"), + }, + })); + })], + dismiss_on_select: true, + search_value: Some(option), + }); + } + + self.bottom_pane.show_selection_view(SelectionViewParams { + title: "Select a base branch".to_string(), + footer_hint: Some(STANDARD_POPUP_HINT_LINE.to_string()), + items, + is_searchable: true, + search_placeholder: Some("Type to search branches".to_string()), + on_escape: Some(Box::new(|tx| tx.send(AppEvent::OpenReviewPopup))), + ..Default::default() + }); + } + + pub(crate) async fn show_review_commit_picker(&mut self, cwd: &Path) { + let commits = codex_core::git_info::recent_commits(cwd, 100).await; + + let mut items: Vec = Vec::with_capacity(commits.len()); + for entry in commits { + let subject = entry.subject.clone(); + let sha = entry.sha.clone(); + let short = sha.chars().take(7).collect::(); + let search_val = format!("{subject} {sha}"); + + items.push(SelectionItem { + name: subject.clone(), + description: None, + is_current: false, + actions: vec![Box::new(move |tx3: &AppEventSender| { + let hint = format!("commit {short}"); + let prompt = format!( + "Review the code changes introduced by commit {sha} (\"{subject}\"). Provide prioritized, actionable findings." + ); + tx3.send(AppEvent::CodexOp(Op::Review { + review_request: ReviewRequest { + prompt, + user_facing_hint: hint, + }, + })); + })], + dismiss_on_select: true, + search_value: Some(search_val), + }); + } + + self.bottom_pane.show_selection_view(SelectionViewParams { + title: "Select a commit to review".to_string(), + footer_hint: Some(STANDARD_POPUP_HINT_LINE.to_string()), + items, + is_searchable: true, + search_placeholder: Some("Type to search commits".to_string()), + on_escape: Some(Box::new(|tx| tx.send(AppEvent::OpenReviewPopup))), + ..Default::default() + }); + } + + pub(crate) fn show_review_custom_prompt(&mut self) { + let tx = self.app_event_tx.clone(); + let view = CustomPromptView::new( + "Custom review instructions".to_string(), + "Type instructions and press Enter".to_string(), + None, + self.app_event_tx.clone(), + Some(Box::new(|tx| tx.send(AppEvent::OpenReviewPopup))), + Box::new(move |prompt: String| { + let trimmed = prompt.trim().to_string(); + if trimmed.is_empty() { + return; + } + tx.send(AppEvent::CodexOp(Op::Review { + review_request: ReviewRequest { + prompt: trimmed.clone(), + user_facing_hint: trimmed, + }, + })); + }), + ); + self.bottom_pane.show_view(Box::new(view)); + } + /// Programmatically submit a user text message as if typed in the /// composer. The text will be added to conversation history and sent to /// the agent. @@ -1731,5 +1914,48 @@ fn extract_first_bold(s: &str) -> Option { None } +#[cfg(test)] +pub(crate) fn show_review_commit_picker_with_entries( + chat: &mut ChatWidget, + entries: Vec, +) { + let mut items: Vec = Vec::with_capacity(entries.len()); + for entry in entries { + let subject = entry.subject.clone(); + let sha = entry.sha.clone(); + let short = sha.chars().take(7).collect::(); + let search_val = format!("{subject} {sha}"); + + items.push(SelectionItem { + name: subject.clone(), + description: None, + is_current: false, + actions: vec![Box::new(move |tx3: &AppEventSender| { + let hint = format!("commit {short}"); + let prompt = format!( + "Review the code changes introduced by commit {sha} (\"{subject}\"). Provide prioritized, actionable findings." + ); + tx3.send(AppEvent::CodexOp(Op::Review { + review_request: ReviewRequest { + prompt, + user_facing_hint: hint, + }, + })); + })], + dismiss_on_select: true, + search_value: Some(search_val), + }); + } + + chat.bottom_pane.show_selection_view(SelectionViewParams { + title: "Select a commit to review".to_string(), + footer_hint: Some(STANDARD_POPUP_HINT_LINE.to_string()), + items, + is_searchable: true, + search_placeholder: Some("Type to search commits".to_string()), + ..Default::default() + }); +} + #[cfg(test)] pub(crate) mod tests; diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 7427bb44..c009cabd 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -801,6 +801,135 @@ fn exec_history_cell_shows_working_then_failed() { assert!(blob.to_lowercase().contains("bloop"), "expected error text"); } +/// Selecting the custom prompt option from the review popup sends +/// OpenReviewCustomPrompt to the app event channel. +#[test] +fn review_popup_custom_prompt_action_sends_event() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + // Open the preset selection popup + chat.open_review_popup(); + + // Move selection down to the fourth item: "Custom review instructions" + chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); + chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); + chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); + // Activate + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + // Drain events and ensure we saw the OpenReviewCustomPrompt request + let mut found = false; + while let Ok(ev) = rx.try_recv() { + if let AppEvent::OpenReviewCustomPrompt = ev { + found = true; + break; + } + } + assert!(found, "expected OpenReviewCustomPrompt event to be sent"); +} + +/// The commit picker shows only commit subjects (no timestamps). +#[test] +fn review_commit_picker_shows_subjects_without_timestamps() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + + // Open the Review presets parent popup. + chat.open_review_popup(); + + // Show commit picker with synthetic entries. + let entries = vec![ + codex_core::git_info::CommitLogEntry { + sha: "1111111deadbeef".to_string(), + timestamp: 0, + subject: "Add new feature X".to_string(), + }, + codex_core::git_info::CommitLogEntry { + sha: "2222222cafebabe".to_string(), + timestamp: 0, + subject: "Fix bug Y".to_string(), + }, + ]; + super::show_review_commit_picker_with_entries(&mut chat, entries); + + // Render the bottom pane and inspect the lines for subjects and absence of time words. + let width = 72; + let height = chat.desired_height(width); + let area = ratatui::layout::Rect::new(0, 0, width, height); + let mut buf = ratatui::buffer::Buffer::empty(area); + (&chat).render_ref(area, &mut buf); + + let mut blob = String::new(); + for y in 0..area.height { + for x in 0..area.width { + let s = buf[(x, y)].symbol(); + if s.is_empty() { + blob.push(' '); + } else { + blob.push_str(s); + } + } + blob.push('\n'); + } + + assert!( + blob.contains("Add new feature X"), + "expected subject in output" + ); + assert!(blob.contains("Fix bug Y"), "expected subject in output"); + + // Ensure no relative-time phrasing is present. + let lowered = blob.to_lowercase(); + assert!( + !lowered.contains("ago") + && !lowered.contains(" second") + && !lowered.contains(" minute") + && !lowered.contains(" hour") + && !lowered.contains(" day"), + "expected no relative time in commit picker output: {blob:?}" + ); +} + +/// Submitting the custom prompt view sends Op::Review with the typed prompt +/// and uses the same text for the user-facing hint. +#[test] +fn custom_prompt_submit_sends_review_op() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + chat.show_review_custom_prompt(); + // Paste prompt text via ChatWidget handler, then submit + chat.handle_paste(" please audit dependencies ".to_string()); + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + // Expect AppEvent::CodexOp(Op::Review { .. }) with trimmed prompt + let evt = rx.try_recv().expect("expected one app event"); + match evt { + AppEvent::CodexOp(Op::Review { review_request }) => { + assert_eq!( + review_request.prompt, + "please audit dependencies".to_string() + ); + assert_eq!( + review_request.user_facing_hint, + "please audit dependencies".to_string() + ); + } + other => panic!("unexpected app event: {other:?}"), + } +} + +/// Hitting Enter on an empty custom prompt view does not submit. +#[test] +fn custom_prompt_enter_empty_does_not_send() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + chat.show_review_custom_prompt(); + // Enter without any text + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + // No AppEvent::CodexOp should be sent + assert!(rx.try_recv().is_err(), "no app event should be sent"); +} + // Snapshot test: interrupting a running exec finalizes the active cell with a red ✗ // marker (replacing the spinner) and flushes it into history. #[test] @@ -830,6 +959,110 @@ fn interrupt_exec_marks_failed_snapshot() { assert_snapshot!("interrupt_exec_marks_failed", exec_blob); } +/// Opening custom prompt from the review popup, pressing Esc returns to the +/// parent popup, pressing Esc again dismisses all panels (back to normal mode). +#[test] +fn review_custom_prompt_escape_navigates_back_then_dismisses() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + // Open the Review presets parent popup. + chat.open_review_popup(); + + // Open the custom prompt submenu (child view) directly. + chat.show_review_custom_prompt(); + + // Verify child view is on top. + let header = render_bottom_first_row(&chat, 60); + assert!( + header.contains("Custom review instructions"), + "expected custom prompt view header: {header:?}" + ); + + // Esc once: child view closes, parent (review presets) remains. + chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + // Process emitted app events to reopen the parent review popup. + while let Ok(ev) = rx.try_recv() { + if let AppEvent::OpenReviewPopup = ev { + chat.open_review_popup(); + break; + } + } + let header = render_bottom_first_row(&chat, 60); + assert!( + header.contains("Select a review preset"), + "expected to return to parent review popup: {header:?}" + ); + + // Esc again: parent closes; back to normal composer state. + chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + assert!( + chat.is_normal_backtrack_mode(), + "expected to be back in normal composer mode" + ); +} + +/// Opening base-branch picker from the review popup, pressing Esc returns to the +/// parent popup, pressing Esc again dismisses all panels (back to normal mode). +#[tokio::test(flavor = "current_thread")] +async fn review_branch_picker_escape_navigates_back_then_dismisses() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + // Open the Review presets parent popup. + chat.open_review_popup(); + + // Open the branch picker submenu (child view). Using a temp cwd with no git repo is fine. + let cwd = std::env::temp_dir(); + chat.show_review_branch_picker(&cwd).await; + + // Verify child view header. + let header = render_bottom_first_row(&chat, 60); + assert!( + header.contains("Select a base branch"), + "expected branch picker header: {header:?}" + ); + + // Esc once: child view closes, parent remains. + chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + // Process emitted app events to reopen the parent review popup. + while let Ok(ev) = rx.try_recv() { + if let AppEvent::OpenReviewPopup = ev { + chat.open_review_popup(); + break; + } + } + let header = render_bottom_first_row(&chat, 60); + assert!( + header.contains("Select a review preset"), + "expected to return to parent review popup: {header:?}" + ); + + // Esc again: parent closes; back to normal composer state. + chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + assert!( + chat.is_normal_backtrack_mode(), + "expected to be back in normal composer mode" + ); +} + +fn render_bottom_first_row(chat: &ChatWidget, width: u16) -> String { + let height = chat.desired_height(width); + let area = Rect::new(0, 0, width, height); + let mut buf = Buffer::empty(area); + (chat).render_ref(area, &mut buf); + let mut row = String::new(); + // Row 0 is the top spacer for the bottom pane; row 1 contains the header line + let y = 1u16.min(height.saturating_sub(1)); + for x in 0..area.width { + let s = buf[(x, y)].symbol(); + if s.is_empty() { + row.push(' '); + } else { + row.push_str(s); + } + } + row +} + #[test] fn exec_history_extends_previous_when_consecutive() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 4d4c8067..6570cf68 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -36,7 +36,7 @@ impl SlashCommand { SlashCommand::New => "start a new chat during a conversation", SlashCommand::Init => "create an AGENTS.md file with instructions for Codex", SlashCommand::Compact => "summarize conversation to prevent hitting the context limit", - SlashCommand::Review => "review my current changes and find issues", + SlashCommand::Review => "review my changes and find issues", SlashCommand::Quit => "exit Codex", SlashCommand::Diff => "show git diff (including untracked files)", SlashCommand::Mention => "mention a file",