diff --git a/codex-rs/tui/src/bottom_pane/hooks_browser_render.rs b/codex-rs/tui/src/bottom_pane/hooks_browser_render.rs index bd71c0f264..a1a3960dd9 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_render.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_render.rs @@ -65,10 +65,9 @@ fn render_line_rows( body.width, /*height*/ 1, ); - line.clone().render(row_area, buf); - if line.style.bg.is_some() { - buf.set_style(row_area, line.style); - } + Paragraph::new(line.clone()) + .style(line.style) + .render(row_area, buf); } picker_style::render_scroll_indicators( area, @@ -160,3 +159,7 @@ impl Renderable for HooksBrowserView { self.render_footer(footer, buf); } } + +#[cfg(test)] +#[path = "hooks_browser_render_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/bottom_pane/hooks_browser_render_tests.rs b/codex-rs/tui/src/bottom_pane/hooks_browser_render_tests.rs new file mode 100644 index 0000000000..1e0b118c04 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_render_tests.rs @@ -0,0 +1,40 @@ +//! Wide hook titles preserve the selection fill and per-span index emphasis. + +use super::*; +use pretty_assertions::assert_eq; +use ratatui::style::Color; +use ratatui::style::Modifier; +use ratatui::style::Style; + +#[test] +fn wide_titles_preserve_reverse_video_selection_and_dim_index() { + let style = Style::default() + .fg(Color::Reset) + .bg(Color::Reset) + .bold() + .not_dim() + .reversed(); + let row = Line::from(vec![ + "› [x] ".into(), + " 1".dim(), + " 检查 🦀 shell commands".into(), + ]) + .style(style); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 40, /*height*/ 1, + ); + let mut buf = Buffer::empty(area); + + render_line_rows(area, &mut buf, vec![row], ScrollState::new()); + + assert_eq!( + buf.content + .iter() + .map(|cell| (cell.fg, cell.bg, cell.modifier.contains(Modifier::REVERSED))) + .collect::>(), + vec![(Color::Reset, Color::Reset, true); usize::from(area.width)] + ); + assert!(buf[(7, 0)].modifier.contains(Modifier::DIM)); + assert!(!buf[(10, 0)].modifier.contains(Modifier::DIM)); + assert_eq!(buf[(10, 0)].symbol(), "检"); +} diff --git a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs index ceedc2faf4..d9468cb811 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -436,6 +436,12 @@ impl HooksBrowserView { #[allow(clippy::disallowed_methods)] fn handler_row_lines(&self, event_name: HookEventName, width: usize) -> Vec> { + let index_width = self + .handlers_for_event(event_name) + .count() + .to_string() + .len() + .max(2); self.handlers_for_event(event_name) .enumerate() .map(|(idx, hook)| { @@ -446,21 +452,27 @@ impl HooksBrowserView { } else { ' ' }; - let row = match hook.trust_status { - HookTrustStatus::Modified => { - format!("[{marker}] {} · modified", hook_title(idx)) - } - HookTrustStatus::Untrusted => format!("[{marker}] {} · new", hook_title(idx)), - HookTrustStatus::Managed | HookTrustStatus::Trusted => { - format!("[{marker}] {}", hook_title(idx)) - } + let title = hook + .status_message + .as_deref() + .map(str::trim) + .filter(|title| !title.is_empty()) + .unwrap_or("Unnamed hook"); + let trust_suffix = match hook.trust_status { + HookTrustStatus::Modified => " · modified", + HookTrustStatus::Untrusted => " · new", + HookTrustStatus::Managed | HookTrustStatus::Trusted => "", }; let prefix = if self.state.selected_idx == Some(idx) { "› " } else { " " }; - let mut line = Line::from(format!("{prefix}{row}")); + let mut line = Line::from(vec![ + format!("{prefix}[{marker}] ").into(), + format!("{:>index_width$}", idx + 1).dim(), + format!(" {title}{trust_suffix}").into(), + ]); line = truncate_line_with_ellipsis_if_overflow(line, width); let needs_review = hook_needs_review(hook); if self.state.selected_idx == Some(idx) { @@ -719,10 +731,6 @@ fn event_description(event_name: HookEventName) -> &'static str { } } -fn hook_title(idx: usize) -> String { - format!("Hook {}", idx + 1) -} - fn hook_source_summary(hook: &HookMetadata) -> String { match hook.source { HookSource::Plugin => hook @@ -839,6 +847,7 @@ mod tests { use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use insta::assert_snapshot; + use pretty_assertions::assert_eq; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::style::Color; @@ -1080,6 +1089,46 @@ mod tests { assert_snapshot!("hooks_browser_handlers", render_lines(&view, /*width*/ 112)); } + #[test] + fn renders_handler_status_messages_with_blank_fallback() { + let mut view = view(); + view.entry.hooks[0].status_message = Some(" 检查 🦀 shell commands ".to_string()); + view.entry.hooks[1].status_message = Some(" \t\n ".to_string()); + view.entry.hooks[2].event_name = HookEventName::PreToolUse; + view.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert_snapshot!( + "hooks_browser_status_messages", + render_lines(&view, /*width*/ 112) + ); + + for (fg, bg) in [ + ((230, 230, 230), (18, 20, 30)), + ((32, 32, 32), (255, 255, 255)), + ] { + crate::terminal_palette::with_test_default_colors( + crate::terminal_probe::DefaultColors { fg, bg }, + || { + let buf = render_buffer(&view, /*width*/ 112); + let selected_y = (0..buf.area.height) + .find(|&y| buf[(0, y)].symbol() == "›") + .expect("selected hook row"); + for y in [selected_y, selected_y + 1] { + assert!(buf[(7, y)].modifier.contains(Modifier::DIM)); + assert!(!buf[(10, y)].modifier.contains(Modifier::DIM)); + } + assert_eq!(buf[(10, selected_y)].symbol(), "检"); + assert_eq!(buf[(10, selected_y)].fg, selection_style().fg.unwrap()); + assert_eq!( + (0..buf.area.width) + .map(|x| buf[(x, selected_y)].bg) + .collect::>(), + vec![selection_style().bg.unwrap(); usize::from(buf.area.width)] + ); + }, + ); + } + } + #[test] fn renders_mcp_tool_handler_with_server_and_tool_details() { let (tx_raw, _rx) = unbounded_channel::(); @@ -1281,9 +1330,9 @@ mod tests { #[test] fn renders_scrolled_handler_window() { let (tx_raw, _rx) = unbounded_channel::(); - let hooks = (0..=MAX_POPUP_ROWS) + let hooks = (0..MAX_POPUP_ROWS + 3) .map(|idx| { - hook( + let mut hook = hook( &format!("path:hook-{idx}"), HookEventName::PreToolUse, HookSource::User, @@ -1292,19 +1341,28 @@ mod tests { /*enabled*/ true, /*is_managed*/ false, idx as i64, - ) + ); + if idx % 2 == 0 { + hook.status_message = Some(format!("Check tool {}", idx + 1)); + } + hook }) .collect(); let mut view = HooksBrowserView::new(hooks, Vec::new(), Vec::new(), AppEventSender::new(tx_raw)); view.handle_key_event(KeyEvent::from(KeyCode::Enter)); - for _ in 0..MAX_POPUP_ROWS { + for _ in 0..MAX_POPUP_ROWS + 1 { view.handle_key_event(KeyEvent::from(KeyCode::Down)); } assert_snapshot!( "hooks_browser_scrolled_handlers", render_lines(&view, /*width*/ 112) ); + assert_eq!( + view.selected_hook(HookEventName::PreToolUse) + .map(|hook| hook.key.as_str()), + Some("path:hook-9") + ); } #[test] @@ -1321,6 +1379,8 @@ mod tests { /*display_order*/ 0, ); capped_command_hook.source_path = test_path_buf("/tmp/h.json").abs(); + capped_command_hook.status_message = + Some("Check shell commands before allowing them to execute".to_string()); let mut view = HooksBrowserView::new( vec![capped_command_hook], Vec::new(), diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_additional_context_limit.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_additional_context_limit.snap index 48bbf99787..8d27510179 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_additional_context_limit.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_additional_context_limit.snap @@ -6,7 +6,7 @@ expression: "render_lines(&view, 112)" PreToolUse hooks Turn hooks on or off. Your changes are saved automatically. -› [x] Hook 1 +› [x] 1 Unnamed hook Event PreToolUse Matcher Bash diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap index e630643ea1..bdd0f43079 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap @@ -7,7 +7,7 @@ expression: "render_lines(&view, 44)" Turn hooks on or off. Your changes are saved automatically. -› [x] Hook 1 +› [x] 1 Check shell commands before allow… Event PreToolUse Matcher Bash diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap index 7da3c9e7c4..ebecbe9d19 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap @@ -6,8 +6,8 @@ expression: "render_lines(&view, 112)" PreToolUse hooks Turn hooks on or off. Your changes are saved automatically. -› [x] Hook 1 - [ ] Hook 2 +› [x] 1 Unnamed hook + [ ] 2 Unnamed hook Event PreToolUse Matcher Bash diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap index e3d9d9a242..56a618a5c7 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap @@ -6,7 +6,7 @@ expression: "render_lines(&view, 112)" PermissionRequest hooks Turn hooks on or off. Your changes are saved automatically. -› [x] Hook 1 +› [x] 1 Unnamed hook Event PermissionRequest Matcher Bash diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_mcp_tool_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_mcp_tool_handler.snap index e9374bf510..c282cbff36 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_mcp_tool_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_mcp_tool_handler.snap @@ -6,7 +6,7 @@ expression: "render_lines(&view, 112)" PreToolUse hooks Turn hooks on or off. Your changes are saved automatically. -› [x] Hook 1 +› [x] 1 Unnamed hook Event PreToolUse Matcher Bash diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap index 2d86de75c6..49b07a4213 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap @@ -6,7 +6,7 @@ expression: "render_lines(&view, 112)" PreToolUse hooks 1 hook needs review before it can run. -› [!] Hook 1 · new +› [!] 1 Unnamed hook · new Event PreToolUse Matcher Bash diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap index 8b807f9f0e..3789e9d744 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap @@ -6,19 +6,19 @@ expression: "render_lines(&view, 112)" PreToolUse hooks Turn hooks on or off. Your changes are saved automatically. ↑ - [x] Hook 2 - [x] Hook 3 - [x] Hook 4 - [x] Hook 5 - [x] Hook 6 - [x] Hook 7 - [x] Hook 8 -› [x] Hook 9 - + [x] 3 Check tool 3 + [x] 4 Unnamed hook + [x] 5 Check tool 5 + [x] 6 Unnamed hook + [x] 7 Check tool 7 + [x] 8 Unnamed hook + [x] 9 Check tool 9 +› [x] 10 Unnamed hook +↓ Event PreToolUse Matcher Bash Source User config - /tmp/hooks.json - Command /tmp/hook-8.sh + Command /tmp/hook-9.sh Mode Sync Timeout 30s Trust Trusted diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap index 9023c899d7..6e7f808d4e 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap @@ -6,8 +6,8 @@ expression: "render_lines(&view, 112)" PreToolUse hooks Turn hooks on or off. Your changes are saved automatically. - [x] Hook 1 -› [x] Hook 2 + [x] 1 Unnamed hook +› [x] 2 Unnamed hook Event PreToolUse Matcher Bash diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_status_messages.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_status_messages.snap new file mode 100644 index 0000000000..e4f0eda1f4 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_status_messages.snap @@ -0,0 +1,22 @@ +--- +source: tui/src/bottom_pane/hooks_browser_view.rs +expression: "render_lines(&view, 112)" +--- + + PreToolUse hooks + Turn hooks on or off. Your changes are saved automatically. + +› [x] 1 检 查 🦀 shell commands + [ ] 2 Unnamed hook + [x] 3 Unnamed hook + + Event PreToolUse + Matcher Bash + Source Plugin - superpowers@openai-curated + Command ${CODEX_PLUGIN_ROOT}/hooks/pre-tool-use-check.sh + Mode Sync + Timeout 30s + Trust Trusted + + + space/enter toggle · esc back diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_untrusted_enabled_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_untrusted_enabled_handler.snap index c9b8cdb97a..087bb66b75 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_untrusted_enabled_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_untrusted_enabled_handler.snap @@ -6,7 +6,7 @@ expression: "render_lines(&view, 112)" PreToolUse hooks 1 hook needs review before it can run. -› [!] Hook 1 · new +› [!] 1 Unnamed hook · new Event PreToolUse Matcher Bash