UI tweaks on skills popup. (#8250)

Only display the skill name (not the folder), and truncate the skill
description to a maximum of two lines.
This commit is contained in:
xl-openai 2025-12-18 17:16:51 -08:00 committed by GitHub
parent 6c76d17713
commit dcc01198e2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 298 additions and 26 deletions

View file

@ -15,6 +15,7 @@ const SYSTEM_SKILLS_DIR: Dir =
const SYSTEM_SKILLS_DIR_NAME: &str = ".system";
const SKILLS_DIR_NAME: &str = "skills";
const SYSTEM_SKILLS_MARKER_FILENAME: &str = ".codex-system-skills.marker";
const SYSTEM_SKILLS_MARKER_SALT: &str = "v1";
/// Returns the on-disk cache location for embedded system skills.
///
@ -103,6 +104,7 @@ fn embedded_system_skills_fingerprint() -> String {
items.sort_unstable_by(|(a, _), (b, _)| a.cmp(b));
let mut hasher = DefaultHasher::new();
SYSTEM_SKILLS_MARKER_SALT.hash(&mut hasher);
for (path, contents_hash) in items {
path.hash(&mut hasher);
contents_hash.hash(&mut hasher);

View file

@ -9,6 +9,7 @@ use ratatui::text::Line;
use ratatui::text::Span;
use ratatui::widgets::Widget;
use unicode_width::UnicodeWidthChar;
use unicode_width::UnicodeWidthStr;
use crate::key_hint::KeyBinding;
@ -25,6 +26,77 @@ pub(crate) struct GenericDisplayRow {
pub wrap_indent: Option<usize>, // optional indent for wrapped lines
}
fn line_width(line: &Line<'_>) -> usize {
line.iter()
.map(|span| UnicodeWidthStr::width(span.content.as_ref()))
.sum()
}
fn truncate_line_to_width(line: Line<'static>, max_width: usize) -> Line<'static> {
if max_width == 0 {
return Line::from(Vec::<Span<'static>>::new());
}
let mut used = 0usize;
let mut spans_out: Vec<Span<'static>> = Vec::new();
for span in line.spans {
let text = span.content.into_owned();
let style = span.style;
let span_width = UnicodeWidthStr::width(text.as_str());
if span_width == 0 {
spans_out.push(Span::styled(text, style));
continue;
}
if used >= max_width {
break;
}
if used + span_width <= max_width {
used += span_width;
spans_out.push(Span::styled(text, style));
continue;
}
let mut truncated = String::new();
for ch in text.chars() {
let ch_width = UnicodeWidthChar::width(ch).unwrap_or(0);
if used + ch_width > max_width {
break;
}
truncated.push(ch);
used += ch_width;
}
if !truncated.is_empty() {
spans_out.push(Span::styled(truncated, style));
}
break;
}
Line::from(spans_out)
}
fn truncate_line_with_ellipsis_if_overflow(line: Line<'static>, max_width: usize) -> Line<'static> {
if max_width == 0 {
return Line::from(Vec::<Span<'static>>::new());
}
let width = line_width(&line);
if width <= max_width {
return line;
}
let truncated = truncate_line_to_width(line, max_width.saturating_sub(1));
let mut spans = truncated.spans;
let ellipsis_style = spans.last().map(|span| span.style).unwrap_or_default();
spans.push(Span::styled("", ellipsis_style));
Line::from(spans)
}
/// Compute a shared description-column start based on the widest visible name
/// plus two spaces of padding. Ensures at least one column is left for the
/// description.
@ -235,6 +307,72 @@ pub(crate) fn render_rows(
}
}
/// Render rows as a single line each (no wrapping), truncating overflow with an ellipsis.
pub(crate) fn render_rows_single_line(
area: Rect,
buf: &mut Buffer,
rows_all: &[GenericDisplayRow],
state: &ScrollState,
max_results: usize,
empty_message: &str,
) {
if rows_all.is_empty() {
if area.height > 0 {
Line::from(empty_message.dim().italic()).render(area, buf);
}
return;
}
let visible_items = max_results
.min(rows_all.len())
.min(area.height.max(1) as usize);
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_items > 0 {
let bottom = start_idx + visible_items - 1;
if sel > bottom {
start_idx = sel + 1 - visible_items;
}
}
}
let desc_col = compute_desc_col(rows_all, start_idx, visible_items, area.width);
let mut cur_y = area.y;
for (i, row) in rows_all
.iter()
.enumerate()
.skip(start_idx)
.take(visible_items)
{
if cur_y >= area.y + area.height {
break;
}
let mut full_line = build_full_line(row, desc_col);
if Some(i) == state.selected_idx {
full_line.spans.iter_mut().for_each(|span| {
span.style = Style::default().fg(Color::Cyan).bold();
});
}
let full_line = truncate_line_with_ellipsis_if_overflow(full_line, area.width as usize);
full_line.render(
Rect {
x: area.x,
y: cur_y,
width: area.width,
height: 1,
},
buf,
);
cur_y = cur_y.saturating_add(1);
}
}
/// Compute the number of terminal rows required to render up to `max_results`
/// items from `rows_all` given the current scroll/selection state and the
/// available `width`. Accounts for description wrapping and alignment so the
@ -281,7 +419,8 @@ pub(crate) fn measure_rows_height(
let opts = RtOptions::new(content_width as usize)
.initial_indent(Line::from(""))
.subsequent_indent(Line::from(" ".repeat(continuation_indent)));
total = total.saturating_add(word_wrap_line(&full_line, opts).len() as u16);
let wrapped_lines = word_wrap_line(&full_line, opts).len();
total = total.saturating_add(wrapped_lines as u16);
}
total.max(1)
}

View file

@ -5,13 +5,14 @@ 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::measure_rows_height;
use super::selection_popup_common::render_rows;
use super::selection_popup_common::render_rows_single_line;
use crate::render::Insets;
use crate::render::RectExt;
use codex_common::fuzzy_match::fuzzy_match;
use codex_core::skills::model::SkillMetadata;
use crate::text_formatting::truncate_text;
pub(crate) struct SkillPopup {
query: String,
skills: Vec<SkillMetadata>,
@ -37,9 +38,10 @@ impl SkillPopup {
self.clamp_selection();
}
pub(crate) fn calculate_required_height(&self, width: u16) -> u16 {
pub(crate) fn calculate_required_height(&self, _width: u16) -> u16 {
let rows = self.rows_from_matches(self.filtered());
measure_rows_height(&rows, &self.state, MAX_POPUP_ROWS, width)
let visible = rows.len().clamp(1, MAX_POPUP_ROWS);
visible as u16
}
pub(crate) fn move_up(&mut self) {
@ -79,13 +81,7 @@ impl SkillPopup {
.into_iter()
.map(|(idx, indices, _score)| {
let skill = &self.skills[idx];
let slug = skill
.path
.parent()
.and_then(|p| p.file_name())
.and_then(|n| n.to_str())
.unwrap_or(&skill.name);
let name = format!("{} ({slug})", skill.name);
let name = truncate_text(&skill.name, 21);
let description = skill
.short_description
.as_ref()
@ -135,7 +131,7 @@ impl SkillPopup {
impl WidgetRef for SkillPopup {
fn render_ref(&self, area: Rect, buf: &mut Buffer) {
let rows = self.rows_from_matches(self.filtered());
render_rows(
render_rows_single_line(
area.inset(Insets::tlbr(0, 2, 0, 0)),
buf,
&rows,

View file

@ -9,6 +9,7 @@ use ratatui::text::Line;
use ratatui::text::Span;
use ratatui::widgets::Widget;
use unicode_width::UnicodeWidthChar;
use unicode_width::UnicodeWidthStr;
use crate::key_hint::KeyBinding;
@ -23,6 +24,77 @@ pub(crate) struct GenericDisplayRow {
pub wrap_indent: Option<usize>, // optional indent for wrapped lines
}
fn line_width(line: &Line<'_>) -> usize {
line.iter()
.map(|span| UnicodeWidthStr::width(span.content.as_ref()))
.sum()
}
fn truncate_line_to_width(line: Line<'static>, max_width: usize) -> Line<'static> {
if max_width == 0 {
return Line::from(Vec::<Span<'static>>::new());
}
let mut used = 0usize;
let mut spans_out: Vec<Span<'static>> = Vec::new();
for span in line.spans {
let text = span.content.into_owned();
let style = span.style;
let span_width = UnicodeWidthStr::width(text.as_str());
if span_width == 0 {
spans_out.push(Span::styled(text, style));
continue;
}
if used >= max_width {
break;
}
if used + span_width <= max_width {
used += span_width;
spans_out.push(Span::styled(text, style));
continue;
}
let mut truncated = String::new();
for ch in text.chars() {
let ch_width = UnicodeWidthChar::width(ch).unwrap_or(0);
if used + ch_width > max_width {
break;
}
truncated.push(ch);
used += ch_width;
}
if !truncated.is_empty() {
spans_out.push(Span::styled(truncated, style));
}
break;
}
Line::from(spans_out)
}
fn truncate_line_with_ellipsis_if_overflow(line: Line<'static>, max_width: usize) -> Line<'static> {
if max_width == 0 {
return Line::from(Vec::<Span<'static>>::new());
}
let width = line_width(&line);
if width <= max_width {
return line;
}
let truncated = truncate_line_to_width(line, max_width.saturating_sub(1));
let mut spans = truncated.spans;
let ellipsis_style = spans.last().map(|span| span.style).unwrap_or_default();
spans.push(Span::styled("", ellipsis_style));
Line::from(spans)
}
/// Compute a shared description-column start based on the widest visible name
/// plus two spaces of padding. Ensures at least one column is left for the
/// description.
@ -217,6 +289,72 @@ pub(crate) fn render_rows(
}
}
/// Render rows as a single line each (no wrapping), truncating overflow with an ellipsis.
pub(crate) fn render_rows_single_line(
area: Rect,
buf: &mut Buffer,
rows_all: &[GenericDisplayRow],
state: &ScrollState,
max_results: usize,
empty_message: &str,
) {
if rows_all.is_empty() {
if area.height > 0 {
Line::from(empty_message.dim().italic()).render(area, buf);
}
return;
}
let visible_items = max_results
.min(rows_all.len())
.min(area.height.max(1) as usize);
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_items > 0 {
let bottom = start_idx + visible_items - 1;
if sel > bottom {
start_idx = sel + 1 - visible_items;
}
}
}
let desc_col = compute_desc_col(rows_all, start_idx, visible_items, area.width);
let mut cur_y = area.y;
for (i, row) in rows_all
.iter()
.enumerate()
.skip(start_idx)
.take(visible_items)
{
if cur_y >= area.y + area.height {
break;
}
let mut full_line = build_full_line(row, desc_col);
if Some(i) == state.selected_idx {
full_line.spans.iter_mut().for_each(|span| {
span.style = Style::default().fg(Color::Cyan).bold();
});
}
let full_line = truncate_line_with_ellipsis_if_overflow(full_line, area.width as usize);
full_line.render(
Rect {
x: area.x,
y: cur_y,
width: area.width,
height: 1,
},
buf,
);
cur_y = cur_y.saturating_add(1);
}
}
/// Compute the number of terminal rows required to render up to `max_results`
/// items from `rows_all` given the current scroll/selection state and the
/// available `width`. Accounts for description wrapping and alignment so the
@ -263,7 +401,8 @@ pub(crate) fn measure_rows_height(
let opts = RtOptions::new(content_width as usize)
.initial_indent(Line::from(""))
.subsequent_indent(Line::from(" ".repeat(continuation_indent)));
total = total.saturating_add(word_wrap_line(&full_line, opts).len() as u16);
let wrapped_lines = word_wrap_line(&full_line, opts).len();
total = total.saturating_add(wrapped_lines as u16);
}
total.max(1)
}

View file

@ -5,13 +5,14 @@ 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::measure_rows_height;
use super::selection_popup_common::render_rows;
use super::selection_popup_common::render_rows_single_line;
use crate::render::Insets;
use crate::render::RectExt;
use codex_common::fuzzy_match::fuzzy_match;
use codex_core::skills::model::SkillMetadata;
use crate::text_formatting::truncate_text;
pub(crate) struct SkillPopup {
query: String,
skills: Vec<SkillMetadata>,
@ -37,9 +38,10 @@ impl SkillPopup {
self.clamp_selection();
}
pub(crate) fn calculate_required_height(&self, width: u16) -> u16 {
pub(crate) fn calculate_required_height(&self, _width: u16) -> u16 {
let rows = self.rows_from_matches(self.filtered());
measure_rows_height(&rows, &self.state, MAX_POPUP_ROWS, width)
let visible = rows.len().clamp(1, MAX_POPUP_ROWS);
visible as u16
}
pub(crate) fn move_up(&mut self) {
@ -79,13 +81,7 @@ impl SkillPopup {
.into_iter()
.map(|(idx, indices, _score)| {
let skill = &self.skills[idx];
let slug = skill
.path
.parent()
.and_then(|p| p.file_name())
.and_then(|n| n.to_str())
.unwrap_or(&skill.name);
let name = format!("{} ({slug})", skill.name);
let name = truncate_text(&skill.name, 21);
let description = skill
.short_description
.as_ref()
@ -134,7 +130,7 @@ impl SkillPopup {
impl WidgetRef for SkillPopup {
fn render_ref(&self, area: Rect, buf: &mut Buffer) {
let rows = self.rows_from_matches(self.filtered());
render_rows(
render_rows_single_line(
area.inset(Insets::tlbr(0, 2, 0, 0)),
buf,
&rows,