-
-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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.mkOptions: allow arbitrary option metadata #341199
base: master
Are you sure you want to change the base?
Conversation
008bb68
to
45b1c5f
Compare
I think this is a good direction.
I'm a little concerned with the UX here because the attribute names won't be checked, or the values for that matter. For it to remain extensible, the type checking would have to added as part of Or would could remove the I think we'll also want to propagate the type information from a module evaluation to all the contained types. We already propagate context like the option prefix, so this would have to follow that pattern. I don't know if this makes the evaluation significantly more expensive. I guess this is the tricky part then. |
I thought of some freeform I'm not sure what would happen in
In the first In the |
I think it makes more sense to allow any arbitrary meta values to be passed in, since
I think |
@roberth What is missing until we can/want to decide if we want to merge this. So should we initially forward the bespoke attributes (description, defaultText, ...) ? Until we actually use them instead of the old ones. Or whats the plan for moving on with this idea? |
It would be up to a few of the many return sub-values of I'm ok to start free-form, if you think it's ok to introduce
I'm not quite certain because I don't have a complete picture for how the migration would go.
So then that means the main code may be ok, but I do have a question. How do you plan to use this field? It doesn't seem that it's propagated to the Could you add a test case showing how you'd like to use this? We have tests in Could you add documentation in |
That makes sense, however I think an additional goal can be to allow module maintainers and/or out-of-tree users to add additional (arbitrary) custom metadata to modules
Sure, it makes sense that well-known attr names would be type-checked where they are used. Such as asserting that
I think that's inevitable, if we want |
Description of changes
Allows adding arbitrary metadata to options.
@roberth @infinisil I am not sure if this is a good idea. But I could need it to build a nice user interface around the modules.
Examples
Justification for the examples:
(Especially when trying to build a more high level user interface)
-_* \/^
) or just where the name of the option might not make sense for a certain user anymore. I.e.title = "FooBar";
would allow to display the option with a more explanatory label without creating an intermediate option, just for the purpose of rendering.I am not sure if we should add those use-cases to the current keywords, or if we should just allow arbitrary names because i am not sure how the above described scenarios would ideally be represented through new keywords.
Alternatives
instead of explicit
meta ? {}
passthru
instead of meta, maybe it is more clear.Could produce less thunks but also is less safe towards future extensions of mkOption attrs.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.