Skip to content

Commit

Permalink
lib/options: Use escapeNixIdentifier for showOption
Browse files Browse the repository at this point in the history
  • Loading branch information
infinisil committed Apr 13, 2020
1 parent f9eb3d1 commit 124cccb
Showing 1 changed file with 1 addition and 8 deletions.
9 changes: 1 addition & 8 deletions lib/options.nix
Original file line number Diff line number Diff line change
Expand Up @@ -193,14 +193,7 @@ rec {
(showOption ["foo" "bar" "baz"]) == "foo.bar.baz"
(showOption ["foo" "bar.baz" "tux"]) == "foo.\"bar.baz\".tux"
*/
showOption = parts: let
escapeOptionPart = part:
let
escaped = lib.strings.escapeNixString part;
in if escaped == "\"${part}\""
then part
else escaped;
in (concatStringsSep ".") (map escapeOptionPart parts);
showOption = parts: concatMapStringsSep "." escapeNixIdentifier parts;
showFiles = files: concatStringsSep " and " (map (f: "`${f}'") files);
unknownModule = "<unknown-file>";

Expand Down

7 comments on commit 124cccb

@eadwu
Copy link
Member

@eadwu eadwu commented on 124cccb Apr 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit seems to have broken the manual for me.

nix-build nixos/release-small.nix -A nixos.manual.x86_64-linux

@jtojnar
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, this produces ton of errors like:

generated/options-db.xml:39: validity error : xml:id : attribute value opt-boot.binfmt.registrations."_name_".fixBinary is not an NCName
;.fixBinary" xml:id="opt-boot.binfmt.registrations.&quot;_name_&quot;.fixBinary"
                                                                               ^

Hydra log: https://hydra.nixos.org/build/116629885/nixlog/5

@jtojnar
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed in c652b64

@LnL7
Copy link
Member

@LnL7 LnL7 commented on 124cccb Apr 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with c652b64 the option generation for darwin is still broken.

generated/options-db.xml:3499: validity error : xml:id : attribute value opt-system.defaults.".GlobalPreferences"."com.apple.sound.beep.sound" is not an NCName
.defaults.&quot;.GlobalPreferences&quot;.&quot;com.apple.sound.beep.sound&quot;"

@LnL7
Copy link
Member

@LnL7 LnL7 commented on 124cccb Apr 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly the examples don't seem to be correct at all, this used to be true, which ensured the value was usable as an identifier.

(showOption ["foo" "bar.baz" "tux"]) == "foo.bar.baz.tux"

@infinisil
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I say let's revert both 124cccb and c652b64 for now until we know how to make it work. This commit wasn't essential for #82461 anyways

@LnL7
Copy link
Member

@LnL7 LnL7 commented on 124cccb Apr 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might want to differentiate between displaying options, where they should be quoted properly and the identifiers used for links which should just be unique.

Please sign in to comment.