From 67880c74805662865e952832888f59dd6920f6ad Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Sat, 22 Aug 2026 02:00:06 +1000 Subject: [PATCH 1/2] feat(cli): scannable outdated report rows The outdated report packed every name into one sentence, so a 13 Skill no-match line ran past 500 characters and nothing could be scanned at a glance. Rows are records now: a glyph, the Skill name, a dim status badge, then aligned detail lines. Name lists render as groups with one row per Skill. Lockfile parse failures name the problem instead of leaking the serde detail, and streamed progress dims so results stand out. Plain output keeps the exact sentence records. --- crates/skilld-command/src/lib.rs | 64 +++++-- crates/skilld-command/src/local_store.rs | 5 +- crates/skilld-command/src/outdated.rs | 70 +++++--- crates/skilld-native/src/main.rs | 1 + crates/skilld-native/src/status.rs | 19 +- crates/skilld-ui/src/lib.rs | 2 +- crates/skilld-ui/src/screen.rs | 210 ++++++++++++++++++++++- 7 files changed, 323 insertions(+), 48 deletions(-) diff --git a/crates/skilld-command/src/lib.rs b/crates/skilld-command/src/lib.rs index fe32e3dd..4c79ca4e 100644 --- a/crates/skilld-command/src/lib.rs +++ b/crates/skilld-command/src/lib.rs @@ -34,7 +34,7 @@ use skilld_core::{ UpdatePlanItem, UpdatePlanV1, UpdateRelation, UpdateRetryAfter, VERSION, classify_update_comparison, select_target_ids, }; -use skilld_ui::{Line, Screen}; +use skilld_ui::{Line, Marker, Screen}; use output::{ OutputMode, SearchItem, SearchOutcome, render_error, render_search, render_update_check, @@ -2317,18 +2317,35 @@ impl LocalHost { }); match state { Ok(RemoteSourceState::Current) => { - vec![Line::success(format!("Current Skill {name}."))] + vec![Line::record( + Marker::Success, + format!("Current Skill {name}."), + name, + Some("current".to_owned()), + Vec::new(), + )] } Ok(RemoteSourceState::Stale { .. }) => { - vec![Line::warn(format!( - "Outdated Skill {name}. Run skilld update {name}{global}." - ))] + let update = format!("skilld update {name}{global}"); + vec![Line::record( + Marker::Warn, + format!("Outdated Skill {name}. Run {update}."), + name, + Some("outdated".to_owned()), + vec![("update", update)], + )] } Err(error) => { - vec![Line::error(format!( - "Source state unavailable for Skill {name}: {}.", - error.message - ))] + vec![Line::record( + Marker::Error, + format!( + "Source state unavailable for Skill {name}: {}.", + error.message + ), + name, + Some("source unavailable".to_owned()), + vec![("error", error.message.clone())], + )] } } } @@ -2340,14 +2357,29 @@ impl LocalHost { .map(|locked| locked.agent) .collect::>(); let agent_flags = outdated::agent_flags(&agents); - vec![Line::warn(format!( - "Unverified Skill {name}. Run skilld install {source} --direct{global}{agent_flags} to update it." - ))] + let install = format!("skilld install {source} --direct{global}{agent_flags}"); + vec![Line::record( + Marker::Warn, + format!("Unverified Skill {name}. Run {install} to update it."), + name, + Some("unverified".to_owned()), + vec![("install", install)], + )] } - (LockedSource::BundledSkilld, _) => { - vec![Line::plain(format!("skilld-maintained Skill {name}."))] - } - _ => vec![Line::plain(format!("Local Skill {name}."))], + (LockedSource::BundledSkilld, _) => vec![Line::record( + Marker::Note, + format!("skilld-maintained Skill {name}."), + name, + Some("skilld-maintained".to_owned()), + Vec::new(), + )], + _ => vec![Line::record( + Marker::Note, + format!("Local Skill {name}."), + name, + Some("local".to_owned()), + Vec::new(), + )], } } diff --git a/crates/skilld-command/src/local_store.rs b/crates/skilld-command/src/local_store.rs index c45b1bc8..811fba43 100644 --- a/crates/skilld-command/src/local_store.rs +++ b/crates/skilld-command/src/local_store.rs @@ -794,8 +794,9 @@ impl LocalStore { )); } let bytes = fs::read(&path).map_err(fs_error)?; - let document: LockDocument = serde_json::from_slice(&bytes) - .map_err(|error| StoreError::InvalidLockfile(error.to_string()))?; + let document: LockDocument = serde_json::from_slice(&bytes).map_err(|_| { + StoreError::InvalidLockfile("the Skill lockfile is not valid JSON".to_owned()) + })?; if document.version != 1 { return Err(StoreError::InvalidLockfile(format!( "unsupported Skill lockfile version: {}", diff --git a/crates/skilld-command/src/outdated.rs b/crates/skilld-command/src/outdated.rs index d6df0a30..ea3d755b 100644 --- a/crates/skilld-command/src/outdated.rs +++ b/crates/skilld-command/src/outdated.rs @@ -3,7 +3,8 @@ use std::fs; use std::path::{Path, PathBuf}; use skilld_core::{AgentTargetId, InstallScope, SkillName}; -use skilld_ui::Line; +use skilld_ui::text::grouped_number; +use skilld_ui::{Line, Marker}; use crate::ResolvedTarget; use crate::local_store::normalize_path; @@ -136,11 +137,16 @@ pub(crate) fn render_no_match(skills: &[&UnmanagedSkill]) -> Vec { if skills.is_empty() { return vec![]; } - vec![Line::warn(format!( - "No Repository match for {} ({}).", - skill_count(skills.len()), - name_list(skills) - ))] + let count = skill_count(skills.len()); + vec![Line::group( + Marker::Warn, + format!("No Repository match for {count} ({}).", name_list(skills)), + format!("No Repository match for {count}"), + skills + .iter() + .map(|skill| (skill.name.clone(), agent_list(skill))) + .collect(), + )] } pub(crate) fn render_search_failures( @@ -149,11 +155,19 @@ pub(crate) fn render_search_failures( failures .iter() .map(|(message, skills)| { - Line::warn(format!( - "Skill search unavailable for {} ({}): {message}.", - skill_count(skills.len()), - name_list(skills) - )) + Line::group( + Marker::Warn, + format!( + "Skill search unavailable for {} ({}): {message}.", + skill_count(skills.len()), + name_list(skills) + ), + format!("Skill search unavailable: {message}"), + skills + .iter() + .map(|skill| (skill.name.clone(), agent_list(skill))) + .collect(), + ) }) .collect() } @@ -184,17 +198,29 @@ pub(crate) fn render_unmanaged( "" }; let agent_flags = agent_flags(&skill.agents); - vec![ - Line::warn(format!( - "Unmanaged Skill {} ({agents}). Candidate source {}, {} stars.", - skill.name, candidate.selector, candidate.stargazer_count - )), - Line::hint(format!( - "Delete {}, then run skilld install {}{global}{agent_flags}.", - skill.path.display(), - candidate.selector - )), - ] + let install = format!("skilld install {}{global}{agent_flags}", candidate.selector); + let plain = format!( + "Unmanaged Skill {} ({agents}). Candidate source {}, {} stars.\nDelete {}, then run {install}.", + skill.name, + candidate.selector, + candidate.stargazer_count, + skill.path.display() + ); + vec![Line::record( + Marker::Warn, + plain, + skill.name.clone(), + Some(format!("{agents} · unmanaged")), + vec![ + ("candidate", candidate.selector.clone()), + ( + "stars", + format!("★ {}", grouped_number(candidate.stargazer_count)), + ), + ("install", install), + ("delete", skill.path.display().to_string()), + ], + )] } pub(crate) fn agent_flags(agents: &[AgentTargetId]) -> String { diff --git a/crates/skilld-native/src/main.rs b/crates/skilld-native/src/main.rs index dc21bfde..68f78b3b 100644 --- a/crates/skilld-native/src/main.rs +++ b/crates/skilld-native/src/main.rs @@ -76,6 +76,7 @@ fn main() -> ExitCode { host.with_outdated_progress(Arc::new(status::OutdatedProgressLine::for_terminal( std::io::stderr().is_terminal(), active_agent_detected(), + !environment_present("NO_COLOR"), ))) } else { host diff --git a/crates/skilld-native/src/status.rs b/crates/skilld-native/src/status.rs index b5a3be9f..922734cc 100644 --- a/crates/skilld-native/src/status.rs +++ b/crates/skilld-native/src/status.rs @@ -200,15 +200,17 @@ where /// it so only results remain. pub struct OutdatedProgressLine { enabled: bool, + color: bool, frame: std::sync::atomic::AtomicUsize, } impl OutdatedProgressLine { /// Progress streams only for humans on a terminal. Agents, CI, and pipes /// get quiet stderr. - pub fn for_terminal(is_terminal: bool, active_agent: bool) -> Self { + pub fn for_terminal(is_terminal: bool, active_agent: bool, color: bool) -> Self { Self { enabled: is_terminal && !active_agent, + color, frame: std::sync::atomic::AtomicUsize::new(0), } } @@ -229,7 +231,11 @@ impl skilld_command::OutdatedProgress for OutdatedProgressLine { } self.erase(); let mut stderr = std::io::stderr().lock(); - let _ = writeln!(stderr, "• {line}"); + let _ = writeln!( + stderr, + "{}", + paint(&format!("• {line}"), Role::Dim, self.color) + ); let _ = stderr.flush(); } @@ -238,8 +244,9 @@ impl skilld_command::OutdatedProgress for OutdatedProgressLine { return; } let frame = spinner::frame(self.frame.fetch_add(1, Ordering::Relaxed)); + let glyph = paint(frame, Role::Brand, self.color); let mut stderr = std::io::stderr().lock(); - let _ = write!(stderr, "\r\x1b[2K{frame} Checking {name}…"); + let _ = write!(stderr, "\r\x1b[2K{glyph} Checking {name}…"); let _ = stderr.flush(); } @@ -379,16 +386,16 @@ mod tests { fn outdated_progress_stays_quiet_for_agents() { use skilld_command::OutdatedProgress; - let agent = super::OutdatedProgressLine::for_terminal(true, true); + let agent = super::OutdatedProgressLine::for_terminal(true, true, true); agent.found("example (project scope)"); agent.checking("example"); agent.finish(); assert!(!agent.enabled); - let human = super::OutdatedProgressLine::for_terminal(true, false); + let human = super::OutdatedProgressLine::for_terminal(true, false, true); assert!(human.enabled); - let piped = super::OutdatedProgressLine::for_terminal(false, false); + let piped = super::OutdatedProgressLine::for_terminal(false, false, true); assert!(!piped.enabled); } diff --git a/crates/skilld-ui/src/lib.rs b/crates/skilld-ui/src/lib.rs index 8fa63030..1ef79667 100644 --- a/crates/skilld-ui/src/lib.rs +++ b/crates/skilld-ui/src/lib.rs @@ -10,7 +10,7 @@ pub mod text; pub mod theme; pub mod time; -pub use screen::{Line, LineKind, Screen, plain_lines}; +pub use screen::{Line, LineKind, Marker, Screen, plain_lines}; pub use theme::{RESET, Role, paint}; pub use time::relative_time; diff --git a/crates/skilld-ui/src/screen.rs b/crates/skilld-ui/src/screen.rs index a1b99a4a..70dd504c 100644 --- a/crates/skilld-ui/src/screen.rs +++ b/crates/skilld-ui/src/screen.rs @@ -13,6 +13,45 @@ pub const GLYPH_SUCCESS: &str = "✓"; pub const GLYPH_WARN: &str = "⚠"; /// The failure glyph prefixing errors and required action. pub const GLYPH_ERROR: &str = "✗"; +/// The neutral glyph prefixing informational rows. +pub const GLYPH_NOTE: &str = "•"; + +/// A marker selects the glyph and role for a row or group heading. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum Marker { + /// Completed work: green check. + Success, + /// Attention: yellow warning. + Warn, + /// Failure: red cross. + Error, + /// Information: brand bullet. + Note, +} + +impl Marker { + const fn glyph(self) -> &'static str { + match self { + Self::Success => GLYPH_SUCCESS, + Self::Warn => GLYPH_WARN, + Self::Error => GLYPH_ERROR, + Self::Note => GLYPH_NOTE, + } + } + + const fn role(self) -> Role { + match self { + Self::Success => Role::Success, + Self::Warn => Role::Warn, + Self::Error => Role::Error, + Self::Note => Role::Brand, + } + } + + fn paint_glyph(self, color: bool) -> String { + paint(self.glyph(), self.role(), color) + } +} /// One rendered output document. #[derive(Clone, Debug, Default, Eq, PartialEq)] @@ -109,6 +148,25 @@ pub enum LineKind { /// A terminal hyperlink target for the value, used when color is on. url: Option, }, + /// One titled row with indented detail rows underneath. The Plain text + /// may span several sentences joined by newlines; Human renders the + /// title row once, then each detail on its own line. + Record { + marker: Marker, + title: String, + /// A dim badge after the title, such as an Agent list or state. + status: Option, + /// Labelled detail rows shown under the title. + details: Vec<(&'static str, String)>, + }, + /// A heading plus one row per item, so long name lists stay scannable. + /// The Plain text is the full sentence with every name inline. + Group { + marker: Marker, + heading: String, + /// One (name, meta) pair per rendered row. + items: Vec<(String, String)>, + }, } impl Line { @@ -213,6 +271,45 @@ impl Line { &self.plain } + /// A record row: `glyph title status` with labelled detail rows + /// underneath. `plain` is the exact machine sentence, which may join + /// several sentences with newlines. + pub fn record( + marker: Marker, + plain: impl Into, + title: impl Into, + status: Option, + details: Vec<(&'static str, String)>, + ) -> Self { + Self { + plain: plain.into(), + kind: LineKind::Record { + marker, + title: title.into(), + status, + details, + }, + } + } + + /// A group heading with one row per item. `plain` is the exact machine + /// sentence with every name inline. + pub fn group( + marker: Marker, + plain: impl Into, + heading: impl Into, + items: Vec<(String, String)>, + ) -> Self { + Self { + plain: plain.into(), + kind: LineKind::Group { + marker, + heading: heading.into(), + items, + }, + } + } + fn field_label(&self) -> Option<&str> { match &self.kind { LineKind::Field { label, .. } => Some(label), @@ -236,6 +333,45 @@ impl Line { }; format!("{}: {value}", paint(&label, Role::Dim, color)) } + LineKind::Record { + marker, + title, + status, + details, + } => { + let mut output = format!("{} {title}", marker.paint_glyph(color)); + if let Some(status) = status { + output.push_str(&format!(" {}", paint(status, Role::Dim, color))); + } + let label_width = details + .iter() + .map(|(label, _)| width(label)) + .max() + .unwrap_or(0); + for (label, value) in details { + let label = pad_to(label, label_width); + output.push('\n'); + output.push_str(&format!(" {} {value}", paint(&label, Role::Dim, color))); + } + output + } + LineKind::Group { + marker, + heading, + items, + } => { + let mut output = format!("{} {heading}", marker.paint_glyph(color)); + let name_width = items.iter().map(|(name, _)| width(name)).max().unwrap_or(0); + for (name, meta) in items { + output.push('\n'); + let name = pad_to(name, name_width); + output.push_str(&format!(" {name}")); + if !meta.is_empty() { + output.push_str(&format!(" {}", paint(meta, Role::Dim, color))); + } + } + output + } } } } @@ -257,7 +393,7 @@ pub(crate) fn has_hyperlink(value: &str) -> bool { #[cfg(test)] mod tests { - use super::{Line, Screen, has_hyperlink}; + use super::{Line, Marker, Screen, has_hyperlink}; #[test] fn plain_rendering_matches_the_machine_contract() { @@ -302,6 +438,78 @@ mod tests { assert_eq!(mono, "Source: skilld-dev/skilld\n"); } + #[test] + fn records_render_a_title_row_with_aligned_details() { + let line = Line::record( + Marker::Warn, + "Unmanaged Skill vue-testing (claude-code). Candidate source sel, 0 stars.\nDelete /tmp/x, then run skilld install sel.", + "vue-testing", + Some("claude-code · unmanaged".to_owned()), + vec![ + ("candidate", "sel".to_owned()), + ("install", "skilld install sel".to_owned()), + ], + ); + + assert_eq!( + line.render_human(false, 0), + concat!( + "⚠ vue-testing claude-code · unmanaged\n", + " candidate sel\n", + " install skilld install sel" + ) + ); + } + + #[test] + fn groups_render_one_row_per_item_with_aligned_names() { + let line = Line::group( + Marker::Warn, + "No Repository match for 2 Skills (b (codex), longer-name (amp)).", + "No Repository match for 2 Skills", + vec![ + ("b".to_owned(), "codex".to_owned()), + ("longer-name".to_owned(), "amp".to_owned()), + ], + ); + + assert_eq!( + line.render_human(false, 0), + concat!( + "⚠ No Repository match for 2 Skills\n", + " b codex\n", + " longer-name amp" + ) + ); + } + + #[test] + fn records_and_groups_keep_their_plain_sentences() { + let screen = Screen::new(vec![ + Line::record( + Marker::Note, + "Local Skill example.", + "example", + Some("local".to_owned()), + Vec::new(), + ), + Line::group( + Marker::Warn, + "No Repository match for 1 Skill (b (codex)).", + "No Repository match for 1 Skill", + vec![("b".to_owned(), "codex".to_owned())], + ), + ]); + + assert_eq!( + screen.render_plain(), + concat!( + "Local Skill example.\n", + "No Repository match for 1 Skill (b (codex)).\n" + ) + ); + } + #[test] fn empty_screens_render_nothing() { let screen = Screen::new(vec![]); From c9cc5e8adb3670fc75a957f44974f1d22100f31f Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Sat, 22 Aug 2026 02:13:05 +1000 Subject: [PATCH 2/2] feat(cli): syntax-highlight commands and shorten displayed paths Commands in output are code the user types, so they highlight like code: brand binary, bold subcommand, dim flags, plain values. Record detail values carry a kind now, and paths inside the working directory display relative while machine records keep absolute paths. The update summary and error lines join the theme; plain text is unchanged. --- crates/skilld-command/src/lib.rs | 16 +-- crates/skilld-command/src/outdated.rs | 13 +-- crates/skilld-command/src/output.rs | 16 ++- crates/skilld-command/tests/output.rs | 2 +- crates/skilld-native/src/main.rs | 6 +- crates/skilld-native/src/status.rs | 1 + crates/skilld-native/src/update_ui.rs | 48 +++++++-- crates/skilld-native/tests/cli.rs | 2 +- crates/skilld-ui/src/lib.rs | 4 +- crates/skilld-ui/src/screen.rs | 75 ++++++++++++-- crates/skilld-ui/src/spans.rs | 134 ++++++++++++++++++++++++++ crates/skilld-ui/src/text.rs | 27 +++++- 12 files changed, 308 insertions(+), 36 deletions(-) create mode 100644 crates/skilld-ui/src/spans.rs diff --git a/crates/skilld-command/src/lib.rs b/crates/skilld-command/src/lib.rs index 4c79ca4e..641cf73f 100644 --- a/crates/skilld-command/src/lib.rs +++ b/crates/skilld-command/src/lib.rs @@ -34,7 +34,7 @@ use skilld_core::{ UpdatePlanItem, UpdatePlanV1, UpdateRelation, UpdateRetryAfter, VERSION, classify_update_comparison, select_target_ids, }; -use skilld_ui::{Line, Marker, Screen}; +use skilld_ui::{Detail, Line, Marker, Screen}; use output::{ OutputMode, SearchItem, SearchOutcome, render_error, render_search, render_update_check, @@ -1868,9 +1868,11 @@ impl Host for LocalHost { let mut failures = BTreeMap::>::new(); for (skill, result) in unmanaged.iter().zip(results) { match result { - Ok(Some(candidate)) => { - lines.extend(outdated::render_unmanaged(skill, Some(&candidate))) - } + Ok(Some(candidate)) => lines.extend(outdated::render_unmanaged( + skill, + Some(&candidate), + &self.project_root, + )), Ok(None) => no_match.push(skill), Err(error) => failures.entry(error.message).or_default().push(skill), } @@ -2332,7 +2334,7 @@ impl LocalHost { format!("Outdated Skill {name}. Run {update}."), name, Some("outdated".to_owned()), - vec![("update", update)], + vec![Detail::command("update", update)], )] } Err(error) => { @@ -2344,7 +2346,7 @@ impl LocalHost { ), name, Some("source unavailable".to_owned()), - vec![("error", error.message.clone())], + vec![Detail::plain("error", error.message.clone())], )] } } @@ -2363,7 +2365,7 @@ impl LocalHost { format!("Unverified Skill {name}. Run {install} to update it."), name, Some("unverified".to_owned()), - vec![("install", install)], + vec![Detail::command("install", install)], )] } (LockedSource::BundledSkilld, _) => vec![Line::record( diff --git a/crates/skilld-command/src/outdated.rs b/crates/skilld-command/src/outdated.rs index ea3d755b..44ccdc8c 100644 --- a/crates/skilld-command/src/outdated.rs +++ b/crates/skilld-command/src/outdated.rs @@ -3,8 +3,8 @@ use std::fs; use std::path::{Path, PathBuf}; use skilld_core::{AgentTargetId, InstallScope, SkillName}; -use skilld_ui::text::grouped_number; -use skilld_ui::{Line, Marker}; +use skilld_ui::text::{display_path, grouped_number}; +use skilld_ui::{Detail, Line, Marker}; use crate::ResolvedTarget; use crate::local_store::normalize_path; @@ -187,6 +187,7 @@ fn skill_count(count: usize) -> String { pub(crate) fn render_unmanaged( skill: &UnmanagedSkill, candidate: Option<&SkillCandidate>, + display_base: &Path, ) -> Vec { let agents = agent_list(skill); let Some(candidate) = candidate else { @@ -212,13 +213,13 @@ pub(crate) fn render_unmanaged( skill.name.clone(), Some(format!("{agents} · unmanaged")), vec![ - ("candidate", candidate.selector.clone()), - ( + Detail::plain("candidate", candidate.selector.clone()), + Detail::plain( "stars", format!("★ {}", grouped_number(candidate.stargazer_count)), ), - ("install", install), - ("delete", skill.path.display().to_string()), + Detail::command("install", install), + Detail::path("delete", display_path(&skill.path, display_base)), ], )] } diff --git a/crates/skilld-command/src/output.rs b/crates/skilld-command/src/output.rs index 70550082..149fd84a 100644 --- a/crates/skilld-command/src/output.rs +++ b/crates/skilld-command/src/output.rs @@ -160,7 +160,17 @@ pub(crate) fn render_error(error: &CommandError, mode: OutputMode) -> Vec { }) .unwrap_or_else(|_| b"OUTPUT_RENDER_FAILED: error output could not be encoded\n".to_vec()); } - format!("{error}\n").into_bytes() + match mode { + OutputMode::Human { color, .. } => format!( + "{} {} {}\n", + skilld_ui::paint("✗", skilld_ui::Role::Error, color), + skilld_ui::paint(&error.message, skilld_ui::Role::Emphasis, color), + skilld_ui::paint(&format!("({})", error.code), skilld_ui::Role::Dim, color), + ) + .into_bytes(), + OutputMode::Plain => format!("{error}\n").into_bytes(), + OutputMode::JsonV1 => unreachable!("JSON errors return early"), + } } fn render_plain(outcome: &SearchOutcome) -> String { @@ -240,10 +250,10 @@ fn render_human(outcome: &SearchOutcome, terminal_width: u16, color: bool) -> St output.push('\n'); } } - let install = format!("Install: skilld install {}", sanitize(&item.selector)); + let install = format!("skilld install {}", sanitize(&item.selector)); for line in wrap(&install, columns.saturating_sub(2)) { output.push_str(" "); - output.push_str(&paint(&line, Role::Dim, color)); + output.push_str(&skilld_ui::paint_command(&line, color)); output.push('\n'); } } diff --git a/crates/skilld-command/tests/output.rs b/crates/skilld-command/tests/output.rs index a80e2206..b7a5048f 100644 --- a/crates/skilld-command/tests/output.rs +++ b/crates/skilld-command/tests/output.rs @@ -237,7 +237,7 @@ fn human_search_is_polished_and_respects_terminal_width() { assert!(stdout.contains("1 of 14 Skills")); assert!(stdout.contains("227,068 stars")); assert!(stdout.contains("skilld:mattpocock/skills/grill-me")); - assert!(stdout.contains("Install: skilld install")); + assert!(stdout.contains("skilld install")); assert!( stdout .lines() diff --git a/crates/skilld-native/src/main.rs b/crates/skilld-native/src/main.rs index 68f78b3b..1c372709 100644 --- a/crates/skilld-native/src/main.rs +++ b/crates/skilld-native/src/main.rs @@ -89,7 +89,11 @@ fn main() -> ExitCode { Ok(summary) => { let exit_code = summary.exit_code(); let mut stdout = std::io::stdout().lock(); - if let Err(error) = write_static_summary(&summary, &mut stdout) { + if let Err(error) = write_static_summary( + &summary, + stdout.is_terminal() && !environment_present("NO_COLOR"), + &mut stdout, + ) { eprintln!("{error}"); ExitCode::from(2) } else if stdout.flush().is_err() { diff --git a/crates/skilld-native/src/status.rs b/crates/skilld-native/src/status.rs index 922734cc..ca2f5226 100644 --- a/crates/skilld-native/src/status.rs +++ b/crates/skilld-native/src/status.rs @@ -245,6 +245,7 @@ impl skilld_command::OutdatedProgress for OutdatedProgressLine { } let frame = spinner::frame(self.frame.fetch_add(1, Ordering::Relaxed)); let glyph = paint(frame, Role::Brand, self.color); + let name = paint(name, Role::Emphasis, self.color); let mut stderr = std::io::stderr().lock(); let _ = write!(stderr, "\r\x1b[2K{glyph} Checking {name}…"); let _ = stderr.flush(); diff --git a/crates/skilld-native/src/update_ui.rs b/crates/skilld-native/src/update_ui.rs index 776655a0..e9e09f54 100644 --- a/crates/skilld-native/src/update_ui.rs +++ b/crates/skilld-native/src/update_ui.rs @@ -621,6 +621,39 @@ impl InteractiveUpdateSummary { summary } + /// The exit summary with the terminal theme; `render` stays plain for + /// tests and non-terminal hosts. + pub fn render_styled(&self, color: bool) -> String { + let glyph = + |symbol: &str, role: skilld_ui::Role| skilld_ui::theme::paint(symbol, role, color); + if self.all_current { + return format!( + "{} All installed Skills are current.\n", + glyph("✓", skilld_ui::Role::Success) + ); + } + if self.cancelled { + return "Skill update cancelled.\n".to_owned(); + } + let updated = skill_count(self.updated); + let mut summary = format!( + "{} Updated {updated}.\n", + glyph("✓", skilld_ui::Role::Success) + ); + for (name, error) in &self.failures { + summary.push_str(&format!( + "{} Failed Skill {name}: {}\n", + glyph("✗", skilld_ui::Role::Error), + error.message + )); + summary.push_str(&format!( + " {}\n", + skilld_ui::theme::paint(&error.code, skilld_ui::Role::Dim, color) + )); + } + summary + } + pub fn exit_code(&self) -> u8 { if self.interrupted { 130 @@ -1755,12 +1788,15 @@ fn terminal_lost(_error: io::Error) -> InteractiveUpdateError { pub fn write_static_summary( summary: &InteractiveUpdateSummary, + color: bool, output: &mut impl Write, ) -> Result<(), InteractiveUpdateError> { - output.write_all(summary.render().as_bytes()).map_err(|_| { - InteractiveUpdateError::terminal( - "TERMINAL_UNAVAILABLE", - "The Skill update summary could not be written.", - ) - }) + output + .write_all(summary.render_styled(color).as_bytes()) + .map_err(|_| { + InteractiveUpdateError::terminal( + "TERMINAL_UNAVAILABLE", + "The Skill update summary could not be written.", + ) + }) } diff --git a/crates/skilld-native/tests/cli.rs b/crates/skilld-native/tests/cli.rs index 0690e64a..91098d3a 100644 --- a/crates/skilld-native/tests/cli.rs +++ b/crates/skilld-native/tests/cli.rs @@ -153,7 +153,7 @@ fn config_directory_alone_keeps_human_output_in_a_terminal() { let output = run_output_probe_in_pty(("CLAUDE_CONFIG_DIR", "/tmp/claude-config"), 40); assert!(output.contains("Skill search output")); - assert!(output.contains("Install: skilld install")); + assert!(output.contains("skilld install")); assert!(output.lines().all(|line| line.chars().count() <= 40)); } diff --git a/crates/skilld-ui/src/lib.rs b/crates/skilld-ui/src/lib.rs index 1ef79667..d55f7c70 100644 --- a/crates/skilld-ui/src/lib.rs +++ b/crates/skilld-ui/src/lib.rs @@ -5,12 +5,14 @@ //! skilld theme. pub mod screen; +pub mod spans; pub mod spinner; pub mod text; pub mod theme; pub mod time; -pub use screen::{Line, LineKind, Marker, Screen, plain_lines}; +pub use screen::{Detail, DetailKind, Line, LineKind, Marker, Screen, plain_lines}; +pub use spans::{Span, command_spans, paint_command, paint_spans}; pub use theme::{RESET, Role, paint}; pub use time::relative_time; diff --git a/crates/skilld-ui/src/screen.rs b/crates/skilld-ui/src/screen.rs index 70dd504c..313e0ff2 100644 --- a/crates/skilld-ui/src/screen.rs +++ b/crates/skilld-ui/src/screen.rs @@ -157,7 +157,7 @@ pub enum LineKind { /// A dim badge after the title, such as an Agent list or state. status: Option, /// Labelled detail rows shown under the title. - details: Vec<(&'static str, String)>, + details: Vec, }, /// A heading plus one row per item, so long name lists stay scannable. /// The Plain text is the full sentence with every name inline. @@ -169,6 +169,58 @@ pub enum LineKind { }, } +/// How a record detail value renders for humans. The class, not the text, +/// decides the look, the way syntax highlighting classes tokens. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum DetailKind { + /// Unstyled text. + Plain, + /// A command the user can type: highlighted as code. + Command, + /// A filesystem path: dimmed. + Path, +} + +/// One labelled row under a record title. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Detail { + /// The dim label. + pub label: &'static str, + /// The row value. + pub value: String, + /// How the value renders. + pub kind: DetailKind, +} + +impl Detail { + /// An unstyled detail row. + pub fn plain(label: &'static str, value: impl Into) -> Self { + Self { + label, + value: value.into(), + kind: DetailKind::Plain, + } + } + + /// A command the user can type. Rendered as highlighted code. + pub fn command(label: &'static str, value: impl Into) -> Self { + Self { + label, + value: value.into(), + kind: DetailKind::Command, + } + } + + /// A filesystem path. Rendered dim. + pub fn path(label: &'static str, value: impl Into) -> Self { + Self { + label, + value: value.into(), + kind: DetailKind::Path, + } + } +} + impl Line { /// Unstyled text whose Plain and Human renderings match. pub fn plain(text: impl Into) -> Self { @@ -279,7 +331,7 @@ impl Line { plain: impl Into, title: impl Into, status: Option, - details: Vec<(&'static str, String)>, + details: Vec, ) -> Self { Self { plain: plain.into(), @@ -345,13 +397,18 @@ impl Line { } let label_width = details .iter() - .map(|(label, _)| width(label)) + .map(|detail| width(detail.label)) .max() .unwrap_or(0); - for (label, value) in details { - let label = pad_to(label, label_width); + for detail in details { + let label = pad_to(detail.label, label_width); + let value = match detail.kind { + DetailKind::Plain => detail.value.clone(), + DetailKind::Command => crate::spans::paint_command(&detail.value, color), + DetailKind::Path => paint(&detail.value, Role::Dim, color), + }; output.push('\n'); - output.push_str(&format!(" {} {value}", paint(&label, Role::Dim, color))); + output.push_str(&format!(" {label} {value}")); } output } @@ -393,7 +450,7 @@ pub(crate) fn has_hyperlink(value: &str) -> bool { #[cfg(test)] mod tests { - use super::{Line, Marker, Screen, has_hyperlink}; + use super::{Detail, Line, Marker, Screen, has_hyperlink}; #[test] fn plain_rendering_matches_the_machine_contract() { @@ -446,8 +503,8 @@ mod tests { "vue-testing", Some("claude-code · unmanaged".to_owned()), vec![ - ("candidate", "sel".to_owned()), - ("install", "skilld install sel".to_owned()), + Detail::plain("candidate", "sel"), + Detail::command("install", "skilld install sel"), ], ); diff --git a/crates/skilld-ui/src/spans.rs b/crates/skilld-ui/src/spans.rs new file mode 100644 index 00000000..8f4e5984 --- /dev/null +++ b/crates/skilld-ui/src/spans.rs @@ -0,0 +1,134 @@ +//! Inline token coloring, modelled on syntax highlighting. +//! +//! A command the user should type is a code literal: it must scan as one +//! visual object, not as prose. [`command_spans`] splits a command into +//! spans the way a highlighter splits a line of code: the binary is the +//! brand, the subcommand is the emphasis, flags dim, values plain. + +use crate::theme::{Role, paint}; + +/// One run of text with one role, or unstyled text. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum Span { + /// Unstyled text. + Text(String), + /// Text painted with a role. + Styled(String, Role), +} + +/// Split a skilld command into highlighted spans. +/// +/// `skilld` is Brand, the first bare word after it is Emphasis (the +/// subcommand), flags starting with `-` are Dim, and everything else stays +/// plain so selectors and Skill names carry the weight. +pub fn command_spans(command: &str) -> Vec { + let mut spans = Vec::new(); + let mut subcommand_used = false; + for word in command.split_whitespace() { + if spans.is_empty() && word == "skilld" { + spans.push(Span::Styled(word.to_owned(), Role::Brand)); + continue; + } + if !subcommand_used && !word.starts_with('-') && spans.len() == 1 { + subcommand_used = true; + spans.push(Span::Styled(word.to_owned(), Role::Emphasis)); + continue; + } + if word.starts_with('-') && word.chars().any(|c| c.is_ascii_alphabetic()) { + spans.push(Span::Styled(word.to_owned(), Role::Dim)); + continue; + } + spans.push(Span::Text(word.to_owned())); + } + spans +} + +/// Join spans into one colored string. Without color, the plain text. +pub fn paint_spans(spans: &[Span], color: bool) -> String { + if !color { + let mut output = String::new(); + for span in spans { + output.push_str(span_text(span)); + output.push(' '); + } + return output.trim_end().to_owned(); + } + let mut output = String::new(); + for span in spans { + match span { + Span::Text(text) => { + output.push_str(text); + output.push(' '); + } + Span::Styled(text, role) => { + output.push_str(&paint(text, *role, true)); + output.push(' '); + } + } + } + output.pop(); + output +} + +/// The span text with any styling stripped. One trailing space per span +/// mirrors the colored join; callers trim. +fn span_text(span: &Span) -> &str { + match span { + Span::Text(text) | Span::Styled(text, _) => text, + } +} + +/// Paint a command string in one step. +pub fn paint_command(command: &str, color: bool) -> String { + paint_spans(&command_spans(command), color) +} + +#[cfg(test)] +mod tests { + use super::{Span, command_spans, paint_command, paint_spans}; + use crate::theme::Role; + + #[test] + fn commands_tokenize_like_code() { + assert_eq!( + command_spans("skilld install skilld:owner/repo/x --agent codex"), + vec![ + Span::Styled("skilld".to_owned(), Role::Brand), + Span::Styled("install".to_owned(), Role::Emphasis), + Span::Text("skilld:owner/repo/x".to_owned()), + Span::Styled("--agent".to_owned(), Role::Dim), + Span::Text("codex".to_owned()), + ] + ); + } + + #[test] + fn update_commands_highlight_their_subcommand() { + assert_eq!( + command_spans("skilld update nuxt-seo --global")[1], + Span::Styled("update".to_owned(), Role::Emphasis) + ); + } + + #[test] + fn plain_commands_render_verbatim() { + assert_eq!( + paint_command("skilld install foo --agent codex", false), + "skilld install foo --agent codex" + ); + } + + #[test] + fn colored_commands_carry_brand_and_dim() { + let colored = paint_command("skilld install foo --agent codex", true); + assert!(colored.starts_with("\u{1b}[1m\u{1b}[36mskilld\u{1b}[0m")); + assert!(colored.contains("\u{1b}[1minstall\u{1b}[0m")); + assert!(colored.contains("\u{1b}[2m--agent\u{1b}[0m")); + assert!(colored.ends_with("codex")); + } + + #[test] + fn empty_input_renders_empty() { + assert_eq!(paint_spans(&command_spans(""), false), ""); + } +} diff --git a/crates/skilld-ui/src/text.rs b/crates/skilld-ui/src/text.rs index 05ee54f5..096a400c 100644 --- a/crates/skilld-ui/src/text.rs +++ b/crates/skilld-ui/src/text.rs @@ -113,10 +113,19 @@ pub fn short_sha(value: &str) -> &str { value.get(..7).unwrap_or(value) } +/// Show `path` relative to `base` when it lives underneath it, otherwise +/// absolute. Human display only; machine records keep absolute paths. +pub fn display_path(path: &std::path::Path, base: &std::path::Path) -> String { + path.strip_prefix(base) + .map(|relative| relative.display().to_string()) + .unwrap_or_else(|_| path.display().to_string()) +} + #[cfg(test)] mod tests { use super::{ - grouped_number, pad_to, sanitize, short_sha, truncate, truncate_ellipsis, width, wrap, + display_path, grouped_number, pad_to, sanitize, short_sha, truncate, truncate_ellipsis, + width, wrap, }; #[test] @@ -162,4 +171,20 @@ mod tests { assert_eq!(short_sha("abc"), "abc"); assert_eq!(width("漢"), 2); } + + #[test] + fn display_paths_strip_the_base_when_inside_it() { + use std::path::Path; + assert_eq!( + display_path( + Path::new("/home/harlan/pkg/skilld/.claude/skills/x"), + Path::new("/home/harlan/pkg/skilld") + ), + ".claude/skills/x" + ); + assert_eq!( + display_path(Path::new("/etc/passwd"), Path::new("/home/harlan")), + "/etc/passwd" + ); + } }