From a84853de121cad8c82f6a7c521190f2ff498d09c Mon Sep 17 00:00:00 2001 From: Mustafa J Date: Wed, 18 Mar 2026 14:02:08 +0300 Subject: [PATCH 1/5] choices: support env-backed values across parse/docs/completion --- cli/src/cli/complete_word.rs | 14 ++- cli/src/cli/generate/fig.rs | 2 +- cli/tests/complete_word.rs | 10 ++ docs/spec/reference/arg.md | 4 + docs/spec/reference/flag.md | 4 + examples/env-choices.usage.kdl | 3 + lib/src/docs/models.rs | 6 +- lib/src/parse.rs | 208 ++++++++++++++++++++++++--------- lib/src/spec/arg.rs | 2 +- lib/src/spec/builder.rs | 22 ++++ lib/src/spec/choices.rs | 95 ++++++++++++++- lib/src/spec/flag.rs | 2 +- lib/tests/dump.rs | 4 + 13 files changed, 306 insertions(+), 70 deletions(-) create mode 100644 examples/env-choices.usage.kdl diff --git a/cli/src/cli/complete_word.rs b/cli/src/cli/complete_word.rs index 971be347..bc00314b 100644 --- a/cli/src/cli/complete_word.rs +++ b/cli/src/cli/complete_word.rs @@ -266,12 +266,14 @@ impl CompleteWord { } if let Some(choices) = &arg.choices { - return Ok(choices - .choices - .iter() - .map(|c| (c.clone(), String::new())) - .filter(|(c, _)| c.starts_with(ctoken)) - .collect()); + let values = choices.values(); + if !values.is_empty() { + return Ok(values + .into_iter() + .map(|c| (c, String::new())) + .filter(|(c, _)| c.starts_with(ctoken)) + .collect()); + } } if let Some(run) = &complete.run { let run = tera::Tera::one_off(run, ctx, false).into_diagnostic()?; diff --git a/cli/src/cli/generate/fig.rs b/cli/src/cli/generate/fig.rs index aa40775a..2a6a0455 100644 --- a/cli/src/cli/generate/fig.rs +++ b/cli/src/cli/generate/fig.rs @@ -222,7 +222,7 @@ impl FigArg { is_optional: !arg.required, template: FigArg::get_template(&arg.name), generators: FigArg::get_generator(&arg.name), - suggestions: arg.choices.clone().map(|c| c.choices).unwrap_or_default(), + suggestions: arg.choices.as_ref().map(|c| c.values()).unwrap_or_default(), debounce: FigArg::get_generator(&arg.name).map(|_| true), } } diff --git a/cli/tests/complete_word.rs b/cli/tests/complete_word.rs index 9d5fbc0e..6d11c47c 100644 --- a/cli/tests/complete_word.rs +++ b/cli/tests/complete_word.rs @@ -68,6 +68,16 @@ fn complete_word_choices() { .stdout("bash\nelvish\nfish\nnu\nxonsh\nzsh\npwsh\n"); } +#[test] +fn complete_word_choices_from_env() { + cmd("env-choices.usage.kdl", Some("fish")) + .env("DEPLOY_ENVS", "foo,bar baz") + .args(["--", "--env", ""]) + .assert() + .success() + .stdout("foo\nbar\nbaz\n"); +} + #[test] fn complete_word_shebang() { assert_cmd("example.sh", &["--", "-"]) diff --git a/docs/spec/reference/arg.md b/docs/spec/reference/arg.md index 8ddff213..9853efcc 100644 --- a/docs/spec/reference/arg.md +++ b/docs/spec/reference/arg.md @@ -43,6 +43,10 @@ arg "" { choices "bash" "zsh" "fish" // must be one of the choices } +arg "" { + choices env="DEPLOY_ENVS" // values from $DEPLOY_ENVS, split on commas and/or whitespace +} + arg "" long_help="longer help for --help (as oppoosed to -h)" // double-dash behavior diff --git a/docs/spec/reference/flag.md b/docs/spec/reference/flag.md index 1965cb92..4db1fa08 100644 --- a/docs/spec/reference/flag.md +++ b/docs/spec/reference/flag.md @@ -36,6 +36,10 @@ flag "--shell " { choices "bash" "zsh" "fish" // must be one of the choices } +flag "--env " { + choices env="DEPLOY_ENVS" // values from $DEPLOY_ENVS, split on commas and/or whitespace +} + flag "--file " long_help="longer help for --help (as opposed to -h)" // this is equivalent to the above but preferred when a lot of space is needed flag "--file " { diff --git a/examples/env-choices.usage.kdl b/examples/env-choices.usage.kdl new file mode 100644 index 00000000..73492d96 --- /dev/null +++ b/examples/env-choices.usage.kdl @@ -0,0 +1,3 @@ +flag "--env " { + choices env="DEPLOY_ENVS" +} diff --git a/lib/src/docs/models.rs b/lib/src/docs/models.rs index b9d08a77..8f019011 100644 --- a/lib/src/docs/models.rs +++ b/lib/src/docs/models.rs @@ -294,7 +294,11 @@ impl From<&crate::SpecArg> for SpecArg { var_max: arg.var_max, hide: arg.hide, default: arg.default.clone(), - choices: arg.choices.clone(), + choices: arg + .choices + .as_ref() + .map(SpecChoices::resolved) + .filter(|choices| !choices.choices.is_empty()), env: arg.env.clone(), rendered: false, help_rendered: None, diff --git a/lib/src/parse.rs b/lib/src/parse.rs index 333fca84..38e33331 100644 --- a/lib/src/parse.rs +++ b/lib/src/parse.rs @@ -12,7 +12,7 @@ use strum::EnumTryAs; use crate::docs; use crate::error::UsageErr; use crate::spec::arg::SpecDoubleDashChoices; -use crate::{Spec, SpecArg, SpecCommand, SpecFlag}; +use crate::{Spec, SpecArg, SpecChoices, SpecCommand, SpecFlag}; /// Extract the flag key from a flag word for lookup in available_flags map /// Handles both long flags (--flag, --flag=value) and short flags (-f) @@ -472,21 +472,19 @@ fn parse_partial_with_env( if !out.flag_awaiting_value.is_empty() { while let Some(flag) = out.flag_awaiting_value.pop() { let arg = flag.arg.as_ref().unwrap(); + if validate_choices( + spec, + &out.cmd, + &mut out.errors, + "option", + &flag.name, + &w, + arg.choices.as_ref(), + custom_env, + )? { + return Ok(out); + } if flag.var { - if let Some(choices) = &arg.choices { - if !choices.choices.contains(&w) { - if is_help_arg(spec, &w) { - out.errors - .push(render_help_err(spec, &out.cmd, w.len() > 2)); - return Ok(out); - } - bail!( - "Invalid choice for option {}: {w}, expected one of {}", - flag.name, - choices.choices.join(", ") - ); - } - } let arr = out .flags .entry(flag) @@ -495,20 +493,6 @@ fn parse_partial_with_env( .unwrap(); arr.push(w); } else { - if let Some(choices) = &arg.choices { - if !choices.choices.contains(&w) { - if is_help_arg(spec, &w) { - out.errors - .push(render_help_err(spec, &out.cmd, w.len() > 2)); - return Ok(out); - } - bail!( - "Invalid choice for option {}: {w}, expected one of {}", - flag.name, - choices.choices.join(", ") - ); - } - } out.flags.insert(flag, ParseValue::String(w)); } w = "".to_string(); @@ -517,21 +501,19 @@ fn parse_partial_with_env( } if let Some(arg) = next_arg { + if validate_choices( + spec, + &out.cmd, + &mut out.errors, + "arg", + &arg.name, + &w, + arg.choices.as_ref(), + custom_env, + )? { + return Ok(out); + } if arg.var { - if let Some(choices) = &arg.choices { - if !choices.choices.contains(&w) { - if is_help_arg(spec, &w) { - out.errors - .push(render_help_err(spec, &out.cmd, w.len() > 2)); - return Ok(out); - } - bail!( - "Invalid choice for arg {}: {w}, expected one of {}", - arg.name, - choices.choices.join(", ") - ); - } - } let arr = out .args .entry(Arc::new(arg.clone())) @@ -543,20 +525,6 @@ fn parse_partial_with_env( next_arg = out.cmd.args.get(out.args.len()); } } else { - if let Some(choices) = &arg.choices { - if !choices.choices.contains(&w) { - if is_help_arg(spec, &w) { - out.errors - .push(render_help_err(spec, &out.cmd, w.len() > 2)); - return Ok(out); - } - bail!( - "Invalid choice for arg {}: {w}, expected one of {}", - arg.name, - choices.choices.join(", ") - ); - } - } out.args .insert(Arc::new(arg.clone()), ParseValue::String(w)); next_arg = out.cmd.args.get(out.args.len()); @@ -667,6 +635,32 @@ fn render_help_err(_spec: &Spec, _cmd: &SpecCommand, _long: bool) -> UsageErr { UsageErr::Help("help".to_string()) } +fn validate_choices( + spec: &Spec, + cmd: &SpecCommand, + errors: &mut Vec, + kind: &str, + name: &str, + value: &str, + choices: Option<&SpecChoices>, + custom_env: Option<&HashMap>, +) -> miette::Result { + if let Some(choices) = choices { + let values = choices.values_with_env(custom_env); + if !values.is_empty() && !values.iter().any(|choice| choice == value) { + if is_help_arg(spec, value) { + errors.push(render_help_err(spec, cmd, value.len() > 2)); + return Ok(true); + } + bail!( + "Invalid choice for {kind} {name}: {value}, expected one of {}", + values.join(", ") + ); + } + } + Ok(false) +} + fn is_help_arg(spec: &Spec, w: &str) -> bool { spec.disable_help != Some(true) && (w == "--help" @@ -1775,6 +1769,106 @@ mod tests { assert!(result.is_err()); } + #[test] + fn test_parser_validates_arg_choices_from_custom_env() { + let cmd = SpecCommand::builder() + .name("test") + .arg( + SpecArg::builder() + .name("env") + .choices_env("USAGE_TEST_DEPLOY_ENVS_ARG") + .build(), + ) + .build(); + let spec = Spec { + name: "test".to_string(), + bin: "test".to_string(), + cmd, + ..Default::default() + }; + + std::env::remove_var("USAGE_TEST_DEPLOY_ENVS_ARG"); + + let env = HashMap::from([( + "USAGE_TEST_DEPLOY_ENVS_ARG".to_string(), + "foo,bar baz".to_string(), + )]); + let input = vec!["test".to_string(), "bar".to_string()]; + let parsed = Parser::new(&spec).with_env(env).parse(&input).unwrap(); + + let value = parsed.args.values().next().unwrap(); + assert_eq!(value.to_string(), "bar"); + } + + #[test] + fn test_parser_rejects_arg_choices_from_custom_env() { + let cmd = SpecCommand::builder() + .name("test") + .arg( + SpecArg::builder() + .name("env") + .choices_env("USAGE_TEST_DEPLOY_ENVS_ARG_ERR") + .build(), + ) + .build(); + let spec = Spec { + name: "test".to_string(), + bin: "test".to_string(), + cmd, + ..Default::default() + }; + + std::env::remove_var("USAGE_TEST_DEPLOY_ENVS_ARG_ERR"); + + let env = HashMap::from([( + "USAGE_TEST_DEPLOY_ENVS_ARG_ERR".to_string(), + "foo,bar baz".to_string(), + )]); + let input = vec!["test".to_string(), "prod".to_string()]; + let err = Parser::new(&spec).with_env(env).parse(&input).unwrap_err(); + + assert_eq!( + format!("{err}"), + "Invalid choice for arg env: prod, expected one of foo, bar, baz" + ); + } + + #[test] + fn test_parser_validates_flag_choices_from_custom_env() { + let cmd = SpecCommand::builder() + .name("test") + .flag( + SpecFlag::builder() + .long("env") + .arg( + SpecArg::builder() + .name("env") + .choices_env("USAGE_TEST_DEPLOY_ENVS_FLAG") + .build(), + ) + .build(), + ) + .build(); + let spec = Spec { + name: "test".to_string(), + bin: "test".to_string(), + cmd, + ..Default::default() + }; + + std::env::remove_var("USAGE_TEST_DEPLOY_ENVS_FLAG"); + + let env = HashMap::from([( + "USAGE_TEST_DEPLOY_ENVS_FLAG".to_string(), + "foo,bar baz".to_string(), + )]); + let input = vec!["test".to_string(), "--env".to_string(), "baz".to_string()]; + let parsed = Parser::new(&spec).with_env(env).parse(&input).unwrap(); + + let value = parsed.flags.values().next().unwrap(); + assert_eq!(value.to_string(), "baz"); + } + #[test] fn test_variadic_arg_captures_unknown_flags_from_spec_string() { let spec: Spec = r#" diff --git a/lib/src/spec/arg.rs b/lib/src/spec/arg.rs index f61e2a66..91672a10 100644 --- a/lib/src/spec/arg.rs +++ b/lib/src/spec/arg.rs @@ -342,7 +342,7 @@ impl From<&clap::Arg> for SpecArg { env: None, }; if !choices.is_empty() { - arg.choices = Some(SpecChoices { choices }); + arg.choices = Some(SpecChoices { choices, env: None }); } arg diff --git a/lib/src/spec/builder.rs b/lib/src/spec/builder.rs index 9c125c91..250052ea 100644 --- a/lib/src/spec/builder.rs +++ b/lib/src/spec/builder.rs @@ -309,10 +309,18 @@ impl SpecArgBuilder { { self.inner.choices = Some(SpecChoices { choices: choices.into_iter().map(Into::into).collect(), + env: None, }); self } + /// Set choices from an environment variable + pub fn choices_env(mut self, env: impl Into) -> Self { + let choices = self.inner.choices.get_or_insert_with(SpecChoices::default); + choices.env = Some(env.into()); + self + } + /// Build the final SpecArg #[must_use] pub fn build(mut self) -> SpecArg { @@ -678,6 +686,20 @@ mod tests { choices.choices, vec!["json".to_string(), "yaml".to_string(), "toml".to_string()] ); + assert_eq!(choices.env, None); + } + + #[test] + fn test_arg_builder_choices_env() { + let arg = SpecArgBuilder::new() + .name("env") + .choices(["local"]) + .choices_env("DEPLOY_ENVS") + .build(); + + let choices = arg.choices.unwrap(); + assert_eq!(choices.choices, vec!["local".to_string()]); + assert_eq!(choices.env, Some("DEPLOY_ENVS".to_string())); } #[test] diff --git a/lib/src/spec/choices.rs b/lib/src/spec/choices.rs index ffe8595a..68cab5a4 100644 --- a/lib/src/spec/choices.rs +++ b/lib/src/spec/choices.rs @@ -1,5 +1,6 @@ -use kdl::KdlNode; +use kdl::{KdlEntry, KdlNode}; use serde::{Deserialize, Serialize}; +use std::collections::HashMap; use crate::error::UsageErr; use crate::spec::context::ParsingContext; @@ -8,18 +9,70 @@ use crate::spec::helpers::NodeHelper; #[derive(Debug, Default, Clone, Serialize, Deserialize)] pub struct SpecChoices { pub choices: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub env: Option, } impl SpecChoices { - pub(crate) fn parse(_ctx: &ParsingContext, node: &NodeHelper) -> Result { + pub(crate) fn parse(ctx: &ParsingContext, node: &NodeHelper) -> Result { let mut config = Self::default(); - node.ensure_arg_len(1..)?; config.choices = node .args() .map(|e| e.ensure_string()) .collect::>()?; + + for (k, v) in node.props() { + match k { + "env" => config.env = Some(v.ensure_string()?), + k => bail_parse!(ctx, v.entry.span(), "unsupported choices key {k}"), + } + } + + if config.choices.is_empty() && config.env.is_none() { + bail_parse!( + ctx, + node.span(), + "choices must have at least 1 argument or env property" + ); + } + Ok(config) } + + pub fn values(&self) -> Vec { + self.values_with_env(None) + } + + pub(crate) fn values_with_env(&self, env: Option<&HashMap>) -> Vec { + let mut values = self.choices.clone(); + + if let Some(env_key) = &self.env { + let env_value = env + .and_then(|env_map| env_map.get(env_key).cloned()) + .or_else(|| std::env::var(env_key).ok()); + + if let Some(env_value) = env_value { + for choice in env_value + .split(|c: char| c == ',' || c.is_whitespace()) + .filter(|choice| !choice.is_empty()) + { + let choice = choice.to_string(); + if !values.contains(&choice) { + values.push(choice); + } + } + } + } + + values + } + + pub fn resolved(&self) -> Self { + Self { + choices: self.values(), + env: None, + } + } } impl From<&SpecChoices> for KdlNode { @@ -28,6 +81,42 @@ impl From<&SpecChoices> for KdlNode { for choice in &arg.choices { node.push(choice.to_string()); } + if let Some(env) = &arg.env { + node.push(KdlEntry::new_prop("env", env.clone())); + } node } } + +#[cfg(test)] +mod tests { + use super::SpecChoices; + use std::collections::HashMap; + + #[test] + fn values_with_env_splits_on_commas_and_whitespace() { + let choices = SpecChoices { + choices: vec!["local".into()], + env: Some("DEPLOY_ENVS".into()), + }; + + let env = HashMap::from([("DEPLOY_ENVS".to_string(), "foo,bar baz\nqux".to_string())]); + + assert_eq!( + choices.values_with_env(Some(&env)), + vec!["local", "foo", "bar", "baz", "qux"] + ); + } + + #[test] + fn values_with_env_deduplicates_existing_choices() { + let choices = SpecChoices { + choices: vec!["foo".into()], + env: Some("DEPLOY_ENVS".into()), + }; + + let env = HashMap::from([("DEPLOY_ENVS".to_string(), "foo,bar foo".to_string())]); + + assert_eq!(choices.values_with_env(Some(&env)), vec!["foo", "bar"]); + } +} diff --git a/lib/src/spec/flag.rs b/lib/src/spec/flag.rs index 2705a209..0807154f 100644 --- a/lib/src/spec/flag.rs +++ b/lib/src/spec/flag.rs @@ -385,7 +385,7 @@ impl From<&clap::Arg> for SpecFlag { .flat_map(|v| v.get_name_and_aliases().map(|s| s.to_string())) .collect::>(); if !choices.is_empty() { - arg.choices = Some(SpecChoices { choices }); + arg.choices = Some(SpecChoices { choices, env: None }); } Some(arg) diff --git a/lib/tests/dump.rs b/lib/tests/dump.rs index 813ae608..c38d2478 100644 --- a/lib/tests/dump.rs +++ b/lib/tests/dump.rs @@ -26,5 +26,9 @@ tests_same! { choices bash fish zsh }"#, + arg_choices_env: r#"arg { + choices env=DEPLOY_ENVS +}"#, + double_dash: r#"arg "<-- shell>…" var=#true"#, } From 55196528b5b290a6e0e006dd73b684c9bc1105b2 Mon Sep 17 00:00:00 2001 From: Mustafa J Date: Wed, 18 Mar 2026 15:13:20 +0300 Subject: [PATCH 2/5] choices,docs,fig: fix env-backed review feedback --- cli/src/cli/generate/fig.rs | 2 +- .../cli/templates/spec_template_long.tera | 10 +++++-- .../cli/templates/spec_template_short.tera | 6 ++-- .../markdown/templates/arg_template.md.tera | 6 +++- .../markdown/templates/flag_template.md.tera | 6 +++- lib/src/docs/models.rs | 6 +--- lib/src/parse.rs | 6 ---- lib/src/spec/choices.rs | 28 ++++++++++++------- 8 files changed, 42 insertions(+), 28 deletions(-) diff --git a/cli/src/cli/generate/fig.rs b/cli/src/cli/generate/fig.rs index 2a6a0455..aa40775a 100644 --- a/cli/src/cli/generate/fig.rs +++ b/cli/src/cli/generate/fig.rs @@ -222,7 +222,7 @@ impl FigArg { is_optional: !arg.required, template: FigArg::get_template(&arg.name), generators: FigArg::get_generator(&arg.name), - suggestions: arg.choices.as_ref().map(|c| c.values()).unwrap_or_default(), + suggestions: arg.choices.clone().map(|c| c.choices).unwrap_or_default(), debounce: FigArg::get_generator(&arg.name).map(|_| true), } } diff --git a/lib/src/docs/cli/templates/spec_template_long.tera b/lib/src/docs/cli/templates/spec_template_long.tera index 93493b20..d960720f 100644 --- a/lib/src/docs/cli/templates/spec_template_long.tera +++ b/lib/src/docs/cli/templates/spec_template_long.tera @@ -36,9 +36,12 @@ Arguments: {{ help | indent(width=2) }} {%- endif %} {%- endif %} -{%- if arg.choices %} +{%- if arg.choices and arg.choices.choices %} [possible values: {{ arg.choices.choices | join(sep=", ") }}] {%- endif %} +{%- if arg.choices and arg.choices.env %} + [choices env: {{ arg.choices.env }}] +{%- endif %} {%- if arg.env %} [env: {{ arg.env }}] {%- endif %} @@ -65,9 +68,12 @@ Flags: {{ help | indent(width=2) }} {%- endif %} {%- endif %} -{%- if flag.arg.choices %} +{%- if flag.arg.choices and flag.arg.choices.choices %} [possible values: {{ flag.arg.choices.choices | join(sep=", ") }}] {%- endif %} +{%- if flag.arg.choices and flag.arg.choices.env %} + [choices env: {{ flag.arg.choices.env }}] +{%- endif %} {%- if flag.env %} [env: {{ flag.env }}] {%- endif %} diff --git a/lib/src/docs/cli/templates/spec_template_short.tera b/lib/src/docs/cli/templates/spec_template_short.tera index ebace42e..20ed4e12 100644 --- a/lib/src/docs/cli/templates/spec_template_short.tera +++ b/lib/src/docs/cli/templates/spec_template_short.tera @@ -20,7 +20,8 @@ Arguments: {%- for arg in cmd.args %} {{ arg.usage | trim }} {%- if arg.help %} {{ arg.help }}{%- endif %} -{%- if arg.choices %} [{{ arg.choices.choices | join(sep=", ") }}]{%- endif %} +{%- if arg.choices and arg.choices.choices %} [{{ arg.choices.choices | join(sep=", ") }}]{%- endif %} +{%- if arg.choices and arg.choices.env %} [choices env: {{ arg.choices.env }}]{%- endif %} {%- if arg.env %} [env: {{ arg.env }}]{%- endif %} {%- if arg.default %} (default: {{ arg.default | join(sep=", ") }}){%- endif %} {%- endfor %} @@ -33,7 +34,8 @@ Flags: {{ flag.usage | trim }} {%- if flag.aliases %} [aliases: {{ flag.aliases | join(sep=", ") }}]{% endif %} {%- if flag.help %} {{ flag.help }}{%- endif %} -{%- if flag.arg.choices %} [{{ flag.arg.choices.choices | join(sep=", ") }}]{%- endif %} +{%- if flag.arg.choices and flag.arg.choices.choices %} [{{ flag.arg.choices.choices | join(sep=", ") }}]{%- endif %} +{%- if flag.arg.choices and flag.arg.choices.env %} [choices env: {{ flag.arg.choices.env }}]{%- endif %} {%- if flag.env %} [env: {{ flag.env }}]{%- endif %} {%- endfor %} {%- endif -%} diff --git a/lib/src/docs/markdown/templates/arg_template.md.tera b/lib/src/docs/markdown/templates/arg_template.md.tera index 66e3f10a..f15fe68d 100644 --- a/lib/src/docs/markdown/templates/arg_template.md.tera +++ b/lib/src/docs/markdown/templates/arg_template.md.tera @@ -2,13 +2,17 @@ {{ arg.help_md | escape_md }} {%- endif %} -{%- if arg.choices %} +{%- if arg.choices and arg.choices.choices %} **Choices:** {% for choice in arg.choices.choices %} - `{{ choice }}` {%- endfor %} {%- endif %} +{%- if arg.choices and arg.choices.env %} + +**Choices Environment Variable:** `{{ arg.choices.env }}` +{%- endif %} {%- if arg.default | length > 0 %} **Default:** {% for d in arg.default %}`{{ d }}`{% if not loop.last %}, {% endif %}{% endfor %} diff --git a/lib/src/docs/markdown/templates/flag_template.md.tera b/lib/src/docs/markdown/templates/flag_template.md.tera index bdcdca0d..a01a665f 100644 --- a/lib/src/docs/markdown/templates/flag_template.md.tera +++ b/lib/src/docs/markdown/templates/flag_template.md.tera @@ -2,13 +2,17 @@ {{ flag.help_md | escape_md }} {%- endif %} -{%- if flag.arg.choices %} +{%- if flag.arg.choices and flag.arg.choices.choices %} **Choices:** {% for choice in flag.arg.choices.choices %} - `{{ choice }}` {%- endfor %} {%- endif %} +{%- if flag.arg.choices and flag.arg.choices.env %} + +**Choices Environment Variable:** `{{ flag.arg.choices.env }}` +{%- endif %} {%- if flag.default | length > 0 %} **Default:** {% for d in flag.default %}`{{ d }}`{% if not loop.last %}, {% endif %}{% endfor %} diff --git a/lib/src/docs/models.rs b/lib/src/docs/models.rs index 8f019011..b9d08a77 100644 --- a/lib/src/docs/models.rs +++ b/lib/src/docs/models.rs @@ -294,11 +294,7 @@ impl From<&crate::SpecArg> for SpecArg { var_max: arg.var_max, hide: arg.hide, default: arg.default.clone(), - choices: arg - .choices - .as_ref() - .map(SpecChoices::resolved) - .filter(|choices| !choices.choices.is_empty()), + choices: arg.choices.clone(), env: arg.env.clone(), rendered: false, help_rendered: None, diff --git a/lib/src/parse.rs b/lib/src/parse.rs index 38e33331..5580dbfe 100644 --- a/lib/src/parse.rs +++ b/lib/src/parse.rs @@ -1787,8 +1787,6 @@ mod tests { ..Default::default() }; - std::env::remove_var("USAGE_TEST_DEPLOY_ENVS_ARG"); - let env = HashMap::from([( "USAGE_TEST_DEPLOY_ENVS_ARG".to_string(), "foo,bar baz".to_string(), @@ -1818,8 +1816,6 @@ mod tests { ..Default::default() }; - std::env::remove_var("USAGE_TEST_DEPLOY_ENVS_ARG_ERR"); - let env = HashMap::from([( "USAGE_TEST_DEPLOY_ENVS_ARG_ERR".to_string(), "foo,bar baz".to_string(), @@ -1856,8 +1852,6 @@ mod tests { ..Default::default() }; - std::env::remove_var("USAGE_TEST_DEPLOY_ENVS_FLAG"); - let env = HashMap::from([( "USAGE_TEST_DEPLOY_ENVS_FLAG".to_string(), "foo,bar baz".to_string(), diff --git a/lib/src/spec/choices.rs b/lib/src/spec/choices.rs index 68cab5a4..05f3568b 100644 --- a/lib/src/spec/choices.rs +++ b/lib/src/spec/choices.rs @@ -47,9 +47,11 @@ impl SpecChoices { let mut values = self.choices.clone(); if let Some(env_key) = &self.env { - let env_value = env - .and_then(|env_map| env_map.get(env_key).cloned()) - .or_else(|| std::env::var(env_key).ok()); + let env_value = if let Some(env_map) = env { + env_map.get(env_key).cloned() + } else { + std::env::var(env_key).ok() + }; if let Some(env_value) = env_value { for choice in env_value @@ -66,13 +68,6 @@ impl SpecChoices { values } - - pub fn resolved(&self) -> Self { - Self { - choices: self.values(), - env: None, - } - } } impl From<&SpecChoices> for KdlNode { @@ -119,4 +114,17 @@ mod tests { assert_eq!(choices.values_with_env(Some(&env)), vec!["foo", "bar"]); } + + #[test] + fn values_with_env_does_not_fallback_when_custom_env_is_present() { + let choices = SpecChoices { + choices: vec!["local".into()], + env: Some("USAGE_TEST_CHOICES_ENV_DOES_NOT_EXIST_A5E0F4D1".into()), + }; + + assert_eq!( + choices.values_with_env(Some(&HashMap::new())), + vec!["local"] + ); + } } From c49eff4d7dfbe551814c8e14563efd97ca36ecbf Mon Sep 17 00:00:00 2001 From: Mustafa J Date: Wed, 18 Mar 2026 18:14:27 +0300 Subject: [PATCH 3/5] parse,builder: fix empty env choices and preserve choices_env --- lib/src/parse.rs | 39 ++++++++++++++++++++++++++++++++++++++- lib/src/spec/builder.rs | 19 +++++++++++++++---- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/lib/src/parse.rs b/lib/src/parse.rs index 5580dbfe..4fa90177 100644 --- a/lib/src/parse.rs +++ b/lib/src/parse.rs @@ -647,11 +647,18 @@ fn validate_choices( ) -> miette::Result { if let Some(choices) = choices { let values = choices.values_with_env(custom_env); - if !values.is_empty() && !values.iter().any(|choice| choice == value) { + if !values.iter().any(|choice| choice == value) { if is_help_arg(spec, value) { errors.push(render_help_err(spec, cmd, value.len() > 2)); return Ok(true); } + if let Some(env) = &choices.env { + if values.is_empty() { + bail!( + "Invalid choice for {kind} {name}: {value}, no choices resolved from env {env}" + ); + } + } bail!( "Invalid choice for {kind} {name}: {value}, expected one of {}", values.join(", ") @@ -1829,6 +1836,36 @@ mod tests { ); } + #[test] + fn test_parser_rejects_arg_choices_when_env_resolves_to_no_values() { + let cmd = SpecCommand::builder() + .name("test") + .arg( + SpecArg::builder() + .name("env") + .choices_env("USAGE_TEST_DEPLOY_ENVS_ARG_EMPTY") + .build(), + ) + .build(); + let spec = Spec { + name: "test".to_string(), + bin: "test".to_string(), + cmd, + ..Default::default() + }; + + let input = vec!["test".to_string(), "prod".to_string()]; + let err = Parser::new(&spec) + .with_env(HashMap::new()) + .parse(&input) + .unwrap_err(); + + assert_eq!( + format!("{err}"), + "Invalid choice for arg env: prod, no choices resolved from env USAGE_TEST_DEPLOY_ENVS_ARG_EMPTY" + ); + } + #[test] fn test_parser_validates_flag_choices_from_custom_env() { let cmd = SpecCommand::builder() diff --git a/lib/src/spec/builder.rs b/lib/src/spec/builder.rs index 250052ea..0bdc036d 100644 --- a/lib/src/spec/builder.rs +++ b/lib/src/spec/builder.rs @@ -307,10 +307,8 @@ impl SpecArgBuilder { I: IntoIterator, S: Into, { - self.inner.choices = Some(SpecChoices { - choices: choices.into_iter().map(Into::into).collect(), - env: None, - }); + let spec_choices = self.inner.choices.get_or_insert_with(SpecChoices::default); + spec_choices.choices = choices.into_iter().map(Into::into).collect(); self } @@ -702,6 +700,19 @@ mod tests { assert_eq!(choices.env, Some("DEPLOY_ENVS".to_string())); } + #[test] + fn test_arg_builder_choices_preserves_choices_env() { + let arg = SpecArgBuilder::new() + .name("env") + .choices_env("DEPLOY_ENVS") + .choices(["local"]) + .build(); + + let choices = arg.choices.unwrap(); + assert_eq!(choices.choices, vec!["local".to_string()]); + assert_eq!(choices.env, Some("DEPLOY_ENVS".to_string())); + } + #[test] fn test_command_builder_subcommands() { let sub1 = SpecCommandBuilder::new().name("sub1").build(); From de8e0d15a5a147e12c8bfbec00a27908dbd746e8 Mon Sep 17 00:00:00 2001 From: Mustafa J Date: Wed, 18 Mar 2026 18:27:49 +0300 Subject: [PATCH 4/5] complete: prevent path fallback for explicit empty choices --- cli/src/cli/complete_word.rs | 22 +++++++++++++--------- cli/tests/complete_word.rs | 10 ++++++++++ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/cli/src/cli/complete_word.rs b/cli/src/cli/complete_word.rs index bc00314b..2e830dc1 100644 --- a/cli/src/cli/complete_word.rs +++ b/cli/src/cli/complete_word.rs @@ -106,6 +106,7 @@ impl CompleteWord { .as_ref() .is_some_and(|rt| prev_token == Some(rt.as_str())); + let mut has_explicit_choices = false; let mut choices = if ctoken == "-" { let shorts = self.complete_short_flag_names(&parsed.available_flags, ""); let longs = self.complete_long_flag_names(&parsed.available_flags, ""); @@ -120,14 +121,18 @@ impl CompleteWord { // but before flag_awaiting_value (since restart clears pending flag values) let mut choices = vec![]; if let Some(arg) = parsed.cmd.args.first() { + has_explicit_choices = arg.choices.is_some(); choices.extend(self.complete_arg(&ctx, spec, &parsed.cmd, arg, &ctoken)?); } choices } else if let Some(flag) = parsed.flag_awaiting_value.first() { - self.complete_arg(&ctx, spec, &parsed.cmd, flag.arg.as_ref().unwrap(), &ctoken)? + let arg = flag.arg.as_ref().unwrap(); + has_explicit_choices = arg.choices.is_some(); + self.complete_arg(&ctx, spec, &parsed.cmd, arg, &ctoken)? } else { let mut choices = vec![]; if let Some(arg) = parsed.cmd.args.get(parsed.args.len()) { + has_explicit_choices = arg.choices.is_some(); choices.extend(self.complete_arg(&ctx, spec, &parsed.cmd, arg, &ctoken)?); } if !parsed.cmd.subcommands.is_empty() { @@ -139,6 +144,7 @@ impl CompleteWord { if let Some(default_cmd) = spec.cmd.find_subcommand(default_name) { // Include completions from default subcommand's first arg if let Some(arg) = default_cmd.args.first() { + has_explicit_choices = has_explicit_choices || arg.choices.is_some(); choices.extend(self.complete_arg( &ctx, spec, @@ -153,7 +159,7 @@ impl CompleteWord { choices }; // Fallback to file completions if nothing is known about this argument and it's not a flag - if choices.is_empty() && !ctoken.starts_with('-') { + if choices.is_empty() && !ctoken.starts_with('-') && !has_explicit_choices { let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let files = self.complete_path(&cwd, &ctoken, |_| true); choices = files.into_iter().map(|n| (n, String::new())).collect(); @@ -267,13 +273,11 @@ impl CompleteWord { if let Some(choices) = &arg.choices { let values = choices.values(); - if !values.is_empty() { - return Ok(values - .into_iter() - .map(|c| (c, String::new())) - .filter(|(c, _)| c.starts_with(ctoken)) - .collect()); - } + return Ok(values + .into_iter() + .map(|c| (c, String::new())) + .filter(|(c, _)| c.starts_with(ctoken)) + .collect()); } if let Some(run) = &complete.run { let run = tera::Tera::one_off(run, ctx, false).into_diagnostic()?; diff --git a/cli/tests/complete_word.rs b/cli/tests/complete_word.rs index 6d11c47c..2cc20ed8 100644 --- a/cli/tests/complete_word.rs +++ b/cli/tests/complete_word.rs @@ -78,6 +78,16 @@ fn complete_word_choices_from_env() { .stdout("foo\nbar\nbaz\n"); } +#[test] +fn complete_word_choices_from_env_unset_returns_empty() { + cmd("env-choices.usage.kdl", Some("fish")) + .env_remove("DEPLOY_ENVS") + .args(["--", "--env", ""]) + .assert() + .success() + .stdout(""); +} + #[test] fn complete_word_shebang() { assert_cmd("example.sh", &["--", "-"]) From 82adee134f03856694c0cd7f3a2c43ddc4c5ae5c Mon Sep 17 00:00:00 2001 From: Mustafa J Date: Thu, 19 Mar 2026 22:03:15 +0300 Subject: [PATCH 5/5] parse,complete: revalidate env/default choices; keep root fallback --- cli/src/cli/complete_word.rs | 1 - cli/tests/complete_word.rs | 6 + ...default-subcommand-root-fallback.usage.kdl | 8 ++ lib/src/parse.rs | 135 ++++++++++++++++++ 4 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 examples/default-subcommand-root-fallback.usage.kdl diff --git a/cli/src/cli/complete_word.rs b/cli/src/cli/complete_word.rs index 2e830dc1..b45de721 100644 --- a/cli/src/cli/complete_word.rs +++ b/cli/src/cli/complete_word.rs @@ -144,7 +144,6 @@ impl CompleteWord { if let Some(default_cmd) = spec.cmd.find_subcommand(default_name) { // Include completions from default subcommand's first arg if let Some(arg) = default_cmd.args.first() { - has_explicit_choices = has_explicit_choices || arg.choices.is_some(); choices.extend(self.complete_arg( &ctx, spec, diff --git a/cli/tests/complete_word.rs b/cli/tests/complete_word.rs index 2cc20ed8..99603f11 100644 --- a/cli/tests/complete_word.rs +++ b/cli/tests/complete_word.rs @@ -88,6 +88,12 @@ fn complete_word_choices_from_env_unset_returns_empty() { .stdout(""); } +#[test] +fn complete_word_default_subcommand_choices_do_not_block_root_file_fallback() { + assert_cmd("default-subcommand-root-fallback.usage.kdl", &["--", "C"]) + .stdout(contains("Cargo.toml")); +} + #[test] fn complete_word_shebang() { assert_cmd("example.sh", &["--", "-"]) diff --git a/examples/default-subcommand-root-fallback.usage.kdl b/examples/default-subcommand-root-fallback.usage.kdl new file mode 100644 index 00000000..b94fb47e --- /dev/null +++ b/examples/default-subcommand-root-fallback.usage.kdl @@ -0,0 +1,8 @@ +arg "" +default_subcommand "run" + +cmd "run" { + arg "" { + choices "task-a" "task-b" + } +} diff --git a/lib/src/parse.rs b/lib/src/parse.rs index 4fa90177..45ce8971 100644 --- a/lib/src/parse.rs +++ b/lib/src/parse.rs @@ -105,12 +105,34 @@ impl<'a> Parser<'a> { for arg in out.cmd.args.iter().skip(out.args.len()) { if let Some(env_var) = arg.env.as_ref() { if let Some(env_value) = get_env(env_var) { + validate_choices( + self.spec, + &out.cmd, + &mut out.errors, + "arg", + &arg.name, + &env_value, + arg.choices.as_ref(), + self.env.as_ref(), + )?; out.args .insert(Arc::new(arg.clone()), ParseValue::String(env_value)); continue; } } if !arg.default.is_empty() { + for default_value in &arg.default { + validate_choices( + self.spec, + &out.cmd, + &mut out.errors, + "arg", + &arg.name, + default_value, + arg.choices.as_ref(), + self.env.as_ref(), + )?; + } // Consider var when deciding the type of default return value if arg.var { // For var=true, always return a vec (MultiString) @@ -136,6 +158,17 @@ impl<'a> Parser<'a> { if let Some(env_var) = flag.env.as_ref() { if let Some(env_value) = get_env(env_var) { if flag.arg.is_some() { + let arg = flag.arg.as_ref().unwrap(); + validate_choices( + self.spec, + &out.cmd, + &mut out.errors, + "option", + &flag.name, + &env_value, + arg.choices.as_ref(), + self.env.as_ref(), + )?; out.flags .insert(Arc::clone(flag), ParseValue::String(env_value)); } else { @@ -153,6 +186,19 @@ impl<'a> Parser<'a> { if flag.var { // For var=true, always return a vec (MultiString for flags with args, MultiBool for boolean flags) if flag.arg.is_some() { + let arg = flag.arg.as_ref().unwrap(); + for default_value in &flag.default { + validate_choices( + self.spec, + &out.cmd, + &mut out.errors, + "option", + &flag.name, + default_value, + arg.choices.as_ref(), + self.env.as_ref(), + )?; + } out.flags.insert( Arc::clone(flag), ParseValue::MultiString(flag.default.clone()), @@ -170,6 +216,17 @@ impl<'a> Parser<'a> { } else { // For var=false, return the first default value if flag.arg.is_some() { + let arg = flag.arg.as_ref().unwrap(); + validate_choices( + self.spec, + &out.cmd, + &mut out.errors, + "option", + &flag.name, + &flag.default[0], + arg.choices.as_ref(), + self.env.as_ref(), + )?; out.flags.insert( Arc::clone(flag), ParseValue::String(flag.default[0].clone()), @@ -186,6 +243,18 @@ impl<'a> Parser<'a> { // Also check nested arg defaults (for flags like --foo where the arg has a default) if let Some(arg) = flag.arg.as_ref() { if !out.flags.contains_key(flag) && !arg.default.is_empty() { + for default_value in &arg.default { + validate_choices( + self.spec, + &out.cmd, + &mut out.errors, + "option", + &flag.name, + default_value, + arg.choices.as_ref(), + self.env.as_ref(), + )?; + } if flag.var { out.flags.insert( Arc::clone(flag), @@ -1866,6 +1935,38 @@ mod tests { ); } + #[test] + fn test_parser_rejects_arg_env_value_not_in_choices_env() { + let cmd = SpecCommand::builder() + .name("test") + .arg( + SpecArg::builder() + .name("env") + .env("CURRENT_ENV") + .choices_env("DEPLOY_ENVS") + .build(), + ) + .build(); + let spec = Spec { + name: "test".to_string(), + bin: "test".to_string(), + cmd, + ..Default::default() + }; + + let env = HashMap::from([ + ("CURRENT_ENV".to_string(), "prod".to_string()), + ("DEPLOY_ENVS".to_string(), "dev,staging".to_string()), + ]); + let input = vec!["test".to_string()]; + let err = Parser::new(&spec).with_env(env).parse(&input).unwrap_err(); + + assert_eq!( + format!("{err}"), + "Invalid choice for arg env: prod, expected one of dev, staging" + ); + } + #[test] fn test_parser_validates_flag_choices_from_custom_env() { let cmd = SpecCommand::builder() @@ -1900,6 +2001,40 @@ mod tests { assert_eq!(value.to_string(), "baz"); } + #[test] + fn test_parser_rejects_flag_default_value_not_in_choices_env() { + let cmd = SpecCommand::builder() + .name("test") + .flag( + SpecFlag::builder() + .long("env") + .arg( + SpecArg::builder() + .name("env") + .choices_env("DEPLOY_ENVS") + .build(), + ) + .default_value("prod") + .build(), + ) + .build(); + let spec = Spec { + name: "test".to_string(), + bin: "test".to_string(), + cmd, + ..Default::default() + }; + + let env = HashMap::from([("DEPLOY_ENVS".to_string(), "dev,staging".to_string())]); + let input = vec!["test".to_string()]; + let err = Parser::new(&spec).with_env(env).parse(&input).unwrap_err(); + + assert_eq!( + format!("{err}"), + "Invalid choice for option env: prod, expected one of dev, staging" + ); + } + #[test] fn test_variadic_arg_captures_unknown_flags_from_spec_string() { let spec: Spec = r#"