Skip to content

Carry clap's suggestion into the json usage error - #11

Merged
mattpodwysocki merged 1 commit into
mainfrom
carry-clap-tip
Sep 14, 2026
Merged

mattpodwysocki merged 1 commit into
mainfrom
carry-clap-tip

Conversation

@mattpodwysocki

Copy link
Copy Markdown
Contributor

Raised while reviewing #8, and worth landing before 0.2.0 rather than after it.

What's wrong

Clap works out what was probably meant and renders it as its own paragraph:

error: unrecognized subcommand 'forward-geocode'

  tip: a similar subcommand exists: 'forward'

message is built from clap's first paragraph — deliberately, so a missing-argument error names the arguments that sit on its later lines — and the tip is in the next paragraph, so take_while stops one line short of it.

Under -o text that costs nothing, because clap prints its whole rendering. Under -o json, only message survived:

{"code":"usage","message":"unrecognized subcommand 'forward-geocode'"}

The one part that says what to do instead was gone — in the rendering scripts and agents read. The consumers least able to guess a new name were the ones told least.

Why now and not after the release

#8 renames nine commands with no aliases, so 0.2.0 is precisely the release where a pinned script breaks. A hint that ships in 0.2.1 arrives after the moment it existed for.

Verified against #8's branch, which is where the motivating cases live:

Typed fix now returned
mapbox geocoder forward-geocode A similar subcommand exists: 'forward'
mapbox static-images get-static-image A similar subcommand exists: 'static'
mapbox tilequery get A similar subcommand exists: 'tilesets'

All three renames now hand back a machine-readable pointer to the command that replaced them.

It helps misspelled flags too, which is the other thing clap suggests about — --usernam answers A similar argument exists: '--username'.

Choices

  • Goes in fix, which already means "why it failed, and what would make it work", rather than inventing a key.
  • Capitalised. Clap's wording starts lowercase; every other fix in this crate reads as a sentence.
  • Absent when there is no suggestion, the rule the other optional keys in this error already follow — {"code":"usage","message":"unexpected argument '--zzz' found"} gains nothing.
  • text is untouched: that path still hands the error to clap to print, tip included.

Tests

Four, and I checked they fail for the right reason rather than trusting green. Reverting the change reddens the two that assert the new behaviour and leaves the two asserting the old behaviour green:

a_misspelled_flag_also_carries_the_suggestion ... FAILED
a_usage_error_carries_claps_suggestion_as_a_fix ... FAILED
a_usage_error_with_no_suggestion_has_no_fix ... ok
text_mode_still_prints_claps_own_tip ... ok

588 tests, cargo fmt --check and cargo clippy --locked --all-targets clean.

Changelog

Per Mofei, the release pipeline stays in mapbox-cli-private, so the canonical CHANGELOG.md is the one at that repo's root. The entry for this goes there — I'll add it to mapbox-cli-private#146, which is already open and is exactly that file.

@zmofei's `37afc38` already carries clap's suggestion into the json error, so
the implementation this branch opened with was a duplicate and is dropped.
What is left is test coverage for three cases its own test does not reach.

- **A misspelled flag.** `--usernam` answers `a similar argument exists:
  '--username'`, through the same path — and a flag is the likelier typo of
  the two. Nothing pinned that it worked.
- **No suggestion at all.** `--zzz` gets no `fix` key rather than an empty
  one, which is the rule the other optional keys in this error follow. Worth
  a test because "absent, not empty" is the kind of thing a later refactor
  turns into `"fix": ""` without noticing.
- **`text` unchanged.** Clap still prints its own rendering there, tip
  included. This is the one that earns its place: the argument for carrying
  the tip into `json` was that `text` already had it, so a change that
  quietly moved it *out* of `text` would undo the reasoning while leaving the
  json test green.

588 tests, fmt and clippy clean.
@mattpodwysocki

Copy link
Copy Markdown
Contributor Author

Reduced this to what is actually additive, because the substance already landed.

@zmofei's 37afc38 does the same fix, and came in with #8 while this was open — same clap_tip helper, same fix field, same reasoning about message taking only clap's first paragraph. I verified main now answers {"code":"usage","fix":"a similar subcommand exists: 'forward'"} for the renames, which is the whole point. So the implementation this branch opened with is a duplicate and I have dropped it.

What is left is three test cases 37afc38's own test does not reach:

  • A misspelled flag — --usernam → a similar argument exists: '--username', through the same path, and the likelier typo of the two.
  • No suggestion — --zzz gets no fix key rather than an empty one. Worth pinning because "absent, not empty" is what a later refactor turns into "fix": "" without noticing.
  • text unchanged — the one I would keep if I could keep only one. The argument for carrying the tip into json was that text already had it, so a change that quietly moved it out of text would undo the reasoning while leaving the json test green.

588 tests, fmt and clippy clean.

The approval on this predates the reduction, so it is no longer approval of what it approved — the content is now strictly a subset plus three tests. Flagging rather than treating it as still valid.

One thing I backed out, and it is yours to decide

I had also capitalised the tip, because every other fix in the crate reads as a sentence and these sit side by side:

Fix: a similar subcommand exists: 'forward'
Fix: The token was passed with `--token`, which outranks both …

Your test asserts the exact lowercase string with assert_eq!, which reads as a deliberate choice — preserving clap's wording verbatim rather than editorialising it is perfectly defensible. So I reverted it rather than overriding you and breaking your test. If you would rather it matched the others, it is two lines in clap_tip plus that assertion; say so and I will add it here.

@mattpodwysocki
mattpodwysocki merged commit 1d4cd30 into main Sep 14, 2026
7 of 8 checks passed
@mattpodwysocki
mattpodwysocki deleted the carry-clap-tip branch September 14, 2026 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants