diff options
| author | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-08-19 13:43:28 +0200 |
|---|---|---|
| committer | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-08-19 13:43:28 +0200 |
| commit | 6bdd5afc5e185b639f9cf5e7c3d662ddc3ecf0e1 (patch) | |
| tree | 0fcef5aa47fa6b58c296ed9655c0c8e3be95ab53 /lib/naming | |
| parent | b26089630a61a54060c86ec26be6dc4fe5418b12 (diff) | |
| download | colitur-6bdd5afc5e185b639f9cf5e7c3d662ddc3ecf0e1.tar.gz colitur-6bdd5afc5e185b639f9cf5e7c3d662ddc3ecf0e1.zip | |
fix(naming): merge duplicate [section] blocks in the language table
F1 (review round 1): of_string's find took only the FIRST section of a
given name (List.find_opt), so a second [celebration] block anywhere in
the file was silently dropped in its entirety -- reproduced with two
blocks (a in the first, b in the second): b resolved to the slug
fallback "b", not its real value.
This is a data-loss footgun aimed squarely at what happens next: Tasks
3/4 write a 595-entry, hand-edited la.ini, and appending a second
[celebration] block is the natural way to paste in a new batch of names.
Worse, the failure surfaces nowhere near its cause -- a coverage check
reports the dropped slugs as missing a Latin name, with nothing pointing
at the parser.
find now folds over every section sharing the name, in file order, so
all blocks merge. This also settles which value wins when the same key
appears in two different blocks: later in the file wins, consistent with
the existing within-one-block behaviour (unchanged, still last SM.add
wins) and with what a reader expects when appending to an INI file.
lang.mli now documents both duplicate policies explicitly, and notes
they run OPPOSITE to Overlay_ini.get's first-match (List.assoc_opt) over
the same section.fields shape -- undocumented before, and a latent trap
since the two modules read the same section type but resolve a
duplicate key in opposite directions.
Three tests added: two [celebration] blocks both resolve (the F1
regression), a key repeated across two blocks resolves to the later
block, and a key repeated within one block still resolves to the later
line (confirms unchanged behaviour). Confirmed the regression test fails
against the pre-fix code (b resolves to "b", the slug fallback) and
passes after.
Diffstat (limited to 'lib/naming')
| -rw-r--r-- | lib/naming/lang.ml | 19 | ||||
| -rw-r--r-- | lib/naming/lang.mli | 18 |
2 files changed, 33 insertions, 4 deletions
diff --git a/lib/naming/lang.ml b/lib/naming/lang.ml index 0d902d5..147a49b 100644 --- a/lib/naming/lang.ml +++ b/lib/naming/lang.ml @@ -66,10 +66,23 @@ let of_string text = match OI.parse_sections text with | Error e -> Error e | Ok sections -> + (* Merge EVERY section sharing [name], not just the first: a hand-edited + 595-entry language file (Tasks 3/4's la.ini) WILL grow duplicate + [section] headers as contributors append entries over time -- a + second [celebration] block is the natural way to paste in a new + batch of names. Taking only the first match (the original + [List.find_opt] here) silently dropped every later block; the + failure then surfaces as a coverage report claiming those slugs have + "no Latin name", with nothing pointing back at the parser. Folding + over all matching sections, in file order, keeps this consistent + with the existing within-section behaviour below (last [SM.add] + wins): a key repeated across two blocks resolves to the later one, + exactly what a reader expects when appending to an INI file. *) let find name = - match List.find_opt (fun (s : OI.section) -> s.OI.name = name) sections with - | Some s -> List.fold_left (fun m (k, v) -> SM.add k v m) empty_table s.OI.fields - | None -> empty_table + List.fold_left + (fun m (s : OI.section) -> + if s.OI.name = name then List.fold_left (fun m (k, v) -> SM.add k v m) m s.OI.fields else m) + empty_table sections in let meta = find "meta" in (match SM.find_opt "lang" meta with diff --git a/lib/naming/lang.mli b/lib/naming/lang.mli index c4432f6..36931d4 100644 --- a/lib/naming/lang.mli +++ b/lib/naming/lang.mli @@ -13,7 +13,23 @@ type t (** Parse INI text. Never raises. [Error] on a malformed file or a missing - [\[meta\] lang]. *) + [\[meta\] lang]. + + Two duplicate policies, both LAST-WINS: + - A section name repeated in the file (e.g. two [\[celebration\]] + blocks) has ALL of its blocks merged, not only the first -- a + 595-entry hand-edited language file WILL grow duplicate section + headers as contributors append entries over time, and dropping a + later block would silently lose real translations. + - Where the same key appears more than once -- within one block or + across two of them -- the value from further down the file wins. + + Both read the same order a reader would: later in the file overrides + earlier. This is the OPPOSITE direction from + {!Colitur_kernel.Overlay_ini.get} ([List.assoc_opt], first match) over + the very same [section.fields] shape -- the two modules resolve a + duplicate key in opposite directions, so do not assume one's behaviour + from the other's. *) val of_string : string -> (t, string) result val code : t -> string |
