From 59fbd3718dbf2721557b97d7b4e87dce38fb745c Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Wed, 19 Aug 2026 13:48:39 +0200 Subject: fix(naming): config fix round 1 -- unknown sections, O(n) accumulate F1: test_unknown_key_is_reported_not_fatal never asserted unknown_keys itself, only that parsing survives -- a no-op accumulator passed it. Now asserts the key is actually collected. F2: a misspelled section name, e.g. [deafults], was silently discarded -- Ok empty, lang and everything else gone, nothing reported. That is the highest-value typo this feature exists to catch. Any section other than [defaults] is now collected into a new Config.unknown_sections, kept separate from unknown_keys so the CLI can word the two warnings differently. Still non-fatal: a newer colitur's added section must not break an older binary. F3: overlays and unknown_keys accumulated with '@ [v]' per line, O(n^2) over the field count. Cons during the fold, List.rev once at the end. F4: documented that lang/template/format are last-wins on a repeated key, the opposite direction from Overlay_ini.get's first-wins over the same section type. --- lib/naming/config.ml | 55 ++++++++++++++++++++++++++++++++++----------------- lib/naming/config.mli | 22 ++++++++++++++++++--- 2 files changed, 56 insertions(+), 21 deletions(-) (limited to 'lib') diff --git a/lib/naming/config.ml b/lib/naming/config.ml index ce1b433..0fafd1f 100644 --- a/lib/naming/config.ml +++ b/lib/naming/config.ml @@ -6,36 +6,55 @@ type t = { template : string option; format : string option; unknown_keys : string list; + unknown_sections : string list; } -let empty = { lang = None; overlays = []; template = None; format = None; unknown_keys = [] } +let empty = + { lang = None; overlays = []; template = None; format = None; unknown_keys = []; + unknown_sections = [] } let lang t = t.lang let overlays t = t.overlays let template t = t.template let format t = t.format let unknown_keys t = t.unknown_keys +let unknown_sections t = t.unknown_sections let of_string text = match OI.parse_sections text with | Error e -> Error e - | Ok sections -> ( - match List.find_opt (fun (s : OI.section) -> s.OI.name = "defaults") sections with - | None -> Ok empty - | Some s -> - 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 = acc.overlays @ [ v ] } - | other -> { acc with unknown_keys = acc.unknown_keys @ [ other ] }) - empty s.OI.fields - in - Ok acc) + | Ok sections -> + (* Any section that is not [defaults] is unrecognised -- including a + plain typo such as [deafults] -- and must be REPORTED, never + silently dropped: that is precisely the failure this feature exists + to surface. *) + let unknown_sections = + List.filter_map + (fun (s : OI.section) -> if s.OI.name = "defaults" then None else Some s.OI.name) + 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 } + in + Ok { acc with unknown_sections } let resolve ~flag ~config ~default = match flag with diff --git a/lib/naming/config.mli b/lib/naming/config.mli index 41ff429..27f0c44 100644 --- a/lib/naming/config.mli +++ b/lib/naming/config.mli @@ -13,16 +13,32 @@ type t val empty : t val of_string : string -> (t, string) result +(** [lang], [template] and [format] are each set from a single field. A + 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". *) val lang : t -> string option + val overlays : t -> string list val template : t -> string option val format : t -> string option -(** Keys present in the file that this build does not understand. Reported, never - fatal: a config written for a newer colitur must still work on an older one, - but silently ignoring a line the user wrote is how a typo becomes invisible. *) +(** Keys present in the [\[defaults\]] section that this build does not + understand. Reported, never fatal: a config written for a newer colitur + must still work on an older one, but silently ignoring a line the user + wrote is how a typo becomes invisible. *) val unknown_keys : t -> string list +(** Section names other than [\[defaults\]], reported separately from + {!unknown_keys} so the CLI can word the two warnings differently (a + misspelled section, e.g. [\[deafults\]], versus a misspelled key inside a + recognised one). Also never fatal, and never silent: a section this build + does not recognise is exactly the highest-value typo this feature exists + to catch, because it silently discards the whole section -- [lang] and + everything else in it -- with no other way for the user to notice. *) +val unknown_sections : t -> string list + (** [resolve ~flag ~config ~default] returns [(value, source)] with source one of ["flag"], ["config"], ["default"]. Precedence is flag > config > default. *) val resolve : flag:string option -> config:string option -> default:string -> string * string -- cgit v1.3