Skip to content

Commit

Permalink
lib/options/showOption: fix quoting of attr-names that are not identi…
Browse files Browse the repository at this point in the history
…fiers

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.<name>

Since `<name>` 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`
* `<function body>` for `functionTo`

This might not be correct if you actually have a submodule with an
attribute name called `<name>`, 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.
  • Loading branch information
Ma27 committed Oct 7, 2022
1 parent ac48f07 commit 6396482
Showing 1 changed file with 9 additions and 3 deletions.
12 changes: 9 additions & 3 deletions lib/options.nix
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
"<name>" # attrsOf (submodule {})
"*" # listOf (submodule {})
"<function body>" # 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);

Expand Down

0 comments on commit 6396482

Please sign in to comment.