Converge the allod tools on one rule for multi-value flags, following gh #131

Closed
opened 2026-07-29 14:39:53 +01:00 by allod-agent · 2 comments
Member

Settle one rule for how a flag takes several values across the allod tools, follow gh wherever it applies, and make cli-design.md state the rule that results.

Primary goals:

  • One documented rule — a reader can predict how any multi-value flag in allod or forge behaves without reading its parser.
  • Follow gh — where gh has an established shape for a case, adopt it rather than inventing a local one.
  • Name the path exception explicitly — commas are legal in filenames, so whatever the rule is for label names cannot be applied blindly to file paths.
  • Memory follows the decisioncli-design.md currently states the space-variadic form as the universal rule, which is not what the tools do.

Current state

Two shapes are in use, and both are internally consistent and tested.

Tool Flags Shape Accepted forms
allod change record -f, --files (allod:536-546, usage at allod:37-38) space-variadic, repeating accumulates --files a b, --files a --files b, -f a b --files c
forge -l/--label, --add-label, --remove-label, --set (forge:867, :982, :1067, :1073, :1149, :1155, :1161, via append_csv_values at forge:267-279) comma-separated, repeating accumulates --label a,b, --label a --label b

forge documents its shape in its own help text as "repeatable, comma-separated" (forge:2144-2145, :2155-2157). allod change record documents its shape as [--files <file>...]. Neither is wrong about itself; there is simply no shared rule above them.

Single-value flags are not at issue. --depends-on (allod:637-642) takes one value and rejects a second explicitly, and every remaining value-taking flag in either tool is single by nature.

What gh does

gh builds multi-value flags on pflag's string-slice type: one occurrence accepts comma-separated values, the flag repeats, and occurrences accumulate — gh issue create --label bug --label urgent and gh issue list --label "bug,urgent" are both valid. gh does not accept space-separated values after a flag.

For lists of files gh does something different: it uses variadic positionals, not a flag — gh release upload <tag> <files>..., gh gist create [<filename>...]. That split is not incidental. A comma is a legal character in a POSIX filename, so a comma-separated --files a,b cannot distinguish two paths from one path containing a comma, while label names never hit that ambiguity in practice.

These claims are from knowledge of gh's flag layer, not from a local run — gh is not installed on the dev VMs. Confirm them against a real gh before ratifying the rule.

Proposed default

Read against gh, the toolchain is closer to consistent than it first looks, and the rule is value-type-dependent rather than universal:

  • Non-path value lists take comma-separated values and repeat. This is what forge's label flags already do, so no forge change falls out of this.
  • Path lists do not use comma separation. allod change record --files a b stays as it is, because the gh-faithful alternative to space-variadic for paths is variadic positionals, not CSV.
  • cli-design.md states both halves and cites the gh precedent, replacing the current text that reads as though space-variadic were the universal rule.

The alternative worth weighing before settling: make the paths case fully gh-shaped by taking them as variadic positionals — allod change record -m msg a.txt b.txt — keeping --files as an accepted spelling so existing call sites and the shared-checkout guidance in allod/tools#118 keep working. change record accepts no positionals today (allod:564-566 is a catch-all that dies), so nothing becomes ambiguous, and the -- terminator it would need already exists. This is a larger change and a second interface for the same thing, which is why it is not the default.

Scope

In: the decision itself, whatever parser and usage-text changes follow from it, and the cli-design.md rewrite. Out: the -f/--files short-flag spelling, which matches forge and is fine; the git add -u fallback; and the end-of-options separator work tracked in allod/tools#53, which this only touches where a value could begin with -.

Validation

Whatever shape is chosen, tests/allod-change.sh and tests/forge/validation.sh should cover it in both tools: a list of two, the repeated form, the two combined, flag order in both directions, an empty list refused rather than falling back to a broader default, and a value that begins with - still reachable.

Settle one rule for how a flag takes several values across the allod tools, follow `gh` wherever it applies, and make `cli-design.md` state the rule that results. Primary goals: - **One documented rule** — a reader can predict how any multi-value flag in `allod` or `forge` behaves without reading its parser. - **Follow `gh`** — where `gh` has an established shape for a case, adopt it rather than inventing a local one. - **Name the path exception explicitly** — commas are legal in filenames, so whatever the rule is for label names cannot be applied blindly to file paths. - **Memory follows the decision** — `cli-design.md` currently states the space-variadic form as the universal rule, which is not what the tools do. ### Current state Two shapes are in use, and both are internally consistent and tested. | Tool | Flags | Shape | Accepted forms | |------|-------|-------|----------------| | `allod change record` | `-f`, `--files` (`allod:536-546`, usage at `allod:37-38`) | space-variadic, repeating accumulates | `--files a b`, `--files a --files b`, `-f a b --files c` | | `forge` | `-l`/`--label`, `--add-label`, `--remove-label`, `--set` (`forge:867`, `:982`, `:1067`, `:1073`, `:1149`, `:1155`, `:1161`, via `append_csv_values` at `forge:267-279`) | comma-separated, repeating accumulates | `--label a,b`, `--label a --label b` | `forge` documents its shape in its own help text as "repeatable, comma-separated" (`forge:2144-2145`, `:2155-2157`). `allod change record` documents its shape as `[--files <file>...]`. Neither is wrong about itself; there is simply no shared rule above them. Single-value flags are not at issue. `--depends-on` (`allod:637-642`) takes one value and rejects a second explicitly, and every remaining value-taking flag in either tool is single by nature. ### What `gh` does `gh` builds multi-value flags on pflag's string-slice type: one occurrence accepts comma-separated values, the flag repeats, and occurrences accumulate — `gh issue create --label bug --label urgent` and `gh issue list --label "bug,urgent"` are both valid. `gh` does not accept space-separated values after a flag. For lists of files `gh` does something different: it uses variadic positionals, not a flag — `gh release upload <tag> <files>...`, `gh gist create [<filename>...]`. That split is not incidental. A comma is a legal character in a POSIX filename, so a comma-separated `--files a,b` cannot distinguish two paths from one path containing a comma, while label names never hit that ambiguity in practice. These claims are from knowledge of `gh`'s flag layer, not from a local run — `gh` is not installed on the dev VMs. Confirm them against a real `gh` before ratifying the rule. ### Proposed default Read against `gh`, the toolchain is closer to consistent than it first looks, and the rule is value-type-dependent rather than universal: - **Non-path value lists take comma-separated values and repeat.** This is what `forge`'s label flags already do, so no `forge` change falls out of this. - **Path lists do not use comma separation.** `allod change record --files a b` stays as it is, because the `gh`-faithful alternative to space-variadic for paths is variadic positionals, not CSV. - **`cli-design.md` states both halves and cites the `gh` precedent**, replacing the current text that reads as though space-variadic were the universal rule. The alternative worth weighing before settling: make the paths case fully `gh`-shaped by taking them as variadic positionals — `allod change record -m msg a.txt b.txt` — keeping `--files` as an accepted spelling so existing call sites and the shared-checkout guidance in allod/tools#118 keep working. `change record` accepts no positionals today (`allod:564-566` is a catch-all that dies), so nothing becomes ambiguous, and the `--` terminator it would need already exists. This is a larger change and a second interface for the same thing, which is why it is not the default. ### Scope In: the decision itself, whatever parser and usage-text changes follow from it, and the `cli-design.md` rewrite. Out: the `-f`/`--files` short-flag spelling, which matches `forge` and is fine; the `git add -u` fallback; and the end-of-options separator work tracked in allod/tools#53, which this only touches where a value could begin with `-`. ### Validation Whatever shape is chosen, `tests/allod-change.sh` and `tests/forge/validation.sh` should cover it in both tools: a list of two, the repeated form, the two combined, flag order in both directions, an empty list refused rather than falling back to a broader default, and a value that begins with `-` still reachable.
Author
Member

Closing — there is nothing here to build.

forge's label flags already match gh exactly: comma-separated values, repetition accumulates. No change falls out of this issue for forge.

allod change record --files is the only divergence, and it is not really one. gh has no file-list flag to conform to — it takes file lists as positionals precisely because a comma is legal in a filename. So there is no gh shape for --files to converge on, and the current space-variadic form is already the right answer for paths.

Both tools work and are covered by tests. This was a stylistic observation written up as if it were a defect.

The one real problem it surfaced was in allod/memory, not in this repo: cli-design.md stated the space-variadic form as the universal rule, which would lead a reader to write forge --label a b and hit unexpected argument: b. That is corrected in allod/memory#32.

Closing — there is nothing here to build. `forge`'s label flags already match `gh` exactly: comma-separated values, repetition accumulates. No change falls out of this issue for `forge`. `allod change record --files` is the only divergence, and it is not really one. `gh` has no file-list flag to conform to — it takes file lists as positionals precisely because a comma is legal in a filename. So there is no `gh` shape for `--files` to converge on, and the current space-variadic form is already the right answer for paths. Both tools work and are covered by tests. This was a stylistic observation written up as if it were a defect. The one real problem it surfaced was in `allod/memory`, not in this repo: `cli-design.md` stated the space-variadic form as the universal rule, which would lead a reader to write `forge --label a b` and hit `unexpected argument: b`. That is corrected in allod/memory#32.
Author
Member

Closed as not planned.

Closed as not planned.
Sign in to join this conversation.
No description provided.