Skip to content

fix(cli): print a friendly error when multiple sources are passed#179

Closed
airvzxf wants to merge 2 commits into
benjajaja:masterfrom
airvzxf:fix/cli-multiple-positional-args
Closed

fix(cli): print a friendly error when multiple sources are passed#179
airvzxf wants to merge 2 commits into
benjajaja:masterfrom
airvzxf:fix/cli-multiple-positional-args

Conversation

@airvzxf

@airvzxf airvzxf commented Jul 3, 2026

Copy link
Copy Markdown

Closes #178

What

When the user passes more than one positional argument (typically because
a shell glob like mdfried **/PRO*.md expanded to multiple paths),
clap returns ErrorKind::UnknownArgument for the second one. mdfried
now detects that specific shape — the offending value does not start
with - — and prints a friendly hint instead of clap's generic
'unexpected argument' message.

Before

$ mdfried a.md b.md
error: unexpected argument 'b.md' found

Usage: mdfried [OPTIONS] [SOURCE]

For more information, try '--help'.

After

$ mdfried a.md b.md
error: mdfried accepts a single source. If you used a shell glob, quote it
(e.g. mdfried '**/*.md') or run mdfried once per file.

Exit code stays 2 (matches clap's default for argument errors).

Why

The [SOURCE] positional is correctly single-valued; the bug is purely
the error message. clap's default does not mention that mdfried takes
one source at a time, so users who reached for a glob to view several
PROPOSAL.md files get a wall of text that does not explain the actual
problem. Unknown flags (--foo) keep clap's normal error, since the
'single source' hint would be misleading there.

How

  • build_cli() is extracted from main() so the new unit test can
    call try_get_matches_from directly.
  • main() now uses try_get_matches_from_mut instead of
    get_matches_mut, intercepts ErrorKind::UnknownArgument whose
    InvalidArg context does not start with -, prints the hint,
    and exits with 2. All other argument errors fall through to
    clap::error::Error::exit, which keeps the well-tested usage /
    --help output for the cases that benefit from it.
  • A unit test (tests::multiple_positional_sources_are_rejected_as_unknown_argument)
    locks the error kind and the offending value in, so the friendly
    message cannot silently regress to clap's default.

Validation

  • cargo fmt --check — clean
  • cargo clippy --all-targets --no-default-features — no new warnings
    (the two #[allow(...)] on document.rs warnings pre-exist on master
    and are unrelated)
  • cargo build --no-default-features
  • cargo build --release --no-default-features
  • cargo test --no-default-features --bin mdfried — new test passes;
    the only failures are the pre-existing insta snapshot tests that
    also fail on base master
  • cargo doc --no-deps
  • End-to-end check on the release binary:
    mdfried a.md b.md → friendly message, exit 2
    mdfried --nope → clap's default error, exit 2
    mdfried --print-config → still works

Running `mdfried **/PRO*.md` from a shell expands the glob to multiple
paths, and clap's default 'error: unexpected argument ...' does not
explain that mdfried takes a single source. Detect the multi-positional
case (ErrorKind::UnknownArgument whose InvalidArg does not start with
'-') and emit a hint that points at quoting the glob or running mdfried
once per file.

All other argument errors fall through to clap's default formatter,
which already includes usage and --help.

The clap::Command builder is extracted into a build_cli() helper so
the new unit test in mod tests can call try_get_matches_from directly
and assert the error kind and offending value, locking the friendly
message in against future regressions.
Copilot AI review requested due to automatic review settings July 3, 2026 15:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the mdfried CLI UX by replacing clap’s generic “unexpected argument” output (when multiple positional sources are provided) with a targeted, user-friendly hint that explains the common glob-expansion cause and how to fix it, while preserving clap’s default behavior for unknown flags.

Changes:

  • Extracted CLI construction into build_cli() to enable direct unit testing of clap parsing behavior.
  • Updated main() to intercept ErrorKind::UnknownArgument that looks like an extra positional (invalid arg doesn’t start with -) and print a friendly error before exiting with code 2.
  • Added a unit test to lock in clap’s error kind and InvalidArg context for the second positional argument.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/main.rs Outdated
…shape

The test only asserts on what clap returns (ErrorKind +
ContextKind::InvalidArg), not on the friendly message printed by
main(). Reword the comment so it accurately describes what is being
locked in, and what would surface a regression.
@airvzxf

airvzxf commented Jul 3, 2026

Copy link
Copy Markdown
Author

Thanks for the review, @copilot-pull-request-reviewer. The one substantive comment has been applied in 57d02be — the test comment now accurately describes what the assertions lock in (clap's error shape, not the friendly message itself).

This PR is ready to merge. The two commits on this branch:

  • 4659f47 fix(cli): print a friendly error when multiple sources are passed
  • 57d02be docs(cli): clarify that the multi-positional test locks clap's error shape

Note: I'm opening this from a fork without write access to benjajaja/mdfried, so the squash-merge needs to happen on your side. Closes #178 is in the PR body, so the issue will auto-close on merge.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

Screenshots, Master References, and Diffs

foot

New:

foot current

Master:

foot reference

Diff:

foot diff

kitty

New:

kitty current

Master:

kitty reference

Diff:

kitty diff

wezterm

New:

wezterm current

Master:

wezterm reference

Diff:

wezterm diff

alacritty

New:

alacritty current

Master:

alacritty reference

Diff:

alacritty diff

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.

mdfried **/*.md fails with cryptic 'unexpected argument' instead of a friendly hint

3 participants