From ac48f07282b1e71614f198a241e8f08b252ed509 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Fri, 7 Oct 2022 09:58:55 +0200 Subject: [PATCH 1/3] lib/types: always use `` instead of `[function body]` to indicate a function inside an option structure The motivation is to have a single identifier for that. Useful for the next commit where I'll try to escape option-parts correctly (options can be any kind of strings, but unless these are Nix identifiers, they must be quoted). Since `` (or ``/`*`) are special identifiers in error messages and the manual, we need a unique way to mark an option part as function call because these are not to be quoted. --- lib/tests/modules.sh | 4 ++-- lib/types.nix | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 57d3b5a76cec1..2be9b5835090c 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -302,11 +302,11 @@ checkConfigOutput '^"baz"$' config.value.nested.bar.baz ./types-anything/mk-mods ## types.functionTo checkConfigOutput '^"input is input"$' config.result ./functionTo/trivial.nix checkConfigOutput '^"a b"$' config.result ./functionTo/merging-list.nix -checkConfigError 'A definition for option .fun.\[function body\]. is not of type .string.. Definition values:\n\s*- In .*wrong-type.nix' config.result ./functionTo/wrong-type.nix +checkConfigError 'A definition for option .fun.. is not of type .string.. Definition values:\n\s*- In .*wrong-type.nix' config.result ./functionTo/wrong-type.nix checkConfigOutput '^"b a"$' config.result ./functionTo/list-order.nix checkConfigOutput '^"a c"$' config.result ./functionTo/merging-attrs.nix checkConfigOutput '^"a bee"$' config.result ./functionTo/submodule-options.nix -checkConfigOutput '^"fun.\[function body\].a fun.\[function body\].b"$' config.optionsResult ./functionTo/submodule-options.nix +checkConfigOutput '^"fun..a fun..b"$' config.optionsResult ./functionTo/submodule-options.nix # moduleType checkConfigOutput '^"a b"$' config.resultFoo ./declare-variants.nix ./define-variant.nix diff --git a/lib/types.nix b/lib/types.nix index 9b2c5e846ad15..db5ae03d6fe92 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -616,8 +616,8 @@ rec { descriptionClass = "composite"; check = isFunction; merge = loc: defs: - fnArgs: (mergeDefinitions (loc ++ [ "[function body]" ]) elemType (map (fn: { inherit (fn) file; value = fn.value fnArgs; }) defs)).mergedValue; - getSubOptions = prefix: elemType.getSubOptions (prefix ++ [ "[function body]" ]); + fnArgs: (mergeDefinitions (loc ++ [ "" ]) elemType (map (fn: { inherit (fn) file; value = fn.value fnArgs; }) defs)).mergedValue; + getSubOptions = prefix: elemType.getSubOptions (prefix ++ [ "" ]); getSubModules = elemType.getSubModules; substSubModules = m: functionTo (elemType.substSubModules m); functor = (defaultFunctor "functionTo") // { wrapped = elemType; }; From 6396482dde41cf3b610f1f56ec77ccf6f82ce502 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Fri, 7 Oct 2022 09:59:01 +0200 Subject: [PATCH 2/3] lib/options/showOption: fix quoting of attr-names that are not identifiers Personally, I think that warnings such as warning: The option `services.redis.enable' defined in `/home/ma27/Projects/nixpkgs/test.nix@node-vm' has been renamed to `services.redis.servers..enable'. are fairly confusing because of the `..` and it's more correct to actually quote that. With this change the warning now looks like this: warning: The option `services.redis.enable' defined in `/home/ma27/Projects/nixpkgs/test.nix@node-vm' has been renamed to `services.redis.servers."".enable'. While implementing that I realized that you'd have a similar problem whenever you use attribute-names that aren't identifiers, e.g. services.nginx.virtualHosts."example.org".locations."/".invalid = 23; now results in the following error: error: The option `interactive.nodes.vm.services.nginx.virtualHosts."example.org".locations."/".invalid' does not exist. Definition values: - In `/home/ma27/Projects/nixpkgs/test.nix@node-vm': 23 Of course there are some corner-cases where this won't work: when generating the manual, you display submodules like this: services.nginx.virtualHosts. Since `` isn't a value, but an indicator for a submodule, it must not be quoted. This also applies to the following identifiers: * `*` for `listOf submodule` * `` for `functionTo` This might not be correct if you actually have a submodule with an attribute name called ``, but I think it's an improvement over the current situation and for this you'd probably need to make even more complex changes to the module system. --- lib/options.nix | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/options.nix b/lib/options.nix index 40c1af667619f..b069cfc7d4f0b 100644 --- a/lib/options.nix +++ b/lib/options.nix @@ -322,10 +322,16 @@ rec { showOption = parts: let escapeOptionPart = part: let - escaped = lib.strings.escapeNixString part; - in if escaped == "\"${part}\"" + # We assume that these are "special values" and not real configuration data. + # If it is real configuration data, it is rendered incorrectly. + specialIdentifiers = [ + "" # attrsOf (submodule {}) + "*" # listOf (submodule {}) + "" # functionTo + ]; + in if builtins.elem part specialIdentifiers then part - else escaped; + else lib.strings.escapeNixIdentifier part; in (concatStringsSep ".") (map escapeOptionPart parts); showFiles = files: concatStringsSep " and " (map (f: "`${f}'") files); From 2480532bd11d1b26659267033345a4812dc063f5 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 9 Oct 2022 10:13:21 +0200 Subject: [PATCH 3/3] nixos/doc: fix build Now we even have options like `services.listmonk.database.settings."app.notify_emails"` shown correctly (i.e. with quotes). --- nixos/lib/make-options-doc/options-to-docbook.xsl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nixos/lib/make-options-doc/options-to-docbook.xsl b/nixos/lib/make-options-doc/options-to-docbook.xsl index 978d5e2468a83..4a5b004f7fe6d 100644 --- a/nixos/lib/make-options-doc/options-to-docbook.xsl +++ b/nixos/lib/make-options-doc/options-to-docbook.xsl @@ -39,8 +39,8 @@ concat('opt-', translate( attr[@name = 'name']/string/@value, - '*< >[]:', - '_______' + '*< >[]:"', + '________' ))" />