Skip to content

CHORE: demo.mjs and install.mjs each have independent, incompatible arg-parsing implementations #126

Description

@devinoldenburg

Summary

Two scripts in the project implement their own argument parsers with different semantics and no shared code. Any fix to argument parsing behavior must be made in two places.

Affected code

scripts/install.mjs lines 36-49 — explicit === "--flag" matching

function parseArgs(argv) {
  const a = { _: [] };
  for (let i = 0; i < argv.length; i++) {
    const t = argv[i];
    if (t === "--local") a.local = true;
    else if (t === "--no-install") a.noInstall = true;
    else if (t === "--dry-run") a.dryRun = true;
    else if (t === "--uninstall" || t === "--remove") a.uninstall = true;
    else if (t === "--print") a.print = true;
    else if (t === "--dir") a.dir = argv[++i];
    else if (t === "--help" || t === "-h") a.help = true;
    else a._.push(t);
  }
  return a;
}

tools/demo.mjs lines 195-205 — startsWith("--") with lookahead

function parseArgs(argv) {
  const out = {};
  for (let i = 0; i < argv.length; i++) {
    const a = argv[i];
    if (a.startsWith("--")) {
      const key = a.slice(2);
      out[key] = argv[i + 1] && !argv[i + 1].startsWith("--") ? argv[(i += 1)] : true;
    }
  }
  return out;
}

Inconsistencies

Aspect install.mjs demo.mjs
Unknown flags Silently dropped into a._ Converted to { [key]: true }
-h shorthand Supported ("-h") Not supported (only "--" prefix)
Value after flag Explicit argv[++i] Lookahead with ternary
Value at end of argv argv[++i] produces undefined (see issue 17) Ternary handles correctly (no next arg -> true)
Aliases Supports --uninstall / --remove None
Boolean flags Explicit fields in object Detected by missing next arg
Return shape { _, local, dryRun, ... } { key: value } dynamic

Why this matters

  • Any new script added to the project must write a third argument parser.
  • Fixing a flag-parsing bug in a shared utility would fix both scripts — currently requires two separate fixes.
  • The demo.mjs parser silently converts unknown flags "--foo" to { foo: true } instead of warning. The install.mjs parser silently drops them into _. Neither warns the user.
  • The value-at-end-of-argv crash in install.mjs (issue 17) also affects demo.mjs, but demo.mjs handles it differently (returns true instead of crashing). Inconsistent behavior for the same edge case.

Proposed fix

  1. Extract a shared parseArgs() into a new utility file (tools/_args.mjs or similar).
  2. Both scripts import from the shared utility.
  3. Shared parser should:
    • Accept a schema of known flags with types (boolean, string)
    • Warn on unknown flags
    • Handle --flag value and --flag (boolean) correctly
    • Handle --flag at end of argv correctly (not crash)

Impact

Low. These are dev/install scripts, not runtime code. The risk is maintainability and the potential for flag-parsing bugs in one script to be fixed but persist in the other.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions