From 9e552e9d15ba52bed7077d5357f3e18e330f8f38 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sat, 11 Jul 2026 20:59:03 +0000 Subject: [PATCH] Use available width for skill names in the toggle view (#32485) ## Why The skill toggle view truncated every display name to 21 characters, even when the popup had enough room to show more. This could hide the part that distinguishes similarly named skills. ## What changed - Pass full skill display names to the shared row renderer so it can fit them to the available width alongside descriptions. - Keep narrow layouts bounded by the renderer's width-aware truncation. ## Testing Added coverage for preserving and filtering full display names, plus snapshots for wide and narrow popup layouts. GitOrigin-RevId: c434237e00440f920d8aa9c365b6bda5e68e0941 --- .../tui/src/bottom_pane/skills_toggle_view.rs | 104 +++++++++++++++++- ...lls_toggle_long_names_at_narrow_width.snap | 14 +++ ...toggle_long_names_use_available_width.snap | 14 +++ codex-rs/tui/src/skills_helpers.rs | 8 -- 4 files changed, 130 insertions(+), 10 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__skills_toggle_view__tests__skills_toggle_long_names_at_narrow_width.snap create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__skills_toggle_view__tests__skills_toggle_long_names_use_available_width.snap diff --git a/codex-rs/tui/src/bottom_pane/skills_toggle_view.rs b/codex-rs/tui/src/bottom_pane/skills_toggle_view.rs index 5a5414ed49..b35088314c 100644 --- a/codex-rs/tui/src/bottom_pane/skills_toggle_view.rs +++ b/codex-rs/tui/src/bottom_pane/skills_toggle_view.rs @@ -23,7 +23,6 @@ use crate::render::RectExt as _; use crate::render::renderable::ColumnRenderable; use crate::render::renderable::Renderable; use crate::skills_helpers::match_skill; -use crate::skills_helpers::truncate_skill_name; use crate::style::user_message_style; use super::CancellationEvent; @@ -145,7 +144,7 @@ impl SkillsToggleView { let is_selected = self.state.selected_idx == Some(visible_idx); let prefix = if is_selected { '›' } else { ' ' }; let marker = if item.enabled { 'x' } else { ' ' }; - let item_name = truncate_skill_name(&item.name); + let item_name = &item.name; let name = format!("{prefix} [{marker}] {item_name}"); GenericDisplayRow { name, @@ -427,6 +426,7 @@ mod tests { use crate::test_support::PathBufExt; use crate::test_support::test_path_buf; use insta::assert_snapshot; + use pretty_assertions::assert_eq; use ratatui::layout::Rect; use tokio::sync::mpsc::unbounded_channel; @@ -453,6 +453,25 @@ mod tests { lines.join("\n") } + fn long_name_items() -> Vec { + vec![ + SkillsToggleItem { + name: "superpowers-systematic-debugging (polish)".to_string(), + skill_name: "polish:superpowers-systematic-debugging".to_string(), + description: "Find root causes before fixing bugs".to_string(), + enabled: true, + path: test_path_buf("/tmp/skills/systematic-debugging/SKILL.md").abs(), + }, + SkillsToggleItem { + name: "superpowers-verification-before-completion (polish)".to_string(), + skill_name: "polish:superpowers-verification-before-completion".to_string(), + description: "Verify completion before claiming success".to_string(), + enabled: false, + path: test_path_buf("/tmp/skills/verification-before-completion/SKILL.md").abs(), + }, + ] + } + #[test] fn renders_basic_popup() { let (tx_raw, _rx) = unbounded_channel::(); @@ -477,6 +496,87 @@ mod tests { assert_snapshot!("skills_toggle_basic", render_lines(&view, /*width*/ 72)); } + #[test] + fn build_rows_preserves_full_skill_display_names() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let view = SkillsToggleView::new( + long_name_items(), + tx, + crate::keymap::RuntimeKeymap::defaults().list, + ); + + let row_names = view + .build_rows() + .into_iter() + .map(|row| row.name) + .collect::>(); + + assert_eq!( + row_names, + vec![ + "› [x] superpowers-systematic-debugging (polish)", + " [ ] superpowers-verification-before-completion (polish)", + ] + ); + } + + #[test] + fn filtering_long_skill_names_preserves_the_matching_row() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut view = SkillsToggleView::new( + long_name_items(), + tx, + crate::keymap::RuntimeKeymap::defaults().list, + ); + view.search_query = "completion".to_string(); + view.apply_filter(); + + let row_names = view + .build_rows() + .into_iter() + .map(|row| row.name) + .collect::>(); + + assert_eq!( + row_names, + vec!["› [ ] superpowers-verification-before-completion (polish)"] + ); + } + + #[test] + fn renders_long_names_using_available_width() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let view = SkillsToggleView::new( + long_name_items(), + tx, + crate::keymap::RuntimeKeymap::defaults().list, + ); + + assert_snapshot!( + "skills_toggle_long_names_use_available_width", + render_lines(&view, /*width*/ 96) + ); + } + + #[test] + fn renders_long_names_at_narrow_width() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let view = SkillsToggleView::new( + long_name_items(), + tx, + crate::keymap::RuntimeKeymap::defaults().list, + ); + + assert_snapshot!( + "skills_toggle_long_names_at_narrow_width", + render_lines(&view, /*width*/ 48) + ); + } + #[test] fn footer_hint_uses_list_keymap_accept_and_cancel() { let (tx_raw, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__skills_toggle_view__tests__skills_toggle_long_names_at_narrow_width.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__skills_toggle_view__tests__skills_toggle_long_names_at_narrow_width.snap new file mode 100644 index 0000000000..d2e4dc3c5f --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__skills_toggle_view__tests__skills_toggle_long_names_at_narrow_width.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/bottom_pane/skills_toggle_view.rs +expression: "render_lines(&view, 48)" +--- + + Enable/Disable Skills + Turn skills on or off. Your changes are save + + Type to search skills + > +› [x] superpowers-systematic-d… Find root cau… + [ ] superpowers-verification… Verify comple… + + Press space or enter to toggle; esc to close diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__skills_toggle_view__tests__skills_toggle_long_names_use_available_width.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__skills_toggle_view__tests__skills_toggle_long_names_use_available_width.snap new file mode 100644 index 0000000000..e2f2f67a7c --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__skills_toggle_view__tests__skills_toggle_long_names_use_available_width.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/bottom_pane/skills_toggle_view.rs +expression: "render_lines(&view, 96)" +--- + + Enable/Disable Skills + Turn skills on or off. Your changes are saved automatically. + + Type to search skills + > +› [x] superpowers-systematic-debugging (polish) Find root causes before fixing bugs + [ ] superpowers-verification-before-completion (polish) Verify completion before claiming … + + Press space or enter to toggle; esc to close diff --git a/codex-rs/tui/src/skills_helpers.rs b/codex-rs/tui/src/skills_helpers.rs index 730acdcaaf..88d879c6a5 100644 --- a/codex-rs/tui/src/skills_helpers.rs +++ b/codex-rs/tui/src/skills_helpers.rs @@ -1,10 +1,6 @@ use codex_core_skills::model::SkillMetadata; use codex_utils_fuzzy_match::fuzzy_match; -use crate::text_formatting::truncate_text; - -pub(crate) const SKILL_NAME_TRUNCATE_LEN: usize = 21; - pub(crate) fn skill_display_name(skill: &SkillMetadata) -> String { if let Some(display_name) = skill .interface @@ -33,10 +29,6 @@ pub(crate) fn skill_description(skill: &SkillMetadata) -> &str { .unwrap_or(&skill.description) } -pub(crate) fn truncate_skill_name(name: &str) -> String { - truncate_text(name, SKILL_NAME_TRUNCATE_LEN) -} - pub(crate) fn match_skill( filter: &str, display_name: &str,