From ef6d9dfd710cf082b90aa78804f0743aa9b1a8a8 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 4 Apr 2022 13:11:00 +0200 Subject: [PATCH 1/2] pkgs.formats.javaProperties: Add implementation note to type --- .../formats/java-properties/default.nix | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/pkgs/pkgs-lib/formats/java-properties/default.nix b/pkgs/pkgs-lib/formats/java-properties/default.nix index b63b96d32769..6ac56bff4bf6 100644 --- a/pkgs/pkgs-lib/formats/java-properties/default.nix +++ b/pkgs/pkgs-lib/formats/java-properties/default.nix @@ -1,6 +1,30 @@ { lib, pkgs }: { javaProperties = { comment ? "Generated with Nix" }: { + + # Design note: + # A nested representation of inevitably leads to bad UX: + # 1. keys like "a.b" must be disallowed, or + # the addition of options in a freeformType module + # become breaking changes + # 2. adding a value for "a" after "a"."b" was already + # defined leads to a somewhat hard to understand + # Nix error, because that's not something you can + # do with attrset syntax. Workaround: "a"."", but + # that's too little too late. Another workaround: + # mkMerge [ { a = ...; } { a.b = ...; } ]. + # + # Choosing a non-nested representation does mean that + # we sacrifice the ability to override at the (conceptual) + # hierarchical levels, _if_ an application exhibits those. + # + # Some apps just use periods instead of spaces in an odd + # mix of attempted categorization and natural language, + # with no meaningful hierarchy. + # + # We _can_ choose to support hierarchical config files + # via nested attrsets, but the module author should + # make sure that problem (2) does not occur. type = lib.types.attrsOf lib.types.str; generate = name: value: From dcca8f5be287bf7efd4867d0e0ed7643434a8236 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 4 Apr 2022 13:11:47 +0200 Subject: [PATCH 2/2] tests.pkgs-lib.formats: Detect when impossible input is fed --- pkgs/pkgs-lib/tests/formats.nix | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkgs/pkgs-lib/tests/formats.nix b/pkgs/pkgs-lib/tests/formats.nix index 1efe9d8686b1..99b88d36ec3c 100644 --- a/pkgs/pkgs-lib/tests/formats.nix +++ b/pkgs/pkgs-lib/tests/formats.nix @@ -9,7 +9,11 @@ let let formatSet = format args; config = formatSet.type.merge [] (imap1 (n: def: { - value = def; + # We check the input values, so that + # - we don't write nonsensical tests that will impede progress + # - the test author has a slightly more realistic view of the + # final format during development. + value = lib.throwIfNot (formatSet.type.check def) (builtins.trace def "definition does not pass the type's check function") def; file = "def${toString n}"; }) [ def ]); in formatSet.generate "test-format-file" config;