mirror of
https://github.com/ultraworkers/claw-code.git
synced 2026-04-24 13:08:11 +08:00
fix(#130c): accept --help / -h in claw diff arm
## What Was Broken (ROADMAP #130c, filed cycle #50) `claw diff --help` was rejected with: [error-kind: unknown] error: unexpected extra arguments after `claw diff`: --help Other local introspection commands accept --help fine: - `claw status --help` → shows help ✅ - `claw mcp --help` → shows help ✅ - `claw export --help` → shows help ✅ - `claw diff --help` → error ❌ (outlier) This is a help-surface parity bug: `diff` is the only local command that rejects --help as "extra arguments" before the help detector gets a chance to run. ## Root Cause (Traced) At main.rs:1063, the `"diff"` parser arm rejected ALL extra args: "diff" => { if rest.len() > 1 { return Err(format!("unexpected extra arguments after `claw diff`: {}", ...)); } Ok(CliAction::Diff { output_format }) } When parsing `["diff", "--help"]`, `rest.len() > 1` was true (length is 2) and `--help` was rejected as extra argument. Other commands (status, sandbox, doctor, init, state, export, etc.) routed through `parse_local_help_action()` which detected `--help` / `-h` and routed to a LocalHelpTopic. The `diff` arm lacked this guard. ## What This Fix Does Three minimal changes: 1. **LocalHelpTopic enum extended** with new `Diff` variant 2. **parse_local_help_action() extended** to map `"diff"` → `LocalHelpTopic::Diff` 3. **diff arm guard added**: check for help flag before extra-args validation 4. **Help topic renderer added**: human-readable help text for diff command Fix locus at main.rs:1063: "diff" => { // #130c: accept --help / -h as first argument and route to help topic if rest.len() == 2 && is_help_flag(&rest[1]) { return Ok(CliAction::HelpTopic(LocalHelpTopic::Diff)); } if rest.len() > 1 { /* existing error */ } Ok(CliAction::Diff { output_format }) } ## Dogfood Verification Before fix: $ claw diff --help [error-kind: unknown] error: unexpected extra arguments after `claw diff`: --help After fix: $ claw diff --help Diff Usage claw diff [--output-format <format>] Purpose show local git staged + unstaged changes Requires workspace must be inside a git repository ... And `claw diff -h` (short form) also works. ## Non-Regression Verification - `claw diff` (no args) → still routes to Diff action correctly - `claw diff foo` (unknown arg) → still rejected as "unexpected extra arguments" - `claw diff --output-format json` (valid flag) → still works - All 180 binary tests pass - All 466 library tests pass ## Regression Tests Added (4 assertions) - `diff --help` → routes to HelpTopic(LocalHelpTopic::Diff) - `diff -h` (short form) → routes to HelpTopic(LocalHelpTopic::Diff) - bare `diff` → still routes to Diff action - `diff foo` (unknown arg) → still errors with "extra arguments" ## Pattern Follows #141 help-consistency work (extending LocalHelpTopic to cover more subcommands). Clean surface-parity fix: identify the outlier, add the missing guard. Low-risk, high-clarity. ## Related - Closes #130c (diff help discoverability gap) - Stacks on #130b (filesystem context) and #251 (session dispatch) - Part of help-consistency thread (#141 audit, #145 plugins wiring)
This commit is contained in:
parent
d49a75cad5
commit
83f744adf0
@ -731,6 +731,8 @@ enum LocalHelpTopic {
|
||||
SystemPrompt,
|
||||
DumpManifests,
|
||||
BootstrapPlan,
|
||||
// #130c: help parity for `claw diff --help`
|
||||
Diff,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
@ -1061,6 +1063,12 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
|
||||
// #146: `diff` is pure-local (shells out to `git diff --cached` +
|
||||
// `git diff`). No session needed to inspect the working tree.
|
||||
"diff" => {
|
||||
// #130c: accept --help / -h as first argument and route to help topic,
|
||||
// matching the behavior of status/sandbox/doctor/etc.
|
||||
// Without this guard, `claw diff --help` was rejected as extra arguments.
|
||||
if rest.len() == 2 && is_help_flag(&rest[1]) {
|
||||
return Ok(CliAction::HelpTopic(LocalHelpTopic::Diff));
|
||||
}
|
||||
if rest.len() > 1 {
|
||||
return Err(format!(
|
||||
"unexpected extra arguments after `claw diff`: {}",
|
||||
@ -1260,6 +1268,8 @@ fn parse_local_help_action(rest: &[String]) -> Option<Result<CliAction, String>>
|
||||
"system-prompt" => LocalHelpTopic::SystemPrompt,
|
||||
"dump-manifests" => LocalHelpTopic::DumpManifests,
|
||||
"bootstrap-plan" => LocalHelpTopic::BootstrapPlan,
|
||||
// #130c: help parity for `claw diff --help`
|
||||
"diff" => LocalHelpTopic::Diff,
|
||||
_ => return None,
|
||||
};
|
||||
Some(Ok(CliAction::HelpTopic(topic)))
|
||||
@ -6083,6 +6093,15 @@ fn render_help_topic(topic: LocalHelpTopic) -> String {
|
||||
Formats text (default), json
|
||||
Related claw doctor · claw status"
|
||||
.to_string(),
|
||||
// #130c: help topic for `claw diff --help`.
|
||||
LocalHelpTopic::Diff => "Diff
|
||||
Usage claw diff [--output-format <format>]
|
||||
Purpose show local git staged + unstaged changes for the current workspace
|
||||
Requires workspace must be inside a git repository
|
||||
Output unified diff (text) or structured diff object (json)
|
||||
Formats text (default), json
|
||||
Related claw status · claw config"
|
||||
.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
@ -10352,6 +10371,47 @@ mod tests {
|
||||
"filesystem_io_error",
|
||||
"#130b: enriched messages must be classifier-recognizable"
|
||||
);
|
||||
// #130c: `claw diff --help` must route to help topic, not reject as extra args.
|
||||
// Regression: `diff` was the outlier among local introspection commands
|
||||
// (status/config/mcp all accepted --help) because its parser arm rejected
|
||||
// all extra args before help detection could run.
|
||||
let diff_help_action = parse_args(&[
|
||||
"diff".to_string(),
|
||||
"--help".to_string(),
|
||||
])
|
||||
.expect("diff --help must parse as help action");
|
||||
assert!(
|
||||
matches!(diff_help_action, CliAction::HelpTopic(LocalHelpTopic::Diff)),
|
||||
"#130c: diff --help must route to LocalHelpTopic::Diff, got: {diff_help_action:?}"
|
||||
);
|
||||
let diff_h_action = parse_args(&[
|
||||
"diff".to_string(),
|
||||
"-h".to_string(),
|
||||
])
|
||||
.expect("diff -h must parse as help action");
|
||||
assert!(
|
||||
matches!(diff_h_action, CliAction::HelpTopic(LocalHelpTopic::Diff)),
|
||||
"#130c: diff -h (short form) must route to LocalHelpTopic::Diff"
|
||||
);
|
||||
// #130c: bare `claw diff` still routes to Diff action, not help.
|
||||
let diff_action = parse_args(&[
|
||||
"diff".to_string(),
|
||||
])
|
||||
.expect("bare diff must parse as diff action");
|
||||
assert!(
|
||||
matches!(diff_action, CliAction::Diff { .. }),
|
||||
"#130c: bare diff must still route to Diff action, got: {diff_action:?}"
|
||||
);
|
||||
// #130c: unknown args still rejected (non-regression).
|
||||
let diff_bad_arg = parse_args(&[
|
||||
"diff".to_string(),
|
||||
"foo".to_string(),
|
||||
])
|
||||
.expect_err("diff foo must still be rejected as extra args");
|
||||
assert!(
|
||||
diff_bad_arg.contains("unexpected extra arguments"),
|
||||
"#130c: diff with unknown arg must still error, got: {diff_bad_arg}"
|
||||
);
|
||||
// #147: empty / whitespace-only positional args must be rejected
|
||||
// with a specific error instead of falling through to the prompt
|
||||
// path (where they surface a misleading "missing Anthropic
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user