From 9122d42de851f9b6260000c93fa193699f4e1f47 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 18 Mar 2026 09:45:25 -0400 Subject: [PATCH] Deduplicate CLI command helpers into shared module Consolidate six duplicated helper functions (tilde_path, color_if, split_run_path, validate_daytona_provider, format_duration_ms, format_size) into commands/shared.rs. Also hoist Utc::now() out of a per-run loop in list_command and avoid an unnecessary Vec allocation in truncate_goal. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-cli/src/commands/asset.rs | 9 +-- lib/crates/fabro-cli/src/commands/cp.rs | 9 +-- lib/crates/fabro-cli/src/commands/mod.rs | 2 +- lib/crates/fabro-cli/src/commands/preview.rs | 16 ++--- lib/crates/fabro-cli/src/commands/rewind.rs | 10 +--- .../fabro-cli/src/commands/run_progress.rs | 6 +- lib/crates/fabro-cli/src/commands/runs.rs | 57 +++--------------- lib/crates/fabro-cli/src/commands/shared.rs | 59 +++++++++++++++++++ lib/crates/fabro-cli/src/commands/ssh.rs | 14 +---- 9 files changed, 81 insertions(+), 101 deletions(-) diff --git a/lib/crates/fabro-cli/src/commands/asset.rs b/lib/crates/fabro-cli/src/commands/asset.rs index 06ce83c6a..ec6629fe1 100644 --- a/lib/crates/fabro-cli/src/commands/asset.rs +++ b/lib/crates/fabro-cli/src/commands/asset.rs @@ -3,6 +3,8 @@ use std::path::{Path, PathBuf}; use anyhow::{bail, Context, Result}; use clap::Args; +use super::shared::split_run_path; + #[derive(Args)] pub struct AssetListArgs { /// Run ID (or prefix) @@ -207,13 +209,6 @@ fn parse_source(source: &str) -> (&str, Option<&str>) { } } -fn split_run_path(s: &str) -> Option<(&str, &str)> { - if s.starts_with('/') || s.starts_with("./") || s.starts_with("../") { - return None; - } - s.split_once(':') -} - fn format_size(bytes: u64) -> String { const KB: u64 = 1024; const MB: u64 = 1024 * KB; diff --git a/lib/crates/fabro-cli/src/commands/cp.rs b/lib/crates/fabro-cli/src/commands/cp.rs index 2445b5f62..cd2f5471c 100644 --- a/lib/crates/fabro-cli/src/commands/cp.rs +++ b/lib/crates/fabro-cli/src/commands/cp.rs @@ -4,6 +4,8 @@ use anyhow::{bail, Context, Result}; use clap::Args; use tracing::{debug, info}; +use super::shared::split_run_path; + #[derive(Args)] pub struct CpArgs { /// Source: : or local path @@ -96,13 +98,6 @@ fn parse_direction(src: &str, dst: &str) -> Result { } } -fn split_run_path(s: &str) -> Option<(&str, &str)> { - if s.starts_with('/') || s.starts_with("./") || s.starts_with("../") { - return None; - } - s.split_once(':') -} - async fn load_sandbox( base: &Path, run_prefix: &str, diff --git a/lib/crates/fabro-cli/src/commands/mod.rs b/lib/crates/fabro-cli/src/commands/mod.rs index 1b412cf23..45de11efc 100644 --- a/lib/crates/fabro-cli/src/commands/mod.rs +++ b/lib/crates/fabro-cli/src/commands/mod.rs @@ -12,7 +12,7 @@ pub mod rewind; pub mod run; mod run_progress; pub mod runs; -mod shared; +pub(crate) mod shared; pub mod ssh; pub mod validate; pub mod workflow; diff --git a/lib/crates/fabro-cli/src/commands/preview.rs b/lib/crates/fabro-cli/src/commands/preview.rs index cd3abe884..323cb5bd4 100644 --- a/lib/crates/fabro-cli/src/commands/preview.rs +++ b/lib/crates/fabro-cli/src/commands/preview.rs @@ -1,7 +1,9 @@ -use anyhow::{bail, Context, Result}; +use anyhow::{Context, Result}; use clap::Args; use tracing::info; +use super::shared::validate_daytona_provider; + #[derive(Args)] pub struct PreviewArgs { /// Run ID or prefix @@ -33,7 +35,7 @@ pub async fn run(args: PreviewArgs) -> Result<()> { "Failed to load sandbox.json — was this run started with a recent version of arc?", )?; - validate_provider(&record)?; + validate_daytona_provider(&record, "Preview URLs")?; let name = record .identifier @@ -70,16 +72,6 @@ pub async fn run(args: PreviewArgs) -> Result<()> { Ok(()) } -fn validate_provider(record: &fabro_workflows::sandbox_record::SandboxRecord) -> Result<()> { - if record.provider != "daytona" { - bail!( - "Preview URLs are only supported for Daytona sandboxes (this run uses '{}')", - record.provider - ); - } - Ok(()) -} - fn format_standard_output(url: &str, token: &str) -> String { let mut out = format!("URL: {url}\nToken: {token}\n"); out.push_str(&format!( diff --git a/lib/crates/fabro-cli/src/commands/rewind.rs b/lib/crates/fabro-cli/src/commands/rewind.rs index 7c98c830d..1b84eb8f7 100644 --- a/lib/crates/fabro-cli/src/commands/rewind.rs +++ b/lib/crates/fabro-cli/src/commands/rewind.rs @@ -7,6 +7,8 @@ use fabro_git_storage::gitobj::Store; use fabro_util::terminal::Styles; use git2::Repository; +use super::shared::color_if; + #[derive(Debug, Args)] pub struct RewindArgs { /// Run ID (or unambiguous prefix) @@ -109,11 +111,3 @@ pub(crate) fn print_timeline( .separator(Separator::builder().build()); let _ = print_stderr(table); } - -fn color_if(use_color: bool, color: Color) -> Option { - if use_color { - Some(color) - } else { - None - } -} diff --git a/lib/crates/fabro-cli/src/commands/run_progress.rs b/lib/crates/fabro-cli/src/commands/run_progress.rs index 038609594..5154c3f4a 100644 --- a/lib/crates/fabro-cli/src/commands/run_progress.rs +++ b/lib/crates/fabro-cli/src/commands/run_progress.rs @@ -12,7 +12,7 @@ use fabro_interview::{Answer, ConsoleInterviewer, Interviewer, Question}; use fabro_workflows::event::{EventEmitter, WorkflowRunEvent}; use fabro_workflows::outcome::StageStatus; -use crate::commands::shared::{format_tokens_human, tilde_path}; +use crate::commands::shared::{format_duration_ms, format_tokens_human, tilde_path}; use fabro_workflows::cost::{compute_stage_cost, format_cost}; // ── Cached styles ─────────────────────────────────────────────────────── @@ -74,10 +74,6 @@ pub(crate) fn format_duration_short(d: Duration) -> String { } } -pub(crate) fn format_duration_ms(ms: u64) -> String { - format_duration_short(Duration::from_millis(ms)) -} - /// Wrap `text` in an OSC 8 terminal hyperlink pointing to `url`. fn terminal_hyperlink(url: &str, text: &str) -> String { format!("\x1b]8;;{url}\x1b\\{text}\x1b]8;;\x1b\\") diff --git a/lib/crates/fabro-cli/src/commands/runs.rs b/lib/crates/fabro-cli/src/commands/runs.rs index c0064c0bb..129b0c5c5 100644 --- a/lib/crates/fabro-cli/src/commands/runs.rs +++ b/lib/crates/fabro-cli/src/commands/runs.rs @@ -1,5 +1,4 @@ use std::path::Path; -use std::time::Duration; use anyhow::{bail, Context, Result}; use chrono::{DateTime, Utc}; @@ -9,6 +8,8 @@ use cli_table::{print_stdout, Cell, CellStruct, Color, Style, Table}; use fabro_util::terminal::Styles; use tracing::{debug, info, warn}; +use super::shared::{color_if, format_duration_ms, format_size, tilde_path}; + #[derive(Args)] pub struct RunFilterArgs { /// Only include runs started before this date (YYYY-MM-DD prefix match) @@ -120,6 +121,7 @@ pub fn list_command(args: &RunsListArgs, styles: &Styles) -> Result<()> { display_runs.reverse(); let use_color = styles.use_color; + let now = Utc::now(); let title = vec![ "RUN ID".cell().bold(true), "WORKFLOW".cell().bold(true), @@ -136,7 +138,7 @@ pub fn list_command(args: &RunsListArgs, styles: &Styles) -> Result<()> { Some(ms) => format_duration_ms(ms), None => match run.start_time_dt { Some(start) => { - let elapsed = Utc::now().signed_duration_since(start); + let elapsed = now.signed_duration_since(start); format_duration_ms(elapsed.num_milliseconds().max(0) as u64) } None => "-".to_string(), @@ -226,43 +228,14 @@ fn short_run_id(id: &str) -> &str { fn truncate_goal(goal: &str, max_len: usize) -> String { let line = goal.lines().next().unwrap_or(""); - let chars: Vec = line.chars().collect(); - if chars.len() <= max_len { + let char_count = line.chars().count(); + if char_count <= max_len { return line.to_string(); } - let truncated: String = chars[..max_len - 3].iter().collect(); + let truncated: String = line.chars().take(max_len - 3).collect(); format!("{truncated}...") } -fn color_if(use_color: bool, color: Color) -> Option { - if use_color { - Some(color) - } else { - None - } -} - -fn tilde_path(path: &Path) -> String { - if let Some(home) = dirs::home_dir() { - if let Ok(suffix) = path.strip_prefix(&home) { - return format!("~/{}", suffix.display()); - } - } - path.display().to_string() -} - -fn format_duration_ms(ms: u64) -> String { - let duration = Duration::from_millis(ms); - let secs = duration.as_secs(); - if secs >= 60 { - format!("{}m{:02}s", secs / 60, secs % 60) - } else if duration.as_millis() >= 1000 { - format!("{secs}s") - } else { - format!("{}ms", duration.as_millis()) - } -} - fn dir_size(path: &Path) -> u64 { walkdir::WalkDir::new(path) .into_iter() @@ -273,22 +246,6 @@ fn dir_size(path: &Path) -> u64 { .sum() } -fn format_size(bytes: u64) -> String { - const KB: u64 = 1024; - const MB: u64 = 1024 * KB; - const GB: u64 = 1024 * MB; - - if bytes >= GB { - format!("{:.1} GB", bytes as f64 / GB as f64) - } else if bytes >= MB { - format!("{:.1} MB", bytes as f64 / MB as f64) - } else if bytes >= KB { - format!("{:.1} KB", bytes as f64 / KB as f64) - } else { - format!("{bytes} B") - } -} - fn df_from(args: &DfArgs, data_dir: &Path, runs_base: &Path, logs_base: &Path) -> Result<()> { let runs = fabro_workflows::run_lookup::scan_runs(runs_base)?; let mut active_count = 0u64; diff --git a/lib/crates/fabro-cli/src/commands/shared.rs b/lib/crates/fabro-cli/src/commands/shared.rs index d7abd3f32..760477017 100644 --- a/lib/crates/fabro-cli/src/commands/shared.rs +++ b/lib/crates/fabro-cli/src/commands/shared.rs @@ -1,5 +1,8 @@ use std::path::Path; +use std::time::Duration; +use anyhow::{bail, Result}; +use cli_table::Color; use fabro_util::terminal::Styles; use fabro_validate::{Diagnostic, Severity}; @@ -66,6 +69,62 @@ pub fn tilde_path(path: &Path) -> String { path.display().to_string() } +pub fn color_if(use_color: bool, color: Color) -> Option { + if use_color { + Some(color) + } else { + None + } +} + +pub fn split_run_path(s: &str) -> Option<(&str, &str)> { + if s.starts_with('/') || s.starts_with("./") || s.starts_with("../") { + return None; + } + s.split_once(':') +} + +pub fn validate_daytona_provider( + record: &fabro_workflows::sandbox_record::SandboxRecord, + feature: &str, +) -> Result<()> { + if record.provider != "daytona" { + bail!( + "{feature} is only supported for Daytona sandboxes (this run uses '{}')", + record.provider + ); + } + Ok(()) +} + +pub fn format_duration_ms(ms: u64) -> String { + let duration = Duration::from_millis(ms); + let secs = duration.as_secs(); + if secs >= 60 { + format!("{}m{:02}s", secs / 60, secs % 60) + } else if duration.as_millis() >= 1000 { + format!("{secs}s") + } else { + format!("{}ms", duration.as_millis()) + } +} + +pub fn format_size(bytes: u64) -> String { + const KB: u64 = 1024; + const MB: u64 = 1024 * KB; + const GB: u64 = 1024 * MB; + + if bytes >= GB { + format!("{:.1} GB", bytes as f64 / GB as f64) + } else if bytes >= MB { + format!("{:.1} MB", bytes as f64 / MB as f64) + } else if bytes >= KB { + format!("{:.1} KB", bytes as f64 / KB as f64) + } else { + format!("{bytes} B") + } +} + #[cfg(test)] mod tests { use super::format_tokens_human; diff --git a/lib/crates/fabro-cli/src/commands/ssh.rs b/lib/crates/fabro-cli/src/commands/ssh.rs index d35a10d1b..0f7a1820a 100644 --- a/lib/crates/fabro-cli/src/commands/ssh.rs +++ b/lib/crates/fabro-cli/src/commands/ssh.rs @@ -2,6 +2,8 @@ use anyhow::{bail, Context, Result}; use clap::Args; use tracing::info; +use super::shared::validate_daytona_provider; + #[derive(Args)] pub struct SshArgs { /// Run ID or prefix @@ -22,7 +24,7 @@ pub async fn run(args: SshArgs) -> Result<()> { "Failed to load sandbox.json — was this run started with a recent version of arc?", )?; - validate_provider(&record)?; + validate_daytona_provider(&record, "SSH access")?; let name = record .identifier @@ -49,16 +51,6 @@ pub async fn run(args: SshArgs) -> Result<()> { Ok(()) } -fn validate_provider(record: &fabro_workflows::sandbox_record::SandboxRecord) -> Result<()> { - if record.provider != "daytona" { - bail!( - "SSH access is only supported for Daytona sandboxes (this run uses '{}')", - record.provider - ); - } - Ok(()) -} - fn format_output(ssh_command: &str) -> String { format!("{ssh_command}\n") }