From 53ccbda97cca9c426eb0618cdd00e277161f4588 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Fri, 21 Aug 2026 19:01:24 -0600 Subject: [PATCH] feat: expand owner/repo shorthand for --install with a --git-host flag --install someuser/repo expands to https://github.com/someuser/repo; --git-host overrides the default host and forces source interpretation even when the value matches an installed bundle name. Two or more path segments are accepted so nested GitLab-style groups work, and #ref pinning applies to shorthand values. --uninstall resolves owner/repo against recorded sources: a single match uninstalls, multiple matches prompt an interactive selection showing each bundle's source, and non-interactive runs bail instead of guessing. --- src/cli/mod.rs | 19 ++ src/config/install_remote.rs | 315 +++++++++++++++++++++++++++++++--- src/config/request_context.rs | 8 + src/main.rs | 7 +- src/repl/mod.rs | 4 +- 5 files changed, 327 insertions(+), 26 deletions(-) diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 45c1b0c..91d66e2 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -187,6 +187,14 @@ pub struct Cli { help_heading = "Installation & Updates" )] pub install: Option, + /// Git host used to expand / shorthand values passed to --install + #[arg( + long, + value_name = "HOST", + requires = "install", + help_heading = "Installation & Updates" + )] + pub git_host: Option, /// Reinstall bundled assets, overwriting any local changes #[arg( long, @@ -593,6 +601,17 @@ mod tests { assert!(parse(&["--install", "https://github.com/x/y", "--install-force"]).install_force); } + #[test] + fn parse_git_host_requires_install() { + assert!(Cli::try_parse_from(["coyote", "--git-host", "git.x.com"]).is_err()); + assert_eq!( + parse(&["--install", "someuser/omc", "--git-host", "git.x.com"]) + .git_host + .as_deref(), + Some("git.x.com") + ); + } + #[test] fn help_omits_install_from_and_shows_install_builtins() { use clap::CommandFactory; diff --git a/src/config/install_remote.rs b/src/config/install_remote.rs index 9253126..21348b6 100644 --- a/src/config/install_remote.rs +++ b/src/config/install_remote.rs @@ -72,9 +72,10 @@ pub fn install_remote(git_url: &str, filter: Option, force: bool) fn parse_repl_install_flags( command: &str, mut iter: impl Iterator, -) -> Result<(Option, bool)> { +) -> Result<(Option, bool, Option)> { let mut filter: Option = None; let mut force = false; + let mut git_host: Option = None; while let Some(tok) = iter.next() { match tok.as_str() { @@ -91,11 +92,20 @@ fn parse_repl_install_flags( s if s.starts_with("--filter=") => { filter = Some(parse_filter(&s["--filter=".len()..])?); } + "--git-host" => { + let val = iter + .next() + .with_context(|| "--git-host requires a value (e.g. git.somedomain.com)")?; + git_host = Some(val); + } + s if s.starts_with("--git-host=") => { + git_host = Some(s["--git-host=".len()..].to_string()); + } other => bail!("Unexpected argument to '{command}': {other}"), } } - Ok((filter, force)) + Ok((filter, force, git_host)) } #[derive(Debug, Clone, PartialEq)] @@ -103,6 +113,7 @@ enum InstallTarget { Category(AssetCategory), InstalledBundle, RemoteSource, + Shorthand, Unknown, } @@ -117,6 +128,9 @@ fn classify_install_target(value: &str, installed_names: &[String]) -> InstallTa if looks_like_remote_source(value) { return InstallTarget::RemoteSource; } + if is_repo_shorthand(value) { + return InstallTarget::Shorthand; + } InstallTarget::Unknown } @@ -137,7 +151,53 @@ fn looks_like_remote_source(value: &str) -> bool { } } -pub fn install_or_update(value: &str, filter: Option, force: bool) -> Result<()> { +const DEFAULT_GIT_HOST: &str = "github.com"; + +fn is_repo_shorthand(value: &str) -> bool { + let path = strip_ref_suffix(value); + if path.contains("://") + || path.contains(':') + || path.contains('\\') + || path.contains(char::is_whitespace) + || path.starts_with(['/', '~', '.', '-']) + { + return false; + } + let mut segments = path.split('/'); + segments.clone().count() >= 2 && segments.all(|segment| !segment.is_empty()) +} + +fn expand_repo_shorthand(value: &str, git_host: Option<&str>) -> Result { + let raw = git_host.unwrap_or(DEFAULT_GIT_HOST); + let host = raw + .strip_prefix("https://") + .or_else(|| raw.strip_prefix("http://")) + .unwrap_or(raw) + .trim_matches('/'); + if host.is_empty() || host.contains('/') || host.chars().any(char::is_whitespace) { + bail!("invalid --git-host '{raw}': expected a bare host like git.somedomain.com"); + } + Ok(format!("https://{host}/{value}")) +} + +pub fn install_or_update( + value: &str, + git_host: Option<&str>, + filter: Option, + force: bool, +) -> Result<()> { + if let Some(host) = git_host { + if !is_repo_shorthand(value) { + bail!( + "--git-host only applies to / shorthand values; \ + '{value}' is not one" + ); + } + let url = expand_repo_shorthand(value, Some(host))?; + println!("Resolved '{value}' to '{url}'"); + return install_remote(&url, filter, force); + } + let store = BundleStore::load()?; let installed: Vec = store .bundle_names() @@ -168,9 +228,15 @@ pub fn install_or_update(value: &str, filter: Option, force: bool update_bundle(value) } InstallTarget::RemoteSource => install_remote(value, filter, force), + InstallTarget::Shorthand => { + let url = expand_repo_shorthand(value, None)?; + println!("Resolved '{value}' to '{url}'"); + install_remote(&url, filter, force) + } InstallTarget::Unknown => { - let hint = "a remote source must be a git URL, an scp-style host:path, \ - or an explicit local path (./dir, /abs, ~)"; + let hint = "a remote source must be a git URL, an / shorthand \ + (expanded against --git-host, default github.com), an scp-style \ + host:path, or an explicit local path (./dir, /abs, ~)"; if installed.is_empty() { bail!("no bundle named '{value}' is installed; none are installed ({hint})"); } @@ -189,13 +255,14 @@ pub fn install_or_update_from_repl_args(args: &str) -> Result<()> { let mut iter = tokens.into_iter(); let value = iter.next().with_context(|| { format!( - "Usage: .install [--filter <{}>] [--force]", + "Usage: .install \ + [--git-host ] [--filter <{}>] [--force]", InstallFilter::NAMES.join("|") ) })?; - let (filter, force) = parse_repl_install_flags(".install", iter)?; - install_or_update(&value, filter, force) + let (filter, force, git_host) = parse_repl_install_flags(".install", iter)?; + install_or_update(&value, git_host.as_deref(), filter, force) } /// The whole remote is always processed — including categories a filtered @@ -414,16 +481,19 @@ pub fn uninstall_bundle(spec: &str, assume_yes: bool) -> Result<()> { Some(_) => spec.to_string(), None => match store.find_by_source(spec) { Some((name, _)) => name.to_string(), - None => { - let installed = store.bundle_names(); - if installed.is_empty() { - bail!("no bundle named '{spec}' is installed; none are installed"); + None => match select_uninstall_candidate(&store, spec)? { + Some(name) => name, + None => { + let installed = store.bundle_names(); + if installed.is_empty() { + bail!("no bundle named '{spec}' is installed; none are installed"); + } + bail!( + "no bundle named '{spec}' is installed; installed bundles: {}", + installed.join(", ") + ); } - bail!( - "no bundle named '{spec}' is installed; installed bundles: {}", - installed.join(", ") - ); - } + }, }, }; let record = store.get(&name).expect("resolved above").clone(); @@ -502,6 +572,49 @@ pub fn uninstall_bundle(spec: &str, assume_yes: bool) -> Result<()> { Ok(()) } +/// Never auto-picks between multiple matches: ambiguity is resolved by an +/// interactive prompt, and non-interactive runs bail even under --yes. +fn select_uninstall_candidate(store: &BundleStore, spec: &str) -> Result> { + if !is_repo_shorthand(spec) { + return Ok(None); + } + let needle = format!("/{}", canonical_source_url(spec)); + let candidates: Vec<(String, String)> = store + .iter() + .filter(|(_, record)| canonical_source_url(&record.source).ends_with(&needle)) + .map(|(name, record)| (name.to_string(), record.source.clone())) + .collect(); + + match candidates.as_slice() { + [] => Ok(None), + [(name, _)] => Ok(Some(name.clone())), + _ => { + let described: Vec = candidates + .iter() + .map(|(name, source)| format!("{name} — {source}")) + .collect(); + if !*IS_STDOUT_TERMINAL { + bail!( + "'{spec}' matches multiple installed bundles: {}; re-run with the \ + exact bundle name or source URL", + described.join(", ") + ); + } + let picked = Select::new( + &format!("Multiple bundles match '{spec}'; which one should be uninstalled?"), + described.clone(), + ) + .prompt() + .with_context(|| "failed to read uninstall selection")?; + let index = described + .iter() + .position(|option| option == &picked) + .expect("selection came from the presented options"); + Ok(Some(candidates[index].0.clone())) + } + } +} + /// A recorded path that could escape `config_dir` (absolute, or containing /// anything but plain components) is never touched; its record survives. fn uninstall_owned_files( @@ -3588,7 +3701,7 @@ mod tests { fn install_or_update_redirects_categories_to_install_builtins() { let _guard = TestVaultConfigGuard::new("iou-category"); - let err = install_or_update("agents", None, false).unwrap_err(); + let err = install_or_update("agents", None, None, false).unwrap_err(); assert!( err.to_string().contains("--install-builtins agents"), @@ -3601,7 +3714,7 @@ mod tests { fn install_or_update_unknown_name_lists_installed_bundles() { let _guard = TestVaultConfigGuard::new("iou-unknown"); - let err = install_or_update("not-a-bundle", None, false).unwrap_err(); + let err = install_or_update("not-a-bundle", None, None, false).unwrap_err(); assert!(err.to_string().contains("none are installed"), "got: {err}"); let src_root = fresh_temp_dir("iou-unknown-src-"); @@ -3611,7 +3724,7 @@ mod tests { init_bundle_repo(&repo); install_remote(repo.to_str().unwrap(), None, false).unwrap(); - let err = install_or_update("not-a-bundle", None, false).unwrap_err(); + let err = install_or_update("not-a-bundle", None, None, false).unwrap_err(); assert!( err.to_string().contains("no bundle named 'not-a-bundle'"), @@ -3644,7 +3757,7 @@ mod tests { ) .unwrap(); - let err = install_or_update("agents", None, false).unwrap_err(); + let err = install_or_update("agents", None, None, false).unwrap_err(); assert!( err.to_string().contains("--install-builtins agents"), @@ -3667,13 +3780,14 @@ mod tests { init_bundle_repo(&repo); install_remote(repo.to_str().unwrap(), None, false).unwrap(); - let err = install_or_update("flag-bundle", Some(InstallFilter::Macros), false).unwrap_err(); + let err = + install_or_update("flag-bundle", None, Some(InstallFilter::Macros), false).unwrap_err(); assert!( err.to_string().contains("only apply to remote installs"), "got: {err}" ); - let err = install_or_update("flag-bundle", None, true).unwrap_err(); + let err = install_or_update("flag-bundle", None, None, true).unwrap_err(); assert!( err.to_string().contains("only apply to remote installs"), "got: {err}" @@ -3681,6 +3795,161 @@ mod tests { let _ = fs::remove_dir_all(&src_root); } + #[test] + fn shorthand_accepts_owner_repo_and_deeper_paths() { + assert!(is_repo_shorthand("someuser/oh-my-coyote")); + assert!(is_repo_shorthand("group/subgroup/repo")); + assert!(is_repo_shorthand("someuser/repo#v2")); + } + + #[test] + fn shorthand_rejects_urls_paths_and_bare_names() { + assert!(!is_repo_shorthand("https://github.com/x/y")); + assert!(!is_repo_shorthand("git@github.com:x/y.git")); + assert!(!is_repo_shorthand("./local/dir")); + assert!(!is_repo_shorthand("/abs/path")); + assert!(!is_repo_shorthand("~/home/path")); + assert!(!is_repo_shorthand("-flag/like")); + assert!(!is_repo_shorthand("bare-name")); + assert!(!is_repo_shorthand("a//b")); + assert!(!is_repo_shorthand("a/b/")); + assert!(!is_repo_shorthand("a\\b")); + assert!(!is_repo_shorthand("a b/c")); + } + + #[test] + fn shorthand_expands_against_default_and_custom_hosts() { + assert_eq!( + expand_repo_shorthand("someuser/omc", None).unwrap(), + "https://github.com/someuser/omc" + ); + assert_eq!( + expand_repo_shorthand("someuser/omc#v2", Some("git.somedomain.com")).unwrap(), + "https://git.somedomain.com/someuser/omc#v2" + ); + assert_eq!( + expand_repo_shorthand("a/b", Some("https://git.x.com/")).unwrap(), + "https://git.x.com/a/b" + ); + assert!(expand_repo_shorthand("a/b", Some("bad/host")).is_err()); + assert!(expand_repo_shorthand("a/b", Some("")).is_err()); + } + + #[test] + fn classify_prefers_installed_names_over_shorthand() { + assert_eq!( + classify_install_target("someuser/omc", &[]), + InstallTarget::Shorthand + ); + assert_eq!( + classify_install_target("someuser/omc", &["someuser/omc".to_string()]), + InstallTarget::InstalledBundle + ); + } + + #[test] + fn install_or_update_git_host_rejects_non_shorthand_values() { + let err = install_or_update("https://github.com/x/y", Some("git.x.com"), None, false) + .unwrap_err(); + assert!( + err.to_string().contains("--git-host only applies"), + "got: {err}" + ); + + let err = install_or_update("bare-name", Some("git.x.com"), None, false).unwrap_err(); + assert!( + err.to_string().contains("--git-host only applies"), + "got: {err}" + ); + } + + #[test] + fn repl_install_flags_parse_git_host() { + let (filter, force, host) = parse_repl_install_flags( + ".install", + vec!["--git-host".to_string(), "git.x.com".to_string()].into_iter(), + ) + .unwrap(); + assert_eq!(host.as_deref(), Some("git.x.com")); + assert!(filter.is_none() && !force); + + let (_, force, host) = parse_repl_install_flags( + ".install", + vec!["--git-host=git.y.com".to_string(), "--force".to_string()].into_iter(), + ) + .unwrap(); + assert_eq!(host.as_deref(), Some("git.y.com")); + assert!(force); + } + + #[test] + #[serial] + fn uninstall_shorthand_resolves_a_single_source_match() { + let _guard = TestVaultConfigGuard::new("uninst-short-one"); + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/someuser/omc")) + .unwrap(); + drop(store); + + uninstall_bundle("someuser/omc", true).unwrap(); + + let store = BundleStore::load().unwrap(); + assert!(store.get("omc").is_none()); + } + + #[test] + #[serial] + fn uninstall_shorthand_with_multiple_matches_bails_non_interactively() { + let _guard = TestVaultConfigGuard::new("uninst-short-multi"); + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/someuser/omc")) + .unwrap(); + store + .upsert_bundle( + "omc-fork", + test_metadata("https://git.somedomain.com/someuser/omc"), + ) + .unwrap(); + drop(store); + + let err = uninstall_bundle("someuser/omc", true).unwrap_err(); + + assert!( + err.to_string() + .contains("matches multiple installed bundles"), + "got: {err}" + ); + assert!(err.to_string().contains("omc-fork"), "got: {err}"); + let store = BundleStore::load().unwrap(); + assert!(store.get("omc").is_some()); + assert!(store.get("omc-fork").is_some()); + } + + #[test] + #[serial] + fn uninstall_exact_name_wins_over_shorthand_source_matches() { + let _guard = TestVaultConfigGuard::new("uninst-short-name"); + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle( + "someuser/omc", + test_metadata("https://git.somedomain.com/other/repo"), + ) + .unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/someuser/omc")) + .unwrap(); + drop(store); + + uninstall_bundle("someuser/omc", true).unwrap(); + + let store = BundleStore::load().unwrap(); + assert!(store.get("someuser/omc").is_none()); + assert!(store.get("omc").is_some()); + } + fn owned_file_record(path: &str, contents: &str) -> FileRecord { FileRecord { path: path.to_string(), diff --git a/src/config/request_context.rs b/src/config/request_context.rs index c2e2489..e255ec9 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -3500,11 +3500,16 @@ impl RequestContext { values = super::map_completion_values( InstallFilter::NAMES.iter().map(|s| s.to_string()).collect(), ); + } else if prev == "--git-host" { + values = super::map_completion_values(vec!["github.com".to_string()]); } else { let has_filter = args.iter().enumerate().any(|(i, a)| { a.starts_with("--filter=") || (*a == "--filter" && i < args.len() - 1) }); let has_force = args.contains(&"--force"); + let has_git_host = args.iter().enumerate().any(|(i, a)| { + a.starts_with("--git-host=") || (*a == "--git-host" && i < args.len() - 1) + }); let mut available: Vec<&str> = vec![]; if !has_filter { @@ -3513,6 +3518,9 @@ impl RequestContext { if !has_force { available.push("--force"); } + if !has_git_host { + available.push("--git-host"); + } values = super::map_completion_values(available); } diff --git a/src/main.rs b/src/main.rs index 402e896..eb2b06d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -135,7 +135,12 @@ async fn main() -> Result<()> { } if let Some(value) = cli.install.as_deref() { - return config::install_or_update(value, cli.filter, cli.install_force); + return config::install_or_update( + value, + cli.git_host.as_deref(), + cli.filter, + cli.install_force, + ); } if let Some(spec) = cli.update_bundle.as_deref() { diff --git a/src/repl/mod.rs b/src/repl/mod.rs index ce25aa9..4e48529 100644 --- a/src/repl/mod.rs +++ b/src/repl/mod.rs @@ -876,8 +876,8 @@ pub async fn run_repl_command( config::install_or_update_from_repl_args(value)?; } ReplInstallDispatch::Usage => println!( - "Usage: .install <{}> | .install \ - [--filter ] [--force]", + "Usage: .install <{}> | .install \ + [--git-host ] [--filter ] [--force]", AssetCategory::NAMES.join("|") ), },