diff --git a/Cargo.lock b/Cargo.lock index 4158e03..443d80c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -377,6 +377,15 @@ dependencies = [ "strsim", ] +[[package]] +name = "clap_complete" +version = "4.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e0a7a9bfdb35811f9e59832f0f05975114d2251b415fb534108e6f34060fd772" +dependencies = [ + "clap", +] + [[package]] name = "clap_derive" version = "4.6.1" @@ -404,6 +413,7 @@ dependencies = [ "bytes", "chrono", "clap", + "clap_complete", "jmespath", "keyring", "open", diff --git a/Cargo.toml b/Cargo.toml index 3db7a98..cdd2126 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,7 @@ url = { version = "2.5.4", optional = true } zeroize = { version = "1.8", optional = true, features = ["derive"] } chrono = { version = "0.4.42", default-features = false, features = ["clock", "serde"] } clap = { version = "4.5.53", features = ["derive", "std", "string"] } +clap_complete = "4" jmespath = "0.5.0" reqwest = { version = "0.12.24", default-features = false, features = ["json", "multipart", "rustls-tls"] } regex = "1.12.2" diff --git a/docs/completion.md b/docs/completion.md new file mode 100644 index 0000000..93b2613 --- /dev/null +++ b/docs/completion.md @@ -0,0 +1,49 @@ +# Shell Completion + +`cli-engine` provides a built-in `completion` command that enables tab-completion for your CLI. +This completion is automatically generated from your CLI's command tree using `clap_complete`, ensuring it stays in sync with your commands, flags, and arguments. + +## Usage + +The `completion` command is a reserved built-in, similar to `help`, `tree`, and `guide`. +It cannot be overridden by consumer-defined commands. + +### Generate Completion Script + +To print the completion script for a specific shell to stdout: + +```bash + completion [bash|zsh|fish|elvish|powershell] +``` + +### Install Completion + +To automatically install the completion script and configure your shell, use the `--install` flag: + +```bash + completion --install [bash|zsh|fish|elvish|powershell] +``` + +This command is **idempotent**. Re-running it replaces any existing managed completion block in your shell configuration. There is no `--uninstall` flag; completion scripts can be removed by deleting the managed block from your shell configuration file. + +## Shell Install Locations + +The completion command manages the installation by appending a block to your shell configuration file. This block is wrapped in managed markers so you can identify and edit it if needed: + +`# >>> completion (managed) >>>` +`# <<< completion (managed) <<<` + +| Shell | Script Location | Shell Configuration File | +| --- | --- | --- | +| **bash** | `$XDG_DATA_HOME/bash-completion/completions/` | `~/.bashrc` | +| **zsh** | `~/.zfunc/_` | `~/.zshrc` | +| **fish** | `$XDG_CONFIG_HOME/fish/completions/.fish` | None (auto-loaded) | +| **elvish** | `$XDG_CONFIG_HOME/elvish/lib/-completion.elv` | `$XDG_CONFIG_HOME/elvish/rc.elv` | +| **powershell** | `~/Documents/PowerShell/-completion.ps1` | `$PROFILE` | + +### Notes + +- **bash**: Adds a managed `source ""` line to `~/.bashrc` that sources the generated completion script directly. +- **zsh**: Adds the script directory to your `fpath` and calls `autoload -Uz compinit && compinit`. +- **fish**: Files placed in the completions directory are auto-loaded by fish; no shell configuration edit is required. +- **powershell**: Adds the dot-source command to your PowerShell profile. diff --git a/src/cli.rs b/src/cli.rs index 81e38eb..dff033b 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -9,6 +9,7 @@ use std::{ }; mod builtins; +mod completion; mod help; mod tree_render; @@ -38,7 +39,9 @@ use crate::{ search::{SearchDocument, SearchIndex}, }; -use builtins::{guide_args, guide_command, help_args, help_command}; +use builtins::{ + completion_args, completion_command, guide_args, guide_command, help_args, help_command, +}; use help::{GROUP_HELP_TEMPLATE, ROOT_HELP_TEMPLATE}; pub use help::{ModuleHelpEntry, build_root_long, render_next_actions_human}; @@ -181,6 +184,11 @@ pub enum Argv0LinkMethod { Script, } +/// Top-level subcommand names that are reserved by the engine and must not be +/// used as module group names. [`Cli::add_module_group`] rejects a group whose +/// name matches a reserved name so the engine's built-in command always wins. +pub(crate) const BUILTIN_COMMAND_NAMES: [&str; 4] = ["help", "guide", "tree", "completion"]; + /// Declarative configuration for a CLI application. /// /// Use [`CliConfig::new`] for the common path and chain `with_*` methods for @@ -398,6 +406,13 @@ impl CliConfig { } /// Adds one domain module. + /// + /// # Reserved group names + /// + /// The top-level group names `help`, `guide`, `tree`, and `completion` are + /// reserved by the engine. A module whose root group uses one of these + /// names will be rejected at registration time (logged as a warning) so + /// the engine's own built-in always takes precedence in the command tree. #[must_use] pub fn with_module(mut self, module: Module) -> Self { self.modules.push(module); @@ -405,6 +420,8 @@ impl CliConfig { } /// Adds several domain modules. + /// + /// See [`with_module`](Self::with_module) for the list of reserved group names. #[must_use] pub fn with_modules(mut self, modules: impl IntoIterator) -> Self { self.modules.extend(modules); @@ -792,7 +809,8 @@ impl Cli { root = register_global_flags(root) .subcommand(help_command()) .subcommand(guide_command()) - .subcommand(Command::new("tree").about("Display full command tree")); + .subcommand(Command::new("tree").about("Display full command tree")) + .subcommand(completion_command()); if let Some(register_flags) = &config.register_flags { root = register_flags(root); } @@ -1020,6 +1038,16 @@ impl Cli { category: impl Into, group: RuntimeGroupSpec, ) -> &mut Self { + // Prevent consumer modules from shadowing engine built-ins in the clap + // command tree. A reserved group name would override the engine's own + // subcommand (last-writer-wins in clap) and corrupt the dispatch path. + if BUILTIN_COMMAND_NAMES.contains(&group.group.name.as_str()) { + tracing::warn!( + name = %group.group.name, + "module group name is reserved by cli-engine built-ins; the group will not be registered" + ); + return self; + } let category = category.into(); if !group.group.hidden { self.module_entries.push(ModuleHelpEntry { @@ -1471,6 +1499,51 @@ impl Cli { } return self.finish_run(self.render_guide(&matches)); } + if command_path == "completion" { + let args = completion_args(&matches); + if let Err(err) = self.run_pre_run(&mut middleware, &command_path, &args) { + return self.finish_run(render_cli_error(&middleware, &err, &self.config.app_id)); + } + let install = args + .get("install") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + let shell_opt = args + .get("shell") + .and_then(|v| v.as_str()) + .map(str::to_owned); + if install { + use crate::cli::completion::{detect_shell, parse_shell}; + let shell = match shell_opt { + Some(ref s) => match parse_shell(s) { + Ok(s) => s, + Err(e) => { + return self.finish_run(render_cli_error( + &middleware, + &e, + &self.config.app_id, + )); + } + }, + None => match detect_shell() { + Ok(s) => s, + Err(e) => { + return self.finish_run(render_cli_error( + &middleware, + &e, + &self.config.app_id, + )); + } + }, + }; + return self.finish_run( + completion::install(&self.root, &self.config.name, shell) + .await + .unwrap_or_else(|e| render_cli_error(&middleware, &e, &self.config.app_id)), + ); + } + return self.finish_run(self.render_completion_print(shell_opt, &middleware)); + } let Some(command) = self.commands.get(&command_path) else { if !command_path.is_empty() && let Some(group) = find_command_by_colon_path(&self.root, &command_path) @@ -1825,6 +1898,31 @@ impl Cli { } } + fn render_completion_print( + &self, + shell_opt: Option, + middleware: &Middleware, + ) -> CliRunOutput { + use crate::cli::completion::{detect_shell, generate_script, parse_shell}; + let shell = match shell_opt { + Some(s) => match parse_shell(&s) { + Ok(s) => s, + Err(e) => return render_cli_error(middleware, &e, &self.config.app_id), + }, + None => match detect_shell() { + Ok(s) => s, + Err(e) => return render_cli_error(middleware, &e, &self.config.app_id), + }, + }; + match generate_script(&self.root, &self.config.name, shell) { + Ok(script) => CliRunOutput { + exit_code: 0, + rendered: script, + }, + Err(e) => render_cli_error(middleware, &e, &self.config.app_id), + } + } + fn render_help_command(&self, matches: &ArgMatches) -> CliRunOutput { let leaf = leaf_matches(matches); let parts = leaf @@ -1868,7 +1966,7 @@ impl Cli { // neither categorized nor an engine built-in, listed under a generic // "Commands" section. This keeps every command discoverable once clap's // auto subcommand list is suppressed by the root help template. - const BUILTINS: [&str; 4] = ["help", "guide", "tree", "completion"]; + let builtins = BUILTIN_COMMAND_NAMES; let categorized: BTreeSet<&str> = self .module_entries .iter() @@ -1878,7 +1976,7 @@ impl Cli { .root .get_subcommands() .filter(|command| !command.is_hide_set()) - .filter(|command| !BUILTINS.contains(&command.get_name())) + .filter(|command| !builtins.contains(&command.get_name())) .filter(|command| !categorized.contains(command.get_name())) .map(|command| ModuleHelpEntry { category: "Commands".to_owned(), @@ -2331,7 +2429,7 @@ fn collect_command_search_documents( aliases: &mut Vec, docs: &mut Vec, ) { - if command.is_hide_set() || command.get_name() == "completion" { + if command.is_hide_set() || BUILTIN_COMMAND_NAMES.contains(&command.get_name()) { return; } if command.get_subcommands().next().is_some() { diff --git a/src/cli/builtins.rs b/src/cli/builtins.rs index 29bf71b..0c261bd 100644 --- a/src/cli/builtins.rs +++ b/src/cli/builtins.rs @@ -1,4 +1,4 @@ -use clap::{Arg, ArgMatches, Command}; +use clap::{Arg, ArgAction, ArgMatches, Command}; use serde_json::Value; use crate::{ @@ -38,3 +38,62 @@ pub(crate) fn guide_args(matches: &ArgMatches) -> ValueMap { value_map([("topic", Value::String(topic.clone()))]) }) } + +pub(crate) fn completion_command() -> Command { + Command::new("completion") + .about("Generate or install shell completion scripts") + .arg(Arg::new("shell").value_name("shell").num_args(0..=1)) + .arg( + Arg::new("install") + .long("install") + .action(ArgAction::SetTrue) + .help("Install completion script into shell config"), + ) +} + +pub(crate) fn completion_args(matches: &ArgMatches) -> ValueMap { + let leaf = leaf_matches(matches); + let shell = leaf.get_one::("shell").cloned(); + let install = leaf.get_flag("install"); + let mut map = value_map([("install", Value::Bool(install))]); + if let Some(s) = shell { + map.insert("shell".to_owned(), Value::String(s)); + } + map +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use super::*; + + #[test] + fn completion_command_parses_shell() { + let m = completion_command() + .try_get_matches_from(["completion", "zsh"]) + .unwrap(); + let leaf = leaf_matches(&m); + assert_eq!( + leaf.get_one::("shell").map(String::as_str), + Some("zsh") + ); + } + + #[test] + fn completion_command_parses_install() { + let m = completion_command() + .try_get_matches_from(["completion", "--install"]) + .unwrap(); + let leaf = leaf_matches(&m); + assert!(leaf.get_flag("install")); + } + + #[test] + fn completion_command_rejects_unknown_flag() { + assert!( + completion_command() + .try_get_matches_from(["completion", "--bogusflag"]) + .is_err() + ); + } +} diff --git a/src/cli/completion.rs b/src/cli/completion.rs new file mode 100644 index 0000000..47dcdbc --- /dev/null +++ b/src/cli/completion.rs @@ -0,0 +1,885 @@ +use std::path::{Path, PathBuf}; + +use clap::Command; +use clap_complete::{Shell as ClapShell, generate}; + +use crate::CliRunOutput; +use crate::error::CliCoreError; + +/// The shells supported by the completion built-in. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +// `PowerShell` ends with the enum name `Shell`; the name is intentional. +#[allow(clippy::enum_variant_names)] +pub(crate) enum Shell { + /// Bourne-Again Shell. + Bash, + /// Z Shell. + Zsh, + /// Friendly Interactive Shell. + Fish, + /// PowerShell. + PowerShell, + /// Elvish shell. + Elvish, +} + +/// Parses a shell name (case-insensitive) into a [`Shell`] variant. +/// +/// Returns an error for unrecognized names rather than panicking. +pub(crate) fn parse_shell(s: &str) -> crate::Result { + match s.to_ascii_lowercase().as_str() { + "bash" => Ok(Shell::Bash), + "zsh" => Ok(Shell::Zsh), + "fish" => Ok(Shell::Fish), + // `pwsh` is the PowerShell Core executable name seen in $SHELL/argv. + "powershell" | "pwsh" => Ok(Shell::PowerShell), + "elvish" => Ok(Shell::Elvish), + _ => Err(CliCoreError::Message(format!( + "unsupported shell: {s}; supported: bash, zsh, fish, powershell, elvish" + ))), + } +} + +/// Basename split on both Unix and Windows separators, with a trailing `.exe` stripped +/// and a version suffix like `-5.9` removed (e.g. `"zsh-5.9"` → `"zsh"`). +fn shell_basename(path: &str) -> &str { + let basename = path.rsplit(['/', '\\']).next().unwrap_or(path); + let name = basename + .strip_suffix(".exe") + .or_else(|| basename.strip_suffix(".EXE")) + .unwrap_or(basename); + // Strip version suffix like "-5.9" so versioned shell paths (e.g. "/usr/bin/zsh-5.9") work. + // Use rfind so a name like "fish-shell-3.7" strips only the trailing "-3.7", yielding + // "fish-shell" rather than "fish". + if let Some(idx) = name.rfind('-') + && name[idx + 1..].starts_with(|c: char| c.is_ascii_digit()) + { + return &name[..idx]; + } + name +} + +/// Detects the current shell by inspecting the `$SHELL` environment variable. +/// +/// On Windows, defaults to [`Shell::PowerShell`] when `$SHELL` is unset. +/// Returns an error if the shell is unset on non-Windows or if the detected +/// name is not a recognized shell. +pub(crate) fn detect_shell() -> crate::Result { + let shell_var = std::env::var("SHELL").ok().filter(|s| !s.is_empty()); + + match shell_var { + Some(path) => { + let basename = shell_basename(&path); + parse_shell(basename).map_err(|_| { + CliCoreError::Message(format!( + "could not detect shell: $SHELL is set to {path:?} but that is not a recognized shell; supported: bash, zsh, fish, powershell, elvish" + )) + }) + } + None => { + if cfg!(windows) { + Ok(Shell::PowerShell) + } else { + Err(CliCoreError::Message( + "could not detect shell: $SHELL is not set".to_owned(), + )) + } + } + } +} + +/// Maps a [`Shell`] to the corresponding [`ClapShell`] variant. +fn to_clap_shell(shell: Shell) -> ClapShell { + match shell { + Shell::Bash => ClapShell::Bash, + Shell::Zsh => ClapShell::Zsh, + Shell::Fish => ClapShell::Fish, + Shell::PowerShell => ClapShell::PowerShell, + Shell::Elvish => ClapShell::Elvish, + } +} + +/// Generates a shell completion script for the given root [`Command`]. +/// +/// Clones the command internally so the caller's instance is not mutated. +/// +/// # Errors +/// +/// Returns an error if `clap_complete` produces non-UTF-8 output (this is a +/// bug in the upstream crate, but surfacing it is safer than silently writing +/// a corrupted script). +pub(crate) fn generate_script( + root: &Command, + bin_name: &str, + shell: Shell, +) -> crate::Result { + let clap_shell = to_clap_shell(shell); + let mut buf: Vec = Vec::new(); + generate(clap_shell, &mut root.clone(), bin_name, &mut buf); + String::from_utf8(buf) + .map_err(|e| CliCoreError::message(format!("completion script is not valid UTF-8: {e}"))) +} + +/// Resolves `$XDG_DATA_HOME` with fallback to `$HOME/.local/share`. +/// +/// Only absolute paths are accepted. +fn xdg_data_dir() -> Option { + let xdg = std::env::var("XDG_DATA_HOME") + .ok() + .filter(|v| !v.is_empty()) + .map(PathBuf::from) + .filter(|p| p.is_absolute()); + xdg.or_else(|| crate::fs::home_dir().map(|h| h.join(".local").join("share"))) +} + +/// Inserts or replaces a managed block delimited by `bin_name`-specific markers +/// inside `content`. +/// +/// - If a complete begin+end pair is found, the span (inclusive) is replaced. +/// - If only the begin marker is found (end was deleted), the orphaned begin +/// line is removed before the new complete block is appended, preventing +/// duplicate markers on re-runs. +/// - If neither marker is present, the block is appended. +fn apply_managed_block(content: &str, bin_name: &str, body: &str) -> String { + let begin = format!("# >>> {bin_name} completion (managed) >>>"); + let end = format!("# <<< {bin_name} completion (managed) <<<"); + let new_block = format!("{begin}\n{body}\n{end}"); + + // Only treat the end marker that follows the begin marker as the span end, + // so a stray earlier end marker cannot delete unrelated content. + if let Some(start_idx) = content.find(&begin) + && let Some(rel_end) = content[start_idx..].find(&end) + { + let end_idx = start_idx + rel_end + end.len(); + format!( + "{}{new_block}{}", + &content[..start_idx], + &content[end_idx..] + ) + } else if let Some(start_idx) = content.find(&begin) { + // Begin marker present but end marker missing; remove the orphaned begin + // line so repeated installs don't accumulate stray markers. Handle + // both LF and CRLF line endings after the marker. + let after_begin = start_idx + begin.len(); + let after_line = if content[after_begin..].starts_with("\r\n") { + after_begin + 2 + } else if content[after_begin..].starts_with('\n') { + after_begin + 1 + } else { + after_begin + }; + format!( + "{}{}\n\n{new_block}\n", + &content[..start_idx], + &content[after_line..] + ) + } else { + format!("{content}\n\n{new_block}\n") + } +} + +/// Reads existing rc file content, resolving symlinks. +/// +/// Returns `(canonical_path, content)`. If the file does not exist, returns +/// the original path with empty content. If the file exists but cannot be +/// read (e.g. permission denied), returns an error so the caller does not +/// accidentally overwrite the file with only the managed block. +fn read_rc(rc_path: &Path) -> crate::Result<(PathBuf, String)> { + match std::fs::canonicalize(rc_path) { + Ok(canonical) => { + // File exists; a read failure here (e.g. permission denied) must + // not be silently treated as "empty" — that would destroy the + // user's config when write_string_atomic overwrites it. + let content = std::fs::read_to_string(&canonical).map_err(|e| { + CliCoreError::message(format!("cannot read {}: {e}", canonical.display())) + })?; + Ok((canonical, content)) + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + // File genuinely does not exist; start with empty content. + Ok((rc_path.to_owned(), String::new())) + } + Err(e) => Err(CliCoreError::message(format!( + "cannot resolve {}: {e}", + rc_path.display() + ))), + } +} + +/// Per-shell rc-file specification, computed on the async thread (env var reads +/// only) and consumed inside `spawn_blocking` where the actual file I/O occurs. +struct RcSpec { + /// Destination rc file path (not yet canonicalized). + path: PathBuf, + /// Body to wrap inside the managed-block markers. + body: String, + /// Normalize the full rc file content to CRLF after applying the block + /// (required by PowerShell). + crlf: bool, +} + +/// Installs shell completions for `bin_name` into the appropriate per-shell +/// location. +/// +/// For shells that source rc files (bash, zsh, elvish, powershell) an idempotent +/// managed block is inserted or updated in the rc file. Fish auto-loads +/// completions from its completions directory so no rc edit is needed. +/// +/// Path resolution (env var reads) happens on the calling async thread; all +/// file I/O is offloaded to a single [`tokio::task::spawn_blocking`] closure so +/// the executor thread is never blocked. +/// +/// # Errors +/// +/// Returns an error if required home/config directories cannot be resolved, if +/// an existing rc file cannot be read, or if any file write fails. +pub(crate) async fn install( + root: &Command, + bin_name: &str, + shell: Shell, +) -> crate::Result { + let script = generate_script(root, bin_name, shell)?; + let bin_name = bin_name.to_owned(); + + // Compute per-shell paths (env var reads only — no blocking I/O here). + let (script_path, rc_spec): (PathBuf, Option) = match shell { + Shell::Bash => { + let data = xdg_data_dir().ok_or_else(|| { + CliCoreError::message("could not resolve XDG_DATA_HOME or HOME for bash completion") + })?; + let script_path = data.join("bash-completion/completions").join(&bin_name); + let rc_path = crate::fs::home_dir() + .ok_or_else(|| CliCoreError::message("could not resolve HOME for .bashrc"))? + .join(".bashrc"); + let body = format!("source \"{}\"", script_path.display()); + ( + script_path, + Some(RcSpec { + path: rc_path, + body, + crlf: false, + }), + ) + } + + Shell::Zsh => { + let home = crate::fs::home_dir().ok_or_else(|| { + CliCoreError::message("could not resolve HOME for zsh completion") + })?; + let script_path = home.join(".zfunc").join(format!("_{bin_name}")); + // When $ZDOTDIR is set, zsh reads its dotfiles from that directory + // instead of $HOME. Write to $ZDOTDIR/.zshrc so the sourced block + // actually takes effect. + let zshrc_dir = std::env::var("ZDOTDIR") + .ok() + .filter(|v| !v.is_empty()) + .map(PathBuf::from) + .filter(|p| p.is_absolute()) + .unwrap_or_else(|| home.clone()); + let rc_path = zshrc_dir.join(".zshrc"); + let body = format!( + "fpath=(\"{home}/.zfunc\" $fpath)\nautoload -Uz compinit && compinit", + home = home.display() + ); + ( + script_path, + Some(RcSpec { + path: rc_path, + body, + crlf: false, + }), + ) + } + + Shell::Fish => { + let config = crate::fs::config_base_dir().ok_or_else(|| { + CliCoreError::message( + "could not resolve XDG_CONFIG_HOME or HOME for fish completion", + ) + })?; + let script_path = config + .join("fish/completions") + .join(format!("{bin_name}.fish")); + // Fish auto-loads from this directory; no rc edit required. + (script_path, None) + } + + Shell::Elvish => { + let config = crate::fs::config_base_dir().ok_or_else(|| { + CliCoreError::message( + "could not resolve XDG_CONFIG_HOME or HOME for elvish completion", + ) + })?; + let script_path = config + .join("elvish/lib") + .join(format!("{bin_name}-completion.elv")); + let rc_path = config.join("elvish/rc.elv"); + let body = format!("use {bin_name}-completion"); + ( + script_path, + Some(RcSpec { + path: rc_path, + body, + crlf: false, + }), + ) + } + + Shell::PowerShell => { + let home = crate::fs::home_dir().ok_or_else(|| { + CliCoreError::message("could not resolve HOME for powershell completion") + })?; + let profile_path = home + .join("Documents") + .join("PowerShell") + .join("Microsoft.PowerShell_profile.ps1"); + let profile_dir = profile_path + .parent() + .ok_or_else(|| { + CliCoreError::message("could not determine PowerShell profile directory") + })? + .to_owned(); + let script_path = profile_dir.join(format!("{bin_name}-completion.ps1")); + let body = format!(". \"{}\"", script_path.display()); + // Normalize to LF first so an existing CRLF profile is not turned into + // `\r\r\n`, then emit CRLF as PowerShell expects. + ( + script_path, + Some(RcSpec { + path: profile_path, + body, + crlf: true, + }), + ) + } + }; + + // All file I/O in one spawn_blocking: write script, read rc, apply block, write rc. + let script_path_clone = script_path.clone(); + let bin_name_for_block = bin_name.clone(); + let written = tokio::task::spawn_blocking(move || -> crate::Result> { + crate::fs::write_string_atomic(&script_path_clone, &script).map_err(|e| { + CliCoreError::message(format!("failed to write completion script: {e}")) + })?; + let mut written = vec![script_path_clone.display().to_string()]; + + if let Some(rc) = rc_spec { + let (canonical_rc, existing) = read_rc(&rc.path)?; + let new_content = apply_managed_block(&existing, &bin_name_for_block, &rc.body); + let final_content = if rc.crlf { + new_content.replace("\r\n", "\n").replace('\n', "\r\n") + } else { + new_content + }; + crate::fs::write_string_atomic(&canonical_rc, &final_content) + .map_err(|e| CliCoreError::message(format!("failed to write rc file: {e}")))?; + written.push(canonical_rc.display().to_string()); + } + + Ok(written) + }) + .await + .map_err(|e| CliCoreError::message(format!("spawn_blocking join error: {e}")))??; + + Ok(CliRunOutput { + exit_code: 0, + rendered: format!( + "Installed {bin_name} completion.\nFiles written:\n{}", + written.join("\n") + ), + }) +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +#[allow(clippy::await_holding_lock)] +mod tests { + use super::*; + + #[test] + fn parse_shell_from_full_path() { + assert_eq!(parse_shell("zsh").unwrap(), Shell::Zsh); + assert_eq!(parse_shell("bash").unwrap(), Shell::Bash); + } + + #[test] + fn parse_shell_case_insensitive() { + assert_eq!(parse_shell("Bash").unwrap(), Shell::Bash); + assert_eq!(parse_shell("ZSH").unwrap(), Shell::Zsh); + assert!(parse_shell("notashell").is_err()); + } + + #[test] + fn parse_shell_accepts_pwsh_alias() { + assert_eq!(parse_shell("pwsh").unwrap(), Shell::PowerShell); + assert_eq!(parse_shell("PWSH").unwrap(), Shell::PowerShell); + assert_eq!(parse_shell("powershell").unwrap(), Shell::PowerShell); + } + + #[test] + fn shell_basename_handles_windows_paths_and_exe() { + assert_eq!(shell_basename("/usr/bin/bash"), "bash"); + assert_eq!( + shell_basename("C:\\Program Files\\PowerShell\\pwsh.exe"), + "pwsh" + ); + assert_eq!(shell_basename("pwsh.EXE"), "pwsh"); + assert_eq!(shell_basename("zsh"), "zsh"); + } + + #[test] + fn shell_basename_strips_version_suffix() { + assert_eq!(shell_basename("/usr/bin/zsh-5.9"), "zsh"); + assert_eq!(shell_basename("bash-5.1"), "bash"); + // Non-version hyphens (no digit after) are left intact. + assert_eq!(shell_basename("my-shell"), "my-shell"); + // A hyphen in the base name before the version suffix is preserved + // because rfind picks the last hyphen, not the first. + assert_eq!(shell_basename("fish-shell-3.7"), "fish-shell"); + } + + #[test] + fn generate_script_returns_nonempty() { + let cmd = Command::new("demo").subcommand(Command::new("list")); + let script = generate_script(&cmd, "demo", Shell::Bash).unwrap(); + assert!(!script.is_empty(), "script should be non-empty"); + assert!(script.contains("demo"), "script should mention bin name"); + } + + #[test] + fn managed_block_appended_when_absent() { + let result = apply_managed_block("existing content", "mybin", "source /path/to/script"); + assert!(result.contains("# >>> mybin completion (managed) >>>")); + assert!(result.contains("# <<< mybin completion (managed) <<<")); + assert!(result.contains("source /path/to/script")); + assert!(result.contains("existing content")); + } + + #[test] + fn managed_block_replaced_when_present() { + let initial = "prefix\n# >>> mybin completion (managed) >>>\nold body\n# <<< mybin completion (managed) <<<\nsuffix"; + let result = apply_managed_block(initial, "mybin", "new body"); + assert_eq!( + result + .matches("# >>> mybin completion (managed) >>>") + .count(), + 1 + ); + assert!(!result.contains("old body"), "old body should be replaced"); + assert!(result.contains("new body"), "new body should appear"); + assert!(result.contains("prefix"), "prefix should be preserved"); + assert!(result.contains("suffix"), "suffix should be preserved"); + } + + #[test] + fn managed_block_ignores_stray_end_marker_before_begin() { + let initial = "# <<< mybin completion (managed) <<<\nimportant user content\n# >>> mybin completion (managed) >>>\nold body\n# <<< mybin completion (managed) <<<"; + let result = apply_managed_block(initial, "mybin", "new body"); + assert!( + result.contains("important user content"), + "content between a stray end marker and the real begin marker must be preserved" + ); + assert!(result.contains("new body"), "new body should appear"); + assert!(!result.contains("old body"), "old body should be replaced"); + } + + #[test] + fn managed_block_replaces_orphaned_begin_marker() { + // User deleted the end marker; re-running install must not accumulate + // a second begin marker. Only the begin line is removed (not any + // content after it) because without the end marker the block extent + // is unknown. + let initial = "prefix\n# >>> mybin completion (managed) >>>\nsuffix"; + let result = apply_managed_block(initial, "mybin", "new body"); + assert_eq!( + result + .matches("# >>> mybin completion (managed) >>>") + .count(), + 1, + "exactly one begin marker after replacing orphaned block" + ); + assert!(result.contains("# <<< mybin completion (managed) <<<")); + assert!(result.contains("new body"), "new body should appear"); + assert!(result.contains("prefix"), "prefix should be preserved"); + assert!(result.contains("suffix"), "suffix should be preserved"); + } + + #[allow(unsafe_code, clippy::await_holding_lock)] + #[tokio::test] + async fn install_bash_writes_script_and_rc() { + use tempfile::TempDir; + let tmp = TempDir::new().unwrap(); + let home = tmp.path().join("home"); + std::fs::create_dir_all(&home).unwrap(); + let data_dir = tmp.path().join("data"); + std::fs::create_dir_all(&data_dir).unwrap(); + let config_dir = tmp.path().join("config"); + + let _lock = crate::config::test_env::lock(); + let prev_home = std::env::var("HOME").ok(); + let prev_data = std::env::var("XDG_DATA_HOME").ok(); + let prev_config = std::env::var("XDG_CONFIG_HOME").ok(); + // SAFETY: caller holds XDG_TEST_MUTEX, serializing all mutation. + unsafe { + std::env::set_var("HOME", home.to_str().unwrap()); + std::env::set_var("XDG_DATA_HOME", data_dir.to_str().unwrap()); + std::env::set_var("XDG_CONFIG_HOME", config_dir.to_str().unwrap()); + } + + let cmd = Command::new("testbin").subcommand(Command::new("list")); + let result = install(&cmd, "testbin", Shell::Bash).await.unwrap(); + assert_eq!(result.exit_code, 0); + + let script = data_dir.join("bash-completion/completions/testbin"); + assert!( + script.exists(), + "script file should exist at {}", + script.display() + ); + + let bashrc = home.join(".bashrc"); + let bashrc_content = std::fs::read_to_string(&bashrc).unwrap(); + assert!(bashrc_content.contains("# >>> testbin completion (managed) >>>")); + assert!(bashrc_content.contains("# <<< testbin completion (managed) <<<")); + + install(&cmd, "testbin", Shell::Bash).await.unwrap(); + let content2 = std::fs::read_to_string(&bashrc).unwrap(); + assert_eq!( + content2 + .matches("# >>> testbin completion (managed) >>>") + .count(), + 1 + ); + + // SAFETY: restoring state while still holding the lock. + unsafe { + match prev_home { + Some(v) => std::env::set_var("HOME", v), + None => std::env::remove_var("HOME"), + } + match prev_data { + Some(v) => std::env::set_var("XDG_DATA_HOME", v), + None => std::env::remove_var("XDG_DATA_HOME"), + } + match prev_config { + Some(v) => std::env::set_var("XDG_CONFIG_HOME", v), + None => std::env::remove_var("XDG_CONFIG_HOME"), + } + } + } + + #[allow(unsafe_code, clippy::await_holding_lock)] + #[tokio::test] + async fn install_zsh_writes_script_and_rc() { + use tempfile::TempDir; + let tmp = TempDir::new().unwrap(); + let home = tmp.path().join("home"); + std::fs::create_dir_all(&home).unwrap(); + + let _lock = crate::config::test_env::lock(); + let prev_home = std::env::var("HOME").ok(); + let prev_data = std::env::var("XDG_DATA_HOME").ok(); + let prev_config = std::env::var("XDG_CONFIG_HOME").ok(); + // SAFETY: caller holds XDG_TEST_MUTEX, serializing all mutation. + unsafe { + std::env::set_var("HOME", home.to_str().unwrap()); + std::env::set_var("XDG_DATA_HOME", tmp.path().join("data").to_str().unwrap()); + std::env::set_var( + "XDG_CONFIG_HOME", + tmp.path().join("config").to_str().unwrap(), + ); + } + + let cmd = Command::new("testbin"); + let result = install(&cmd, "testbin", Shell::Zsh).await.unwrap(); + assert_eq!(result.exit_code, 0); + + let script = home.join(".zfunc/_testbin"); + assert!( + script.exists(), + "zsh script should exist at {}", + script.display() + ); + + let zshrc = home.join(".zshrc"); + let content = std::fs::read_to_string(&zshrc).unwrap(); + assert!(content.contains("# >>> testbin completion (managed) >>>")); + assert!(content.contains("fpath=")); + assert!(content.contains("autoload -Uz compinit")); + + // SAFETY: restoring state while still holding the lock. + unsafe { + match prev_home { + Some(v) => std::env::set_var("HOME", v), + None => std::env::remove_var("HOME"), + } + match prev_data { + Some(v) => std::env::set_var("XDG_DATA_HOME", v), + None => std::env::remove_var("XDG_DATA_HOME"), + } + match prev_config { + Some(v) => std::env::set_var("XDG_CONFIG_HOME", v), + None => std::env::remove_var("XDG_CONFIG_HOME"), + } + } + } + + #[allow(unsafe_code, clippy::await_holding_lock)] + #[tokio::test] + async fn install_fish_writes_no_rc() { + use tempfile::TempDir; + let tmp = TempDir::new().unwrap(); + let config_dir = tmp.path().join("config"); + + let _lock = crate::config::test_env::lock(); + let prev_home = std::env::var("HOME").ok(); + let prev_config = std::env::var("XDG_CONFIG_HOME").ok(); + let prev_data = std::env::var("XDG_DATA_HOME").ok(); + // SAFETY: caller holds XDG_TEST_MUTEX, serializing all mutation. + unsafe { + std::env::set_var("HOME", tmp.path().to_str().unwrap()); + std::env::set_var("XDG_CONFIG_HOME", config_dir.to_str().unwrap()); + std::env::set_var("XDG_DATA_HOME", tmp.path().join("data").to_str().unwrap()); + } + + let cmd = Command::new("testbin"); + let result = install(&cmd, "testbin", Shell::Fish).await.unwrap(); + assert_eq!(result.exit_code, 0); + + let script = config_dir.join("fish/completions/testbin.fish"); + assert!( + script.exists(), + "fish script should exist at {}", + script.display() + ); + + let rc_candidates = ["config.fish", "init.fish"]; + for rc in rc_candidates { + assert!( + !config_dir.join("fish").join(rc).exists(), + "{rc} should not exist" + ); + } + + // SAFETY: restoring state while still holding the lock. + unsafe { + match prev_home { + Some(v) => std::env::set_var("HOME", v), + None => std::env::remove_var("HOME"), + } + match prev_config { + Some(v) => std::env::set_var("XDG_CONFIG_HOME", v), + None => std::env::remove_var("XDG_CONFIG_HOME"), + } + match prev_data { + Some(v) => std::env::set_var("XDG_DATA_HOME", v), + None => std::env::remove_var("XDG_DATA_HOME"), + } + } + } + + #[allow(unsafe_code, clippy::await_holding_lock)] + #[tokio::test] + async fn install_zsh_respects_zdotdir() { + use tempfile::TempDir; + let tmp = TempDir::new().unwrap(); + let home = tmp.path().join("home"); + let zdotdir = tmp.path().join("zdotdir"); + std::fs::create_dir_all(&home).unwrap(); + std::fs::create_dir_all(&zdotdir).unwrap(); + + let _lock = crate::config::test_env::lock(); + let prev_home = std::env::var("HOME").ok(); + let prev_zdotdir = std::env::var("ZDOTDIR").ok(); + let prev_data = std::env::var("XDG_DATA_HOME").ok(); + let prev_config = std::env::var("XDG_CONFIG_HOME").ok(); + // SAFETY: caller holds XDG_TEST_MUTEX, serializing all mutation. + unsafe { + std::env::set_var("HOME", home.to_str().unwrap()); + std::env::set_var("ZDOTDIR", zdotdir.to_str().unwrap()); + std::env::set_var("XDG_DATA_HOME", tmp.path().join("data").to_str().unwrap()); + std::env::set_var( + "XDG_CONFIG_HOME", + tmp.path().join("config").to_str().unwrap(), + ); + } + + let cmd = Command::new("testbin"); + let result = install(&cmd, "testbin", Shell::Zsh).await.unwrap(); + assert_eq!(result.exit_code, 0); + + // rc should be in ZDOTDIR, not HOME + let zshrc_in_zdotdir = zdotdir.join(".zshrc"); + assert!( + zshrc_in_zdotdir.exists(), + "zshrc should be written to $ZDOTDIR, not $HOME" + ); + let zshrc_in_home = home.join(".zshrc"); + assert!( + !zshrc_in_home.exists(), + "zshrc must NOT be written to $HOME when $ZDOTDIR is set" + ); + + let content = std::fs::read_to_string(&zshrc_in_zdotdir).unwrap(); + assert!(content.contains("# >>> testbin completion (managed) >>>")); + + // SAFETY: restoring state while still holding the lock. + #[allow(clippy::items_after_statements)] + unsafe { + match prev_home { + Some(v) => std::env::set_var("HOME", v), + None => std::env::remove_var("HOME"), + } + match prev_zdotdir { + Some(v) => std::env::set_var("ZDOTDIR", v), + None => std::env::remove_var("ZDOTDIR"), + } + match prev_data { + Some(v) => std::env::set_var("XDG_DATA_HOME", v), + None => std::env::remove_var("XDG_DATA_HOME"), + } + match prev_config { + Some(v) => std::env::set_var("XDG_CONFIG_HOME", v), + None => std::env::remove_var("XDG_CONFIG_HOME"), + } + } + } + + #[allow(unsafe_code, clippy::await_holding_lock)] + #[tokio::test] + async fn install_elvish_writes_script_and_rc() { + use tempfile::TempDir; + let tmp = TempDir::new().unwrap(); + let config_dir = tmp.path().join("config"); + + let _lock = crate::config::test_env::lock(); + let prev_home = std::env::var("HOME").ok(); + let prev_config = std::env::var("XDG_CONFIG_HOME").ok(); + let prev_data = std::env::var("XDG_DATA_HOME").ok(); + // SAFETY: caller holds XDG_TEST_MUTEX, serializing all mutation. + unsafe { + std::env::set_var("HOME", tmp.path().to_str().unwrap()); + std::env::set_var("XDG_CONFIG_HOME", config_dir.to_str().unwrap()); + std::env::set_var("XDG_DATA_HOME", tmp.path().join("data").to_str().unwrap()); + } + + let cmd = Command::new("testbin"); + let result = install(&cmd, "testbin", Shell::Elvish).await.unwrap(); + assert_eq!(result.exit_code, 0); + + let script = config_dir.join("elvish/lib/testbin-completion.elv"); + assert!( + script.exists(), + "elvish script should exist at {}", + script.display() + ); + + let rc = config_dir.join("elvish/rc.elv"); + let content = std::fs::read_to_string(&rc).unwrap(); + assert!(content.contains("# >>> testbin completion (managed) >>>")); + assert!( + content.contains("use testbin-completion"), + "rc must contain the use line; got:\n{content}" + ); + + // SAFETY: restoring state while still holding the lock. + unsafe { + match prev_home { + Some(v) => std::env::set_var("HOME", v), + None => std::env::remove_var("HOME"), + } + match prev_config { + Some(v) => std::env::set_var("XDG_CONFIG_HOME", v), + None => std::env::remove_var("XDG_CONFIG_HOME"), + } + match prev_data { + Some(v) => std::env::set_var("XDG_DATA_HOME", v), + None => std::env::remove_var("XDG_DATA_HOME"), + } + } + } + + #[allow(unsafe_code, clippy::await_holding_lock)] + #[tokio::test] + async fn install_powershell_writes_script_and_profile_with_crlf() { + use tempfile::TempDir; + let tmp = TempDir::new().unwrap(); + let home = tmp.path().join("home"); + std::fs::create_dir_all(&home).unwrap(); + + let _lock = crate::config::test_env::lock(); + let prev_home = std::env::var("HOME").ok(); + let prev_data = std::env::var("XDG_DATA_HOME").ok(); + let prev_config = std::env::var("XDG_CONFIG_HOME").ok(); + // SAFETY: caller holds XDG_TEST_MUTEX, serializing all mutation. + unsafe { + std::env::set_var("HOME", home.to_str().unwrap()); + std::env::set_var("XDG_DATA_HOME", tmp.path().join("data").to_str().unwrap()); + std::env::set_var( + "XDG_CONFIG_HOME", + tmp.path().join("config").to_str().unwrap(), + ); + } + + let cmd = Command::new("testbin"); + let result = install(&cmd, "testbin", Shell::PowerShell).await.unwrap(); + assert_eq!(result.exit_code, 0); + + let script = home.join("Documents/PowerShell/testbin-completion.ps1"); + assert!( + script.exists(), + "powershell script should exist at {}", + script.display() + ); + + let profile = home.join("Documents/PowerShell/Microsoft.PowerShell_profile.ps1"); + let content = std::fs::read_to_string(&profile).unwrap(); + + // Profile must use CRLF line endings. + assert!( + content.contains("\r\n"), + "profile must use CRLF line endings; got:\n{content:?}" + ); + assert!( + !content.contains("\r\r\n"), + "profile must not contain double-CR (CRLF normalization bug)" + ); + assert!(content.contains("# >>> testbin completion (managed) >>>")); + assert!( + content.contains(". \""), + "profile must contain dot-source line" + ); + + // Second install must be idempotent. + install(&cmd, "testbin", Shell::PowerShell).await.unwrap(); + let content2 = std::fs::read_to_string(&profile).unwrap(); + assert_eq!( + content2 + .matches("# >>> testbin completion (managed) >>>") + .count(), + 1, + "re-install must not duplicate the managed block" + ); + // Must still use CRLF after re-run on a CRLF file. + assert!( + !content2.contains("\r\r\n"), + "re-install must not produce double-CR" + ); + + // SAFETY: restoring state while still holding the lock. + unsafe { + match prev_home { + Some(v) => std::env::set_var("HOME", v), + None => std::env::remove_var("HOME"), + } + match prev_data { + Some(v) => std::env::set_var("XDG_DATA_HOME", v), + None => std::env::remove_var("XDG_DATA_HOME"), + } + match prev_config { + Some(v) => std::env::set_var("XDG_CONFIG_HOME", v), + None => std::env::remove_var("XDG_CONFIG_HOME"), + } + } + } +} diff --git a/src/fs.rs b/src/fs.rs index 0d3dfb0..01bff94 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -51,6 +51,25 @@ pub fn config_base_dir() -> Option { .filter(|p| p.is_absolute()) } +/// Returns the user's home directory. +/// +/// On non-Windows platforms this reads `$HOME`. On Windows, `%USERPROFILE%` is +/// tried first, then `$HOME` as a fallback (matching shell environments such as +/// Git Bash that set `HOME`). +/// +/// Only absolute paths are accepted; a relative value is rejected so files +/// never land relative to the current working directory. Returns `None` when +/// no suitable variable is set or the resolved path is relative. +#[must_use] +pub fn home_dir() -> Option { + if cfg!(windows) { + env_path("USERPROFILE").or_else(|| env_path("HOME")) + } else { + env_path("HOME") + } + .filter(|p| p.is_absolute()) +} + /// Returns true only when `s` is a single, non-traversal path component that is /// valid on all supported platforms. /// @@ -97,9 +116,12 @@ pub fn is_safe_path_component(s: &str) -> bool { } /// Writes `contents` to `path` via a uniquely-named temp file then renames it -/// into place. On Unix the rename is atomic, the file is created `0600`, and the -/// parent directory is best-effort restricted to `0700`. On Windows the rename -/// replaces an existing destination but is not crash-atomic. +/// into place. On Unix the rename is atomic, the file is created `0600`, and +/// **newly-created** parent directories are best-effort restricted to `0700`. +/// Pre-existing parent directories are left unchanged so callers that write +/// into established locations (e.g. `$HOME`) do not alter their permissions. +/// On Windows the rename replaces an existing destination but is not +/// crash-atomic. /// /// **Blocking**: this function uses synchronous filesystem I/O. Call it from /// within [`tokio::task::spawn_blocking`] when used in an async context to @@ -110,10 +132,14 @@ pub fn is_safe_path_component(s: &str) -> bool { /// fails. pub fn write_string_atomic(path: &Path, contents: &str) -> crate::Result<()> { if let Some(parent) = path.parent() { + // Record whether the parent already existed so we only restrict + // permissions on directories we create, not on pre-existing ones + // such as $HOME (which other users need to traverse). + let parent_existed = parent.is_dir(); std::fs::create_dir_all(parent) .map_err(|e| CliCoreError::message(format!("failed to create directory: {e}")))?; #[cfg(unix)] - { + if !parent_existed { use std::os::unix::fs::PermissionsExt as _; if let Err(e) = std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)) { @@ -166,7 +192,13 @@ fn write_tmp_file(tmp_path: &Path, contents: &str) -> crate::Result<()> { #[cfg(test)] mod tests { use super::*; - use crate::config::test_env::with_xdg_config_home; + use crate::config::test_env::{EnvVarGuard, lock, with_xdg_config_home}; + + fn with_home R, R>(value: &Path, f: F) -> R { + let _lock = lock(); + let _restore = EnvVarGuard::set("HOME", Some(value)); + f() + } #[test] fn safe_path_component_basic() { @@ -222,6 +254,21 @@ mod tests { }); } + #[test] + fn home_dir_honors_home_env() { + let dir = std::env::temp_dir().join("cli-engine-fs-home-test"); + with_home(&dir, || { + assert_eq!(home_dir(), Some(dir.clone())); + }); + } + + #[test] + fn home_dir_rejects_relative() { + with_home(Path::new("."), || { + assert!(home_dir().is_none(), "relative HOME should be rejected"); + }); + } + #[tokio::test] async fn write_string_atomic_round_trip_creates_dirs() { let tmp = tempfile::tempdir().expect("tempdir"); diff --git a/tests/completion.rs b/tests/completion.rs new file mode 100644 index 0000000..72efe8c --- /dev/null +++ b/tests/completion.rs @@ -0,0 +1,356 @@ +/// Integration tests for the completion built-in via `Cli::run`. +/// +/// These tests exercise the full dispatch path rather than the internal +/// `completion::generate_script` / `completion::install` functions directly. +/// They assert: +/// - every supported shell prints a raw script (no JSON envelope) that +/// references the binary name +/// - `$SHELL` auto-detection picks the correct shell +/// - `--install` writes the expected files under a TempDir HOME +/// - a second `--install` run is idempotent (single managed block) +/// - an unknown shell name exits non-zero without panicking +#[allow(clippy::unwrap_used)] +#[allow(clippy::await_holding_lock)] +mod completion_integration { + use std::sync::{Mutex, MutexGuard}; + + use cli_engine::{BuildInfo, Cli, CliConfig, GroupSpec, Module, RuntimeGroupSpec}; + use serde_json::Value; + use tempfile::TempDir; + + // --------------------------------------------------------------------------- + // Env mutation lock – integration tests are compiled into a separate crate + // and cannot access `pub(crate) config::test_env` from the library. We + // re-implement the minimal env-var isolation helpers here. If the pattern + // changes (e.g. Rust makes set_var safe), update both this file and + // src/config.rs where the canonical version lives. + // --------------------------------------------------------------------------- + static INSTALL_MUTEX: Mutex<()> = Mutex::new(()); + + fn env_lock() -> MutexGuard<'static, ()> { + INSTALL_MUTEX.lock().unwrap_or_else(|e| e.into_inner()) + } + + /// RAII guard that sets an env var and restores its prior value on drop, + /// so a panicking assertion cannot leak state into other tests. + struct EnvVarGuard { + key: &'static str, + prev: Option, + } + + impl EnvVarGuard { + #[allow(unsafe_code)] + fn set(key: &'static str, value: &str) -> Self { + let prev = std::env::var(key).ok(); + // SAFETY: every caller holds INSTALL_MUTEX for the guard's lifetime. + unsafe { std::env::set_var(key, value) }; + Self { key, prev } + } + } + + impl Drop for EnvVarGuard { + #[allow(unsafe_code)] + fn drop(&mut self) { + // SAFETY: the INSTALL_MUTEX guard outlives every EnvVarGuard. + unsafe { + match self.prev.take() { + Some(v) => std::env::set_var(self.key, v), + None => std::env::remove_var(self.key), + } + } + } + } + + // --------------------------------------------------------------------------- + // Minimal test CLI – no real commands needed; completion generates scripts + // from the clap Command tree, which just needs the binary to exist. + // --------------------------------------------------------------------------- + fn demo_cli() -> Cli { + Cli::new( + CliConfig::new("demo", "Demo CLI for completion tests", "demo") + .with_build(BuildInfo::new("0.1.0")) + .with_module(Module::new("Demo", |_ctx| { + RuntimeGroupSpec::new(GroupSpec::new("widget", "Manage widgets")) + })), + ) + } + + // ========================================================================= + // (a) Print tests: each supported shell exits 0 with a raw script containing + // the binary name and no JSON envelope. + // ========================================================================= + + #[tokio::test] + async fn completion_print_bash_is_raw_script() { + let cli = demo_cli(); + let out = cli.run(["demo", "completion", "bash"]).await; + assert_eq!(out.exit_code, 0, "bash: {}", out.rendered); + assert!(!out.rendered.is_empty(), "bash script should be non-empty"); + assert!( + out.rendered.contains("demo"), + "bash script should mention bin name; got: {}", + out.rendered + ); + // Must NOT be a JSON envelope. + assert!( + serde_json::from_str::(&out.rendered).is_err(), + "bash output must not be a JSON envelope; got: {}", + out.rendered + ); + } + + #[tokio::test] + async fn completion_print_zsh_is_raw_script() { + let cli = demo_cli(); + let out = cli.run(["demo", "completion", "zsh"]).await; + assert_eq!(out.exit_code, 0, "zsh: {}", out.rendered); + assert!(!out.rendered.is_empty(), "zsh script should be non-empty"); + assert!( + out.rendered.contains("demo"), + "zsh script should mention bin name; got: {}", + out.rendered + ); + assert!( + serde_json::from_str::(&out.rendered).is_err(), + "zsh output must not be a JSON envelope" + ); + } + + #[tokio::test] + async fn completion_print_fish_is_raw_script() { + let cli = demo_cli(); + let out = cli.run(["demo", "completion", "fish"]).await; + assert_eq!(out.exit_code, 0, "fish: {}", out.rendered); + assert!(!out.rendered.is_empty(), "fish script should be non-empty"); + assert!( + out.rendered.contains("demo"), + "fish script should mention bin name; got: {}", + out.rendered + ); + assert!( + serde_json::from_str::(&out.rendered).is_err(), + "fish output must not be a JSON envelope" + ); + } + + #[tokio::test] + async fn completion_print_powershell_is_raw_script() { + let cli = demo_cli(); + let out = cli.run(["demo", "completion", "powershell"]).await; + assert_eq!(out.exit_code, 0, "powershell: {}", out.rendered); + assert!( + !out.rendered.is_empty(), + "powershell script should be non-empty" + ); + assert!( + out.rendered.contains("demo"), + "powershell script should mention bin name; got: {}", + out.rendered + ); + assert!( + serde_json::from_str::(&out.rendered).is_err(), + "powershell output must not be a JSON envelope" + ); + } + + #[tokio::test] + async fn completion_print_elvish_is_raw_script() { + let cli = demo_cli(); + let out = cli.run(["demo", "completion", "elvish"]).await; + assert_eq!(out.exit_code, 0, "elvish: {}", out.rendered); + assert!( + !out.rendered.is_empty(), + "elvish script should be non-empty" + ); + assert!( + out.rendered.contains("demo"), + "elvish script should mention bin name; got: {}", + out.rendered + ); + assert!( + serde_json::from_str::(&out.rendered).is_err(), + "elvish output must not be a JSON envelope" + ); + } + + // ========================================================================= + // (b) Auto-detect: set $SHELL, call `completion` with no shell arg. + // ========================================================================= + + #[tokio::test] + async fn completion_autodetect_picks_bash_from_shell_env() { + let cli = demo_cli(); + + let _lock = env_lock(); + let _shell = EnvVarGuard::set("SHELL", "/usr/bin/bash"); + + let out = cli.run(["demo", "completion"]).await; + + assert_eq!(out.exit_code, 0, "autodetect bash: {}", out.rendered); + assert!( + out.rendered.contains("demo"), + "auto-detected bash script should mention bin name; got: {}", + out.rendered + ); + assert!( + serde_json::from_str::(&out.rendered).is_err(), + "auto-detected output must not be a JSON envelope" + ); + } + + #[tokio::test] + async fn completion_autodetect_picks_zsh_from_shell_env() { + let cli = demo_cli(); + + let _lock = env_lock(); + let _shell = EnvVarGuard::set("SHELL", "/bin/zsh"); + + let out = cli.run(["demo", "completion"]).await; + + assert_eq!(out.exit_code, 0, "autodetect zsh: {}", out.rendered); + assert!( + out.rendered.contains("demo"), + "auto-detected zsh script should mention bin name; got: {}", + out.rendered + ); + assert!( + serde_json::from_str::(&out.rendered).is_err(), + "auto-detected zsh output must not be a JSON envelope" + ); + } + + // ========================================================================= + // (c) & (d) Install + idempotency — all env-mutating install tests run in a + // single serialized test function to avoid races between them. + // ========================================================================= + + #[allow(unsafe_code)] + #[tokio::test] + async fn completion_install_bash_writes_files_and_is_idempotent() { + let cli = demo_cli(); + + let tmp = TempDir::new().unwrap(); + let home = tmp.path().join("home"); + std::fs::create_dir_all(&home).unwrap(); + let data_dir = tmp.path().join("data"); + std::fs::create_dir_all(&data_dir).unwrap(); + let config_dir = tmp.path().join("config"); + std::fs::create_dir_all(&config_dir).unwrap(); + + let _lock = env_lock(); + let _home = EnvVarGuard::set("HOME", home.to_str().unwrap()); + let _data = EnvVarGuard::set("XDG_DATA_HOME", data_dir.to_str().unwrap()); + let _config = EnvVarGuard::set("XDG_CONFIG_HOME", config_dir.to_str().unwrap()); + + // First install. + let out = cli.run(["demo", "completion", "--install", "bash"]).await; + + assert_eq!(out.exit_code, 0, "install bash first run: {}", out.rendered); + + // The completion script must land under the tempdir data dir. + let script = data_dir.join("bash-completion/completions/demo"); + assert!( + script.exists(), + "bash completion script should exist at {}", + script.display() + ); + assert!( + script.starts_with(tmp.path()), + "script path must be under tempdir, not real HOME" + ); + + // The .bashrc managed block must exist. + let bashrc = home.join(".bashrc"); + let content1 = std::fs::read_to_string(&bashrc).unwrap(); + assert!( + content1.contains("# >>> demo completion (managed) >>>"), + ".bashrc must contain opening managed block marker; got:\n{content1}" + ); + assert!( + content1.contains("# <<< demo completion (managed) <<<"), + ".bashrc must contain closing managed block marker; got:\n{content1}" + ); + assert_eq!( + content1 + .matches("# >>> demo completion (managed) >>>") + .count(), + 1, + "first install: exactly one managed block" + ); + + // Second install: idempotent. + let out2 = cli.run(["demo", "completion", "--install", "bash"]).await; + assert_eq!( + out2.exit_code, 0, + "install bash second run: {}", + out2.rendered + ); + + let content2 = std::fs::read_to_string(&bashrc).unwrap(); + assert_eq!( + content2 + .matches("# >>> demo completion (managed) >>>") + .count(), + 1, + "re-install must not duplicate the managed block" + ); + } + + #[tokio::test] + async fn completion_install_fish_writes_script_under_config_home() { + let cli = demo_cli(); + + let tmp = TempDir::new().unwrap(); + let home = tmp.path().join("home"); + std::fs::create_dir_all(&home).unwrap(); + let data_dir = tmp.path().join("data"); + let config_dir = tmp.path().join("config"); + + let _lock = env_lock(); + let _home = EnvVarGuard::set("HOME", home.to_str().unwrap()); + let _data = EnvVarGuard::set("XDG_DATA_HOME", data_dir.to_str().unwrap()); + let _config = EnvVarGuard::set("XDG_CONFIG_HOME", config_dir.to_str().unwrap()); + + let out = cli.run(["demo", "completion", "--install", "fish"]).await; + assert_eq!(out.exit_code, 0, "install fish: {}", out.rendered); + + let fish_script = config_dir.join("fish/completions/demo.fish"); + assert!( + fish_script.exists(), + "fish completion script should exist at {}", + fish_script.display() + ); + assert!( + fish_script.starts_with(tmp.path()), + "fish script must be under tempdir, not real HOME" + ); + } + + // ========================================================================= + // (e) Unknown shell → non-zero exit, no panic. + // ========================================================================= + + #[tokio::test] + async fn completion_unknown_shell_exits_nonzero_no_panic() { + let cli = demo_cli(); + let out = cli.run(["demo", "completion", "notashell"]).await; + assert_ne!(out.exit_code, 0, "unknown shell must exit non-zero"); + assert!( + out.rendered.contains("notashell") || out.rendered.contains("unsupported"), + "error should mention the bad shell name; got: {}", + out.rendered + ); + // Must not be empty (error was surfaced, not silently swallowed). + assert!(!out.rendered.is_empty(), "error output must not be empty"); + } + + #[tokio::test] + async fn completion_install_unknown_shell_exits_nonzero_no_panic() { + let cli = demo_cli(); + let out = cli + .run(["demo", "completion", "--install", "notashell"]) + .await; + assert_ne!(out.exit_code, 0, "install unknown shell must exit non-zero"); + assert!(!out.rendered.is_empty(), "error output must not be empty"); + } +} diff --git a/tests/consumer_cli.rs b/tests/consumer_cli.rs index bac61f5..3ee55aa 100644 --- a/tests/consumer_cli.rs +++ b/tests/consumer_cli.rs @@ -601,3 +601,31 @@ async fn view_id_takes_precedence_over_inline_view() { assert!(human.rendered.contains("active"), "{}", human.rendered); assert!(!human.rendered.contains("ID"), "{}", human.rendered); } + +#[tokio::test] +async fn completion_print_bash_exits_zero_with_script() { + let cli = consumer_cli(); + let out = cli.run(["my-cli", "completion", "bash"]).await; + assert_eq!(out.exit_code, 0, "rendered: {}", out.rendered); + assert!( + out.rendered.contains("my-cli"), + "script should reference binary name; got: {}", + out.rendered + ); + assert!( + !out.rendered.is_empty(), + "script should be non-empty; got empty string" + ); +} + +#[tokio::test] +async fn completion_print_unknown_shell_exits_nonzero() { + let cli = consumer_cli(); + let out = cli.run(["my-cli", "completion", "notashell"]).await; + assert_ne!(out.exit_code, 0, "should fail for unknown shell"); + assert!( + out.rendered.contains("notashell") || out.rendered.contains("unsupported"), + "error should mention the bad shell; got: {}", + out.rendered + ); +}