change record: --files rejects the list of paths its name and usage string promise #128
Labels
No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
allod/tools#128
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Make
allod change record's--filesaccept the list of paths its name and its usage string both promise, so--files a bstages both instead of aborting on the second.Primary goals:
--filesconsumes every following path up to the next option, and repeating the flag still accumulates, so both spellings work and combine.[--files <file>...]is what the usage string has always said; the parser is the half that disagrees with it.change recordtakes no positional arguments, so consuming up to the next--prefixed token cannot swallow anything else.Current state
allod change record -m <message> [--files <file>...](allod:37, and identically for-Mat:38). The trailing...reads as "this option takes one or more values", which is what a flag named--filesshould do.The parser consumes exactly one:
at
allod:530-533. A second path therefore reaches the catch-all atallod:542-543and aborts withunexpected argument for change record: <path>. Repeating the flag works, becausefiles+=accumulates across occurrences:Observed while recording a two-file change in
allod/nexus.Why it is worth fixing
git-workflow.mdinstructs agents to name files when recording in a shared checkout, becauserecordotherwise runsgit add -u(allod:552-556), which stages another agent's tracked edits — the concern behind allod/tools#118. So the one invocation shape the memory prescribes as the safe path is also the shape whose documented form is rejected.The failure itself is benign and loud: it dies before
git add, touches nothing, and names the offending argument. This is a usability defect, not a safety one, and should not be triaged as though staging were at risk.Change
Have
-f|--filesconsume paths until the next--prefixed token, so--files a b,--files a --files b, and-f a b --files call name the same set. Order stays free, because a following option ends the list:--files a b -m msgand-m msg --files a bare equivalent. A list that consumed nothing keeps failing with the existingrequires a valuewording rather than falling through to thegit add -usweep, and--ends option parsing so a path beginning with-stays reachable. The usage strings at:37and:38are already accurate and stay as they are.This reverses the direction first recorded in this issue, which named correcting the usage string as the default and the variadic parser as the alternative. That was backwards. The usage string described what a flag named
--filesshould do; the parser was the defect, and editing the documentation to match it would have documented the bug.Scope
In: the
-f|--filesparser forchange record, the--terminator it needs, and tests covering the variadic form, the repeated form, their combination, flag order in both directions, an empty list, and a path beginning with-. Out: the-f/--filesshort-flag spelling itself, which is fine and matchesforge; thegit add -ufallback behavior; and the shared-checkout sweeping concern in allod/tools#118, which this only touches through its file-naming flag.Residual risk
R0. Argument parsing for one subcommand, failing loud today and before any git mutation. Every invocation that works today keeps working — a single
--files aand the repeated--files a --files bparse exactly as before — and the forms this adds all abort today, so nothing can regress from working to broken.change record: --files usage string implies a variadic list the parser rejectsto change record: --files rejects the list of paths its name and usage string promise