Skip to content

refactor(parser): consolidate global-flag long-option resolution between argv-parser and subcommand-scanner #400

@toiroakr

Description

@toiroakr

Background

Politty parses argv in two phases:

  1. Subcommand scan (src/parser/subcommand-scanner.ts): walk argv with only the global schema available, skip over global flags and their values, locate the subcommand name. The local schema is not available because lazy-loaded commands make eager loading infeasible.
  2. Full parse (src/parser/argv-parser.ts): with both global and local schemas resolved, parse all argv tokens, assign values, and emit warnings/errors.

Both phases share the same long-option vocabulary against the global schema:

  • default negation: --no-<flag> (kebab) and --no<Flag> (camelCase)
  • custom negation (introduced in feat: support custom negation names for boolean args #393): --<negation> and its camelCase variant via negationMap
  • customNegatedFields suppression of the default --no-<X> form
  • literal-name disambiguation: when a global is literally named no-foo / noBar, recognize it as itself rather than as the negation of foo / bar (argv-parser.ts:147 / :167 and the mirror in subcommand-scanner.ts:86-96)

Today, the rules are implemented twice:

  • argv-parser inlined inside parseArgv (lines ~120-180 of argv-parser.ts)
  • subcommand-scanner mirrored inside resolveGlobalLongOption (subcommand-scanner.ts)

When a rule changes in one place the other must be updated by hand, which has already caused review churn during #393. The risk is silent drift — a scanner that stops on a token the parser actually accepts (or vice versa) is hard to detect without a fixture that exercises both phases together.

Proposal

Extract a single resolver that both phases call:

// e.g. src/parser/global-long-option.ts
export interface LongOptionResolution {
  resolvedName: string;
  withoutDashes: string;
  isNegated: boolean;
  isCustomNegation: boolean;
  isGlobal: boolean;       // global-only flag, not also defined locally
  // (omit `isGlobal` when consumed by argv-parser, which doesn't need it)
}

export function resolveLongOption(
  arg: string,
  lookup: GlobalFlagLookup,
): LongOptionResolution;
  • argv-parser uses it to decide negation/custom-negation/default branching before assignment.
  • subcommand-scanner uses it via the existing resolveGlobalLongOption wrapper, adding the isGlobal check on top.
  • Disambiguation rules (literal no-foo, customNegatedFields suppression, camelCase negation form) live in exactly one place.

buildGlobalFlagLookup is already shared — only the resolver itself needs extraction.

Scope

  • Pure refactor; no observable behavior change.
  • Existing scanner tests (src/parser/subcommand-scanner.test.ts) and parser tests (src/parser/arg-parser.test.ts, tests/e2e.test.ts) must pass unchanged.
  • Add a fixture that explicitly checks scanner / parser symmetry: a small property-style test that exercises a corpus of long-option shapes (--flag, --no-flag, --noFlag, --no-foo where no-foo is a literal field, --disable-cache where cache has custom negation, --flag=value) and asserts both phases agree on whether each token is a recognized global flag, negation, or unknown.

Non-goals

  • Sharing more than long-option resolution. Short options (-x) and value-consumption logic stay phase-local for now.
  • Removing the two-phase architecture. The scanner exists because the local schema is lazy; that constraint isn't going away.

Context

Surfaced during review of #393. The custom-negation feature required mirroring four distinct rules (custom-negation map, default kebab negation, default camelCase negation, literal-name disambiguation) across both phases. Each addition is small in isolation, but the cumulative duplication makes the parser harder to reason about and easier to drift.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions