diff --git a/src/config/bundles.rs b/src/config/bundles.rs index 3499227..a378f7e 100644 --- a/src/config/bundles.rs +++ b/src/config/bundles.rs @@ -626,7 +626,10 @@ pub fn list_installed_bundles() -> Result<()> { let store = BundleStore::load()?; let rows = bundle_list_rows(&store, &paths::config_dir()); if rows.is_empty() { - println!("No bundles installed. Install one with `coyote --install `."); + println!( + "No bundles installed. Install one with `coyote --install ` \ + or, from the REPL, `.install ` (see `.install --help`)." + ); return Ok(()); } diff --git a/src/config/install_remote.rs b/src/config/install_remote.rs index c590e9e..6a43966 100644 --- a/src/config/install_remote.rs +++ b/src/config/install_remote.rs @@ -71,10 +71,21 @@ pub fn install_remote(git_url: &str, filter: Option, force: bool) Ok(()) } +#[derive(Debug)] +struct ReplInstallArgs { + value: Option, + filter: Option, + force: bool, + git_host: Option, +} + +/// Flags may appear before or after the positional value, matching how the +/// completer offers them at any argument position. fn parse_repl_install_flags( command: &str, mut iter: impl Iterator, -) -> Result<(Option, bool, Option)> { +) -> Result { + let mut value: Option = None; let mut filter: Option = None; let mut force = false; let mut git_host: Option = None; @@ -103,11 +114,24 @@ fn parse_repl_install_flags( s if s.starts_with("--git-host=") => { git_host = Some(s["--git-host=".len()..].to_string()); } - other => bail!("Unexpected argument to '{command}': {other}"), + other if other.starts_with('-') => { + bail!("Unexpected argument to '{command}': {other}") + } + other => { + if value.is_some() { + bail!("Unexpected argument to '{command}': {other}"); + } + value = Some(other.to_string()); + } } } - Ok((filter, force, git_host)) + Ok(ReplInstallArgs { + value, + filter, + force, + git_host, + }) } #[derive(Debug, Clone, PartialEq)] @@ -159,7 +183,7 @@ fn looks_like_remote_source(value: &str) -> bool { } } -pub(crate) const DEFAULT_GIT_HOST: &str = "github.com"; +pub const DEFAULT_GIT_HOST: &str = "github.com"; fn is_repo_shorthand(value: &str) -> bool { let path = strip_ref_suffix(value); @@ -266,17 +290,20 @@ pub fn install_or_update_from_repl_args(args: &str) -> Result<()> { let tokens = shell_words::split(args) .with_context(|| format!("failed to parse '.install' args: {args}"))?; - let mut iter = tokens.into_iter(); - let value = iter.next().with_context(|| { + let parsed = parse_repl_install_flags(".install", tokens.into_iter())?; + let value = parsed.value.with_context(|| { format!( "Usage: .install \ [--git-host ] [--filter <{}>] [--force]", InstallFilter::NAMES.join("|") ) })?; - - let (filter, force, git_host) = parse_repl_install_flags(".install", iter)?; - install_or_update(&value, git_host.as_deref(), filter, force) + install_or_update( + &value, + parsed.git_host.as_deref(), + parsed.filter, + parsed.force, + ) } /// The whole remote is always processed, including categories a filtered @@ -4194,21 +4221,50 @@ mod tests { #[test] fn repl_install_flags_parse_git_host() { - let (filter, force, host) = parse_repl_install_flags( + let parsed = 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); + assert_eq!(parsed.git_host.as_deref(), Some("git.x.com")); + assert!(parsed.filter.is_none() && !parsed.force); - let (_, force, host) = parse_repl_install_flags( + let parsed = 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); + assert_eq!(parsed.git_host.as_deref(), Some("git.y.com")); + assert!(parsed.force); + } + + #[test] + fn repl_install_flags_accept_any_argument_order() { + let parsed = parse_repl_install_flags( + ".install", + vec![ + "--git-host".to_string(), + "git.x.com".to_string(), + "owner/repo".to_string(), + "--force".to_string(), + ] + .into_iter(), + ) + .unwrap(); + assert_eq!(parsed.value.as_deref(), Some("owner/repo")); + assert_eq!(parsed.git_host.as_deref(), Some("git.x.com")); + assert!(parsed.filter.is_none() && parsed.force); + + let err = parse_repl_install_flags( + ".install", + vec!["one".to_string(), "two".to_string()].into_iter(), + ) + .unwrap_err(); + assert!(err.to_string().contains("Unexpected argument")); + + let err = parse_repl_install_flags(".install", vec!["--bogus".to_string()].into_iter()) + .unwrap_err(); + assert!(err.to_string().contains("Unexpected argument")); } #[test] diff --git a/src/config/mod.rs b/src/config/mod.rs index 762ca5e..f1749ce 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -33,7 +33,8 @@ pub use self::app_state::AppState; pub use self::bundles::list_installed_bundles; pub use self::input::Input; pub use self::install_remote::{ - install_or_update, install_or_update_from_repl_args, uninstall_bundle, update_bundle, + DEFAULT_GIT_HOST, install_or_update, install_or_update_from_repl_args, uninstall_bundle, + update_bundle, }; pub use self::macro_policy::{ MacroAllowlistLevel, MacroPolicy, MacroSource, MacroState, RESERVED_MACRO_NAMES, ResolvedMacro, diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 2a0a9a8..e6e9e41 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -3278,10 +3278,19 @@ impl RequestContext { .map(|(name, desc)| (name, if desc.is_empty() { None } else { Some(desc) })) .collect(), ".install" => { - let mut values: Vec = + let mut names: Vec = AssetCategory::NAMES.iter().map(|s| s.to_string()).collect(); - values.extend(installed_bundle_names()); - super::map_completion_values(values) + names.extend(installed_bundle_names()); + let mut values = super::map_completion_values(names); + values.push(( + "--git-host".to_string(), + Some("Host the owner/repo shorthand expands against".to_string()), + )); + values.push(( + "--help".to_string(), + Some("Show usage for .install".to_string()), + )); + values } ".uninstall" => { let mut values = super::map_completion_values(installed_bundle_names()); @@ -3289,6 +3298,10 @@ impl RequestContext { "--yes".to_string(), Some("Skip the uninstall confirmation".to_string()), )); + values.push(( + "--help".to_string(), + Some("Show usage for .uninstall".to_string()), + )); values } @@ -3525,6 +3538,9 @@ impl RequestContext { if !has_git_host { available.push("--git-host"); } + if !args.contains(&"--help") { + available.push("--help"); + } values = super::map_completion_values(available); } diff --git a/src/repl/mod.rs b/src/repl/mod.rs index 12c68a9..3c159f2 100644 --- a/src/repl/mod.rs +++ b/src/repl/mod.rs @@ -875,9 +875,10 @@ pub async fn run_repl_command( ReplInstallDispatch::Unified(value) => { config::install_or_update_from_repl_args(value)?; } + ReplInstallDispatch::Help => println!("{}", repl_install_help()), ReplInstallDispatch::Usage => println!( "Usage: .install <{}> | .install \ - [--git-host ] [--filter ] [--force]", + [--git-host ] [--filter ] [--force] (see `.install --help`)", AssetCategory::NAMES.join("|") ), }, @@ -1198,8 +1199,13 @@ pub async fn run_repl_command( } }, ".uninstall" => match parse_repl_uninstall(args) { - Some((name, assume_yes)) => config::uninstall_bundle(&name, assume_yes)?, - None => println!("Usage: .uninstall [--yes]"), + ReplUninstallDispatch::Run(name, assume_yes) => { + config::uninstall_bundle(&name, assume_yes)? + } + ReplUninstallDispatch::Help => println!("{}", repl_uninstall_help()), + ReplUninstallDispatch::Usage => { + println!("Usage: .uninstall [--yes] (see `.uninstall --help`)") + } }, ".list" => match args { Some(args) => { @@ -1573,11 +1579,18 @@ fn unknown_command() -> Result<()> { enum ReplInstallDispatch<'a> { Builtins(AssetCategory), Unified(&'a str), + Help, Usage, } fn parse_repl_install(args: Option<&str>) -> ReplInstallDispatch<'_> { let trimmed = args.map(str::trim).unwrap_or(""); + if trimmed + .split_whitespace() + .any(|token| token == "--help" || token == "-h") + { + return ReplInstallDispatch::Help; + } let mut parts = trimmed.splitn(2, char::is_whitespace); match parts.next() { Some(name) if !name.is_empty() => { @@ -1592,19 +1605,65 @@ fn parse_repl_install(args: Option<&str>) -> ReplInstallDispatch<'_> { } } -fn parse_repl_uninstall(args: Option<&str>) -> Option<(String, bool)> { +fn repl_install_help() -> String { + format!( + r#"Install built-in assets, install a bundle from a Git source, or update an installed bundle. + +Usage: + .install Reinstall built-in assets ({categories}) + .install [#ref] Install a bundle from {default_host} (change the host with --git-host) + .install [#ref] Install a bundle from any Git URL, scp-style path, or local path + .install [#ref] Update an installed bundle from its recorded source + +Flags: + --git-host Host the shorthand expands against (default {default_host}) + --filter Restrict a remote install to one category ({filters}) + --force Overwrite all conflicts without prompting (remote installs only) + +Suffix # to pin a branch, tag, or commit. List installed bundles with +`.list bundles`; remove one with `.uninstall `."#, + categories = AssetCategory::NAMES.join("|"), + default_host = config::DEFAULT_GIT_HOST, + filters = config::InstallFilter::NAMES.join("|"), + ) +} + +fn repl_uninstall_help() -> String { + r#"Remove an installed bundle: delete the files it owns and the mcp.json entries it added. + +Usage: + .uninstall [--yes] + +Flags: + --yes, -y Skip the confirmation prompt + +Files you modified after install are prompted for individually and kept by +default; --yes never deletes modified files. List installed bundles with +`.list bundles`."# + .to_string() +} + +#[derive(Debug, PartialEq)] +enum ReplUninstallDispatch { + Run(String, bool), + Help, + Usage, +} + +fn parse_repl_uninstall(args: Option<&str>) -> ReplUninstallDispatch { let mut assume_yes = false; let mut names = Vec::new(); for token in args.unwrap_or("").split_whitespace() { match token { + "--help" | "-h" => return ReplUninstallDispatch::Help, "--yes" | "-y" => assume_yes = true, - other if other.starts_with('-') => return None, + other if other.starts_with('-') => return ReplUninstallDispatch::Usage, other => names.push(other), } } match names.as_slice() { - [name] => Some((name.to_string(), assume_yes)), - _ => None, + [name] => ReplUninstallDispatch::Run(name.to_string(), assume_yes), + _ => ReplUninstallDispatch::Usage, } } @@ -1871,6 +1930,70 @@ mod tests { ); } + #[test] + fn parse_repl_install_routes_help_from_any_position() { + assert_eq!( + parse_repl_install(Some("--help")), + ReplInstallDispatch::Help + ); + assert_eq!(parse_repl_install(Some("-h")), ReplInstallDispatch::Help); + assert_eq!( + parse_repl_install(Some("agents --help")), + ReplInstallDispatch::Help + ); + assert_eq!( + parse_repl_install(Some("owner/repo --help")), + ReplInstallDispatch::Help + ); + } + + #[test] + fn parse_repl_uninstall_routes_run_help_and_usage() { + assert_eq!( + parse_repl_uninstall(Some("my-bundle --yes")), + ReplUninstallDispatch::Run("my-bundle".to_string(), true) + ); + assert_eq!( + parse_repl_uninstall(Some("my-bundle")), + ReplUninstallDispatch::Run("my-bundle".to_string(), false) + ); + assert_eq!( + parse_repl_uninstall(Some("--help")), + ReplUninstallDispatch::Help + ); + assert_eq!( + parse_repl_uninstall(Some("my-bundle -h")), + ReplUninstallDispatch::Help + ); + assert_eq!( + parse_repl_uninstall(Some("--force my-bundle")), + ReplUninstallDispatch::Usage + ); + assert_eq!(parse_repl_uninstall(None), ReplUninstallDispatch::Usage); + } + + #[test] + fn repl_install_and_uninstall_help_text_cover_the_full_surface() { + let install = repl_install_help(); + for needle in [ + "--git-host", + "--filter", + "--force", + "#ref", + "owner/repo", + ".list bundles", + ] { + assert!(install.contains(needle), "install help missing {needle}"); + } + let uninstall = repl_uninstall_help(); + for needle in ["--yes", ".list bundles", ""] { + assert!( + uninstall.contains(needle), + "uninstall help missing {needle}" + ); + } + } + #[test] fn builtin_command_names_are_sorted_deduped_first_words_without_dots() { let names = builtin_command_names();