Skip to content

Commit 4f8234f

Browse files
xerialclaude
andcommitted
Preserve root context in subcommand help; split MissingOption error
Fourth codex pass: - dispatch: subcommand help now shows the full invocation context. The new `compose_help_schema` walks the root → subcommand path and produces a help-only schema named e.g. `git clone` with the root's global options merged into the subcommand's own. Users copying the usage line get a string that actually parses. - error/parser: add Error::MissingOption for required options that were never typed. ParseResult::require now chooses the variant based on the name shape — dash-prefixed → MissingOption, else MissingArgument. Commands that make an option mandatory via `require::<T>("--num")` now get a diagnostic mentioning the option rather than a phantom positional. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 09b1117 commit 4f8234f

3 files changed

Lines changed: 67 additions & 5 deletions

File tree

runi-cli/src/launcher/dispatch.rs

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,8 @@ fn report_error(err: Error, root: &CommandSchema) -> i32 {
183183
0
184184
}
185185
Error::InSubcommand { path, source } => {
186-
let schema = resolve_schema(root, &path).unwrap_or(root);
186+
let composed = compose_help_schema(root, &path);
187+
let schema = composed.as_ref().unwrap_or(root);
187188
match *source {
188189
Error::HelpRequested => {
189190
HelpPrinter::print(schema);
@@ -204,12 +205,24 @@ fn report_error(err: Error, root: &CommandSchema) -> i32 {
204205
}
205206
}
206207

207-
fn resolve_schema<'a>(root: &'a CommandSchema, path: &[String]) -> Option<&'a CommandSchema> {
208+
/// Build a help-only schema that represents `root ... path` as a single
209+
/// command, so the usage line reads e.g. `git clone [OPTIONS] <url>` and
210+
/// includes the root's global options alongside the subcommand's own. The
211+
/// returned schema is only suitable for help printing — it is not used for
212+
/// parsing.
213+
fn compose_help_schema(root: &CommandSchema, path: &[String]) -> Option<CommandSchema> {
214+
let mut options = root.options.clone();
208215
let mut schema = root;
216+
let mut parts = vec![root.name.clone()];
209217
for name in path {
210218
schema = schema.subcommands.iter().find(|s| s.name == *name)?;
219+
options.extend(schema.options.iter().cloned());
220+
parts.push(name.clone());
211221
}
212-
Some(schema)
222+
let mut composed = schema.clone();
223+
composed.name = parts.join(" ");
224+
composed.options = options;
225+
Some(composed)
213226
}
214227

215228
fn env_args() -> Vec<String> {
@@ -449,6 +462,21 @@ mod tests {
449462
}
450463
}
451464

465+
#[test]
466+
fn compose_help_schema_prefixes_root_name_and_options() {
467+
let root = CommandSchema::new("git", "").flag("-v,--verbose", "Verbose");
468+
let sub = CommandSchema::new("clone", "Clone a repo").argument("url", "URL");
469+
let mut with_sub = root.clone();
470+
with_sub.subcommands.push(sub);
471+
let composed =
472+
compose_help_schema(&with_sub, &["clone".to_string()]).expect("must resolve");
473+
assert_eq!(composed.name, "git clone");
474+
// Root's --verbose must appear alongside clone's own options.
475+
assert!(composed.options.iter().any(|o| o.matches_long("verbose")));
476+
// Clone's own positional must be preserved.
477+
assert!(composed.arguments.iter().any(|a| a.name == "url"));
478+
}
479+
452480
#[test]
453481
fn invalid_value_error_is_informative() {
454482
let launcher = Launcher::<NeedsInt>::of();

runi-cli/src/launcher/error.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ pub enum Error {
1010
},
1111
MissingValue(String),
1212
UnexpectedValue(String),
13+
/// A required option was not provided at all (distinct from
14+
/// `MissingValue`, which fires when the option name was typed without a
15+
/// following value).
16+
MissingOption(String),
1317
MissingArgument(String),
1418
MissingSubcommand {
1519
available: Vec<String>,
@@ -59,6 +63,7 @@ impl Error {
5963
| Error::UnknownSubcommand { .. }
6064
| Error::MissingValue(_)
6165
| Error::UnexpectedValue(_)
66+
| Error::MissingOption(_)
6267
| Error::MissingArgument(_)
6368
| Error::MissingSubcommand { .. }
6469
| Error::ExtraArgument(_)
@@ -82,6 +87,7 @@ impl fmt::Display for Error {
8287
}
8388
Error::MissingValue(opt) => write!(f, "missing value for option: {opt}"),
8489
Error::UnexpectedValue(opt) => write!(f, "option {opt} does not take a value"),
90+
Error::MissingOption(name) => write!(f, "missing required option: {name}"),
8591
Error::MissingArgument(name) => write!(f, "missing required argument: <{name}>"),
8692
Error::MissingSubcommand { available } => {
8793
write!(f, "a subcommand is required")?;

runi-cli/src/launcher/parser.rs

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,20 @@ impl ParseResult {
6464
}
6565

6666
/// Like [`ParseResult::get`] but errors if the value is missing.
67+
///
68+
/// The error variant depends on the name shape: dash-prefixed names
69+
/// (e.g. `--num`, `-n`) become `MissingOption`, everything else becomes
70+
/// `MissingArgument`. That way a command that marks an option as
71+
/// required via `require::<T>("--num")` gets a diagnostic mentioning
72+
/// the option, not a positional argument.
6773
pub fn require<T: FromArg>(&self, name: &str) -> Result<T> {
68-
self.get::<T>(name)?
69-
.ok_or_else(|| Error::MissingArgument(name.to_string()))
74+
self.get::<T>(name)?.ok_or_else(|| {
75+
if name.starts_with('-') {
76+
Error::MissingOption(name.to_string())
77+
} else {
78+
Error::MissingArgument(name.to_string())
79+
}
80+
})
7081
}
7182

7283
/// Get all values supplied for a repeatable option.
@@ -347,6 +358,23 @@ mod tests {
347358
assert!(matches!(err, Error::MissingArgument(ref n) if n == "file"));
348359
}
349360

361+
#[test]
362+
fn require_on_missing_option_reports_missing_option() {
363+
let schema = CommandSchema::new("app", "").option("--num", "");
364+
let r = OptionParser::parse(&schema, &args(&[])).unwrap();
365+
let err = r.require::<u32>("--num").unwrap_err();
366+
assert!(matches!(err, Error::MissingOption(ref n) if n == "--num"));
367+
}
368+
369+
#[test]
370+
fn require_on_missing_positional_reports_missing_argument() {
371+
// Mirror of the above: positional uses MissingArgument.
372+
let schema = CommandSchema::new("app", "").optional_argument("file", "");
373+
let r = OptionParser::parse(&schema, &args(&[])).unwrap();
374+
let err = r.require::<String>("file").unwrap_err();
375+
assert!(matches!(err, Error::MissingArgument(ref n) if n == "file"));
376+
}
377+
350378
#[test]
351379
fn optional_argument_absent_is_ok() {
352380
let schema = CommandSchema::new("app", "").optional_argument("out", "output");

0 commit comments

Comments
 (0)