From 325ca38486ee1ca97b6c1468763b7800af709a33 Mon Sep 17 00:00:00 2001 From: Blankeos Date: Thu, 15 Oct 2026 00:22:54 +0800 Subject: [PATCH 1/2] feat(skills): add skill info view, mention insertion, and path-free catalog - Skills dialog gains an info view (ctrl+i) with source open in the configured editor - Enter on a skill inserts `@name` at the cursor - Skill mentions get the same color as agent mentions; agent names win on collision - Skill catalog in the system prompt drops `` paths; the skill tool loads bodies by name - Add `estimate_skill_catalog_tokens` for catalog token counts - Extract file actions (copy, reveal, open) into `App::perform_file_action` - Add `title_note` to `Dialog` for muted secondary header text --- src/agent/subagent.rs | 4 +- src/app.rs | 216 ++++++--- src/prompt/mod.rs | 29 +- src/ui/components/dialog.rs | 19 +- src/ui/components/input.rs | 83 +++- src/views/home.rs | 5 +- src/views/skills_dialog.rs | 849 +++++++++++++++++++++++++++++++++--- 7 files changed, 1054 insertions(+), 151 deletions(-) diff --git a/src/agent/subagent.rs b/src/agent/subagent.rs index eb8553c..1a584d8 100644 --- a/src/agent/subagent.rs +++ b/src/agent/subagent.rs @@ -822,9 +822,7 @@ mod tests { assert!(system .content .contains("Use test-workflow for focused testing.")); - assert!(system - .content - .contains("file:///skills/test-workflow/SKILL.md")); + assert!(!system.content.contains("")); assert!(system .content .contains("use the skill tool to load it before responding")); diff --git a/src/app.rs b/src/app.rs index 3adc029..9459dd4 100644 --- a/src/app.rs +++ b/src/app.rs @@ -5426,64 +5426,7 @@ impl App { let Some(popup) = self.file_actions.take() else { return; }; - if action == FileAction::Copy { - self.copy_text_with_toast(&popup.copy_payload(), "Copied absolute path"); - return; - } - let target = popup.target; - if action == FileAction::Reveal { - // push_toast uses the global Mutex, so the - // worker can report its outcome without borrowing the app. - std::thread::spawn(move || { - match crate::utils::file_opener::reveal_file_path(&target.path) { - Ok(()) => push_toast(Toast::new( - format!("Revealed {}", target.path.display()), - ToastLevel::Info, - None, - )), - Err(err) => push_toast(Toast::new( - format!("File action failed: {err}"), - ToastLevel::Error, - None, - )), - } - }); - return; - } - let result = match action { - FileAction::Open => { - let result = if let Some(line) = target.line { - crate::utils::file_opener::open_file_path_at_location( - &target.path, - line, - target.column.unwrap_or(1), - &self.editor, - ) - } else { - crate::utils::file_opener::open_file_path(&target.path, &self.editor) - }; - result.map(|outcome| { - if let crate::utils::file_opener::OpenOutcome::Suspend(command) = - outcome - { - self.pending_editor_suspend = Some(command); - } - }) - } - FileAction::Reveal | FileAction::Copy => unreachable!(), - }; - match result { - Ok(()) => push_toast(Toast::new( - format!("Opened {}", target.path.display()), - ToastLevel::Info, - None, - )), - Err(err) => push_toast(Toast::new( - format!("File action failed: {err}"), - ToastLevel::Error, - None, - )), - } + self.perform_file_action(action, popup.target); } } } @@ -5633,7 +5576,9 @@ impl App { OverlayFocus::SessionsDialog if self.sessions_dialog_state.item_menu.is_none() => { Some(&self.sessions_dialog_state.dialog) } - OverlayFocus::SkillsDialog => Some(&self.skills_dialog_state.dialog), + OverlayFocus::SkillsDialog if !self.skills_dialog_state.info_is_open() => { + Some(&self.skills_dialog_state.dialog) + } OverlayFocus::McpDialog => Some(&self.mcp_dialog_state.dialog), OverlayFocus::TimelineDialog => Some(&self.timeline_dialog_state.dialog), OverlayFocus::CommandPalette => Some(&self.command_palette_state.dialog), @@ -6400,7 +6345,7 @@ impl App { .join(""), ); } - (_, OverlayFocus::SkillsDialog) => { + (_, OverlayFocus::SkillsDialog) if !self.skills_dialog_state.info_is_open() => { self.skills_dialog_state .dialog .search_textarea @@ -9293,13 +9238,91 @@ impl App { self.overlay_focus = OverlayFocus::SkillsDialog; } + fn perform_file_action(&mut self, action: FileAction, target: FileHyperlinkTarget) { + if action == FileAction::Copy { + match crate::utils::file_opener::absolute_file_path(&target.path) { + Ok(path) => { + self.copy_text_with_toast(&path.to_string_lossy(), "Copied absolute path") + } + Err(err) => push_toast(Toast::new( + format!("Failed to resolve file: {err}"), + ToastLevel::Error, + None, + )), + } + return; + } + if action == FileAction::Reveal { + // push_toast uses the global Mutex, so the + // worker can report its outcome without borrowing the app. + std::thread::spawn(move || { + match crate::utils::file_opener::reveal_file_path(&target.path) { + Ok(()) => push_toast(Toast::new( + format!("Revealed {}", target.path.display()), + ToastLevel::Info, + None, + )), + Err(err) => push_toast(Toast::new( + format!("File action failed: {err}"), + ToastLevel::Error, + None, + )), + } + }); + return; + } + let result = if let Some(line) = target.line { + crate::utils::file_opener::open_file_path_at_location( + &target.path, + line, + target.column.unwrap_or(1), + &self.editor, + ) + } else { + crate::utils::file_opener::open_file_path(&target.path, &self.editor) + }; + let result = result.map(|outcome| { + if let crate::utils::file_opener::OpenOutcome::Suspend(command) = outcome { + self.pending_editor_suspend = Some(command); + } + }); + match result { + Ok(()) => push_toast(Toast::new( + format!("Opened {}", target.path.display()), + ToastLevel::Info, + None, + )), + Err(err) => push_toast(Toast::new( + format!("File action failed: {err}"), + ToastLevel::Error, + None, + )), + } + } + fn handle_skills_dialog_action( &mut self, action: crate::views::skills_dialog::SkillsDialogAction, ) { - let crate::views::skills_dialog::SkillsDialogAction::Toggle { skill_id } = action else { - return; - }; + use crate::views::skills_dialog::SkillsDialogAction; + match action { + SkillsDialogAction::None => {} + SkillsDialogAction::InsertMention { name } => { + self.input.insert_str(&format!("@{name}")); + } + SkillsDialogAction::File { action, path } => self.perform_file_action( + action, + FileHyperlinkTarget { + path, + line: None, + column: None, + }, + ), + SkillsDialogAction::Toggle { skill_id } => self.toggle_skill(&skill_id), + } + } + + fn toggle_skill(&mut self, skill_id: &str) { let result = (|| -> anyhow::Result<()> { let store = crate::skill::get_skill_store() .ok_or_else(|| anyhow::anyhow!("Skill store not initialized"))?; @@ -18194,6 +18217,73 @@ mod tests { } } + #[test] + fn enter_on_a_skill_inserts_the_mention_at_the_cursor() { + let mut app = test_app(); + app.input.set_text("xy"); + assert!(app + .input + .handle_event(KeyEvent::new(KeyCode::Left, KeyModifiers::NONE))); + app.skills_dialog_state = crate::views::skills_dialog::init_skills_dialog( + "Skills", + vec![crate::ui::components::dialog::DialogItem { + id: "computer-use".to_string(), + name: "computer-use".to_string(), + group: "Skills".to_string(), + description: String::new(), + tip: None, + provider_id: String::new(), + active: true, + }], + ); + app.skills_dialog_state.dialog.show(); + app.overlay_focus = OverlayFocus::SkillsDialog; + app.handle_keys(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert_eq!(app.input.get_text(), "x@computer-usey"); + assert_eq!(app.overlay_focus, OverlayFocus::None); + } + + #[test] + fn skill_source_open_uses_the_configured_editor() { + let root = tempfile::tempdir().unwrap(); + let path = root.path().join("SKILL.md"); + std::fs::write(&path, "# skill\n").unwrap(); + let store = crate::skill::SkillStore::for_test([crate::skill::SkillInfo { + name: "alpha".to_string(), + description: None, + location: path.clone(), + content: String::new(), + }]); + let mut app = test_app(); + app.editor.open = Some("ed {pathname}".to_string()); + app.editor.suspend = true; + app.skills_dialog_state = crate::views::skills_dialog::init_skills_dialog("Skills", vec![]); + app.skills_dialog_state.refresh(&store); + app.skills_dialog_state.dialog.show(); + app.overlay_focus = OverlayFocus::SkillsDialog; + + app.handle_keys(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::CONTROL)); + assert!(app.skills_dialog_state.info_is_open()); + assert_eq!(app.input.get_text(), ""); + + app.handle_keys(KeyEvent::new(KeyCode::Char('o'), KeyModifiers::NONE)); + let expected = + crate::utils::file_opener::expand_editor_open_command("ed {pathname}", &path, 1, 1) + .unwrap(); + assert_eq!(app.take_editor_suspend(), Some(expected)); + assert!(app.skills_dialog_state.info_is_open()); + assert_eq!(app.overlay_focus, OverlayFocus::SkillsDialog); + assert_eq!(app.skills_dialog_state.dialog.title, "Skills"); + assert!( + app.skills_dialog_state + .dialog + .title_note + .starts_with("1 ~"), + "{}", + app.skills_dialog_state.dialog.title_note + ); + } + #[test] fn skill_suggestions_fill_without_submitting_and_match_remote_discovery() { let root = tempfile::tempdir().unwrap(); diff --git a/src/prompt/mod.rs b/src/prompt/mod.rs index d28a84f..07e509d 100644 --- a/src/prompt/mod.rs +++ b/src/prompt/mod.rs @@ -14,10 +14,9 @@ pub(crate) fn render_skill_guidance<'a>( .into_iter() .map(|skill| { format!( - " \n {}\n {}\n file://{}\n ", + " \n {}\n {}\n ", skill.name, - skill.description.as_deref().unwrap_or(""), - skill.location.display() + skill.description.as_deref().unwrap_or("") ) }) .collect::>() @@ -29,6 +28,17 @@ pub(crate) fn render_skill_guidance<'a>( format!("{SKILL_GUIDANCE_PREFIX}{skills_xml}\n") } +/// Tokens the enabled catalog adds to every prompt. +/// +/// Matches the current OpenCode catalog: name and description only. Paths and +/// skill bodies stay out; the skill tool loads a body by name. The count is +/// the rendered block at ~4 characters per token, not a provider tokenizer. +pub(crate) fn estimate_skill_catalog_tokens<'a>( + skills: impl IntoIterator, +) -> usize { + render_skill_guidance(skills).chars().count().div_ceil(4) +} + /// Refresh only our generated catalog in a restored system prompt. Do not /// rewrite skill content already loaded in conversation history or user rules. pub(crate) fn refresh_skill_guidance<'a>( @@ -491,8 +501,19 @@ mod tests { assert_eq!(guidance.matches("").count(), 1); assert!(guidance.contains("test-skill")); assert!(guidance.contains("")); - assert!(guidance.contains("file:///skills/test-skill/SKILL.md")); + assert!(!guidance.contains("")); assert!(!guidance.contains(&skill.content)); + assert_eq!( + estimate_skill_catalog_tokens([&skill]), + guidance.chars().count().div_ceil(4) + ); + assert_eq!(estimate_skill_catalog_tokens([]), 0); + let mut with_body = skill.clone(); + with_body.content = "x".repeat(10_000); + assert_eq!( + estimate_skill_catalog_tokens([&skill]), + estimate_skill_catalog_tokens([&with_body]) + ); } #[test] diff --git a/src/ui/components/dialog.rs b/src/ui/components/dialog.rs index 77ef4c4..e31541a 100644 --- a/src/ui/components/dialog.rs +++ b/src/ui/components/dialog.rs @@ -123,6 +123,8 @@ fn dialog_action_key(hint: &str) -> Option { #[derive(Debug)] pub struct Dialog { pub title: String, + /// Secondary header text, drawn after `title` in the muted color. + pub title_note: String, pub items: Vec, pub grouped_items: HashMap>, pub filtered_items: Vec<(String, Vec)>, @@ -164,6 +166,7 @@ impl Dialog { search_textarea.set_placeholder_text("Search"); Self { title, + title_note: String::new(), items: Vec::new(), grouped_items: HashMap::new(), filtered_items: Vec::new(), @@ -1764,13 +1767,20 @@ impl Dialog { ]) .split(chunks[0]); - let title_paragraph = Paragraph::new(Line::from(vec![Span::styled( - &self.title, + let mut title_spans = vec![Span::styled( + self.title.clone(), Style::default() .fg(colors.text) .add_modifier(Modifier::BOLD), - )])) - .alignment(ratatui::layout::Alignment::Left); + )]; + if !self.title_note.is_empty() { + title_spans.push(Span::styled( + format!(" {}", self.title_note), + Style::default().fg(colors.text_weak), + )); + } + let title_paragraph = + Paragraph::new(Line::from(title_spans)).alignment(ratatui::layout::Alignment::Left); frame.render_widget(title_paragraph, header_chunks[0]); let esc_paragraph = Paragraph::new(Line::from(vec![Span::styled( @@ -2081,6 +2091,7 @@ impl Clone for Dialog { fn clone(&self) -> Self { Self { title: self.title.clone(), + title_note: self.title_note.clone(), items: self.items.clone(), grouped_items: self.grouped_items.clone(), filtered_items: self.filtered_items.clone(), diff --git a/src/ui/components/input.rs b/src/ui/components/input.rs index 66ee8bf..d9d415a 100644 --- a/src/ui/components/input.rs +++ b/src/ui/components/input.rs @@ -1834,17 +1834,27 @@ impl Input { } } - fn configured_agent_names(&self) -> Vec { - self.autocomplete - .as_ref() - .map(|autocomplete| { - autocomplete - .agents - .iter() - .map(|agent| agent.name.clone()) - .collect() - }) - .unwrap_or_default() + /// Agent names, then skill names that don't collide with an agent. + /// Collisions keep the agent so `@explore` stays an agent mention. + fn configured_mention_names(&self) -> Vec { + let Some(autocomplete) = &self.autocomplete else { + return Vec::new(); + }; + let mut names: Vec = autocomplete + .agents + .iter() + .map(|agent| agent.name.clone()) + .collect(); + for skill in &autocomplete.skills { + if names + .iter() + .any(|name| name.eq_ignore_ascii_case(&skill.name)) + { + continue; + } + names.push(skill.name.clone()); + } + names } fn style_agent_mention_ranges( @@ -1858,7 +1868,7 @@ impl Input { return; } - let agent_names = self.configured_agent_names(); + let agent_names = self.configured_mention_names(); if agent_names.is_empty() { return; } @@ -3677,6 +3687,55 @@ mod tests { ); } + #[test] + fn test_skill_mentions_render_with_the_same_mention_colors_as_agents() { + use crate::autocomplete::{AutoComplete, CommandAuto, Suggestion}; + use crate::command::registry::Registry; + use crate::theme::agent_color; + use ratatui::{backend::TestBackend, Terminal}; + + let mut input = Input::new(); + input.autocomplete = Some( + AutoComplete::new(CommandAuto::new(&Registry::new())) + .with_agents(vec![Suggestion::agent("explore", "Explore agent")]) + .with_skills(vec![ + Suggestion::skill("explore", "Name collision stays an agent"), + Suggestion::skill("codebase-design", "Design deep modules"), + ]), + ); + input.insert_str("@explore and @codebase-design"); + + let colors = test_colors(); + let mut terminal = Terminal::new(TestBackend::new(48, 6)).unwrap(); + terminal + .draw(|frame| { + input.render( + frame, + Rect::new(0, 0, 48, 6), + "Plan", + "model", + "provider", + None, + false, + &colors, + true, + ); + }) + .unwrap(); + + let buffer = terminal.backend().buffer(); + let explore_pos = find_buffer_text(buffer, 48, 6, "@explore").expect("explore mention"); + let skill_pos = find_buffer_text(buffer, 48, 6, "@codebase-design").expect("skill mention"); + assert_eq!( + buffer.cell(explore_pos).expect("explore cell").style().fg, + Some(colors.warning) + ); + assert_eq!( + buffer.cell(skill_pos).expect("skill cell").style().fg, + Some(agent_color("codebase-design", &colors)) + ); + } + #[test] fn test_alt_shift_left_extends_selection_by_word() { let mut input = Input::new(); diff --git a/src/views/home.rs b/src/views/home.rs index 383a2fe..b6741e6 100644 --- a/src/views/home.rs +++ b/src/views/home.rs @@ -406,7 +406,10 @@ mod tests { assert_eq!(text_in(&buffer, area), label); assert_eq!(area.height, 1); let suffix = Rect::new(area.right(), area.y, 35, 1); - assert_eq!(text_in(&buffer, suffix), " shift-tab agents ctrl+p commands"); + assert_eq!( + text_in(&buffer, suffix), + " shift-tab agents ctrl+p commands" + ); let colors = crate::theme::Theme::load_builtin_default().get_colors(true); assert_eq!(buffer[(area.x, area.y)].fg, colors.warning); assert!(buffer[(area.x, area.y)].modifier.contains(Modifier::BOLD)); diff --git a/src/views/skills_dialog.rs b/src/views/skills_dialog.rs index 3b53553..ba59a3e 100644 --- a/src/views/skills_dialog.rs +++ b/src/views/skills_dialog.rs @@ -1,34 +1,69 @@ -use ratatui::crossterm::event::{KeyCode, KeyEvent, MouseButton, MouseEvent, MouseEventKind}; -use ratatui::{layout::Rect, Frame}; +use std::collections::HashMap; +use std::path::PathBuf; + +use ratatui::crossterm::event::{ + KeyCode, KeyEvent, KeyModifiers, MouseButton, MouseEvent, MouseEventKind, +}; +use ratatui::layout::{Position, Rect}; +use ratatui::style::{Modifier, Style}; +use ratatui::text::{Line, Span}; +use ratatui::widgets::{Clear, Paragraph}; +use ratatui::Frame; +use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; use crate::skill::SkillStore; -use crate::theme::ThemeColors; -use crate::ui::components::dialog::{Dialog, DialogAction, DialogItem}; +use crate::theme::{contrast_text, ThemeColors}; +use crate::ui::components::dialog::{dim_backdrop, Dialog, DialogAction, DialogItem}; +use crate::ui::components::file_actions::{reveal_label, FileAction}; -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum SkillsDialogAction { - Toggle { skill_id: String }, + Toggle { + skill_id: String, + }, + /// Insert `@name` at the chat cursor. The dialog closes; the input is kept. + InsertMention { + name: String, + }, + /// Open, reveal, or copy a skill file. Uses the same file-action config as chat links. + File { + action: FileAction, + path: PathBuf, + }, None, } #[derive(Debug)] pub struct SkillsDialogState { pub dialog: Dialog, + locations: HashMap, + source: Option, +} + +#[derive(Debug)] +struct SkillSource { + path: PathBuf, + selected: usize, + area: Rect, + esc_area: Rect, + action_rows: [Rect; 3], } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SourceEvent { + None, + Close, + Choose(FileAction), +} + +const SOURCE_ACTIONS: [FileAction; 3] = [FileAction::Open, FileAction::Reveal, FileAction::Copy]; + impl SkillsDialogState { pub fn new(dialog: Dialog) -> Self { Self { - dialog: dialog.with_actions(vec![ - DialogAction { - label: "toggle".to_string(), - key: "space".to_string(), - }, - DialogAction { - label: "close".to_string(), - key: "esc".to_string(), - }, - ]), + dialog: dialog.with_actions(list_actions()), + locations: HashMap::new(), + source: None, } } @@ -38,8 +73,236 @@ impl SkillsDialogState { /// Update activation labels without losing search, selection, or scroll. pub fn refresh(&mut self, store: &SkillStore) { + let installed = store.installed(); + self.locations = installed + .iter() + .map(|skill| (skill.name.clone(), skill.location.clone())) + .collect(); + let enabled = store.all(); + let tokens = crate::prompt::estimate_skill_catalog_tokens(enabled.iter().copied()); + self.dialog.title = "Skills".to_string(); + self.dialog.title_note = skill_title_note(enabled.len(), installed.len(), tokens); self.dialog.set_items_preserve_ui(skill_dialog_items(store)); } + + pub fn info_is_open(&self) -> bool { + self.source.is_some() + } +} + +impl SkillSource { + fn new(path: PathBuf) -> Self { + Self { + path, + selected: 0, + area: Rect::default(), + esc_area: Rect::default(), + action_rows: [Rect::default(); 3], + } + } + + fn handle_key(&mut self, event: KeyEvent) -> SourceEvent { + match event.code { + KeyCode::Esc => SourceEvent::Close, + KeyCode::Up | KeyCode::Left | KeyCode::BackTab => { + self.selected = (self.selected + 2) % 3; + SourceEvent::None + } + KeyCode::Tab if event.modifiers.contains(KeyModifiers::SHIFT) => { + self.selected = (self.selected + 2) % 3; + SourceEvent::None + } + KeyCode::Down | KeyCode::Right | KeyCode::Tab => { + self.selected = (self.selected + 1) % 3; + SourceEvent::None + } + KeyCode::Home => { + self.selected = 0; + SourceEvent::None + } + KeyCode::End => { + self.selected = 2; + SourceEvent::None + } + KeyCode::Enter | KeyCode::Char(' ') if event.modifiers == KeyModifiers::NONE => { + SourceEvent::Choose(SOURCE_ACTIONS[self.selected]) + } + KeyCode::Char(ch) if event.modifiers == KeyModifiers::NONE => source_action_for_key(ch) + .map(SourceEvent::Choose) + .unwrap_or(SourceEvent::None), + _ => SourceEvent::None, + } + } + + fn action_at(&self, position: Position) -> Option { + self.action_rows + .iter() + .enumerate() + .find_map(|(index, row)| row.contains(position).then_some(SOURCE_ACTIONS[index])) + } + + fn render(&mut self, frame: &mut Frame, screen: Rect, name: &str, colors: ThemeColors) { + let width = screen.width.min(70).max(1); + let pad_x = 3.min(width / 4); + let inner_width = width.saturating_sub(pad_x.saturating_mul(2)).max(1); + let path_lines = wrap_path(&self.path.display().to_string(), inner_width as usize); + let path_height = path_lines.len().max(1) as u16; + // pad, title, gap, path, gap, three actions, pad + let height = (8 + path_height).min(screen.height).max(1); + let area = Rect { + x: screen.x + screen.width.saturating_sub(width) / 2, + y: screen.y + screen.height.saturating_sub(height) / 2, + width, + height, + }; + self.area = area; + dim_backdrop(frame, screen, area); + frame.render_widget(Clear, area); + frame.render_widget( + Paragraph::new("").style(Style::default().bg(colors.dialog_background)), + area, + ); + + let content = Rect { + x: area.x + pad_x.min(area.width.saturating_sub(1)), + y: area.y + 1.min(area.height.saturating_sub(1)), + width: inner_width.min(area.width), + height: area.height.saturating_sub(2).max(1), + }; + let name = truncate_to_width(name, content.width.saturating_sub(4) as usize); + let gap = (content.width as usize) + .saturating_sub(name.width()) + .saturating_sub("esc".width()); + let title = Line::from(vec![ + Span::styled( + name, + Style::default() + .fg(colors.primary) + .add_modifier(Modifier::BOLD), + ), + Span::raw(" ".repeat(gap)), + Span::styled( + "esc", + Style::default() + .fg(colors.primary) + .add_modifier(Modifier::BOLD), + ), + ]); + frame.render_widget( + Paragraph::new(title), + Rect::new(content.x, content.y, content.width, 1), + ); + self.esc_area = Rect::new( + content.right().saturating_sub(3), + content.y, + 3.min(content.width), + 1, + ); + + let path_area = Rect::new( + content.x, + content.y.saturating_add(2), + content.width, + path_height.min(content.height.saturating_sub(2)), + ); + let path_style = Style::default() + .fg(colors.text_weak) + .bg(colors.dialog_background); + frame.render_widget( + Paragraph::new( + path_lines + .into_iter() + .map(|line| Line::from(Span::styled(line, path_style))) + .collect::>(), + ), + path_area, + ); + + let actions_y = path_area.bottom().saturating_add(1); + let base = Style::default() + .fg(colors.text) + .bg(colors.dialog_background); + let selected_fg = contrast_text(colors.primary); + for (index, action) in SOURCE_ACTIONS.iter().enumerate() { + let row = Rect::new( + content.x, + actions_y.saturating_add(index as u16), + content.width, + 1, + ); + self.action_rows[index] = row; + if row.y >= area.bottom() { + continue; + } + let style = if index == self.selected { + Style::default() + .fg(selected_fg) + .bg(colors.primary) + .add_modifier(Modifier::BOLD) + } else { + base + }; + let line = Line::from(vec![ + Span::styled( + source_key(*action).to_string(), + style.add_modifier(Modifier::BOLD), + ), + Span::styled(format!(" {} ", source_label(*action)), style), + ]); + frame.render_widget(Paragraph::new(line).style(style), row); + } + } +} + +/// `25` when every installed skill is enabled, `23/25` otherwise. +/// The token figure is the enabled catalog only, so a fully disabled list +/// shows no estimate. +fn skill_title_note(enabled: usize, installed: usize, tokens: usize) -> String { + let count = if enabled == installed { + installed.to_string() + } else { + format!("{enabled}/{installed}") + }; + if tokens == 0 { + count + } else { + format!("{count} {}", format_tokens(tokens)) + } +} + +fn format_tokens(tokens: usize) -> String { + if tokens < 1_000 { + return format!("~{tokens}"); + } + let tenths = tokens.saturating_add(50) / 100; + let whole = tenths / 10; + let frac = tenths % 10; + if frac == 0 { + format!("~{whole}k") + } else { + format!("~{whole}.{frac}k") + } +} + +fn list_actions() -> Vec { + vec![ + DialogAction { + label: "toggle".to_string(), + key: "space".to_string(), + }, + DialogAction { + label: "insert".to_string(), + key: "enter".to_string(), + }, + DialogAction { + label: "info".to_string(), + key: "ctrl+i".to_string(), + }, + DialogAction { + label: "close".to_string(), + key: "esc".to_string(), + }, + ] } fn skill_dialog_items(store: &SkillStore) -> Vec { @@ -64,17 +327,119 @@ fn skill_dialog_items(store: &SkillStore) -> Vec { .collect() } +fn is_info_key(event: KeyEvent) -> bool { + event.modifiers.contains(KeyModifiers::CONTROL) + && !event.modifiers.contains(KeyModifiers::ALT) + && matches!(event.code, KeyCode::Char('i') | KeyCode::Char('I')) +} + +fn source_action_for_key(ch: char) -> Option { + match ch { + 'o' | 'O' => Some(FileAction::Open), + 'r' | 'R' => Some(FileAction::Reveal), + 'y' | 'Y' => Some(FileAction::Copy), + _ => None, + } +} + +fn source_key(action: FileAction) -> &'static str { + match action { + FileAction::Open => "o", + FileAction::Reveal => "r", + FileAction::Copy => "y", + } +} + +fn source_label(action: FileAction) -> String { + match action { + FileAction::Open => "Open".to_string(), + FileAction::Reveal => reveal_label().to_string(), + FileAction::Copy => "Copy absolute path".to_string(), + } +} + +fn truncate_to_width(text: &str, max_width: usize) -> String { + if text.width() <= max_width { + return text.to_string(); + } + let mut out = String::new(); + let mut width = 0; + for ch in text.chars() { + let ch_width = UnicodeWidthChar::width(ch).unwrap_or(0); + if width + ch_width + 3 > max_width { + break; + } + out.push(ch); + width += ch_width; + } + out.push_str("..."); + out +} + +/// Wrap a path on `/` when it fits, so the filename stays on one line. +fn wrap_path(text: &str, width: usize) -> Vec { + if width == 0 || text.is_empty() { + return vec![text.to_string()]; + } + let mut lines = Vec::new(); + let mut current = String::new(); + let mut current_width = 0usize; + for ch in text.chars() { + let ch_width = UnicodeWidthChar::width(ch).unwrap_or(0).max(1); + if current_width + ch_width > width && !current.is_empty() { + if let Some(slash) = current.rfind('/') { + let break_at = slash + '/'.len_utf8(); + if break_at < current.len() { + let rest = current[break_at..].to_string(); + current.truncate(break_at); + lines.push(std::mem::take(&mut current)); + current = rest; + current_width = current.width(); + if current_width + ch_width <= width { + current.push(ch); + current_width += ch_width; + continue; + } + lines.push(std::mem::take(&mut current)); + current_width = 0; + } + } + if current_width + ch_width > width && !current.is_empty() { + lines.push(std::mem::take(&mut current)); + current_width = 0; + } + } + current.push(ch); + current_width += ch_width; + } + if !current.is_empty() { + lines.push(current); + } + if lines.is_empty() { + lines.push(String::new()); + } + lines +} + pub fn init_skills_dialog(title: impl Into, items: Vec) -> SkillsDialogState { SkillsDialogState::with_items(title, items) } pub fn render_skills_dialog( - f: &mut Frame, + frame: &mut Frame, dialog_state: &mut SkillsDialogState, area: Rect, colors: ThemeColors, ) { - dialog_state.dialog.render(f, area, colors); + dialog_state.dialog.render(frame, area, colors); + if let Some(source) = dialog_state.source.as_mut() { + let name = dialog_state + .dialog + .get_selected() + .map(|item| item.name.clone()) + .unwrap_or_default(); + source.render(frame, area, &name, colors); + } } pub fn handle_skills_dialog_key_event( @@ -84,6 +449,13 @@ pub fn handle_skills_dialog_key_event( if !dialog_state.dialog.is_visible() { return SkillsDialogAction::None; } + if dialog_state.source.is_some() { + return handle_source_key(dialog_state, event); + } + + if is_info_key(event) { + return open_source(dialog_state); + } match event.code { KeyCode::Char(' ') => { @@ -93,18 +465,68 @@ pub fn handle_skills_dialog_key_event( }; } } + KeyCode::Enter if event.modifiers == KeyModifiers::NONE => { + if let Some(name) = dialog_state + .dialog + .get_selected() + .map(|item| item.id.clone()) + { + dialog_state.dialog.hide(); + dialog_state.source = None; + return SkillsDialogAction::InsertMention { name }; + } + } _ => { dialog_state.dialog.handle_key_event(event); + if !dialog_state.dialog.is_visible() { + dialog_state.source = None; + } } } SkillsDialogAction::None } +fn open_source(dialog_state: &mut SkillsDialogState) -> SkillsDialogAction { + let Some(selected) = dialog_state.dialog.get_selected() else { + return SkillsDialogAction::None; + }; + let Some(path) = dialog_state.locations.get(&selected.id).cloned() else { + return SkillsDialogAction::None; + }; + dialog_state.source = Some(SkillSource::new(path)); + SkillsDialogAction::None +} + +fn handle_source_key(dialog_state: &mut SkillsDialogState, event: KeyEvent) -> SkillsDialogAction { + let outcome = { + let Some(source) = dialog_state.source.as_mut() else { + return SkillsDialogAction::None; + }; + let path = source.path.clone(); + match source.handle_key(event) { + SourceEvent::None => return SkillsDialogAction::None, + SourceEvent::Close => None, + SourceEvent::Choose(action) => Some(SkillsDialogAction::File { action, path }), + } + }; + match outcome { + None => { + dialog_state.source = None; + SkillsDialogAction::None + } + Some(action) => action, + } +} + pub fn handle_skills_dialog_mouse_event( dialog_state: &mut SkillsDialogState, event: MouseEvent, ) -> SkillsDialogAction { + if dialog_state.source.is_some() { + return handle_source_mouse(dialog_state, event); + } + // Scrolling, hovering, and scrollbar dragging must never toggle a skill. let clicked_item = matches!(event.kind, MouseEventKind::Down(MouseButton::Left)) && dialog_state @@ -112,6 +534,10 @@ pub fn handle_skills_dialog_mouse_event( .item_index_at_position(event.column, event.row) .is_some(); dialog_state.dialog.handle_mouse_event(event); + if !dialog_state.dialog.is_visible() { + dialog_state.source = None; + return SkillsDialogAction::None; + } if clicked_item && dialog_state.dialog.is_visible() { if let Some(selected) = dialog_state.dialog.get_selected() { return SkillsDialogAction::Toggle { @@ -122,6 +548,52 @@ pub fn handle_skills_dialog_mouse_event( SkillsDialogAction::None } +fn handle_source_mouse( + dialog_state: &mut SkillsDialogState, + event: MouseEvent, +) -> SkillsDialogAction { + let outcome = { + let Some(source) = dialog_state.source.as_mut() else { + return SkillsDialogAction::None; + }; + let position = Position::new(event.column, event.row); + match event.kind { + MouseEventKind::Moved => { + if let Some(index) = source + .action_rows + .iter() + .position(|row| row.contains(position)) + { + source.selected = index; + } + return SkillsDialogAction::None; + } + MouseEventKind::Down(MouseButton::Left) + if source.esc_area.contains(position) || !source.area.contains(position) => + { + None + } + MouseEventKind::Down(MouseButton::Left) => { + return source + .action_at(position) + .map(|action| SkillsDialogAction::File { + action, + path: source.path.clone(), + }) + .unwrap_or(SkillsDialogAction::None); + } + _ => return SkillsDialogAction::None, + } + }; + match outcome { + None => { + dialog_state.source = None; + SkillsDialogAction::None + } + Some(action) => action, + } +} + #[cfg(test)] mod tests { use super::*; @@ -160,6 +632,40 @@ mod tests { } } + fn key(code: KeyCode) -> KeyEvent { + KeyEvent::new(code, KeyModifiers::NONE) + } + + fn rendered(state: &mut SkillsDialogState) -> String { + let mut terminal = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(100, 30)).unwrap(); + terminal + .draw(|frame| { + render_skills_dialog( + frame, + state, + frame.area(), + crate::theme::Theme::load_builtin_default().get_colors(true), + ); + }) + .unwrap(); + terminal + .backend() + .buffer() + .content() + .iter() + .map(|cell| cell.symbol()) + .collect() + } + + #[test] + fn title_note_drops_the_fraction_when_nothing_is_disabled() { + assert_eq!(skill_title_note(25, 25, 2_400), "25 ~2.4k"); + assert_eq!(skill_title_note(23, 25, 980), "23/25 ~980"); + assert_eq!(skill_title_note(0, 25, 0), "0/25"); + assert_eq!(skill_title_note(0, 0, 0), "0"); + } + #[test] fn skills_are_alphabetical_regardless_of_case_or_activation() { let store = @@ -182,6 +688,12 @@ mod tests { .flat_map(|(_, items)| items.iter().map(|item| item.name.as_str())) .collect::>(); assert_eq!(names, ["Alpha", "alpha", "Beta", "delta", "zebra"]); + assert_eq!(state.dialog.title, "Skills"); + assert!( + state.dialog.title_note.starts_with("4/5 ~"), + "{}", + state.dialog.title_note + ); assert_eq!( store .all() @@ -196,60 +708,60 @@ mod tests { fn space_toggles_without_closing() { let mut state = state(); assert_eq!( - handle_skills_dialog_key_event( - &mut state, - KeyEvent::new(KeyCode::Char(' '), KeyModifiers::NONE) - ), + handle_skills_dialog_key_event(&mut state, key(KeyCode::Char(' '))), SkillsDialogAction::Toggle { skill_id: "alpha".to_string() } ); assert!(state.dialog.is_visible()); - handle_skills_dialog_key_event(&mut state, KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + handle_skills_dialog_key_event(&mut state, key(KeyCode::Esc)); assert!(!state.dialog.is_visible()); assert_eq!( - handle_skills_dialog_key_event( - &mut state, - KeyEvent::new(KeyCode::Char(' '), KeyModifiers::NONE) - ), + handle_skills_dialog_key_event(&mut state, key(KeyCode::Char(' '))), SkillsDialogAction::None ); } #[test] - fn enter_does_not_toggle_or_close() { + fn enter_inserts_mention_and_closes_without_toggling() { let mut state = state(); assert_eq!( - handle_skills_dialog_key_event( - &mut state, - KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE) - ), + handle_skills_dialog_key_event(&mut state, key(KeyCode::Enter)), + SkillsDialogAction::InsertMention { + name: "alpha".to_string() + } + ); + assert!(!state.dialog.is_visible()); + assert!(!state.info_is_open()); + assert_eq!( + handle_skills_dialog_key_event(&mut state, key(KeyCode::Enter)), SkillsDialogAction::None ); - assert!(state.dialog.search_query.is_empty()); - assert!(state.dialog.is_visible()); } #[test] - fn empty_or_filtered_out_list_does_not_toggle() { + fn empty_or_filtered_out_list_does_not_toggle_or_insert() { let mut state = state(); state.dialog.set_search_query("no-such-skill"); assert_eq!( - handle_skills_dialog_key_event( - &mut state, - KeyEvent::new(KeyCode::Char(' '), KeyModifiers::NONE) - ), + handle_skills_dialog_key_event(&mut state, key(KeyCode::Char(' '))), + SkillsDialogAction::None + ); + assert_eq!( + handle_skills_dialog_key_event(&mut state, key(KeyCode::Enter)), SkillsDialogAction::None ); assert!(state.dialog.is_visible()); state.dialog.set_items(vec![]); assert_eq!( - handle_skills_dialog_key_event( - &mut state, - KeyEvent::new(KeyCode::Char(' '), KeyModifiers::NONE) - ), + handle_skills_dialog_key_event(&mut state, key(KeyCode::Char(' '))), SkillsDialogAction::None ); + assert_eq!( + handle_skills_dialog_key_event(&mut state, key(KeyCode::Enter)), + SkillsDialogAction::None + ); + assert!(state.dialog.is_visible()); } #[test] @@ -263,6 +775,12 @@ mod tests { store.set_enabled("beta", false, &prefs).unwrap(); state.refresh(&store); assert_eq!(state.dialog.items.len(), 2); + assert_eq!(state.dialog.title, "Skills"); + assert!( + state.dialog.title_note.starts_with("1/2 ~"), + "{}", + state.dialog.title_note + ); assert_eq!(state.dialog.search_query, "beta"); let selected = state.dialog.get_selected().unwrap(); assert_eq!(selected.id, "beta"); @@ -272,6 +790,12 @@ mod tests { store.set_enabled("beta", true, &prefs).unwrap(); state.refresh(&store); + assert!( + state.dialog.title_note.starts_with("2 ~"), + "{}", + state.dialog.title_note + ); + assert!(!state.dialog.title_note.contains('/')); assert_eq!( state.dialog.get_selected().unwrap().tip.as_deref(), Some("enabled") @@ -316,36 +840,233 @@ mod tests { } #[test] - fn rendered_skill_dialog_shows_activation_and_controls() { + fn rendered_skill_dialog_shows_activation_controls_and_count() { let store = store(); store .set_enabled("beta", false, &PrefsDAO::in_memory()) .unwrap(); let mut state = state_for(&store); - let mut terminal = - ratatui::Terminal::new(ratatui::backend::TestBackend::new(100, 30)).unwrap(); - terminal - .draw(|f| { - render_skills_dialog( - f, - &mut state, - f.area(), - crate::theme::Theme::load_builtin_default().get_colors(true), - ); - }) - .unwrap(); - let buffer = terminal.backend().buffer(); - let text = buffer - .content() - .iter() - .map(|cell| cell.symbol()) - .collect::(); + let text = rendered(&mut state); for label in [ - "alpha", "beta", "enabled", "disabled", "toggle", "space", "close", + "Skills", "1/2", "~", "alpha", "beta", "enabled", "disabled", "toggle", "space", + "insert", "enter", "info", "ctrl+i", "close", ] { assert!(text.contains(label), "missing label: {label}"); } assert_eq!(text.matches("toggle").count(), 1); - assert!(!text.contains("enter")); + + let colors = crate::theme::Theme::load_builtin_default().get_colors(true); + let mut terminal = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(100, 30)).unwrap(); + terminal + .draw(|frame| render_skills_dialog(frame, &mut state, frame.area(), colors)) + .unwrap(); + let buffer = terminal.backend().buffer().clone(); + let skills = find_cell(&buffer, "Skills").expect("title"); + let count = find_cell(&buffer, "1/2").expect("count"); + assert_eq!(buffer[skills].style().fg, Some(colors.text)); + assert_eq!(buffer[count].style().fg, Some(colors.text_weak)); + assert_ne!(colors.text, colors.text_weak); + } + + fn find_cell(buffer: &ratatui::buffer::Buffer, needle: &str) -> Option<(u16, u16)> { + let width = buffer.area.width; + (0..buffer.area.height).find_map(|y| { + let row: String = (0..width).map(|x| buffer[(x, y)].symbol()).collect(); + row.find(needle).map(|x| (x as u16, y)) + }) + } + + #[test] + fn ctrl_i_opens_source_panel_and_letters_do_not_search() { + let mut state = state(); + state.dialog.set_search_query("alp"); + assert_eq!( + handle_skills_dialog_key_event( + &mut state, + KeyEvent::new(KeyCode::Char('i'), KeyModifiers::CONTROL) + ), + SkillsDialogAction::None + ); + assert!(state.info_is_open()); + assert!(state.dialog.is_visible()); + assert_eq!(state.dialog.search_query, "alp"); + + assert_eq!( + handle_skills_dialog_key_event(&mut state, key(KeyCode::Char('z'))), + SkillsDialogAction::None + ); + assert_eq!(state.dialog.search_query, "alp"); + assert!(state.info_is_open()); + + let colors = crate::theme::Theme::load_builtin_default().get_colors(true); + let mut terminal = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(100, 30)).unwrap(); + terminal + .draw(|frame| render_skills_dialog(frame, &mut state, frame.area(), colors)) + .unwrap(); + let buffer = terminal.backend().buffer().clone(); + let text: String = buffer.content().iter().map(|cell| cell.symbol()).collect(); + assert!(text.contains("/skills/alpha/SKILL.md")); + assert!(text.contains("Open")); + assert!(text.contains(reveal_label())); + assert!(text.contains("Copy absolute path")); + assert!(text.contains(&state.dialog.search_query)); + let path = find_cell(&buffer, "/skills/alpha").expect("path"); + let action = find_cell(&buffer, "Reveal").expect("unselected action"); + let title = state.source.as_ref().unwrap(); + let title = buffer[(title.area.x + 3, title.area.y + 1)].style(); + assert_eq!(buffer[path].style().fg, Some(colors.text_weak)); + assert_eq!(buffer[action].style().fg, Some(colors.text)); + assert_eq!(title.fg, Some(colors.primary)); + assert!(title + .add_modifier + .contains(ratatui::style::Modifier::BOLD)); + assert_ne!(colors.text, colors.text_weak); + } + + #[test] + fn source_panel_navigates_and_chooses_file_actions() { + let mut state = state(); + handle_skills_dialog_key_event( + &mut state, + KeyEvent::new(KeyCode::Char('i'), KeyModifiers::CONTROL), + ); + let path = PathBuf::from("/skills/alpha/SKILL.md"); + assert_eq!( + handle_skills_dialog_key_event(&mut state, key(KeyCode::Down)), + SkillsDialogAction::None + ); + assert_eq!( + handle_skills_dialog_key_event(&mut state, key(KeyCode::Enter)), + SkillsDialogAction::File { + action: FileAction::Reveal, + path: path.clone(), + } + ); + assert!(state.info_is_open()); + assert_eq!( + handle_skills_dialog_key_event(&mut state, key(KeyCode::Char('y'))), + SkillsDialogAction::File { + action: FileAction::Copy, + path: path.clone(), + } + ); + assert_eq!( + handle_skills_dialog_key_event(&mut state, key(KeyCode::Char('o'))), + SkillsDialogAction::File { + action: FileAction::Open, + path: path.clone(), + } + ); + assert_eq!( + handle_skills_dialog_key_event(&mut state, key(KeyCode::End)), + SkillsDialogAction::None + ); + assert_eq!( + handle_skills_dialog_key_event(&mut state, key(KeyCode::Char(' '))), + SkillsDialogAction::File { + action: FileAction::Copy, + path, + } + ); + } + + #[test] + fn source_panel_escape_returns_to_the_skill_list() { + let mut state = state(); + handle_skills_dialog_key_event( + &mut state, + KeyEvent::new(KeyCode::Char('I'), KeyModifiers::CONTROL), + ); + assert!(state.info_is_open()); + assert_eq!( + handle_skills_dialog_key_event(&mut state, key(KeyCode::Esc)), + SkillsDialogAction::None + ); + assert!(!state.info_is_open()); + assert!(state.dialog.is_visible()); + assert_eq!(state.dialog.search_query, ""); + handle_skills_dialog_key_event(&mut state, key(KeyCode::Esc)); + assert!(!state.dialog.is_visible()); + } + + #[test] + fn source_panel_click_outside_closes_and_click_on_action_chooses() { + let mut state = state(); + handle_skills_dialog_key_event( + &mut state, + KeyEvent::new(KeyCode::Char('i'), KeyModifiers::CONTROL), + ); + let text_before = rendered(&mut state); + assert!(text_before.contains("/skills/alpha/SKILL.md")); + let open_row = state.source.as_ref().unwrap().action_rows[0]; + assert_eq!( + handle_skills_dialog_mouse_event( + &mut state, + mouse( + MouseEventKind::Down(MouseButton::Left), + open_row.x, + open_row.y + ) + ), + SkillsDialogAction::File { + action: FileAction::Open, + path: PathBuf::from("/skills/alpha/SKILL.md"), + } + ); + assert!(state.info_is_open()); + assert_eq!( + handle_skills_dialog_mouse_event( + &mut state, + mouse(MouseEventKind::Down(MouseButton::Left), 0, 0) + ), + SkillsDialogAction::None + ); + assert!(!state.info_is_open()); + assert!(state.dialog.is_visible()); + } + + #[test] + fn wrapped_source_path_keeps_the_filename() { + let store = SkillStore::for_test([SkillInfo { + name: "alpha".to_string(), + description: None, + location: format!("/{}/SKILL.md", "directory/".repeat(12)).into(), + content: String::new(), + }]); + let mut state = state_for(&store); + handle_skills_dialog_key_event( + &mut state, + KeyEvent::new(KeyCode::Char('i'), KeyModifiers::CONTROL), + ); + let text = rendered(&mut state); + assert!(text.contains("SKILL.md"), "{text}"); + assert!(text.contains("directory/"), "{text}"); + } + + #[test] + fn ctrl_i_without_a_known_location_does_nothing() { + let mut state = init_skills_dialog( + "Skills", + vec![DialogItem { + id: "alpha".to_string(), + name: "alpha".to_string(), + group: "Skills".to_string(), + description: String::new(), + tip: None, + provider_id: String::new(), + active: true, + }], + ); + state.dialog.show(); + assert_eq!( + handle_skills_dialog_key_event( + &mut state, + KeyEvent::new(KeyCode::Char('i'), KeyModifiers::CONTROL) + ), + SkillsDialogAction::None + ); + assert!(!state.info_is_open()); } } From f997178efa5068a341b1f1ad303a2d2361f4fd12 Mon Sep 17 00:00:00 2001 From: Blankeos Date: Thu, 15 Oct 2026 00:58:45 +0800 Subject: [PATCH 2/2] test(skills): cover skill info panel wrapping and search isolation - Assert skill file paths and file:// URLs stay out of subagent and prompt skill guidance - Add test that paste while skill info is open does not change the search query - Verify long skill paths wrap after "/" and filenames stay on one line - Drop assertions on title note, token estimate, and color inequality --- src/agent/subagent.rs | 2 +- src/app.rs | 40 +++++++++++++++++++------ src/prompt/mod.rs | 6 ++-- src/views/skills_dialog.rs | 61 +++++++++++++++++++++++++++----------- 4 files changed, 78 insertions(+), 31 deletions(-) diff --git a/src/agent/subagent.rs b/src/agent/subagent.rs index 1a584d8..1a42606 100644 --- a/src/agent/subagent.rs +++ b/src/agent/subagent.rs @@ -822,7 +822,7 @@ mod tests { assert!(system .content .contains("Use test-workflow for focused testing.")); - assert!(!system.content.contains("")); + assert!(!system.content.contains(skill.location.to_str().unwrap())); assert!(system .content .contains("use the skill tool to load it before responding")); diff --git a/src/app.rs b/src/app.rs index 9459dd4..e3ffbc5 100644 --- a/src/app.rs +++ b/src/app.rs @@ -18273,15 +18273,37 @@ mod tests { assert_eq!(app.take_editor_suspend(), Some(expected)); assert!(app.skills_dialog_state.info_is_open()); assert_eq!(app.overlay_focus, OverlayFocus::SkillsDialog); - assert_eq!(app.skills_dialog_state.dialog.title, "Skills"); - assert!( - app.skills_dialog_state - .dialog - .title_note - .starts_with("1 ~"), - "{}", - app.skills_dialog_state.dialog.title_note - ); + } + + #[test] + fn paste_while_skill_info_is_open_does_not_change_search() { + let root = tempfile::tempdir().unwrap(); + let path = root.path().join("SKILL.md"); + std::fs::write(&path, "# skill\n").unwrap(); + let store = crate::skill::SkillStore::for_test([crate::skill::SkillInfo { + name: "alpha".to_string(), + description: None, + location: path, + content: String::new(), + }]); + let mut app = test_app(); + app.skills_dialog_state = crate::views::skills_dialog::init_skills_dialog("Skills", vec![]); + app.skills_dialog_state.refresh(&store); + app.skills_dialog_state.dialog.show(); + app.skills_dialog_state.dialog.set_search_query("alp"); + app.overlay_focus = OverlayFocus::SkillsDialog; + + app.handle_keys(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::CONTROL)); + app.handle_paste("nope".to_string()); + assert!(app.skills_dialog_state.info_is_open()); + assert_eq!(app.skills_dialog_state.dialog.search_query, "alp"); + assert_eq!(app.input.get_text(), ""); + + app.handle_keys(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + assert!(!app.skills_dialog_state.info_is_open()); + app.handle_paste("ha".to_string()); + assert_eq!(app.skills_dialog_state.dialog.search_query, "alpha"); + assert_eq!(app.input.get_text(), ""); } #[test] diff --git a/src/prompt/mod.rs b/src/prompt/mod.rs index 07e509d..7e6c8b4 100644 --- a/src/prompt/mod.rs +++ b/src/prompt/mod.rs @@ -502,11 +502,9 @@ mod tests { assert!(guidance.contains("test-skill")); assert!(guidance.contains("")); assert!(!guidance.contains("")); + assert!(!guidance.contains(skill.location.to_str().unwrap())); + assert!(!guidance.contains("file://")); assert!(!guidance.contains(&skill.content)); - assert_eq!( - estimate_skill_catalog_tokens([&skill]), - guidance.chars().count().div_ceil(4) - ); assert_eq!(estimate_skill_catalog_tokens([]), 0); let mut with_body = skill.clone(); with_body.content = "x".repeat(10_000); diff --git a/src/views/skills_dialog.rs b/src/views/skills_dialog.rs index ba59a3e..608c357 100644 --- a/src/views/skills_dialog.rs +++ b/src/views/skills_dialog.rs @@ -694,14 +694,6 @@ mod tests { "{}", state.dialog.title_note ); - assert_eq!( - store - .all() - .iter() - .map(|skill| skill.name.as_str()) - .collect::>(), - ["Alpha", "alpha", "delta", "zebra"] - ); } #[test] @@ -866,7 +858,6 @@ mod tests { let count = find_cell(&buffer, "1/2").expect("count"); assert_eq!(buffer[skills].style().fg, Some(colors.text)); assert_eq!(buffer[count].style().fg, Some(colors.text_weak)); - assert_ne!(colors.text, colors.text_weak); } fn find_cell(buffer: &ratatui::buffer::Buffer, needle: &str) -> Option<(u16, u16)> { @@ -911,7 +902,6 @@ mod tests { assert!(text.contains("Open")); assert!(text.contains(reveal_label())); assert!(text.contains("Copy absolute path")); - assert!(text.contains(&state.dialog.search_query)); let path = find_cell(&buffer, "/skills/alpha").expect("path"); let action = find_cell(&buffer, "Reveal").expect("unselected action"); let title = state.source.as_ref().unwrap(); @@ -919,10 +909,7 @@ mod tests { assert_eq!(buffer[path].style().fg, Some(colors.text_weak)); assert_eq!(buffer[action].style().fg, Some(colors.text)); assert_eq!(title.fg, Some(colors.primary)); - assert!(title - .add_modifier - .contains(ratatui::style::Modifier::BOLD)); - assert_ne!(colors.text, colors.text_weak); + assert!(title.add_modifier.contains(ratatui::style::Modifier::BOLD)); } #[test] @@ -1040,9 +1027,49 @@ mod tests { &mut state, KeyEvent::new(KeyCode::Char('i'), KeyModifiers::CONTROL), ); - let text = rendered(&mut state); - assert!(text.contains("SKILL.md"), "{text}"); - assert!(text.contains("directory/"), "{text}"); + let mut terminal = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(100, 30)).unwrap(); + terminal + .draw(|frame| { + render_skills_dialog( + frame, + &mut state, + frame.area(), + crate::theme::Theme::load_builtin_default().get_colors(true), + ); + }) + .unwrap(); + let buffer = terminal.backend().buffer(); + let panel = state.source.as_ref().unwrap().area; + let rows = (panel.y..panel.bottom()) + .map(|y| { + (panel.x..panel.right()) + .map(|x| buffer[(x, y)].symbol()) + .collect::() + .trim() + .to_string() + }) + .filter(|row| row.contains("directory") || row.contains("SKILL")) + .collect::>(); + assert!( + rows.iter().any(|row| row == "SKILL.md"), + "filename should stay intact on its own line: {rows:?}" + ); + assert!( + rows.iter() + .any(|row| row.contains("directory/") && row.ends_with('/')), + "long paths should break after a slash: {rows:?}" + ); + assert!( + rows.iter().all(|row| { + row == "SKILL.md" + || row + .split('/') + .filter(|segment| !segment.is_empty()) + .all(|segment| segment == "directory") + }), + "wrap should not split a path segment: {rows:?}" + ); } #[test]