computer/git: make per-command help match the parsers

The usage lines went in by hand and several described a command
that does not exist. hash-object advertised a positional path when
it only reads --stdin, reset offered --soft and --mixed when both
are explicitly refused, and log omitted the --format it had just
gained. A caller following that help got exit 129 for doing what
it said.

Correct every line against the flags its parser actually declares,
including the short spellings that were missing, and add a test
that walks each advertised long flag back through the command it
belongs to. Drift now fails a test rather than reaching a caller.

Normalize --pretty to --format while parsing, rather than reading
whichever key happened to be set. They are one option in real git,
so the last one written should win; before this, --format always
did regardless of order.

Author: aron-cf

packages/computer/src/git/cli.test.ts

@@ -2441,9 +2441,83 @@ describe("runGitCli — log --format", () => {
     expect(res.stdout).toBe("aaaaaaa %zz\n");
   });
 
+  it("lets the last of --format and --pretty win, in either order", async () => {
+    const { client } = fakeClient({}, { log: () => [sample("a".repeat(40), "release")] });
+    const formatThenPretty = await runGitCli(client, {
+      argv: ["log", "--format=%s", "--pretty=%h"],
+    });
+    expect(formatThenPretty.stdout).toBe("aaaaaaa\n");
+    const prettyThenFormat = await runGitCli(client, {
+      argv: ["log", "--pretty=%h", "--format=%s"],
+    });
+    expect(prettyThenFormat.stdout).toBe("release\n");
+  });
+
   it("rejects --format combined with --oneline", async () => {
     const { client } = fakeClient();
     const res = await runGitCli(client, { argv: ["log", "--oneline", "--format=%h"] });
     expect(res.exitCode).toBe(129);
   });
 });
+
+describe("runGitCli — help stays honest about the flags each command takes", () => {
+  // The usage lines are maintained by hand, so they can drift from the
+  // parsers they describe. Rather than re-check every line by eye, take
+  // each long flag the help advertises and confirm the command does not
+  // reject it as unknown or unsupported. A flag that no longer exists,
+  // or was never accepted, fails here instead of misleading a caller.
+  const commands = [
+    "add",
+    "branch",
+    "cat-file",
+    "checkout",
+    "clean",
+    "clone",
+    "commit",
+    "config",
+    "diff",
+    "fetch",
+    "hash-object",
+    "init",
+    "log",
+    "ls-files",
+    "ls-tree",
+    "merge",
+    "pull",
+    "push",
+    "remote",
+    "reset",
+    "rev-parse",
+    "rm",
+    "show",
+    "stash",
+    "status",
+    "switch",
+    "symbolic-ref",
+    "tag",
+    "update-ref",
+  ];
+
+  it("advertises only flags the command's parser accepts", async () => {
+    const offenders: string[] = [];
+    for (const command of commands) {
+      const { client } = fakeClient();
+      const help = await runGitCli(client, { argv: ["help", command] });
+      expect(help.exitCode, `git help ${command}`).toBe(0);
+
+      for (const flag of help.stdout.match(/--[a-z][a-z-]*/g) ?? []) {
+        const { client: probe } = fakeClient();
+        const res = await runGitCli(probe, { argv: [command, flag] });
+        // The parser rejects an unknown flag at 129 with a message naming
+        // it. Any other failure means the flag was understood and the
+        // command merely wanted different arguments, which is fine here.
+        const unknown =
+          res.stderr.includes(`unknown option '${flag}'`) ||
+          res.stderr.includes(`${flag} is not supported`) ||
+          res.stderr.includes(`only --stdin is supported`);
+        if (unknown) offenders.push(`${command} ${flag}: ${res.stderr.trim()}`);
+      }
+    }
+    expect(offenders).toEqual([]);
+  });
+});

packages/computer/src/git/cli.ts

@@ -159,36 +159,37 @@ export async function runGitCli(
 // actually accepts are listed: the whole point is to tell a caller what
 // works here, rather than what real git would take.
 const COMMAND_USAGE: Record<string, string> = {
-  add: "git add [-A] <path>...",
-  branch: "git branch [-d|-D <name>] [--show-current] [<name>]",
+  add: "git add [-A|--all] [-f|--force] <path>...",
+  branch: "git branch [-d|-D|--delete] [-f|--force] [--show-current] [<name>]",
   "cat-file": "git cat-file (-p|-t|-s) <oid>[:<path>]",
-  checkout: "git checkout [-b <name>] [-f] <ref> [--] [<path>...]",
-  clean: "git clean [-f] [-d] [-n] [<path>...]",
+  checkout: "git checkout [-b] [-f|--force] <ref> [--] [<path>...]",
+  clean: "git clean [-f|--force] [-d] [-n|--dry-run] [<path>...]",
   clone:
-    "git clone [--depth <n>] [--branch <ref>] [--single-branch|--no-single-branch] <url> [<dir>]",
-  commit: "git commit -m <message> [-a]",
-  config: "git config [--get] <key> [<value>]",
-  diff: "git diff [--stat] [--name-only] [<ref>] [--] [<path>...]",
-  fetch: "git fetch [<remote>] [<ref>]",
-  "hash-object": "git hash-object [-w] [-t <type>] <path>",
-  init: "git init [<dir>]",
-  log: "git log [-n <count>] [-<count>] [--oneline] [<ref>]",
-  "ls-files": "git ls-files [<ref>]",
+    "git clone [--depth <n>] [-b|--branch <ref>] [--single-branch|--no-single-branch] [--tags|--no-tags] <url> [<dir>]",
+  commit: 'git commit -m|--message <message> [-a|--all] [--amend] [--author "Name <email>"]',
+  config: "git config [--get|--get-all|--add|--unset] <key> [<value>]",
+  diff: "git diff [--stat] [--name-only] [--name-status] [<ref>] [--] [<path>...]",
+  fetch:
+    "git fetch [--depth <n>] [--single-branch|--no-single-branch] [--tags|--no-tags] [--prune] [<remote>] [<ref>]",
+  "hash-object": "git hash-object --stdin [-w]",
+  init: "git init [-b|--initial-branch <name>] [--bare] [<dir>]",
+  log: "git log [-n <count>] [-<count>] [--oneline] [--format|--pretty=<spec>] [<ref>]",
+  "ls-files": "git ls-files [--ref <ref>]",
   "ls-tree": "git ls-tree <ref> [<path>]",
-  merge: "git merge [--ff-only] [--no-ff] <ref>",
-  pull: "git pull [<remote>] [<ref>]",
-  push: "git push [-f|--force] [--delete] [<remote>] [<refspec>]",
-  remote: "git remote [-v] [add <name> <url>] [remove <name>]",
-  reset: "git reset [--hard|--soft|--mixed] [<ref>] [--] [<path>...]",
-  "rev-parse": "git rev-parse <rev>",
-  rm: "git rm [--cached] [-r] <path>...",
+  merge: "git merge [--ff-only] [--no-ff] [-m|--message <message>] <ref>",
+  pull: "git pull [--ff-only] [--no-ff] [<remote>] [<ref>]",
+  push: "git push [-f|--force] [-d|--delete] [<remote>] [<refspec>]",
+  remote: "git remote [add <name> <url>] [remove <name>]",
+  reset: "git reset [--hard] [<ref>] [--] [<path>...]",
+  "rev-parse": "git rev-parse [--abbrev-ref] [--show-toplevel] <rev>",
+  rm: "git rm [--cached] <path>...",
   show: "git show [<ref>]",
   stash: "git stash [push|pop|apply|list|drop]",
-  status: "git status [-s|--short|--porcelain]",
-  switch: "git switch [-c <name>] <ref>",
-  "symbolic-ref": "git symbolic-ref <name> [<ref>]",
-  tag: "git tag [-d <name>] [<name> [<ref>]]",
-  "update-ref": "git update-ref <ref> <oid>",
+  status: "git status [-s|--short] [--porcelain[=<version>]]",
+  switch: "git switch [-c] <ref>",
+  "symbolic-ref": "git symbolic-ref [--short] [-q|--quiet] <name> [<ref>]",
+  tag: "git tag [-d|--delete] [-f|--force] [<name> [<ref>]]",
+  "update-ref": "git update-ref [--force] <ref> <oid>",
   help: "git help [<command>]",
   version: "git version",
 };
@@ -814,19 +815,25 @@ async function runLog(
       shorthandDepth = m[1];
       continue;
     }
+    // --pretty is the same option as --format in real git. Normalize it
+    // to the one key so the parser's last-write behavior decides which
+    // wins: a caller appending an override to a command it did not build
+    // gets the last one written, whichever spelling either of them used.
+    if (arg === "--pretty" || arg.startsWith("--pretty=")) {
+      rewritten.push(`--format${arg.slice("--pretty".length)}`);
+      continue;
+    }
     rewritten.push(arg);
   }
   const parsed = parseFlags(rewritten, {
     n: { kind: "value" },
     oneline: { kind: "bool" },
     format: { kind: "value" },
-    pretty: { kind: "value" },
   });
   if ("error" in parsed) {
     return { stdout: "", stderr: `git log: ${parsed.error}\n`, exitCode: 129 };
   }
-  // --pretty and --format are the same option in real git.
-  const formatSpec = (parsed.flags.format ?? parsed.flags.pretty) as string | undefined;
+  const formatSpec = parsed.flags.format as string | undefined;
   if (formatSpec !== undefined && parsed.flags.oneline === true) {
     return {
       stdout: "",