Skip to content

Commit 5a6adef

Browse files
Smaug123claude
andcommitted
ArgParser: correct map spellability, seen-set scaling, and help syntax
Three follow-ups to the Map support. Only an unspellable enumerated case is rejected. Enumerated values are matched with OrdinalIgnoreCase while an entry is split ordinally, so every casing of a case name is an accepted spelling: a case named `Apple` survives the separator 'A' because it may be spelled `apple`. The check now rejects a case only when some character has no casing which avoids the separators, which is the actual condition for unrepresentability. It also now considers the separators jointly, so a case named `A` is rejected when 'a' and 'A' are the two separators, though neither alone would do it. The seen-key set is carried between occurrences. A map with no entry separator is filled by repeating the argument, and rebuilding the set from every accumulated key on each occurrence made parsing N entries quadratic. Each map field now has a persistent-set slot beside its accumulator, seeded from it in O(1); both are committed only once every entry of the occurrence has parsed and been found fresh, so a failure partway still leaves the field untouched. Help text describes each half of an entry in the syntax that half accepts. A flag DU is spelled true/false and an enumerated value by case name, so rendering the raw type left the user guessing: a map now reports e.g. `map<Severity [one of: Low|High], bool>` rather than `map<Severity, Enabled>`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 76f887f commit 5a6adef

7 files changed

Lines changed: 532 additions & 77 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ An unsupplied `Map` is empty, so (like a `list`) it may not be an `option` or ca
1010
Supplying the same key twice is an error rather than an overwrite.
1111
Each entry is split at its *first* key-value separator, so a value may contain that separator but a key may not.
1212
This means e.g. that some `Map<string, string>` are inexpressible (if the key contains the key-value separator); use `string list` and parse it yourself into a map if you need something smarter.
13-
Where the spellings are known at generation time we check them: an enumerated key or value whose case name contains a separator it must avoid is rejected rather than silently misparsed.
13+
Where the spellings are known at generation time we check them: an enumerated key or value with a case that no spelling can express is rejected rather than silently misparsed.
14+
Help text describes each half of an entry in the syntax that half accepts, so a flag-valued map advertises `map<..., bool>` and an enumerated one lists its case names.
1415

1516
# WoofWare.Myriad.Plugins 10.3.1
1617

ConsumePlugin/GeneratedMapArgs.fs

Lines changed: 278 additions & 6 deletions
Large diffs are not rendered by default.

ConsumePlugin/MapArgs.fs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,34 @@ type MapArgs =
3737
Switches : Map<string, bool>
3838
}
3939

40+
/// A flag-valued union: its values are spelled `true` and `false`, so help text must advertise
41+
/// `bool` rather than the union's name.
42+
type Enabled =
43+
| [<ArgumentFlag true>] Yes
44+
| [<ArgumentFlag false>] No
45+
46+
/// An enumerated type with a case containing a cased letter, so that a cased letter can be used
47+
/// as a separator without making the case unspellable.
48+
type Alpha =
49+
| Apple
50+
| Pear
51+
52+
/// Help text must describe each side of a map in the syntax that side actually accepts, and a
53+
/// cased separator must not be mistaken for one which makes a case unrepresentable.
54+
[<ArgParser>]
55+
type MapDisplayArgs =
56+
{
57+
/// An enumerated key and a flag-valued value: `--features=Low:true`.
58+
[<ArgumentKeyValueSeparator ':'>]
59+
Features : Map<Severity, Enabled>
60+
61+
/// `Apple` contains an `A`, but enumerated values are matched case-insensitively, so the
62+
/// key may be spelled `apple` — which avoids the separator. Every `Map<Alpha, _>` is
63+
/// therefore expressible, e.g. `--casing=appleAvalue`.
64+
[<ArgumentKeyValueSeparator 'A'>]
65+
Casing : Map<Alpha, string>
66+
}
67+
4068
type DeployArgs =
4169
{
4270
[<ArgumentKeyValueSeparator ':'>]

README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,9 @@ For key types which cannot spell the separator in the first place (like an `int`
289289
Do note that some types spell themselves with punctuation: `TimeSpan` uses colons and `Guid` uses hyphens, so choose a separator accordingly.
290290
If you need to express keys containing arbitrary characters, take a `string list` field and do the splitting yourself with whatever logic is appropriate to your domain.
291291

292-
Where the spellings are known at generation time, we check rather than trust: a key or value which is an enumerated union has its case names checked against the separators, and generation fails if one of them contains a separator it must avoid.
293-
(Case names are arbitrary identifiers, so ``` ``a:b`` ``` is a perfectly legal case name which no command line could express as a key when `:` is the separator.)
292+
Where the spellings are known at generation time, we check rather than trust: a key or value which is an enumerated union has its case names checked against the separators, and generation fails for a case which no spelling can express.
293+
(Case names are arbitrary identifiers, so ``` ``a:b`` ``` is a perfectly legal case name, and no command line could express it as a key when `:` is the separator.)
294+
Note "no spelling": enumerated values are matched case-insensitively while entries are split case-sensitively, so a case named `Apple` survives the separator `A` — spell it `apple` — and only a character whose every casing is a separator is fatal.
294295

295296
### Help text
296297

WoofWare.Myriad.Plugins.Test/TestArgParser/TestArgParserMap.fs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,51 @@ module TestArgParserMap =
226226

227227
exc.Message |> shouldContainText "map<string, bool>"
228228

229+
// ------------------------------------------------------- component syntax in help
230+
231+
[<Test>]
232+
let ``Help text describes each side of a map in the syntax that side accepts`` () =
233+
let exc =
234+
Assert.Throws<exn> (fun () -> MapDisplayArgs.parse' noEnv [ "--help" ] |> ignore<MapDisplayArgs>)
235+
236+
// A flag DU is spelled true/false, and an enumerated value by case name, exactly as they
237+
// would be if they were scalar leaves rather than halves of a map entry.
238+
exc.Message |> shouldContainText "map<Severity [one of: Low|High], bool>"
239+
240+
[<Test>]
241+
let ``A cased separator does not make an enumerated case unspellable`` () =
242+
// 'A' is the separator and the case is named `Apple`, but values are matched
243+
// case-insensitively, so `apple` names the case while avoiding the separator.
244+
let args = MapDisplayArgs.parse' noEnv [ "--casing=appleAvalue" ]
245+
246+
args.Casing |> shouldEqual (Map.ofList [ Alpha.Apple, "value" ])
247+
248+
[<Test>]
249+
let ``A flag-valued map parses its values as booleans`` () =
250+
let args =
251+
MapDisplayArgs.parse' noEnv [ "--features=Low:true" ; "--features=High:false" ]
252+
253+
args.Features
254+
|> shouldEqual (Map.ofList [ Severity.Low, Enabled.Yes ; Severity.High, Enabled.No ])
255+
256+
// ------------------------------------------------------- accumulating many occurrences
257+
258+
[<Test>]
259+
let ``Many occurrences accumulate, and a late duplicate is still caught`` () =
260+
// The seen-key set is carried between occurrences rather than rebuilt from the
261+
// accumulator each time; this exercises that it is actually kept up to date.
262+
let entries = List.init 500 (fun i -> $"k%i{i}", $"v%i{i}")
263+
264+
parseLabels entries |> shouldEqual (Map.ofList entries)
265+
266+
let exc =
267+
Assert.Throws<exn> (fun () ->
268+
MapArgs.parse' noEnv (renderLabels entries @ [ "--labels=k0:again" ])
269+
|> ignore<MapArgs>
270+
)
271+
272+
exc.Message |> shouldContainText "Key 'k0' was supplied more than once"
273+
229274
// ------------------------------------------------------- reference implementation
230275

231276
[<Test>]

0 commit comments

Comments
 (0)