mirror of
https://github.com/openai/codex.git
synced 2026-10-21 09:54:56 +00:00
Show hook status messages as titles in the hooks browser (#51433)
## What changed Replace generic `Hook 1` labels with trimmed `status_message` titles, falling back to `Unnamed hook` when the message is missing or blank. Show aligned, dimmed row numbers alongside the titles and retain trust-status suffixes. Render rows with `Paragraph` to preserve full-row selection styling and per-span dimming, including titles containing wide characters and emoji. ## Testing Add snapshot and buffer assertions for title trimming, blank and missing messages, wide characters, selection styling, and dimmed indices. Update snapshots for long-title truncation and scrolling across two-digit row numbers. GitOrigin-RevId: a9b20ea91e7252b6399d1c8a9fa849186f16a651
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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<_>>(),
|
||||
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(), "检");
|
||||
}
|
||||
@@ -436,6 +436,12 @@ impl HooksBrowserView {
|
||||
|
||||
#[allow(clippy::disallowed_methods)]
|
||||
fn handler_row_lines(&self, event_name: HookEventName, width: usize) -> Vec<Line<'static>> {
|
||||
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<_>>(),
|
||||
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::<AppEvent>();
|
||||
@@ -1281,9 +1330,9 @@ mod tests {
|
||||
#[test]
|
||||
fn renders_scrolled_handler_window() {
|
||||
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
|
||||
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(),
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
+2
-2
@@ -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
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
+10
-10
@@ -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
|
||||
|
||||
+2
-2
@@ -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
|
||||
|
||||
+22
@@ -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
|
||||
+1
-1
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user