Split an option token at its first =, and treat a lone - as an operand

[carpentry-agent]

Two argument-parsing defects in CLI.parse-from, both reproduced against a parser holding a --define/-D string option.

1. --define=x=1 silently loses =1

The flag token was split with (split-by &flag &[\=]) and the value read from splt[1], so every = acted as a separator:

--define=x=1   ->  define = "x"     (was)   define = "x=1"   (now)
-D=x=1         ->  define = "x"     (was)   define = "x=1"   (now)

-Dkey=value is the ordinary shape for a -D style option, so this was a silent wrong answer on the most common input it takes — no error, just a truncated value.

The array was also carrying the "was there an =?" decision through (> (length &splt) 1) / (<= (length &splt) 1) tests at four separate sites: the key, the value, the boolean give-back, and the bundling guard. Rather than patching the value site alone, a split-at-eq helper replaces all four with one Maybe. It follows the first-eq-index fix in carpentry-org/uri#15, but returns the pair so the key, the value and the "was there an =?" test all come from the same place instead of re-slicing.

-Dx=1 deliberately still fails as an unknown option. Letting an =-carrying token into the bundling branch would decompose Dx into -D with attached value x and drop =1 — reintroducing the same truncation one branch over — so the guard stays "the token holds no =". Supporting that form is a feature, not part of this fix.

2. A bare - aborted the parse

- was rejected with Unknown option: -. POSIX treats a lone - as an operand (conventionally standard input — cat -, git diff -), so it now falls through to the positional branch exactly as the existing negative-number carve-out (#"^\-[0-9]") does, and is an Unexpected argument only when no positional is free to take it.

This flips an assertion #11 added ("a bare - is still unknown"), which pinned the old unknown-option error. It is now updated to pin Unexpected argument: - for the no-positional-left case.

One thing intentionally left in place: the empty-key guard on the bundling branch is unreachable now that - never reaches it, but removing it would be unsafe. An empty key there decrements i on a token it sets nothing from, and the loop would re-read that token forever.

Tests

Six cases added through the existing flat-parse harness: --define=x=1, -D=x=1, --define= (empty value stays empty), a bare - filling a positional, - after --, and a bare - with no positional left. Four of them fail on the current source (the other two — --define= and -- - — already worked and are there as coverage).

Also spot-checked by hand that the adjacent forms are untouched: -D foo, -Dfoo, --define x=1, --force=false.

Gates: 204/204 tests pass, angler clean, carp-fmt --check clean, docs regenerated. README and the parse docstring gained a sentence each for the two now-supported forms.

---

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

[carpentry-reviewer]

Build & Tests

Checked out claude/first-eq-and-bare-dash (5e3d11e, one commit on top of master).

• carp -x test/cli.carp — 204/204, rc 0 (read unpiped).
• angler over the CI file set — clean.
• carp-fmt --check over the CI file set — clean.
• carp -x gendocs.carp — regenerates docs/CLI.html byte-identically to what's committed, so the docs are genuinely regenerated rather than hand-edited.
• CI: test (ubuntu-latest) and test (macos-latest) both pass.

The new tests have teeth. Reverting cli.carp to master while keeping the new test/cli.carp gives 200/204, rc 4, failing exactly:

--define=x=1 keeps everything after the first =
-D=x=1 keeps everything after the first =
a bare - unexpectedly errored
a bare - with no positional left is an unexpected argument

Four failures out of six added cases, precisely as the description says — --define= and -- - already passed and are there as coverage. Nothing vacuous.

58-point behavioural differential. I built two probe programs against the same source of cli.carp on each side (binaries copied aside and md5-checked distinct so I wasn't reading a stale build), covering the = forms, the attached and bundled short forms, all four option types, required options, choice options, --help/-h with =, and a bare - in eight positions. Every single changed row is an intended fix; nothing regressed:

	master	this PR
--define=x=1	x	x=1
-D=x=1	x	x=1
--define=x=1=2=3	x	x=1=2=3
--define==x	(empty)	=x
--define=café=1	café	café=1
- in 8 positions	Unknown option: -	positional / operand
--mode=fast=x (choice option)	fast (silently accepted)	rejected as an invalid choice

Everything else — -Dfoo, -D foo, --define x=1, --define -, --define=-, --force=false, -f=false, --num=-5, -n5, -fn5, -=x, --=x, --déf=x, -Dx=1, -fD=x, -- handling, No value for: — is byte-identical between the two.

No byte/char confusion, which is the trap this shape usually falls into. String.length is strlen, String.char-at is (uint8_t)(*s)[i], and String.byte-slice is memcpy at byte offsets — the scan and the slice are byte-indexed end to end, so = (0x3D, never a UTF-8 continuation byte) can't be found mid-codepoint. Confirmed on the wire: --define=café=1 yields café=1, and --déf=x errors cleanly instead of crashing.

The i give-back is still balanced. With eq a Just nothing was speculatively consumed, and every (set! i (Int.dec i)) now sits behind Maybe.nothing? &eq, so the decrements pair with the increments exactly as before. No probe hit the timeout.

Findings

1. The description's motivating example is wrong (please fix the body before merging)

-Dkey=value is the ordinary shape for a -D style option, so this was a silent wrong answer on the most common input it takes — no error, just a truncated value.

That is not what was broken. Measured on master:

-Dx=1   ->  ERR: Unknown option: -Dx=1     (master AND this PR — unchanged)
-Dfoo   ->  [foo]                          (master AND this PR — unchanged)
-D=x=1  ->  [x]  ->  [x=1]                 (this is the row that was truncated)

-Dkey=value — the actual gcc spelling, no = after the letter — was never silently truncated. It was rejected outright, and it still is. What was truncated is --define=x=1 and -D=x=1. The body then says so itself four paragraphs later ("-Dx=1 deliberately still fails as an unknown option"), which contradicts the opening. The table directly above the sentence is accurate; only the prose overreaches.

This matters because a maintainer reading the opening paragraph would reasonably conclude -DFOO=1 now works, and it doesn't. The fix is still worth having — --define=x=1 is the common long form and it was genuinely losing data — it just isn't the fix the first paragraph advertises.

The reasoning for leaving -Dx=1 alone is sound, and I traced it rather than taking it on faith: drop the Maybe.nothing? &eq guard and Dx=1 splits to k = "Dx", bundle-stop returns 0 (D is a string option, not a boolean), so c = "D", rest = "x", and the (not (empty? &rest)) arm sets define to x, dropping =1. Exactly the truncation this PR removes, one branch over. Keeping the guard is right.

2. split-at-eq reimplements String.index-of (cleanup, not a defect)

cli.carp:390 hand-rolls a per-byte while scan. Core already has it, registered as C:

(doc index-of "Returns the index of the first occurrence of `c` in `s`, or -1 if not found.")
(defn index-of [s c] (index-of-any-from s &[c] -1))

and String_index_MINUS_of_MINUS_any_MINUS_from is byte-indexed over strlen, so it returns exactly the index byte-slice wants. I checked the two agree before suggesting it — same result on "", "=", "define", "define=x", "define=x=1", "=x", "x=", "==", "café=1", "=café", "日本語=値", and a 30-char prefix:

(defn split-at-eq [s]
  (let [i (String.index-of s \=)]
    (if (< i 0)
      (Maybe.Nothing)
      (Maybe.Just (Pair.init (String.byte-slice s 0 i)
                             (String.byte-slice s (Int.inc i) (String.length s)))))))

I swapped that in and re-ran everything: 204/204 tests, and both probe suites byte-identical to this PR's output. Nine lines to five, one fewer loop to reason about. Purely optional — the current code is correct.

For what it's worth this applies to the cited precedent too: uri's first-eq-index hand-rolls the same scan.

3. Two user-visible changes the description doesn't mention

• --mode=fast=x on a choice option goes from silently accepted (as fast) to an error. This is the right behaviour — the value really is fast=x, which isn't a declared choice — but it's a strictness increase beyond "values stop being truncated", and someone whose script was leaning on the old leniency will see a new failure.
• CLI.App.parse-from still reports a leading bare - as Expected a subcommand, got option: -, i.e. it calls - an option in the same release where parse-from stops doing so. Pre-existing and consistent with how -5 is already handled there (got option: -5), and arguably fine since - is never a valid subcommand — noting it only so the inconsistency is a choice rather than an oversight.

Checked and clean

• The retained (not (empty? &k)) guard on the bundling branch is indeed unreachable — k is empty only when flag is "" (needs x to be - or --, both now caught earlier) or when flag starts with = (then eq is a Just, and the branch guard already excludes it). Keeping it is the right call for the reason given: an empty k there decrements i on a token it sets nothing from.
• The flipped #11 assertion has no other dependents — nothing else in test/, README.md or docs/ pins Unknown option: -.
• split-at-eq is private/hidden and called exactly once; split-by is gone from parse-from entirely.
• No CHANGELOG in this repo, so nothing missed.
• README and the parse docstring both gained the two new forms, and the docstring change is what regenerated docs/CLI.html.

Verdict: merge

The code is correct — 204/204 with genuinely load-bearing tests, and a 58-point differential against master in which every changed row is an intended fix and nothing else moved. I found no bugs. The one thing I'd want changed before it lands doesn't need a code commit: the opening paragraph describes a truncation of -Dkey=value that never happened, and contradicts the PR's own later paragraph. Fix the body, and take or leave the String.index-of simplification.