New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
lib.options: automatically generate example for booleans #18816
Conversation
This reduces the boilerplate that is written for boolean options.
We have to check if !default is valid, otherwise mkOption { type = types.enum [ false "x" ]; default = false; } would yield example = true;
👍 but please also add documentation for these defaults! |
ping @nbp @shlevy do you have an opinion on this? we could remove all explicit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is an interesting idea.
Can you submit a page for the documentation such that we can check if reading such documentation is more pleasing?
docOptionExample = | ||
if opt ? example then { example = scrubOptionValue opt.example; } | ||
else if opt.type == lib.types.bool then | ||
{ example = !docOptionDefault.default or false; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: !(docOptionDefault.default or false)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll change that to improve readability.
if opt ? example then { example = scrubOptionValue opt.example; } | ||
else if opt.type == lib.types.bool then | ||
{ example = !docOptionDefault.default or false; } | ||
else if opt ? type && isBool (docOptionDefault.default or null) && opt.type.check (!docOptionDefault.default) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In opt.type.check (!docOptionDefault.default)
, the attribute default
might not exists, and raise an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should never happen since it's only evaluated if isBool (docOptionDefault.default or null)
else if opt.type == lib.types.bool then | ||
{ example = !docOptionDefault.default or false; } | ||
else if opt ? type && isBool (docOptionDefault.default or null) && opt.type.check (!docOptionDefault.default) then | ||
{ example = !docOptionDefault.default or false; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: !(… or …)
nit: factor this expression in a let … in …
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check if I can factor that out, but I'd rather not add a let.
Reading it again it's strange I check for opt.type == bool
and in the else case check opt ? type
. This looks wrong 😕
IMO, examples for bools contain no useful information. I vote to remove all examples for bools instead. |
I agree with @joachifm, and every example adds 2 lines to |
I think there is no sufficient interest in this and we should rather remove the boolean examples. Closing this for now. RFC sounds useful if people want this. |
They contain no useful information and increase the length of the autogenerated options documentation. See discussion in #18816.
Motivation for this change
Remove the need to provide an example for boolean options. Since there are only two possibilities, setting
example = !default
is reasonable. This should remove the amount of boilerplate in nixpkgs by a few hundred lines.Also catches cases where
type == bool
butdefault
is not defined and providesexample = true
.The second commit catches cases where
type != bool
butisBool default
holds (type may be undefined or something likenullOr bool
) and!default
is a valid value.If this is too expensive we can also drop it, but since this is only executed for the manual generation, it should be ok.
The first commit adds 853 example values to options-db.xml, the second one another 3.
ref #18803 could use this 😉
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"