From f6a65c917091957a0337e0cb528a5d8649839495 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 1 May 2026 18:40:42 +0200 Subject: [PATCH 1/2] feat: improve apply and clean output --- src/main.rs | 356 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 330 insertions(+), 26 deletions(-) diff --git a/src/main.rs b/src/main.rs index 58b28904..f170e058 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,12 +8,201 @@ use colored::Colorize; use std::env; use std::path::PathBuf; -use agentsync::{Linker, SyncOptions, config::Config, gitignore, init}; +use agentsync::{Linker, SyncOptions, SyncResult, config::Config, gitignore, init}; mod commands; mod output; use commands::doctor::run_doctor; use commands::skill::{SkillCommand, run_skill}; use commands::status::{StatusArgs, run_status}; +use output::{HumanFormatter, LabelKind, output_mode}; + +fn human_use_color() -> bool { + match output_mode(false) { + output::OutputMode::Human { use_color } => use_color, + output::OutputMode::Json => false, + } +} + +fn render_phase(title: &str, detail: &str, use_color: bool) -> Vec { + let formatter = HumanFormatter::new(use_color); + vec![ + formatter.format_heading(&format!("➤ {title}")), + format!(" {detail}"), + ] +} + +fn render_dry_run_notice(use_color: bool) -> Vec { + let formatter = HumanFormatter::new(use_color); + vec![ + formatter.format_label("!", "Dry run", LabelKind::Warning), + " No filesystem changes will be made.".to_string(), + ] +} + +#[cfg(test)] +fn render_clean_phase(dry_run: bool) -> Vec { + render_clean_phase_with_color(dry_run, false) +} + +fn render_clean_phase_with_color(dry_run: bool, use_color: bool) -> Vec { + render_phase( + "Clean", + if dry_run { + "Previewing managed symlink removals" + } else { + "Removing managed symlinks" + }, + use_color, + ) +} + +#[cfg(test)] +fn render_sync_phase(dry_run: bool, clean_first: bool) -> Vec { + render_sync_phase_with_color(dry_run, clean_first, false) +} + +fn render_sync_phase_with_color(dry_run: bool, clean_first: bool, use_color: bool) -> Vec { + let detail = match (dry_run, clean_first) { + (true, true) => "Previewing clean and sync changes", + (true, false) => "Previewing agent configuration changes", + (false, true) => "Cleaning existing symlinks before syncing", + (false, false) => "Syncing agent configurations", + }; + render_phase("Sync", detail, use_color) +} + +#[cfg(test)] +fn render_gitignore_phase(enabled: bool, dry_run: bool) -> Vec { + render_gitignore_phase_with_color(enabled, dry_run, false) +} + +fn render_gitignore_phase_with_color(enabled: bool, dry_run: bool, use_color: bool) -> Vec { + let detail = match (enabled, dry_run) { + (true, true) => "Previewing .gitignore update", + (true, false) => "Updating .gitignore", + (false, true) => "Previewing .gitignore cleanup", + (false, false) => "Cleaning .gitignore", + }; + render_phase("Gitignore", detail, use_color) +} + +fn render_mcp_phase(dry_run: bool, use_color: bool) -> Vec { + render_phase( + "MCP", + if dry_run { + "Previewing MCP configuration changes" + } else { + "Syncing MCP configurations" + }, + use_color, + ) +} + +fn render_count(label: &str, value: usize, kind: LabelKind, use_color: bool) -> String { + let formatter = HumanFormatter::new(use_color); + format!( + " {}", + formatter.format_summary_line(label, &value.to_string(), kind) + ) +} + +#[cfg(test)] +fn render_apply_summary(dry_run: bool, result: &SyncResult) -> Vec { + render_apply_summary_with_color(dry_run, result, false) +} + +fn render_apply_summary_with_color( + dry_run: bool, + result: &SyncResult, + use_color: bool, +) -> Vec { + let formatter = HumanFormatter::new(use_color); + let mut lines = vec![formatter.format_label( + "✔", + if dry_run { + "Sync dry run complete" + } else { + "Sync complete" + }, + LabelKind::Success, + )]; + lines.push(render_count( + "Created", + result.created, + LabelKind::Success, + use_color, + )); + lines.push(render_count( + "Updated", + result.updated, + LabelKind::Warning, + use_color, + )); + lines.push(render_count( + "Skipped", + result.skipped, + LabelKind::Muted, + use_color, + )); + lines.push(render_count( + "Errors", + result.errors, + if result.errors > 0 { + LabelKind::Failure + } else { + LabelKind::Muted + }, + use_color, + )); + lines +} + +#[cfg(test)] +fn render_clean_summary(dry_run: bool, removed: usize) -> Vec { + render_clean_summary_with_color(dry_run, removed, false) +} + +fn render_clean_summary_with_color(dry_run: bool, removed: usize, use_color: bool) -> Vec { + let formatter = HumanFormatter::new(use_color); + vec![ + formatter.format_label( + "✔", + if dry_run { + "Clean dry run complete" + } else { + "Clean complete" + }, + LabelKind::Success, + ), + render_count( + if dry_run { "Would remove" } else { "Removed" }, + removed, + LabelKind::Success, + use_color, + ), + ] +} + +#[cfg(test)] +fn render_mcp_summary(result: &agentsync::mcp::McpSyncResult) -> Vec { + render_mcp_summary_with_color(result, false) +} + +fn render_mcp_summary_with_color( + result: &agentsync::mcp::McpSyncResult, + use_color: bool, +) -> Vec { + vec![ + render_count("Created", result.created, LabelKind::Success, use_color), + render_count("Updated", result.updated, LabelKind::Warning, use_color), + ] +} + +fn print_lines(lines: &[String]) { + for line in lines { + println!("{line}"); + } +} fn init_next_steps_lines(wizard: bool) -> Option> { if wizard { @@ -192,16 +381,22 @@ fn main() -> Result<()> { } let config = Config::load(&config_path)?; let linker = Linker::new(config, config_path); + let use_color = human_use_color(); + if dry_run { + print_lines(&render_dry_run_notice(use_color)); + println!(); + } if clean { - println!("{}", "➤ Cleaning existing symlinks".cyan().bold()); + print_lines(&render_clean_phase_with_color(dry_run, use_color)); let clean_opts = SyncOptions { dry_run, verbose, ..Default::default() }; linker.clean(&clean_opts)?; + println!(); } - println!("{}", "➤ Syncing agent configurations".cyan().bold()); + print_lines(&render_sync_phase_with_color(dry_run, clean, use_color)); let options = SyncOptions { clean: false, dry_run, @@ -211,7 +406,8 @@ fn main() -> Result<()> { let mut result = linker.sync(&options)?; if !no_gitignore { if linker.config().gitignore.enabled { - println!("\n{}", "➤ Updating .gitignore".cyan().bold()); + println!(); + print_lines(&render_gitignore_phase_with_color(true, dry_run, use_color)); let entries = linker.config().all_gitignore_entries(); gitignore::update_gitignore( linker.project_root(), @@ -220,7 +416,10 @@ fn main() -> Result<()> { dry_run, )?; } else { - println!("\n{}", "➤ Cleaning .gitignore".cyan().bold()); + println!(); + print_lines(&render_gitignore_phase_with_color( + false, dry_run, use_color, + )); gitignore::cleanup_gitignore( linker.project_root(), &linker.config().gitignore.marker, @@ -229,15 +428,12 @@ fn main() -> Result<()> { } } if linker.config().mcp.enabled && !linker.config().mcp_servers.is_empty() { - println!("\n{}", "➤ Syncing MCP configurations".cyan().bold()); + println!(); + print_lines(&render_mcp_phase(dry_run, use_color)); match linker.sync_mcp(dry_run, options.agents.as_ref()) { Ok(mcp_result) => { if mcp_result.created > 0 || mcp_result.updated > 0 { - println!( - " MCP configs: Created {}, Updated {}", - mcp_result.created.to_string().green(), - mcp_result.updated.to_string().yellow(), - ); + print_lines(&render_mcp_summary_with_color(&mcp_result, use_color)); } } Err(e) => { @@ -246,18 +442,10 @@ fn main() -> Result<()> { } } } - println!("\n{}", "✨ Sync complete!".green().bold()); - println!( - " Created: {}, Updated: {}, Skipped: {}, Errors: {}", - result.created.to_string().green(), - result.updated.to_string().yellow(), - result.skipped.to_string().dimmed(), - if result.errors > 0 { - result.errors.to_string().red() - } else { - result.errors.to_string().dimmed() - } - ); + println!(); + print_lines(&render_apply_summary_with_color( + dry_run, &result, use_color, + )); } Commands::Clean { path, @@ -273,14 +461,24 @@ fn main() -> Result<()> { }; let config = Config::load(&config_path)?; let linker = Linker::new(config, config_path); + let use_color = human_use_color(); + if dry_run { + print_lines(&render_dry_run_notice(use_color)); + println!(); + } + print_lines(&render_clean_phase_with_color(dry_run, use_color)); let options = SyncOptions { dry_run, verbose, ..Default::default() }; let result = linker.clean(&options)?; - println!("\n{}", "✨ Clean complete!".green().bold()); - println!(" Removed: {} symlinks", result.removed.to_string().green()); + println!(); + print_lines(&render_clean_summary_with_color( + dry_run, + result.removed, + use_color, + )); } Commands::DevInstall { skill_id, json } => { let project_root = env::current_dir().unwrap(); @@ -304,7 +502,113 @@ fn print_header() { #[cfg(test)] mod tests { - use super::init_next_steps_lines; + use super::{ + init_next_steps_lines, render_apply_summary, render_clean_phase, render_clean_summary, + render_dry_run_notice, render_gitignore_phase, render_mcp_summary, render_sync_phase, + }; + use agentsync::{SyncResult, mcp::McpSyncResult}; + + #[test] + fn test_render_dry_run_notice_is_explicit() { + assert_eq!( + render_dry_run_notice(false), + vec![ + "! Dry run".to_string(), + " No filesystem changes will be made.".to_string() + ] + ); + } + + #[test] + fn test_render_sync_phase_names_dry_run_preview() { + assert_eq!( + render_sync_phase(true, false), + vec![ + "➤ Sync".to_string(), + " Previewing agent configuration changes".to_string() + ] + ); + } + + #[test] + fn test_render_gitignore_phase_distinguishes_update_and_clean() { + assert_eq!( + render_gitignore_phase(true, false), + vec![ + "➤ Gitignore".to_string(), + " Updating .gitignore".to_string() + ] + ); + assert_eq!( + render_gitignore_phase(false, true), + vec![ + "➤ Gitignore".to_string(), + " Previewing .gitignore cleanup".to_string() + ] + ); + } + + #[test] + fn test_render_apply_summary_uses_consistent_counts() { + let summary = render_apply_summary( + false, + &SyncResult { + created: 2, + updated: 1, + skipped: 3, + removed: 0, + errors: 1, + }, + ); + + assert_eq!( + summary, + vec![ + "✔ Sync complete".to_string(), + " Created: 2".to_string(), + " Updated: 1".to_string(), + " Skipped: 3".to_string(), + " Errors: 1".to_string(), + ] + ); + } + + #[test] + fn test_render_clean_phase_and_summary_make_dry_run_clear() { + assert_eq!( + render_clean_phase(true), + vec![ + "➤ Clean".to_string(), + " Previewing managed symlink removals".to_string() + ] + ); + assert_eq!( + render_clean_summary(false, 3), + vec!["✔ Clean complete".to_string(), " Removed: 3".to_string()] + ); + assert_eq!( + render_clean_summary(true, 3), + vec![ + "✔ Clean dry run complete".to_string(), + " Would remove: 3".to_string() + ] + ); + } + + #[test] + fn test_render_mcp_summary_reports_created_and_updated() { + let summary = render_mcp_summary(&McpSyncResult { + created: 1, + updated: 2, + skipped: 0, + errors: 0, + }); + + assert_eq!( + summary, + vec![" Created: 1".to_string(), " Updated: 2".to_string()] + ); + } #[test] fn test_init_next_steps_lines_suppresses_generic_footer_for_wizard_runs() { From cbe3bded34d4e8e442ef0deed5af7a484553b379 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 1 May 2026 18:52:03 +0200 Subject: [PATCH 2/2] test: align module map clean output expectation --- tests/test_module_map_cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_module_map_cli.rs b/tests/test_module_map_cli.rs index 027d5c11..e598f577 100644 --- a/tests/test_module_map_cli.rs +++ b/tests/test_module_map_cli.rs @@ -147,7 +147,7 @@ fn test_module_map_cli_placeholder_happy_path() { ui_dest.display() ); assert!( - String::from_utf8_lossy(&clean.stdout).contains("Removed: 2 symlinks"), + String::from_utf8_lossy(&clean.stdout).contains("Removed: 2"), "{}", String::from_utf8_lossy(&clean.stdout) );