From 6396482dde41cf3b610f1f56ec77ccf6f82ce502 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Fri, 7 Oct 2022 09:59:01 +0200 Subject: [PATCH] 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 40c1af667619..b069cfc7d4f0 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);