Skip to content

bug: unknown flags after the positional arguments are silently ignored #211

Description

@szhekpisov

What

An unknown flag is only reported when it appears before the <from> <to> arguments. After them, it is silently dropped:

$ diffyml --typo a.yaml b.yaml
Error: flag provided but not defined: -typo        # exit 255, correct

$ diffyml a.yaml b.yaml --typo
Found two differences ...                          # exit 0, typo ignored

$ diffyml a.yaml b.yaml --typo --help
diffyml - A diff tool for YAML files ...           # exit 0, typo ignored

Why it happens

reorderArgs (pkg/diffyml/cli/cli.go) moves recognized flags ahead of positional arguments so interspersed flags parse. An unrecognized flag is deliberately left in the positional list so fs.Parse reports it:

f := fs.Lookup(name)
if f == nil {
    // Unknown flag — keep as positional so fs.Parse reports the error.
    positional = append(positional, arg)
    continue
}

That only works when nothing else precedes it, because flag.Parse stops at the first non-flag argument. With a.yaml b.yaml ahead of it in the positional list, parsing stops at a.yaml and --typo ends up in fs.Args()[2:], which ParseArgs never inspects — it reads remaining[0] and remaining[1] and returns.

A related symptom from the same place: a value-taking flag given last with no value swallows a file path, producing a confusing error.

$ diffyml a.yaml b.yaml --mask-placeholder
Error: requires two file arguments: <from> <to>    # a.yaml became the placeholder

Suggested fix

After the GIT_EXTERNAL_DIFF detection and the two-argument extraction in ParseArgs, reject leftover arguments that look like flags. Points needing care:

  • Everything after a literal -- must stay exempt — reorderArgs already preserves it verbatim.
  • The 7–9 argument GIT_EXTERNAL_DIFF convention returns early and must keep doing so.
  • A real file named -something is legitimate but only reachable after --, which the exemption above covers.
  • kubectl diff places KUBECTL_EXTERNAL_DIFF flags after the directory paths, so the check must run on unrecognized flags only, never on trailing recognized ones.

Context

Found while reviewing #210, which changed how flag errors are reported (parse now happens before help, and the error is reported once with a pointer to --help). This behavior is older than that PR and independent of it — #210's note that "a mistyped flag no longer falls through to the help text" holds only for the pre-positional case above.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions