diff --git a/Cargo.lock b/Cargo.lock index f66e451d..6dab71f4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2015,6 +2015,7 @@ dependencies = [ "serde_json", "sha2 0.11.0", "skilld-core", + "skilld-ui", "tempfile", "unicode-width 0.2.0", "url", @@ -2042,6 +2043,7 @@ dependencies = [ "skilld-auth", "skilld-command", "skilld-core", + "skilld-ui", "tempfile", "terminal_size", "unicode-width 0.2.0", @@ -2049,6 +2051,13 @@ dependencies = [ "url", ] +[[package]] +name = "skilld-ui" +version = "3.0.0-beta.1" +dependencies = [ + "unicode-width 0.2.0", +] + [[package]] name = "skilld-wasi" version = "3.0.0-beta.1" diff --git a/Cargo.toml b/Cargo.toml index e7aa17c2..c7f49dec 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,7 @@ members = [ "crates/skilld-command", "crates/skilld-core", "crates/skilld-native", + "crates/skilld-ui", "crates/skilld-wasi", ] resolver = "3" @@ -27,6 +28,7 @@ sha2 = "0.11.0" skilld-auth = { path = "crates/skilld-auth" } skilld-command = { path = "crates/skilld-command" } skilld-core = { path = "crates/skilld-core" } +skilld-ui = { path = "crates/skilld-ui" } subtle = "2.6.1" tempfile = "3.27.0" terminal_size = "0.4.4" diff --git a/crates/skilld-command/Cargo.toml b/crates/skilld-command/Cargo.toml index 7438b487..5eaa545c 100644 --- a/crates/skilld-command/Cargo.toml +++ b/crates/skilld-command/Cargo.toml @@ -24,6 +24,8 @@ sha2.workspace = true skilld-core.workspace = true +skilld-ui.workspace = true + tempfile.workspace = true unicode-width.workspace = true diff --git a/crates/skilld-command/src/config.rs b/crates/skilld-command/src/config.rs index 8440538b..ef392059 100644 --- a/crates/skilld-command/src/config.rs +++ b/crates/skilld-command/src/config.rs @@ -4,6 +4,7 @@ use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; use skilld_core::{AgentTargetId, InstallMode}; +use skilld_ui::Line; use crate::CommandError; @@ -53,17 +54,17 @@ impl LocalConfig { } } - pub fn entries(&self) -> Vec { + pub fn entries(&self) -> Vec { + let targets = self + .agent_targets + .iter() + .map(|target| target.as_str()) + .collect::>() + .join(","); + let mode = self.install_mode.as_str(); vec![ - format!( - "agent.targets={}", - self.agent_targets - .iter() - .map(|target| target.as_str()) - .collect::>() - .join(",") - ), - format!("install.mode={}", self.install_mode.as_str()), + Line::field_plain(format!("agent.targets={targets}"), "agent.targets", targets), + Line::field_plain(format!("install.mode={mode}"), "install.mode", mode), ] } } diff --git a/crates/skilld-command/src/lib.rs b/crates/skilld-command/src/lib.rs index 3890cc41..fe32e3dd 100644 --- a/crates/skilld-command/src/lib.rs +++ b/crates/skilld-command/src/lib.rs @@ -34,6 +34,7 @@ use skilld_core::{ UpdatePlanItem, UpdatePlanV1, UpdateRelation, UpdateRetryAfter, VERSION, classify_update_comparison, select_target_ids, }; +use skilld_ui::{Line, Screen}; use output::{ OutputMode, SearchItem, SearchOutcome, render_error, render_search, render_update_check, @@ -200,23 +201,19 @@ pub trait Host { )) } - fn verify(&self, _name: Option<&str>) -> Result, CommandError> { + fn verify(&self, _name: Option<&str>) -> Result, CommandError> { Err(CommandError::unsupported_host( "source verification is unavailable on this host", )) } - fn update( - &self, - _name: Option<&str>, - _scope: InstallScope, - ) -> Result, CommandError> { + fn update(&self, _name: Option<&str>, _scope: InstallScope) -> Result, CommandError> { Err(CommandError::unsupported_host( "Skill update is unavailable on this host", )) } - fn update_selected(&self, items: &[UpdatePlanItem]) -> Result, CommandError> { + fn update_selected(&self, items: &[UpdatePlanItem]) -> Result, CommandError> { validate_update_selection(items)?; Err(CommandError::unsupported_host( "Selected Skill updates are unavailable on this host", @@ -229,7 +226,7 @@ pub trait Host { )) } - fn outdated(&self, _all: bool) -> Result, CommandError> { + fn outdated(&self, _all: bool) -> Result, CommandError> { Err(CommandError::unsupported_host( "Outdated Skill reports are unavailable on this host", )) @@ -247,7 +244,7 @@ pub trait Host { )) } - fn config_list(&self) -> Result, CommandError> { + fn config_list(&self) -> Result, CommandError> { Err(CommandError::unsupported_host( "configuration is unavailable on this host", )) @@ -402,7 +399,7 @@ where } enum CommandOutput { - Lines(Vec), + Screen(Screen), Search(SearchOutcome), UpdateCheck(UpdatePlanV1), } @@ -503,13 +500,12 @@ where } match dispatch(cli.command, host) { - Ok(CommandOutput::Lines(lines)) => { - let mut bytes = Vec::new(); - for line in lines { - bytes.extend_from_slice(line.as_bytes()); - bytes.push(b'\n'); - } - write_success(&bytes, mode, stdout, stderr) + Ok(CommandOutput::Screen(screen)) => { + let bytes = match mode { + OutputMode::Human { color, .. } => screen.render_human(color), + OutputMode::Plain | OutputMode::JsonV1 => screen.render_plain(), + }; + write_success(bytes.as_bytes(), mode, stdout, stderr) } Ok(CommandOutput::Search(outcome)) => match render_search(&outcome, mode) { Ok(bytes) => write_success(&bytes, mode, stdout, stderr), @@ -697,56 +693,67 @@ fn dispatch(command: Command, host: &H) -> Result>(); if direct { - lines.push("Review the unverified Skill before use.".to_owned()); + lines.push(Line::hint("Review the unverified Skill before use.")); } - Ok(CommandOutput::Lines(lines)) - } - Command::List { global } => host.list(scope(global)).map(CommandOutput::Lines), - Command::View { skill, global } => { - render_view(host.view(&skill, scope(global))?).map(CommandOutput::Lines) + Ok(CommandOutput::Screen(Screen::new(lines))) } + Command::List { global } => host.list(scope(global)).map(|names| { + CommandOutput::Screen(Screen::new(names.into_iter().map(Line::item).collect())) + }), + Command::View { skill, global } => render_view(host.view(&skill, scope(global))?) + .map(|lines| CommandOutput::Screen(Screen::new(lines))), Command::Remove { skill, global } => { host.remove(&skill, scope(global))?; - Ok(CommandOutput::Lines(vec![format!( - "Removed Skill {skill}." - )])) + Ok(CommandOutput::Screen(Screen::new(vec![Line::success( + format!("Removed Skill {skill}."), + )]))) } Command::Auth { command: AuthCommand::Status, - } => Ok(CommandOutput::Lines(vec![if host.auth_status()? { - "Authenticated.".to_owned() - } else { - "Not authenticated.".to_owned() - }])), + } => Ok(CommandOutput::Screen(Screen::new(vec![ + if host.auth_status()? { + Line::success("Authenticated.") + } else { + Line::plain("Not authenticated.") + }, + ]))), Command::Auth { command: AuthCommand::Login, } => { host.auth_login()?; - Ok(CommandOutput::Lines(vec![ - "Authentication started.".to_owned(), - ])) + Ok(CommandOutput::Screen(Screen::new(vec![Line::plain( + "Authentication started.", + )]))) } Command::Auth { command: AuthCommand::Logout, } => { host.auth_logout()?; - Ok(CommandOutput::Lines(vec!["Logged out.".to_owned()])) + Ok(CommandOutput::Screen(Screen::new(vec![Line::success( + "Logged out.", + )]))) } Command::Config { command: ConfigCommand::Get { key }, - } => Ok(CommandOutput::Lines(vec![host.config_get(&key)?])), + } => Ok(CommandOutput::Screen(Screen::new(vec![Line::plain( + host.config_get(&key)?, + )]))), Command::Config { command: ConfigCommand::Set { key, value }, } => { host.config_set(&key, &value)?; - Ok(CommandOutput::Lines(vec![format!("Set {key}.")])) + Ok(CommandOutput::Screen(Screen::new(vec![Line::success( + format!("Set {key}."), + )]))) } Command::Config { command: ConfigCommand::List, - } => host.config_list().map(CommandOutput::Lines), + } => host + .config_list() + .map(|lines| CommandOutput::Screen(Screen::new(lines))), Command::Search { query } => { let query = query.join(" ").trim().to_owned(); if query.is_empty() || query.len() > 200 { @@ -790,19 +797,26 @@ fn dispatch(command: Command, host: &H) -> Result host.verify(skill.as_deref()).map(CommandOutput::Lines), - Command::Outdated { all } => host.outdated(all).map(CommandOutput::Lines), + Command::Verify { skill } => host + .verify(skill.as_deref()) + .map(|lines| CommandOutput::Screen(Screen::new(lines))), + Command::Outdated { all } => host + .outdated(all) + .map(|lines| CommandOutput::Screen(Screen::new(lines))), } } -fn render_view(view: SkillView) -> Result, CommandError> { +fn render_view(view: SkillView) -> Result, CommandError> { let source = match view.skill.source { - LockedSource::Local { path } => format!("local {path}"), - LockedSource::BundledSkilld => "skilld-maintained Skill".to_owned(), - LockedSource::Remote { source, .. } => source, + LockedSource::Local { path } => Line::field("Source", format!("local {path}")), + LockedSource::BundledSkilld => Line::field("Source", "skilld-maintained Skill"), + LockedSource::Remote { source, .. } => match github_url(&source) { + Some(url) => Line::linked_field("Source", source, url), + None => Line::field("Source", source), + }, }; let targets = if view.skill.targets.is_empty() { "none".to_owned() @@ -815,14 +829,25 @@ fn render_view(view: SkillView) -> Result, CommandError> { .join(", ") }; Ok(vec![ - format!("Name: {}", view.name), - format!("Path: {}", view.canonical_path.display()), - format!("Source: {source}"), - format!("Source status: {}", view.skill.source_status.as_str()), - format!("Agent targets: {targets}"), + Line::field("Name", view.name), + Line::field("Path", view.canonical_path.display().to_string()), + source, + Line::field("Source status", view.skill.source_status.as_str()), + Line::field("Agent targets", targets), ]) } +/// A GitHub repository URL for a remote Skill source, when the source names +/// one. +fn github_url(source: &str) -> Option { + let body = source.split_once(':')?.1; + let mut segments = body.split('/'); + let owner = segments.next()?; + let repository = segments.next()?; + (!owner.is_empty() && !repository.is_empty()) + .then(|| format!("https://github.com/{owner}/{repository}")) +} + fn scope(global: bool) -> InstallScope { if global { InstallScope::Global @@ -1301,7 +1326,7 @@ impl Host for LocalHost { store.write(&config) } - fn config_list(&self) -> Result, CommandError> { + fn config_list(&self) -> Result, CommandError> { Ok(self.config_store().read()?.entries()) } @@ -1332,7 +1357,7 @@ impl Host for LocalHost { .map_err(CommandError::remote) } - fn verify(&self, requested: Option<&str>) -> Result, CommandError> { + fn verify(&self, requested: Option<&str>) -> Result, CommandError> { let scope = InstallScope::Project; let known = self.known_targets(scope)?; let store = self.store(scope); @@ -1363,7 +1388,7 @@ impl Host for LocalHost { .map_err(CommandError::remote)? { RemoteSourceState::Current => { - lines.push(format!("Verified Skill {}.", name.as_str())); + lines.push(Line::success(format!("Verified Skill {}.", name.as_str()))); } RemoteSourceState::Stale { .. } => { return Err(CommandError::operation( @@ -1379,7 +1404,10 @@ impl Host for LocalHost { format!("Skill {} has an unverified source", name.as_str()), )); } - _ => lines.push(format!("Checked local Skill {}.", name.as_str())), + _ => lines.push(Line::plain(format!( + "Checked local Skill {}.", + name.as_str() + ))), } } Ok(lines) @@ -1389,7 +1417,7 @@ impl Host for LocalHost { &self, requested: Option<&str>, scope: InstallScope, - ) -> Result, CommandError> { + ) -> Result, CommandError> { let known = self.known_targets(scope)?; let store = self.store(scope); let names = selected_names(&store, &known, requested)?; @@ -1565,11 +1593,11 @@ impl Host for LocalHost { .map_err(CommandError::store)?; Ok(selected .into_iter() - .map(|selection| format!("Updated Skill {}.", selection.name)) + .map(|selection| Line::success(format!("Updated Skill {}.", selection.name))) .collect()) } - fn update_selected(&self, items: &[UpdatePlanItem]) -> Result, CommandError> { + fn update_selected(&self, items: &[UpdatePlanItem]) -> Result, CommandError> { validate_update_selection(items)?; let scope = InstallScope::Project; let known = self.known_targets(scope)?; @@ -1734,14 +1762,14 @@ impl Host for LocalHost { Ok(UpdatePlanV1::new(plan)) } - fn outdated(&self, all: bool) -> Result, CommandError> { + fn outdated(&self, all: bool) -> Result, CommandError> { let scopes = if all { vec![InstallScope::Project, InstallScope::Global] } else { vec![InstallScope::Project] }; let progress = self.outdated_progress.as_ref(); - let mut lines = Vec::new(); + let mut lines: Vec = Vec::new(); let mut managed = BTreeMap::>::new(); let mut store_roots = Vec::new(); let mut scan = Vec::new(); @@ -1754,11 +1782,11 @@ impl Host for LocalHost { Ok(names) => names, Err(error) => { // Without a readable lockfile, managed copies cannot be told from unmanaged ones. - lines.push(format!( + lines.push(Line::error(format!( "Skill store unavailable in {} scope: {}", scope.as_str(), CommandError::store(error).message - )); + ))); if all { // The ancestor scan must not report Skills this scope cannot verify. suppressed_roots.extend(known.iter().map(|target| target.root.clone())); @@ -1772,10 +1800,10 @@ impl Host for LocalHost { let view = match store.view(&skill_name, &known) { Ok(view) => view, Err(error) => { - lines.push(format!( + lines.push(Line::error(format!( "Skill {name} details unavailable: {}", CommandError::store(error).message - )); + ))); continue; } }; @@ -1852,7 +1880,7 @@ impl Host for LocalHost { } progress.finish(); if lines.is_empty() { - lines.push("No installed Skills found.".to_owned()); + lines.push(Line::plain("No installed Skills found.")); } Ok(lines) } @@ -1932,7 +1960,7 @@ fn apply_update_selection( items: &[UpdatePlanItem], store: LocalStore, known: Vec, -) -> Result, CommandError> { +) -> Result, CommandError> { let provider = host.remote_provider()?; let mut pending = Vec::new(); for item in items { @@ -2113,7 +2141,7 @@ fn apply_update_selection( .map_err(CommandError::store)?; Ok(selected .into_iter() - .map(|selection| format!("Updated Skill {}.", selection.name)) + .map(|selection| Line::success(format!("Updated Skill {}.", selection.name))) .collect()) } @@ -2266,7 +2294,7 @@ fn update_apply_failure(name: &str, outcome: RemoteComparisonOutcome) -> Command } impl LocalHost { - fn report_outdated_view(&self, view: &SkillView, scope: InstallScope) -> Vec { + fn report_outdated_view(&self, view: &SkillView, scope: InstallScope) -> Vec { let name = &view.name; let global = if scope == InstallScope::Global { " --global" @@ -2289,18 +2317,18 @@ impl LocalHost { }); match state { Ok(RemoteSourceState::Current) => { - vec![format!("Current Skill {name}.")] + vec![Line::success(format!("Current Skill {name}."))] } Ok(RemoteSourceState::Stale { .. }) => { - vec![format!( + vec![Line::warn(format!( "Outdated Skill {name}. Run skilld update {name}{global}." - )] + ))] } Err(error) => { - vec![format!( + vec![Line::error(format!( "Source state unavailable for Skill {name}: {}.", error.message - )] + ))] } } } @@ -2312,12 +2340,14 @@ impl LocalHost { .map(|locked| locked.agent) .collect::>(); let agent_flags = outdated::agent_flags(&agents); - vec![format!( + vec![Line::warn(format!( "Unverified Skill {name}. Run skilld install {source} --direct{global}{agent_flags} to update it." - )] + ))] + } + (LockedSource::BundledSkilld, _) => { + vec![Line::plain(format!("skilld-maintained Skill {name}."))] } - (LockedSource::BundledSkilld, _) => vec![format!("skilld-maintained Skill {name}.")], - _ => vec![format!("Local Skill {name}.")], + _ => vec![Line::plain(format!("Local Skill {name}."))], } } diff --git a/crates/skilld-command/src/outdated.rs b/crates/skilld-command/src/outdated.rs index 4440222b..d6df0a30 100644 --- a/crates/skilld-command/src/outdated.rs +++ b/crates/skilld-command/src/outdated.rs @@ -3,6 +3,7 @@ use std::fs; use std::path::{Path, PathBuf}; use skilld_core::{AgentTargetId, InstallScope, SkillName}; +use skilld_ui::Line; use crate::ResolvedTarget; use crate::local_store::normalize_path; @@ -131,28 +132,28 @@ pub(crate) fn found_line(skill: &UnmanagedSkill) -> String { ) } -pub(crate) fn render_no_match(skills: &[&UnmanagedSkill]) -> Vec { +pub(crate) fn render_no_match(skills: &[&UnmanagedSkill]) -> Vec { if skills.is_empty() { return vec![]; } - vec![format!( + vec![Line::warn(format!( "No Repository match for {} ({}).", skill_count(skills.len()), name_list(skills) - )] + ))] } pub(crate) fn render_search_failures( failures: &BTreeMap>, -) -> Vec { +) -> Vec { failures .iter() .map(|(message, skills)| { - format!( + Line::warn(format!( "Skill search unavailable for {} ({}): {message}.", skill_count(skills.len()), name_list(skills) - ) + )) }) .collect() } @@ -172,7 +173,7 @@ fn skill_count(count: usize) -> String { pub(crate) fn render_unmanaged( skill: &UnmanagedSkill, candidate: Option<&SkillCandidate>, -) -> Vec { +) -> Vec { let agents = agent_list(skill); let Some(candidate) = candidate else { return vec![]; @@ -184,15 +185,15 @@ pub(crate) fn render_unmanaged( }; let agent_flags = agent_flags(&skill.agents); vec![ - format!( + Line::warn(format!( "Unmanaged Skill {} ({agents}). Candidate source {}, {} stars.", skill.name, candidate.selector, candidate.stargazer_count - ), - format!( + )), + Line::hint(format!( "Delete {}, then run skilld install {}{global}{agent_flags}.", skill.path.display(), candidate.selector - ), + )), ] } diff --git a/crates/skilld-command/src/output.rs b/crates/skilld-command/src/output.rs index f32c0c04..70550082 100644 --- a/crates/skilld-command/src/output.rs +++ b/crates/skilld-command/src/output.rs @@ -1,7 +1,8 @@ use clap::error::ErrorKind; use serde::Serialize; use skilld_core::UpdatePlanV1; -use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; +use skilld_ui::text::{grouped_number, sanitize, width, wrap}; +use skilld_ui::{Role, paint}; use crate::{CommandError, CommandErrorKind}; @@ -179,12 +180,12 @@ fn render_plain(outcome: &SearchOutcome) -> String { output } -fn render_human(outcome: &SearchOutcome, width: u16, color: bool) -> String { - let width = usize::from(width); +fn render_human(outcome: &SearchOutcome, terminal_width: u16, color: bool) -> String { + let columns = usize::from(terminal_width); let mut output = String::new(); - let heading = format!("Skill search {}", terminal_text(&outcome.query)); - for line in wrap(&heading, width) { - output.push_str(&styled(&line, "\u{1b}[1m\u{1b}[36m", color)); + let heading = format!("Skill search {}", sanitize(&outcome.query)); + for line in wrap(&heading, columns) { + output.push_str(&paint(&line, Role::Brand, color)); output.push('\n'); } let shown = outcome.items.len(); @@ -201,8 +202,8 @@ fn render_human(outcome: &SearchOutcome, width: u16, color: bool) -> String { if outcome.items.is_empty() { output.push('\n'); - let empty = format!("No Skills found for {}.", terminal_text(&outcome.query)); - for line in wrap(&empty, width) { + let empty = format!("No Skills found for {}.", sanitize(&outcome.query)); + for line in wrap(&empty, columns) { output.push_str(&line); output.push('\n'); } @@ -212,37 +213,37 @@ fn render_human(outcome: &SearchOutcome, width: u16, color: bool) -> String { for item in &outcome.items { output.push('\n'); - let name = terminal_text(&item.name); + let name = sanitize(&item.name); let stars = format!("{} stars", grouped_number(item.stargazer_count)); - if 2 + display_width(&name) + 2 + display_width(&stars) <= width { - let gap = width - 2 - display_width(&name) - display_width(&stars); + if 2 + width(&name) + 2 + width(&stars) <= columns { + let gap = columns - 2 - width(&name) - width(&stars); output.push_str(" "); - output.push_str(&styled(&name, "\u{1b}[1m", color)); + output.push_str(&paint(&name, Role::Emphasis, color)); output.push_str(&" ".repeat(gap)); - output.push_str(&styled(&stars, "\u{1b}[33m", color)); + output.push_str(&paint(&stars, Role::Warn, color)); output.push('\n'); } else { - for line in wrap(&name, width.saturating_sub(2)) { + for line in wrap(&name, columns.saturating_sub(2)) { output.push_str(" "); - output.push_str(&styled(&line, "\u{1b}[1m", color)); + output.push_str(&paint(&line, Role::Emphasis, color)); output.push('\n'); } output.push_str(" "); - output.push_str(&styled(&stars, "\u{1b}[33m", color)); + output.push_str(&paint(&stars, Role::Warn, color)); output.push('\n'); } if let Some(description) = &item.description { - for line in wrap(&terminal_text(description), width.saturating_sub(2)) { + for line in wrap(&sanitize(description), columns.saturating_sub(2)) { output.push_str(" "); output.push_str(&line); output.push('\n'); } } - let install = format!("Install: skilld install {}", terminal_text(&item.selector)); - for line in wrap(&install, width.saturating_sub(2)) { + let install = format!("Install: skilld install {}", sanitize(&item.selector)); + for line in wrap(&install, columns.saturating_sub(2)) { output.push_str(" "); - output.push_str(&styled(&line, "\u{1b}[2m", color)); + output.push_str(&paint(&line, Role::Dim, color)); output.push('\n'); } } @@ -266,77 +267,6 @@ fn escape_plain(value: &str) -> String { output } -fn terminal_text(value: &str) -> String { - value - .chars() - .map(|character| { - if character.is_control() { - ' ' - } else { - character - } - }) - .collect() -} - -fn wrap(value: &str, width: usize) -> Vec { - let width = width.max(1); - let mut lines = Vec::new(); - let mut current = String::new(); - - for word in value.split_whitespace() { - let separator = usize::from(!current.is_empty()); - if display_width(¤t) + separator + display_width(word) > width && !current.is_empty() - { - lines.push(std::mem::take(&mut current)); - } - if !current.is_empty() { - current.push(' '); - } - if display_width(word) <= width { - current.push_str(word); - } else { - let mut chunk = String::new(); - for character in word.chars() { - let character_width = UnicodeWidthChar::width(character).unwrap_or(0); - if !chunk.is_empty() && display_width(&chunk) + character_width > width { - lines.push(std::mem::take(&mut chunk)); - } - chunk.push(character); - } - current = chunk; - } - } - if !current.is_empty() || lines.is_empty() { - lines.push(current); - } - lines -} - -fn display_width(value: &str) -> usize { - UnicodeWidthStr::width(value) -} - -fn grouped_number(value: u64) -> String { - let digits = value.to_string(); - let mut output = String::new(); - for (index, character) in digits.chars().enumerate() { - if index > 0 && (digits.len() - index) % 3 == 0 { - output.push(','); - } - output.push(character); - } - output -} - -fn styled(value: &str, style: &str, color: bool) -> String { - if color { - format!("{style}{value}\u{1b}[0m") - } else { - value.to_owned() - } -} - #[derive(Serialize)] #[serde(rename_all = "camelCase")] struct JsonSuccess { diff --git a/crates/skilld-command/tests/outdated.rs b/crates/skilld-command/tests/outdated.rs index a6503fe1..d4a0aaa6 100644 --- a/crates/skilld-command/tests/outdated.rs +++ b/crates/skilld-command/tests/outdated.rs @@ -298,7 +298,7 @@ fn outdated_reports_current_and_stale_project_skills() { } #[test] -fn outdated_system_reports_a_stale_global_skill_with_the_global_update() { +fn outdated_all_reports_a_stale_global_skill_with_the_global_update() { let temporary = tempfile::tempdir().unwrap(); let project = temporary.path().join("project"); fs::create_dir_all(&project).unwrap(); @@ -327,7 +327,7 @@ fn outdated_system_reports_a_stale_global_skill_with_the_global_update() { } #[test] -fn outdated_system_links_unmanaged_skills_to_a_repository() { +fn outdated_all_links_unmanaged_skills_to_a_repository() { let temporary = tempfile::tempdir().unwrap(); let project = temporary.path().join("project"); fs::create_dir_all(&project).unwrap(); @@ -356,7 +356,7 @@ fn outdated_system_links_unmanaged_skills_to_a_repository() { } #[test] -fn outdated_system_reports_unmanaged_skills_without_a_match() { +fn outdated_all_reports_unmanaged_skills_without_a_match() { let temporary = tempfile::tempdir().unwrap(); let project = temporary.path().join("project"); fs::create_dir_all(&project).unwrap(); @@ -383,7 +383,7 @@ fn outdated_system_reports_unmanaged_skills_without_a_match() { } #[test] -fn outdated_system_surfaces_a_search_failure_and_keeps_scanning() { +fn outdated_all_surfaces_a_search_failure_and_keeps_scanning() { let temporary = tempfile::tempdir().unwrap(); let project = temporary.path().join("project"); fs::create_dir_all(&project).unwrap(); @@ -412,7 +412,7 @@ fn outdated_system_surfaces_a_search_failure_and_keeps_scanning() { } #[test] -fn outdated_system_reports_a_managed_skill_once() { +fn outdated_all_reports_a_managed_skill_once() { let temporary = tempfile::tempdir().unwrap(); let project = temporary.path().join("project"); fs::create_dir_all(&project).unwrap(); @@ -517,7 +517,7 @@ fn outdated_survives_a_source_state_failure() { } #[test] -fn outdated_system_survives_a_corrupt_global_store() { +fn outdated_all_survives_a_corrupt_global_store() { let temporary = tempfile::tempdir().unwrap(); let project = temporary.path().join("project"); fs::create_dir_all(&project).unwrap(); @@ -553,7 +553,7 @@ fn outdated_system_survives_a_corrupt_global_store() { } #[test] -fn outdated_system_groups_agents_sharing_one_directory() { +fn outdated_all_groups_agents_sharing_one_directory() { let temporary = tempfile::tempdir().unwrap(); let project = temporary.path().join("project"); fs::create_dir_all(&project).unwrap(); @@ -651,7 +651,7 @@ fn outdated_reports_the_bundled_skill_by_its_source() { } #[test] -fn outdated_system_runs_candidate_searches_in_parallel_with_a_bound() { +fn outdated_all_runs_candidate_searches_in_parallel_with_a_bound() { use std::time::Instant; let temporary = tempfile::tempdir().unwrap(); @@ -715,7 +715,7 @@ fn ancestor_roots_continue_to_the_root_outside_home() { } #[test] -fn outdated_system_finds_skills_in_parent_directories() { +fn outdated_all_finds_skills_in_parent_directories() { let temporary = tempfile::tempdir().unwrap(); let nested = temporary.path().join("work/app"); fs::create_dir_all(&nested).unwrap(); diff --git a/crates/skilld-command/tests/remote.rs b/crates/skilld-command/tests/remote.rs index a1d122fe..5da64fce 100644 --- a/crates/skilld-command/tests/remote.rs +++ b/crates/skilld-command/tests/remote.rs @@ -1721,7 +1721,13 @@ fn multi_skill_update_prepares_then_commits_every_artifact() { let lines = host.update(None, InstallScope::Project).unwrap(); - assert_eq!(lines, ["Updated Skill alpha.", "Updated Skill beta."]); + assert_eq!( + lines + .iter() + .map(skilld_ui::Line::plain_text) + .collect::>(), + ["Updated Skill alpha.", "Updated Skill beta."] + ); assert_eq!(*provider.prepared_names.lock().unwrap(), ["alpha", "beta"]); for name in ["alpha", "beta"] { assert_eq!( @@ -1863,7 +1869,13 @@ fn selected_skill_update_commits_only_the_exact_subset() { let lines = host.update_selected(&reviewed).unwrap(); - assert_eq!(lines, ["Updated Skill gamma.", "Updated Skill alpha."]); + assert_eq!( + lines + .iter() + .map(skilld_ui::Line::plain_text) + .collect::>(), + ["Updated Skill gamma.", "Updated Skill alpha."] + ); assert_eq!(*provider.prepared_names.lock().unwrap(), ["gamma", "alpha"]); for (name, version) in [("alpha", "second"), ("beta", "first"), ("gamma", "second")] { assert_eq!( @@ -2035,7 +2047,11 @@ fn remote_install_verify_and_failed_update_use_the_normal_transaction() { assert_eq!(host.install_request(request).unwrap(), ["example"]); assert_eq!( - host.verify(Some("example")).unwrap(), + host.verify(Some("example")) + .unwrap() + .iter() + .map(skilld_ui::Line::plain_text) + .collect::>(), ["Verified Skill example."] ); let before = fs::read(project.join(".skills/example/SKILL.md")).unwrap(); diff --git a/crates/skilld-native/Cargo.toml b/crates/skilld-native/Cargo.toml index f88e0036..f23284f0 100644 --- a/crates/skilld-native/Cargo.toml +++ b/crates/skilld-native/Cargo.toml @@ -22,6 +22,8 @@ skilld-command.workspace = true skilld-core.workspace = true +skilld-ui.workspace = true + tempfile.workspace = true terminal_size.workspace = true @@ -41,3 +43,6 @@ unicode-width.workspace = true [target.'cfg(unix)'.dev-dependencies] nix = { version = "0.29.0", features = [ "term" ] } + +[dev-dependencies] +skilld-ui.workspace = true diff --git a/crates/skilld-native/src/main.rs b/crates/skilld-native/src/main.rs index 2687eec9..dc21bfde 100644 --- a/crates/skilld-native/src/main.rs +++ b/crates/skilld-native/src/main.rs @@ -75,6 +75,7 @@ fn main() -> ExitCode { { host.with_outdated_progress(Arc::new(status::OutdatedProgressLine::for_terminal( std::io::stderr().is_terminal(), + active_agent_detected(), ))) } else { host diff --git a/crates/skilld-native/src/status.rs b/crates/skilld-native/src/status.rs index 7545e94a..b5a3be9f 100644 --- a/crates/skilld-native/src/status.rs +++ b/crates/skilld-native/src/status.rs @@ -1,4 +1,6 @@ use skilld_command::OutputContext; +use skilld_ui::spinner; +use skilld_ui::theme::{RESET, Role, paint}; use std::io::Write; use std::sync::Arc; @@ -10,12 +12,25 @@ use std::time::Duration; use std::time::Instant; const ERASE_LINE: &[u8] = b"\r\x1b[2K"; +/// The delay before the spinner first paints, so fast operations never flash. +const START_DELAY: Duration = Duration::from_millis(500); struct StatusState { stopped: bool, out: Box, label: String, started: Instant, + frame: usize, + color: bool, +} + +/// The spinner line for one frame. Pure so tests can pin the exact bytes. +fn frame_line(label: &str, frame: usize, seconds: u64, color: bool) -> String { + let glyph = paint(spinner::frame(frame), Role::Brand, color); + let text = paint(label, Role::Emphasis, color); + let clock = paint(&format!("{seconds}s"), Role::Dim, color); + let reset = if color { RESET } else { "" }; + format!("\r\x1b[2K{glyph} {text}\u{2026} {clock}{reset}") } pub struct StatusLine { @@ -36,16 +51,26 @@ impl StatusLine { self.shared.is_none() } - pub fn begin(label: &str, tick: Duration, out: Box) -> Self { + pub fn begin( + label: &str, + tick: Duration, + delay: Duration, + color: bool, + out: Box, + ) -> Self { let shared = Arc::new(Mutex::new(StatusState { stopped: false, out, label: label.to_owned(), started: Instant::now(), + frame: 0, + color, })); let worker = Arc::clone(&shared); let thread = thread::spawn(move || { loop { + // Sleep in short slices so a stop is noticed even with a long tick. + thread::sleep(tick.min(Duration::from_millis(250))); let mut state = match worker.lock() { Ok(state) => state, Err(_) => return, @@ -53,12 +78,18 @@ impl StatusLine { if state.stopped { return; } - let seconds = state.started.elapsed().as_secs(); - let line = format!("\r\x1b[2K{}… {seconds}s", state.label); - let _ = state.out.write_all(line.as_bytes()); - let _ = state.out.flush(); + if state.started.elapsed() >= delay { + let line = frame_line( + &state.label, + state.frame, + state.started.elapsed().as_secs(), + state.color, + ); + let _ = state.out.write_all(line.as_bytes()); + let _ = state.out.flush(); + state.frame += 1; + } drop(state); - thread::sleep(tick); } }); Self { @@ -68,12 +99,14 @@ impl StatusLine { } pub fn for_terminal(label: &str, context: OutputContext) -> Self { - if !matches!(context, OutputContext::HumanTerminal { .. }) { + let OutputContext::HumanTerminal { color, .. } = context else { return Self::disabled(); - } + }; Self::begin( label, - Duration::from_millis(500), + Duration::from_millis(100), + START_DELAY, + color, Box::new(std::io::stderr()), ) } @@ -162,8 +195,6 @@ where } } -const SPINNER_FRAMES: [&str; 10] = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"]; - /// Streams `skilld outdated` progress to a terminal: found Skills print as /// lines, remote verification rewrites one spinner line, and `finish` erases /// it so only results remain. @@ -173,9 +204,11 @@ pub struct OutdatedProgressLine { } impl OutdatedProgressLine { - pub fn for_terminal(is_terminal: bool) -> Self { + /// 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 { Self { - enabled: is_terminal, + enabled: is_terminal && !active_agent, frame: std::sync::atomic::AtomicUsize::new(0), } } @@ -204,8 +237,7 @@ impl skilld_command::OutdatedProgress for OutdatedProgressLine { if !self.enabled { return; } - let frame = - SPINNER_FRAMES[self.frame.fetch_add(1, Ordering::Relaxed) % SPINNER_FRAMES.len()]; + let frame = spinner::frame(self.frame.fetch_add(1, Ordering::Relaxed)); let mut stderr = std::io::stderr().lock(); let _ = write!(stderr, "\r\x1b[2K{frame} Checking {name}…"); let _ = stderr.flush(); @@ -218,9 +250,10 @@ impl skilld_command::OutdatedProgress for OutdatedProgressLine { #[cfg(test)] mod tests { - use super::{GatedStderr, OutputContext, StatusLine, status_label}; + use super::{GatedStderr, OutputContext, StatusLine, frame_line, status_label}; use std::io::Write; use std::sync::Mutex; + use std::thread; use std::time::Duration; fn never() -> Duration { @@ -236,10 +269,42 @@ mod tests { StatusLine::disabled().finish(); } + #[test] + fn frame_lines_lead_with_the_spinner_glyph() { + assert_eq!( + frame_line("Searching", 0, 2, false), + "\r\x1b[2K⠋ Searching… 2s" + ); + let colored = frame_line("Searching", 1, 2, true); + assert!(colored.starts_with("\r\x1b[2K\u{1b}[1m\u{1b}[36m⠙")); + assert!(colored.contains("Searching")); + } + + #[test] + fn nothing_paints_before_the_start_delay() { + let buffer = std::sync::Arc::new(Mutex::new(Vec::new())); + let status = StatusLine::begin( + "Searching", + Duration::from_millis(2), + Duration::from_secs(30), + false, + Box::new(Writer(buffer.clone())), + ); + thread::sleep(Duration::from_millis(20)); + status.finish(); + assert_eq!(output(&buffer), "\r\x1b[2K"); + } + #[test] fn finish_erases_the_line() { let buffer = std::sync::Arc::new(Mutex::new(Vec::new())); - let status = StatusLine::begin("Searching", never(), Box::new(Writer(buffer.clone()))); + let status = StatusLine::begin( + "Searching", + never(), + never(), + false, + Box::new(Writer(buffer.clone())), + ); status.finish(); assert_eq!(output(&buffer), "\r\x1b[2K"); } @@ -247,7 +312,13 @@ mod tests { #[test] fn stop_erases_once_and_is_idempotent() { let buffer = std::sync::Arc::new(Mutex::new(Vec::new())); - let mut status = StatusLine::begin("Searching", never(), Box::new(Writer(buffer.clone()))); + let mut status = StatusLine::begin( + "Searching", + never(), + never(), + false, + Box::new(Writer(buffer.clone())), + ); status.stop(); status.stop(); status.finish(); @@ -257,7 +328,13 @@ mod tests { #[test] fn a_gated_writer_stops_the_line_before_forwarding() { let buffer = std::sync::Arc::new(Mutex::new(Vec::new())); - let status = StatusLine::begin("Searching", never(), Box::new(Writer(buffer.clone()))); + let status = StatusLine::begin( + "Searching", + never(), + never(), + false, + Box::new(Writer(buffer.clone())), + ); let mut sink = Vec::new(); let mut gated = GatedStderr::new(&mut sink, status); gated.write_all(b"done").unwrap(); @@ -286,6 +363,33 @@ mod tests { StatusLine::for_terminal("Searching", OutputContext::Plain), line if line.is_disabled() )); + assert!(matches!( + StatusLine::for_terminal( + "Searching", + OutputContext::HumanTerminal { + width: 80, + color: true + } + ), + line if !line.is_disabled() + )); + } + + #[test] + fn outdated_progress_stays_quiet_for_agents() { + use skilld_command::OutdatedProgress; + + let agent = super::OutdatedProgressLine::for_terminal(true, true); + agent.found("example (project scope)"); + agent.checking("example"); + agent.finish(); + assert!(!agent.enabled); + + let human = super::OutdatedProgressLine::for_terminal(true, false); + assert!(human.enabled); + + let piped = super::OutdatedProgressLine::for_terminal(false, false); + assert!(!piped.enabled); } struct Writer(std::sync::Arc>>); diff --git a/crates/skilld-native/src/update_ui.rs b/crates/skilld-native/src/update_ui.rs index ad3919dc..776655a0 100644 --- a/crates/skilld-native/src/update_ui.rs +++ b/crates/skilld-native/src/update_ui.rs @@ -21,11 +21,11 @@ use ratatui::text::{Line, Text}; use ratatui::widgets::{Block, List, ListItem, ListState, Paragraph, Tabs}; use skilld_command::{CommandError, Host}; use skilld_core::{CommitHistory, UpdatePlanItem, UpdatePlanV1, UpdateRelation, UpdateRetryAfter}; +use skilld_ui::spinner; +use skilld_ui::time::relative_time; use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; use url::Url; -const SPINNER: [&str; 4] = ["⠋", "⠙", "⠹", "⠸"]; - #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] pub struct ComparisonId(String); @@ -837,7 +837,7 @@ pub fn update(mut model: Model, message: Message) -> Transition { } } Message::Tick => { - model.spinner = (model.spinner + 1) % SPINNER.len(); + model.spinner = model.spinner.wrapping_add(1); } Message::Key(key) => update_key(&mut model, key, &mut effects), } @@ -1029,8 +1029,8 @@ fn render_body(frame: &mut ratatui::Frame<'_>, model: &Model, color: bool, area: Phase::LoadingCandidates => { frame.render_widget( Paragraph::new(format!( - "{} Loading outdated Skills...", - SPINNER[model.spinner] + "{} Loading outdated Skills…", + spinner::frame(model.spinner) )), area, ); @@ -1073,18 +1073,18 @@ fn render_outdated(frame: &mut ratatui::Frame<'_>, model: &Model, color: bool, a commit_count, .. } => { - let checked = if row.selected { 'x' } else { ' ' }; + let checked = if row.selected { "◉" } else { "◯" }; if compact { - ListItem::new(truncate(&format!("[{checked}] {name}"), width)) + ListItem::new(truncate(&format!("{checked} {name}"), width)) } else { - let first = format!("[{checked}] {name:<28} {repository}"); + let first = format!("{checked} {name:<28} {repository}"); let commits = if *commit_count == 1 { "1 commit".to_owned() } else { format!("{commit_count} commits") }; let second = format!( - " {} -> {} {commits}", + " {} → {} {commits}", short_sha(locked_commit_sha), short_sha(latest_commit_sha) ); @@ -1096,10 +1096,10 @@ fn render_outdated(frame: &mut ratatui::Frame<'_>, model: &Model, color: bool, a } UpdateCandidate::Unavailable { name, error } => { if compact { - ListItem::new(truncate(&format!("[!] {name}: {}", error.code), width)) + ListItem::new(truncate(&format!("⚠ {name}: {}", error.code), width)) } else { ListItem::new(Text::from(vec![ - Line::from(truncate(&format!("[!] {name}: {}", error.code), width)), + Line::from(truncate(&format!("⚠ {name}: {}", error.code), width)), Line::from(truncate(&format!(" {}", error.message), width)), ])) .style(Style::default().fg(theme(color, Color::Red))) @@ -1159,8 +1159,8 @@ fn render_commits(frame: &mut ratatui::Frame<'_>, model: &Model, color: bool, ar } Some(CommitState::Loading) => { lines.push(Line::from(format!( - " {} Loading commits...", - SPINNER[model.spinner] + " {} Loading commits…", + spinner::frame(model.spinner) ))); } Some(CommitState::Failed(error)) => { @@ -1171,11 +1171,11 @@ fn render_commits(frame: &mut ratatui::Frame<'_>, model: &Model, color: bool, ar } Some(CommitState::Ready(page)) => { for commit in &page.commits { - let date = commit.timestamp.get(..10).unwrap_or(&commit.timestamp); + let date = relative_time(&commit.timestamp, std::time::SystemTime::now()); let details = if model.width >= 100 { format!("{} {date}", commit.author) } else { - date.to_owned() + date }; lines.push(Line::from(truncate( &format!( @@ -1217,8 +1217,8 @@ fn status_text(model: &Model) -> String { match model.phase { Phase::LoadingCandidates => "Loading update plan".to_owned(), Phase::Applying => format!( - "{} Updating {}...", - SPINNER[model.spinner], + "{} Updating {}…", + spinner::frame(model.spinner), skill_count(model.selected_names().len()) ), Phase::FailedLoad(_) => "Update plan failed".to_owned(), diff --git a/crates/skilld-native/tests/update_ui.rs b/crates/skilld-native/tests/update_ui.rs index debe72ac..ee072600 100644 --- a/crates/skilld-native/tests/update_ui.rs +++ b/crates/skilld-native/tests/update_ui.rs @@ -54,7 +54,7 @@ fn model_selects_outdated_skills_and_generates_visible_key_help() { let snapshot = render_snapshot(&transition.model, false); assert!(transition.effects.is_empty()); - assert!(snapshot.contains("> [ ] review-skill")); + assert!(snapshot.contains("> ◯ review-skill")); assert!(snapshot.contains("2 selected")); assert!(snapshot.contains("↑/↓ move")); assert!(snapshot.contains("space select")); @@ -124,7 +124,7 @@ fn view_reflows_at_narrow_width_and_keeps_the_cursor_in_the_viewport() { .model; let snapshot = render_snapshot(&model, false); - assert!(snapshot.contains("> [x] web-perf")); + assert!(snapshot.contains("> ◉ web-perf")); assert!(!snapshot.contains("cloudflare/skills")); assert!( snapshot @@ -236,7 +236,7 @@ fn one_unavailable_skill_keeps_other_updates_selectable() { .model; let snapshot = render_snapshot(&model, false); - assert!(snapshot.contains("[!] review-skill: RATE_LIMITED")); + assert!(snapshot.contains("⚠ review-skill: RATE_LIMITED")); assert!(snapshot.contains("1 selected. 1 unavailable")); assert!(snapshot.contains("r retry")); @@ -417,14 +417,19 @@ impl Host for PlanHost { Ok(self.plan.clone()) } - fn update_selected(&self, items: &[UpdatePlanItem]) -> Result, CommandError> { + fn update_selected( + &self, + items: &[UpdatePlanItem], + ) -> Result, CommandError> { self.selections.lock().unwrap().push(items.to_vec()); if let Some(error) = self.apply_error.clone() { return Err(error); } Ok(items .iter() - .map(|item| format!("Updated Skill {}.", item.name().as_str())) + .map(|item| { + skilld_ui::Line::success(format!("Updated Skill {}.", item.name().as_str())) + }) .collect()) } } diff --git a/crates/skilld-ui/Cargo.toml b/crates/skilld-ui/Cargo.toml new file mode 100644 index 00000000..c7f70f67 --- /dev/null +++ b/crates/skilld-ui/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "skilld-ui" + +version.workspace = true + +edition.workspace = true + +license.workspace = true + +repository.workspace = true + +rust-version.workspace = true + +[dependencies] +unicode-width.workspace = true diff --git a/crates/skilld-ui/src/lib.rs b/crates/skilld-ui/src/lib.rs new file mode 100644 index 00000000..8fa63030 --- /dev/null +++ b/crates/skilld-ui/src/lib.rs @@ -0,0 +1,90 @@ +//! Shared terminal presentation primitives for the skilld CLI. +//! +//! Every command output is a [`Screen`] of semantic [`Line`]s. Plain mode +//! flattens to machine records, Human mode renders the same data with the +//! skilld theme. + +pub mod screen; +pub mod spinner; +pub mod text; +pub mod theme; +pub mod time; + +pub use screen::{Line, LineKind, Screen, plain_lines}; +pub use theme::{RESET, Role, paint}; +pub use time::relative_time; + +#[cfg(test)] +mod tests { + use super::{Line, Role, Screen, paint}; + + #[test] + fn paint_matches_legacy_escape_sequences() { + assert_eq!( + paint("x", Role::Brand, true), + "\u{1b}[1m\u{1b}[36mx\u{1b}[0m" + ); + assert_eq!(paint("x", Role::Emphasis, true), "\u{1b}[1mx\u{1b}[0m"); + assert_eq!(paint("x", Role::Warn, true), "\u{1b}[33mx\u{1b}[0m"); + assert_eq!(paint("x", Role::Dim, true), "\u{1b}[2mx\u{1b}[0m"); + assert_eq!(paint("x", Role::Brand, false), "x"); + } + + #[test] + fn plain_render_keeps_the_exact_machine_text() { + let screen = Screen::new(vec![ + Line::success("Installed Skill grill-me."), + Line::hint("Review the unverified Skill before use."), + Line::field("Name", "grill-me"), + ]); + + assert_eq!( + screen.render_plain(), + concat!( + "Installed Skill grill-me.\n", + "Review the unverified Skill before use.\n", + "Name: grill-me\n" + ) + ); + } + + #[test] + fn human_render_adds_glyphs_and_styles() { + let screen = Screen::new(vec![ + Line::success("Installed Skill grill-me."), + Line::warn("Outdated Skill grill-me."), + Line::error("Unverified Skill grill-me."), + Line::field_plain("agent.targets=claude-code", "agent.targets", "claude-code"), + ]); + + assert_eq!( + screen.render_human(true), + concat!( + "\u{1b}[32m✓\u{1b}[0m Installed Skill grill-me.\n", + "\u{1b}[33m⚠\u{1b}[0m Outdated Skill grill-me.\n", + "\u{1b}[31m✗\u{1b}[0m Unverified Skill grill-me.\n", + "\u{1b}[2magent.targets\u{1b}[0m: claude-code\n" + ) + ); + } + + #[test] + fn human_without_color_keeps_glyphs_and_drops_escapes() { + let screen = Screen::new(vec![Line::success("Installed Skill grill-me.")]); + + assert_eq!(screen.render_human(false), "✓ Installed Skill grill-me.\n"); + } + + #[test] + fn fields_align_labels_in_human_mode() { + let screen = Screen::new(vec![ + Line::field("Name", "grill-me"), + Line::field("Source status", "verified"), + ]); + + assert_eq!( + screen.render_human(false), + concat!("Name : grill-me\n", "Source status: verified\n") + ); + } +} diff --git a/crates/skilld-ui/src/screen.rs b/crates/skilld-ui/src/screen.rs new file mode 100644 index 00000000..a1b99a4a --- /dev/null +++ b/crates/skilld-ui/src/screen.rs @@ -0,0 +1,312 @@ +//! Screens: command output as semantic lines with two renderings. +//! +//! A [`Line`] carries the exact Plain-mode text plus how it should look for +//! humans. Plain mode prints the text untouched for machines; Human mode adds +//! glyphs, theme roles, aligned fields, and optional hyperlinks. + +use crate::text::{pad_to, width}; +use crate::theme::{Role, paint}; + +/// The success glyph prefixing completed work. +pub const GLYPH_SUCCESS: &str = "✓"; +/// The attention glyph prefixing degraded or outdated results. +pub const GLYPH_WARN: &str = "⚠"; +/// The failure glyph prefixing errors and required action. +pub const GLYPH_ERROR: &str = "✗"; + +/// One rendered output document. +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct Screen { + /// A Human-only heading, rendered with the brand role. + pub header: Option, + /// The output body. + pub lines: Vec, +} + +impl Screen { + /// A screen with no heading. + pub fn new(lines: Vec) -> Self { + Self { + header: None, + lines, + } + } + + /// A screen with a Human-only heading. + pub fn with_header(header: impl Into, lines: Vec) -> Self { + Self { + header: Some(header.into()), + lines, + } + } + + /// The machine rendering: one exact line per record, heading excluded. + pub fn render_plain(&self) -> String { + let mut output = String::new(); + for line in &self.lines { + output.push_str(line.plain_text()); + output.push('\n'); + } + output + } + + /// The terminal rendering with glyphs, theme roles, and aligned fields. + pub fn render_human(&self, color: bool) -> String { + let mut output = String::new(); + if let Some(header) = &self.header { + output.push_str(&paint(header, Role::Brand, color)); + output.push('\n'); + if !self.lines.is_empty() { + output.push('\n'); + } + } + let label_width = self + .lines + .iter() + .filter_map(Line::field_label) + .map(width) + .max() + .unwrap_or(0); + for line in &self.lines { + output.push_str(&line.render_human(color, label_width)); + output.push('\n'); + } + output + } +} + +/// The plain text of every line, for tests and machine consumers. +pub fn plain_lines(lines: &[Line]) -> Vec<&str> { + lines.iter().map(Line::plain_text).collect() +} + +/// One semantic output line. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Line { + plain: String, + kind: LineKind, +} + +/// How a line renders for humans. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum LineKind { + /// Unstyled text. + Plain, + /// A highlighted entry, such as an installed Skill name. + Item, + /// Completed work, prefixed with the success glyph. + Success, + /// Attention, prefixed with the warn glyph. + Warn, + /// Failure, prefixed with the error glyph. + Error, + /// A dimmed hint or follow-up step. + Hint, + /// A label and value pair, aligned across the screen. + Field { + label: String, + value: String, + /// A terminal hyperlink target for the value, used when color is on. + url: Option, + }, +} + +impl Line { + /// Unstyled text whose Plain and Human renderings match. + pub fn plain(text: impl Into) -> Self { + let text = text.into(); + Self { + plain: text, + kind: LineKind::Plain, + } + } + + /// A highlighted entry. + pub fn item(text: impl Into) -> Self { + let text = text.into(); + Self { + plain: text, + kind: LineKind::Item, + } + } + + /// Completed work; Plain text is the sentence without the glyph. + pub fn success(text: impl Into) -> Self { + let text = text.into(); + Self { + plain: text, + kind: LineKind::Success, + } + } + + /// Attention; Plain text is the sentence without the glyph. + pub fn warn(text: impl Into) -> Self { + let text = text.into(); + Self { + plain: text, + kind: LineKind::Warn, + } + } + + /// Failure; Plain text is the sentence without the glyph. + pub fn error(text: impl Into) -> Self { + let text = text.into(); + Self { + plain: text, + kind: LineKind::Error, + } + } + + /// A dimmed hint. + pub fn hint(text: impl Into) -> Self { + let text = text.into(); + Self { + plain: text, + kind: LineKind::Hint, + } + } + + /// A label and value pair; Plain text is `label: value`. + pub fn field(label: impl Into, value: impl Into) -> Self { + let label = label.into(); + let value = value.into(); + Self::field_plain(format!("{label}: {value}"), label, value) + } + + /// A label and value pair with an explicit Plain rendering, for records + /// like `key=value`. + pub fn field_plain( + plain: impl Into, + label: impl Into, + value: impl Into, + ) -> Self { + Self { + plain: plain.into(), + kind: LineKind::Field { + label: label.into(), + value: value.into(), + url: None, + }, + } + } + + /// A label and value pair whose value links to `url` in Human mode. + pub fn linked_field( + label: impl Into, + value: impl Into, + url: impl Into, + ) -> Self { + let label = label.into(); + let value = value.into(); + Self { + plain: format!("{label}: {value}"), + kind: LineKind::Field { + label, + value, + url: Some(url.into()), + }, + } + } + + /// The exact Plain-mode text. + pub fn plain_text(&self) -> &str { + &self.plain + } + + fn field_label(&self) -> Option<&str> { + match &self.kind { + LineKind::Field { label, .. } => Some(label), + _ => None, + } + } + + fn render_human(&self, color: bool, label_width: usize) -> String { + match &self.kind { + LineKind::Plain => self.plain.clone(), + LineKind::Item => paint(&self.plain, Role::Emphasis, color), + LineKind::Success => glyphed(GLYPH_SUCCESS, Role::Success, &self.plain, color), + LineKind::Warn => glyphed(GLYPH_WARN, Role::Warn, &self.plain, color), + LineKind::Error => glyphed(GLYPH_ERROR, Role::Error, &self.plain, color), + LineKind::Hint => paint(&self.plain, Role::Dim, color), + LineKind::Field { label, value, url } => { + let label = pad_to(label, label_width); + let value = match url { + Some(url) if color => hyperlink(value, url), + _ => value.clone(), + }; + format!("{}: {value}", paint(&label, Role::Dim, color)) + } + } + } +} + +fn glyphed(glyph: &str, role: Role, text: &str, color: bool) -> String { + format!("{} {text}", paint(glyph, role, color)) +} + +/// Wrap `value` in an OSC 8 terminal hyperlink. +fn hyperlink(value: &str, url: &str) -> String { + format!("\u{1b}]8;;{url}\u{1b}\\{value}\u{1b}]8;;\u{1b}\\") +} + +/// True when the value contains an OSC 8 hyperlink. +#[cfg(test)] +pub(crate) fn has_hyperlink(value: &str) -> bool { + value.contains("\u{1b}]8;;") +} + +#[cfg(test)] +mod tests { + use super::{Line, Screen, has_hyperlink}; + + #[test] + fn plain_rendering_matches_the_machine_contract() { + let screen = Screen::new(vec![ + Line::field_plain("agent.targets=codex", "agent.targets", "codex"), + Line::success("Installed Skill grill-me."), + ]); + + assert_eq!( + screen.render_plain(), + "agent.targets=codex\nInstalled Skill grill-me.\n" + ); + } + + #[test] + fn human_header_is_brand_colored_and_excluded_from_plain() { + let screen = Screen::with_header("Installed Skills", vec![Line::item("grill-me")]); + + assert_eq!(screen.render_plain(), "grill-me\n"); + assert_eq!( + screen.render_human(true), + "\u{1b}[1m\u{1b}[36mInstalled Skills\u{1b}[0m\n\n\u{1b}[1mgrill-me\u{1b}[0m\n" + ); + assert_eq!(screen.render_human(false), "Installed Skills\n\ngrill-me\n"); + } + + #[test] + fn linked_fields_hyperlink_only_with_color() { + let screen = Screen::new(vec![Line::linked_field( + "Source", + "skilld-dev/skilld", + "https://github.com/skilld-dev/skilld", + )]); + + let colored = screen.render_human(true); + let mono = screen.render_human(false); + + assert_eq!(screen.render_plain(), "Source: skilld-dev/skilld\n"); + assert!(has_hyperlink(&colored)); + assert!(!has_hyperlink(&mono)); + assert!(colored.contains("https://github.com/skilld-dev/skilld")); + assert_eq!(mono, "Source: skilld-dev/skilld\n"); + } + + #[test] + fn empty_screens_render_nothing() { + let screen = Screen::new(vec![]); + + assert_eq!(screen.render_plain(), ""); + assert_eq!(screen.render_human(true), ""); + } +} diff --git a/crates/skilld-ui/src/spinner.rs b/crates/skilld-ui/src/spinner.rs new file mode 100644 index 00000000..54a87ba6 --- /dev/null +++ b/crates/skilld-ui/src/spinner.rs @@ -0,0 +1,9 @@ +//! Braille spinner frames shared by status lines and the TUI. + +/// One full rotation of the skilld spinner. +pub const FRAMES: [&str; 10] = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"]; + +/// The frame at `step` in the shared rotation. +pub const fn frame(step: usize) -> &'static str { + FRAMES[step % FRAMES.len()] +} diff --git a/crates/skilld-ui/src/text.rs b/crates/skilld-ui/src/text.rs new file mode 100644 index 00000000..05ee54f5 --- /dev/null +++ b/crates/skilld-ui/src/text.rs @@ -0,0 +1,165 @@ +//! Terminal text measurement and shaping, shared by every renderer. + +use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; + +/// Replace terminal control characters with spaces so untrusted text can +/// never move a cursor or forge an escape sequence. +pub fn sanitize(value: &str) -> String { + value + .chars() + .map(|character| { + if character.is_control() { + ' ' + } else { + character + } + }) + .collect() +} + +/// The display width of `value` in terminal cells. +pub fn width(value: &str) -> usize { + UnicodeWidthStr::width(value) +} + +/// Pad `value` with spaces until it fills `columns` cells. +pub fn pad_to(value: &str, columns: usize) -> String { + let used = width(value); + if used >= columns { + return value.to_owned(); + } + format!("{value}{}", " ".repeat(columns - used)) +} + +/// Greedy word wrap that never splits below one column. +pub fn wrap(value: &str, limit: usize) -> Vec { + let limit = limit.max(1); + let mut lines = Vec::new(); + let mut current = String::new(); + + for word in value.split_whitespace() { + let separator = usize::from(!current.is_empty()); + if width(¤t) + separator + width(word) > limit && !current.is_empty() { + lines.push(std::mem::take(&mut current)); + } + if !current.is_empty() { + current.push(' '); + } + if width(word) <= limit { + current.push_str(word); + } else { + let mut chunk = String::new(); + for character in word.chars() { + let character_width = UnicodeWidthChar::width(character).unwrap_or(0); + if !chunk.is_empty() && width(&chunk) + character_width > limit { + lines.push(std::mem::take(&mut chunk)); + } + chunk.push(character); + } + current = chunk; + } + } + if !current.is_empty() || lines.is_empty() { + lines.push(current); + } + lines +} + +/// Truncate `value` to `limit` cells without an ellipsis. +pub fn truncate(value: &str, limit: usize) -> String { + if limit == 0 { + return String::new(); + } + let mut used: usize = 0; + let mut output = String::new(); + for character in value.chars() { + let character_width = UnicodeWidthChar::width(character).unwrap_or(0); + if used.saturating_add(character_width) > limit { + break; + } + output.push(character); + used = used.saturating_add(character_width); + } + output +} + +/// Truncate `value` to `limit` cells, marking the cut with an ellipsis. +pub fn truncate_ellipsis(value: &str, limit: usize) -> String { + if width(value) <= limit { + return value.to_owned(); + } + let mut output = truncate(value, limit.saturating_sub(1)); + if limit > 1 { + output.push('…'); + } + output +} + +/// Group digits with commas: `227068` becomes `227,068`. +pub fn grouped_number(value: u64) -> String { + let digits = value.to_string(); + let mut output = String::new(); + for (index, character) in digits.chars().enumerate() { + if index > 0 && (digits.len() - index) % 3 == 0 { + output.push(','); + } + output.push(character); + } + output +} + +/// The short form of a commit SHA, or the whole value when it is shorter. +pub fn short_sha(value: &str) -> &str { + value.get(..7).unwrap_or(value) +} + +#[cfg(test)] +mod tests { + use super::{ + grouped_number, pad_to, sanitize, short_sha, truncate, truncate_ellipsis, width, wrap, + }; + + #[test] + fn sanitize_replaces_control_characters() { + assert_eq!(sanitize("a\u{1b}[31mb"), "a [31mb"); + assert_eq!(sanitize("a\tb"), "a b"); + } + + #[test] + fn wrap_breaks_on_words_then_characters() { + assert_eq!(wrap("aaa bbb", 5), vec!["aaa", "bbb"]); + assert_eq!(wrap("aaaaaaaa", 3), vec!["aaa", "aaa", "aa"]); + assert_eq!(wrap("", 5), vec![String::new()]); + } + + #[test] + fn truncate_respects_display_cells() { + assert_eq!(truncate("漢字abc", 4), "漢字"); + assert_eq!(truncate("abc", 0), ""); + } + + #[test] + fn truncate_ellipsis_marks_the_cut() { + assert_eq!(truncate_ellipsis("abcdef", 4), "abc…"); + assert_eq!(truncate_ellipsis("ab", 4), "ab"); + } + + #[test] + fn pad_to_fills_display_cells() { + assert_eq!(pad_to("漢", 4), "漢 "); + assert_eq!(pad_to("abc", 2), "abc"); + } + + #[test] + fn numbers_group_by_thousands() { + assert_eq!(grouped_number(227_068), "227,068"); + assert_eq!(grouped_number(999), "999"); + } + + #[test] + fn shas_shorten_to_seven_characters() { + assert_eq!(short_sha("0123456789abcdef"), "0123456"); + assert_eq!(short_sha("abc"), "abc"); + assert_eq!(width("漢"), 2); + } +} diff --git a/crates/skilld-ui/src/theme.rs b/crates/skilld-ui/src/theme.rs new file mode 100644 index 00000000..02a847b5 --- /dev/null +++ b/crates/skilld-ui/src/theme.rs @@ -0,0 +1,48 @@ +//! The skilld color theme. +//! +//! Roles map to fixed escape sequences so every surface (screens, the TUI, +//! status lines) shares one look. When color is off, painting is a no-op. + +/// The escape sequence that clears every style. +pub const RESET: &str = "\u{1b}[0m"; + +/// A semantic slot in the skilld theme. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum Role { + /// Bold cyan. The skilld brand: headings and highlights. + Brand, + /// Bold. Emphasis inside body text. + Emphasis, + /// Green. Completed work. + Success, + /// Yellow. Attention: outdated or degraded results. + Warn, + /// Red. Failures and required action. + Error, + /// Dim. Hints, footers, and labels. + Dim, +} + +impl Role { + /// The escape prefix for this role. + pub const fn prefix(self) -> &'static str { + match self { + Self::Brand => "\u{1b}[1m\u{1b}[36m", + Self::Emphasis => "\u{1b}[1m", + Self::Success => "\u{1b}[32m", + Self::Warn => "\u{1b}[33m", + Self::Error => "\u{1b}[31m", + Self::Dim => "\u{1b}[2m", + } + } +} + +/// Paint `value` with `role` when `color` is enabled, otherwise return it +/// unchanged. +pub fn paint(value: &str, role: Role, color: bool) -> String { + if color { + format!("{}{value}{RESET}", role.prefix()) + } else { + value.to_owned() + } +} diff --git a/crates/skilld-ui/src/time.rs b/crates/skilld-ui/src/time.rs new file mode 100644 index 00000000..032e9028 --- /dev/null +++ b/crates/skilld-ui/src/time.rs @@ -0,0 +1,123 @@ +//! Relative time formatting for commit and artifact timestamps. + +use std::time::SystemTime; + +const MINUTE: i64 = 60; +const HOUR: i64 = 60 * MINUTE; +const DAY: i64 = 24 * HOUR; +const MONTH: i64 = 30 * DAY; + +/// Describe `timestamp` relative to `now`. +/// +/// Timestamps use GitHub ISO 8601 form (`2026-08-21T00:00:00Z`). Values the +/// parser rejects fall back to the date portion, then to the raw value. +pub fn relative_time(timestamp: &str, now: SystemTime) -> String { + let Some(utc) = parse_utc_seconds(timestamp) else { + return timestamp.get(..10).unwrap_or(timestamp).to_owned(); + }; + let Some(elapsed) = now.elapsed().ok().map(|duration| duration.as_secs() as i64) else { + return timestamp.get(..10).unwrap_or(timestamp).to_owned(); + }; + match elapsed - utc { + delta if delta < MINUTE => "just now".to_owned(), + delta if delta < HOUR => format!("{}m ago", delta / MINUTE), + delta if delta < DAY => format!("{}h ago", delta / HOUR), + delta if delta < MONTH => format!("{}d ago", delta / DAY), + _ => timestamp.get(..10).unwrap_or(timestamp).to_owned(), + } +} + +/// Parse `YYYY-MM-DDTHH:MM:SS` UTC timestamps into seconds since the epoch. +fn parse_utc_seconds(value: &str) -> Option { + let bytes = value.as_bytes(); + if bytes.len() < 19 || bytes[4] != b'-' || bytes[7] != b'-' { + return None; + } + let year = digits(value.get(0..4)?)?; + let month = digits(value.get(5..7)?)?; + let day = digits(value.get(8..10)?)?; + if !(1..=12).contains(&month) || !(1..=31).contains(&day) { + return None; + } + let (hour, minute, second) = match bytes[10] { + b'T' | b' ' | b't' => ( + digits(value.get(11..13)?)?, + digits(value.get(14..16)?)?, + digits(value.get(17..19)?)?, + ), + _ => (0, 0, 0), + }; + if hour > 23 || minute > 59 || second > 60 { + return None; + } + Some(days_from_civil(year, month, day) * DAY + hour * HOUR + minute * MINUTE + second) +} + +fn digits(value: &str) -> Option { + value.parse().ok() +} + +/// Days since the Unix epoch for a civil date (Howard Hinnant's algorithm). +fn days_from_civil(year: i64, month: i64, day: i64) -> i64 { + let year = if month <= 2 { year - 1 } else { year }; + let era = if year >= 0 { year } else { year - 399 } / 400; + let year_of_era = year - era * 400; + let month_shift = if month > 2 { month - 3 } else { month + 9 }; + let day_of_year = (153 * month_shift + 2) / 5 + day - 1; + let day_of_era = year_of_era * 365 + year_of_era / 4 - year_of_era / 100 + day_of_year; + era * 146_097 + day_of_era - 719_468 +} + +#[cfg(test)] +mod tests { + use super::{parse_utc_seconds, relative_time}; + use std::time::{Duration, SystemTime}; + + fn now_plus(delta: i64) -> SystemTime { + let base = SystemTime::now() + .checked_sub(Duration::from_secs(2)) + .unwrap(); + if delta >= 0 { + base.checked_sub(Duration::from_secs(delta as u64)).unwrap() + } else { + base.checked_add(Duration::from_secs(delta.unsigned_abs())) + .unwrap() + } + } + + #[test] + fn parses_epoch_correctly() { + assert_eq!(parse_utc_seconds("1970-01-01T00:00:00Z"), Some(0)); + assert_eq!( + parse_utc_seconds("2026-08-21T00:00:00Z"), + Some(1_787_270_400) + ); + assert_eq!(parse_utc_seconds("not-a-date"), None); + assert_eq!(parse_utc_seconds("2026-08-21"), None); + } + + #[test] + fn buckets_relative_times() { + const NOON: i64 = 1_787_313_600; + let timestamp = "2026-08-21T12:00:00Z"; + assert_eq!(relative_time(timestamp, now_plus(NOON + 30)), "just now"); + assert_eq!(relative_time(timestamp, now_plus(NOON + 5 * 60)), "5m ago"); + assert_eq!( + relative_time(timestamp, now_plus(NOON + 3 * 3600)), + "3h ago" + ); + assert_eq!( + relative_time(timestamp, now_plus(NOON + 2 * 86_400)), + "2d ago" + ); + assert_eq!( + relative_time(timestamp, now_plus(NOON + 400 * 86_400)), + "2026-08-21" + ); + } + + #[test] + fn unparsable_values_fall_back_to_the_date() { + assert_eq!(relative_time("garbage", SystemTime::now()), "garbage"); + } +}