From d869a4410a88333d328358c723609577d32f3380 Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Wed, 19 Aug 2026 14:09:09 +0200 Subject: fix(naming): config.ml merges every [defaults] block, like lang.ml config.ml and lang.ml both parse the INI format through the same reader, Colitur_kernel.Overlay_ini.parse_sections, but resolved a repeated [section] header oppositely: lang.ml folds over every section sharing a name, while config.ml used List.find_opt and silently discarded every [defaults] block after the first. Two modules parsing one file format must not disagree about what a duplicate section header means. of_string now folds a single accumulator across every section named [defaults], in file order, matching lang.ml's of_string shape. A scalar key (lang/template/format) repeated across two blocks resolves to the later value, consistent with the existing within-section last-wins rule; overlay keeps accumulating across every block, not only the first; and unknown_sections still excludes every [defaults] block, merged or not, since merging it is the point. config.mli's lang doc comment is extended to say the last-wins rule holds across block boundaries too, cross-referencing lang.ml's own duplicate-section policy so the two do not drift again unnoticed. --- lib/naming/config.ml | 56 ++++++++++++++++++++++++++++++++++----------------- lib/naming/config.mli | 7 ++++++- 2 files changed, 43 insertions(+), 20 deletions(-) (limited to 'lib/naming') diff --git a/lib/naming/config.ml b/lib/naming/config.ml index 0fafd1f..7d87d62 100644 --- a/lib/naming/config.ml +++ b/lib/naming/config.ml @@ -33,27 +33,45 @@ let of_string text = (fun (s : OI.section) -> if s.OI.name = "defaults" then None else Some s.OI.name) sections in + (* Merge EVERY section named [defaults], not just the first: [lang.ml]'s + [of_string] was fixed this morning to fold over all matching + sections rather than take [List.find_opt]'s first match, because a + hand-edited file WILL grow duplicate headers as a user appends to it + over time. Both modules parse the same reader + ([Overlay_ini.parse_sections]) over the same file format, so they + must not disagree about what a duplicate [section] header means -- + taking only the first [defaults] block here silently discarded every + later one, with nothing pointing back at the parser. Folding a + single accumulator across every matching section, in file order, + keeps this consistent with the within-section behaviour below (last + [k]-match wins): a scalar key repeated across two blocks resolves to + the LATER value, and [overlay] keeps accumulating across every + block, not only its first. *) + let defaults_sections = + List.filter (fun (s : OI.section) -> s.OI.name = "defaults") sections + in let acc = - match List.find_opt (fun (s : OI.section) -> s.OI.name = "defaults") sections with - | None -> empty - | Some s -> - (* Cons then reverse once at the end, not `@ [v]` per line: the - latter is O(n^2) over the field count, a real hang on a - machine-generated file with many overlay lines. *) - let acc = - List.fold_left - (fun acc (k, v) -> - match k with - | "lang" -> { acc with lang = Some v } - | "template" -> { acc with template = Some v } - | "format" -> { acc with format = Some v } - (* accumulates: a user has more than one overlay *) - | "overlay" -> { acc with overlays = v :: acc.overlays } - | other -> { acc with unknown_keys = other :: acc.unknown_keys }) - empty s.OI.fields - in - { acc with overlays = List.rev acc.overlays; unknown_keys = List.rev acc.unknown_keys } + (* Cons then reverse once at the very end, not `@ [v]` per line: the + latter is O(n^2) over the field count, a real hang on a + machine-generated file with many overlay lines. Reversing only + after every section has been folded (not once per section) is + what keeps [overlay] and [unknown_keys] in file order across + block boundaries, not merely within one block. *) + List.fold_left + (fun acc (s : OI.section) -> + List.fold_left + (fun acc (k, v) -> + match k with + | "lang" -> { acc with lang = Some v } + | "template" -> { acc with template = Some v } + | "format" -> { acc with format = Some v } + (* accumulates: a user has more than one overlay *) + | "overlay" -> { acc with overlays = v :: acc.overlays } + | other -> { acc with unknown_keys = other :: acc.unknown_keys }) + acc s.OI.fields) + empty defaults_sections in + let acc = { acc with overlays = List.rev acc.overlays; unknown_keys = List.rev acc.unknown_keys } in Ok { acc with unknown_sections } let resolve ~flag ~config ~default = diff --git a/lib/naming/config.mli b/lib/naming/config.mli index 27f0c44..a84f95b 100644 --- a/lib/naming/config.mli +++ b/lib/naming/config.mli @@ -17,7 +17,12 @@ val of_string : string -> (t, string) result repeated key is LAST-WINS -- the opposite direction from {!Colitur_kernel.Overlay_ini.get}'s first-wins over the same [section] type -- because the natural reading of a config file a user edited by - hand and appended to is "the bottom line is the one that took effect". *) + hand and appended to is "the bottom line is the one that took effect". + This holds whether the repeat is within one [\[defaults\]] block or + across two of them: every section named [defaults] is merged, not only + the first, the same duplicate-section policy {!Lang.of_string} documents + for its own sections -- the two modules read the same underlying format + and must not disagree about what a repeated header means. *) val lang : t -> string option val overlays : t -> string list -- cgit v1.3