diff options
| -rw-r--r-- | lib/naming/config.ml | 55 | ||||
| -rw-r--r-- | lib/naming/config.mli | 22 | ||||
| -rw-r--r-- | test/test_config.ml | 22 |
3 files changed, 75 insertions, 24 deletions
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 diff --git a/test/test_config.ml b/test/test_config.ml index 9100380..067b255 100644 --- a/test/test_config.ml +++ b/test/test_config.ml @@ -46,11 +46,25 @@ let test_malformed_is_error_not_crash () = (* An unknown key is a WARNING case, not a hard error: a config written for a newer colitur must still work on an older one. But it must be reportable, so - it is not silently dropped either. *) + it is not silently dropped either -- assert it is actually COLLECTED, not + only that parsing survives it: a no-op accumulator would also pass a test + that checked survival alone. *) let test_unknown_key_is_reported_not_fatal () = match C.of_string "[defaults]\nlang = la\nnonsense = 1\n" with | Error _ -> Alcotest.fail "an unknown key must not be fatal" - | Ok c -> Alcotest.(check (option string)) "known key still read" (Some "la") (C.lang c) + | Ok c -> + Alcotest.(check (option string)) "known key still read" (Some "la") (C.lang c); + Alcotest.(check (list string)) "unknown key reported" [ "nonsense" ] (C.unknown_keys c) + +(* The highest-value case: a misspelled SECTION name (not merely a misspelled + key inside a recognised one). Before this fix the whole section, lang + included, vanished with nothing reported -- exactly the typo this feature + exists to surface, in its worst form: a user whose config silently does + nothing has no way to discover why. *) +let test_misspelled_section_is_reported_not_silently_dropped () = + let c = ok (C.of_string "[deafults]\nlang = pl\n") in + Alcotest.(check (option string)) "lang not read from the wrong section" None (C.lang c); + Alcotest.(check (list string)) "section reported" [ "deafults" ] (C.unknown_sections c) let suite = ( "Config", @@ -59,4 +73,6 @@ let suite = Alcotest.test_case "empty is all none" `Quick test_empty_config_is_all_none; Alcotest.test_case "precedence and provenance" `Quick test_precedence_and_provenance; Alcotest.test_case "malformed is error" `Quick test_malformed_is_error_not_crash; - Alcotest.test_case "unknown key not fatal" `Quick test_unknown_key_is_reported_not_fatal ] ) + Alcotest.test_case "unknown key not fatal" `Quick test_unknown_key_is_reported_not_fatal; + Alcotest.test_case "misspelled section reported" + `Quick test_misspelled_section_is_reported_not_silently_dropped ] ) |