lib.generators.toPlist: add floats #42938
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matthewbauer
requested review from
Profpatsch and
LnL7
and removed request for
edolstra,
nbp and
Profpatsch
LnL7
suggested changes
Jul 3, 2018
| if isAttrs x then attrs ind x else | ||
| if isList x then list ind x else | ||
| if isAttrs x then attrs ind x else | ||
| if isFloat x then float ind x else |
There was a problem hiding this comment.
builtins.isFloat was not available in 1.11 and we should keep nixpkgs compatible until 18.09. But we can add a fallback as long as nothing in nixpkgs itself uses floats.
eg.
let
toFloat = builtins.toFloat or (_: false);
...
LnL7
reviewed
Jul 3, 2018
| @@ -177,13 +177,14 @@ rec { | |||
|
|
|||
| # PLIST handling | |||
| toPlist = {}: v: let | |||
There was a problem hiding this comment.
Guess it's a bit late, but I kind of disagree with the casing plist is an acronym. It should be toPLIST and toPropertyList IMHO.
matthewbauer
force-pushed
the
add-float-to-plist
branch
from
ba40373
to
da87cb1
Compare
LnL7
reviewed
Jul 3, 2018
lib/generators.nix
Outdated
| abort "generators.toPlist: should never happen (v = ${v})"; | ||
|
|
||
| literal = ind: x: ind + x; | ||
|
|
||
| bool = ind: x: literal ind (if x then "<true/>" else "<false/>"); | ||
| int = ind: x: literal ind "<integer>${toString x}</integer>"; |
There was a problem hiding this comment.
I'm pretty sure this is necessary, same with floats.
There was a problem hiding this comment.
Really? I'll see what happens but I thought that ${} would convert it to a string automatically.
Nix now supports floats & we can pretty easily map them to Plist’s <real></real> type. Note that I am unsure how this affects older version of Nix that may or may not have builtins.isFloat available. Make sure this satisfies minver.nix’s "1.11" requirement.
matthewbauer
force-pushed
the
add-float-to-plist
branch
from
da87cb1
to
dc72e8a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Nix now supports floats & we can pretty easily map them to Plist’s
type. Note that I am unsure how this affects older
version of Nix that may or may not have builtins.isFloat available.
Make sure this satisfies minver.nix’s "1.11" requirement.
/cc @3noch