By analyzing the blame information on this pull request, we identified @edolstra, @maggesi and @shlevy to be potential reviewers

This PR doesn't really describe what it does. What does the generated manpage look like?

👎 on function names like mkWheneverToPkgOption, I have no idea what that does. In general, I don't like adding more abstraction/obfuscation to the NixOS module system - when I see mkOption I know what it does; functions like mkEnableOption' just make it harder to understand a module.

This PR doesn't really describe what it does. What does the generated manpage look like?

It (mostly) does #9306, but I guess it's easier to show, than to explain. Added a screenshot to the top.

👎 on function names like mkWheneverToPkgOption, I have no idea what that does. In general, I don't like adding more abstraction/obfuscation to the NixOS module system - when I see mkOption I know what it does; functions like mkEnableOption' just make it harder to understand a module.

Yep, names are subject to change. mkWheneverToPkgOption would probably
be gone too. This is just an experiment for me and a preview for you
that should hopefully show me whether there's a chance to merge this
upstream (or should I just relax and implement it on top of my own
branch full of awesome rejected things).

NixOS already has mkEnableOption, I just implemented a better one
(currently by copy-paste, but I plan to drop/deprecate current
mkEnableOption after I refactor everything). Currently
mkEnableOption is actually enforced in new PRs.

So, you can safely assume that the current set of patches won't add any
new mkOption-like functions (or one, in case of mkEnableOption
deprecation).

Moreover, I actually plan to add another mkOption variant for
whatever.package'-like options. E.g. considerfonts.fonts` option for
a moment. Wouldn't it be awesome to list most of (or even all) the
available fonts in the man page? My current implementation (out of this
branch yet) does this by:

font.fonts = mkPackageListOption {
  name = "List of primary font paths.";
  default = literalPackages pkgs [ "pkgs.some" "pkgs.font" ];
  example = literalPackages pkgs [ "pkgs.dejavu_fonts" ];
  packages = literalPackages pkgs [ "all" "other" "fonts" ];
};

which reduces to

font.fonts = mkOption {
  name = "List of primary font paths.";
  default = [ pkgs.some pkgs.font ];
  defaultText = ''[ "pkgs.some" "pkgs.font" ]'';
  example = literalExample ''[ "pkgs.dejavu_fonts" ]'';
  packages = literalPackages pkgs [ "pkgs.some" "pkgs.font" "pkgs.dejavu_fonts" "all" "other" "fonts" ];
};

Moreover, if one to group all the fonts in top-level/all-packages.nix
into a common attrset ''"all" "other" "fonts"'' part could become
completely automatic.

Can we agree that this is cool? (I would then also try to drop
`defaultText' thing altogether, at least from the nixos modules, because
it's only ever used in this type of use case.)

Ask away if something is still unclear.

/cc @nbp @bjornfor from #9306

I've just read through the patch (and your comments), and I honestly don't get what it's doing. Is there a 5-word summary for us mere mortals? A better mkEnableOption sounds great.

You write this
https://github.com/NixOS/nixpkgs/pull/14860/files#diff-d82ed2c982beffa445203678d076e1edR16
and get this

After thinking some more about this, I think I can push all of this into
types. How does

whatever.enable = mkOption {
  type = types.service;
  default = true;
  description = "whatever service";
  package = whatever.package.default;
};

whatever.package = mkOption {
  type = types.package;
  default = literalPackage pkgs "pkgs.whatever";
  example = literalPackage pkgs "pkgs.whatever-alternative";
  packages = literalPackages pkgs [ "other" "options" ];
  description = "whatever service package";
};

look? We can probably even specify default defaults [sic!] in types if
desired. Then mkOption alone would be sufficient and we could throw
away mkEnableOption and all the others.

@edolstra?

I agree with @edolstra , adding additional mk*Option is a bad idea, and replacing mkEnableOption by the simple mkOption with a proper type sounds like a better approach to me.

In #14860 (comment) I will suggest that you rename package to descriptionPackage. On the other hand the option declaration should not depend on options definitions. Thus descriptionPackage should default to one version of the package, and not one set by whatever option.

I do not understand what packages is supposed to be used for.

Okay then, type hackery it is.

"> I do not understand what packages is supposed to be used for."

Multiple possible options and/or types.listOf types.path. See e.g.
fonts.fonts example above, same should, imho, be done for xorg modules
and other similar stuff.

(bah, github mail reply cites broke)

In relation to the font example, you might be interested by how the input methods plugins option description is generated. (It use the attribute set trick you described)

Also the input method packages option use an extended package type that check a meta attribute of the package so an unrelated package cannot be used. (the main motivation was that different input methods have similar plugins names so it is easy to mistake a ibus plugin with a fcitx one)

In the input method case, it would be nice and more flexible to generate the option description by automatically selecting packages with the right meta attribute, but as far as I know there is no cheap way to do that at the moment.

@oxij

I do not understand what packages is supposed to be used for.

Multiple possible options and/or types.listOf types.path. See e.g. fonts.fonts example above, same should, imho, be done for xorg modules and other similar stuff.

I still do not understand what this is supposed to mean. Is that a way to limit the set of packages that you can use as argument, like an enum?

I really like the idea of listing the supported alternatives for module options 👍 personally I'd prefer nixos-option to list them and not having to look them up in a man page but that would probably be easy to add.

I'm just not sure if the options should be hard-coded into the module. Maybe the package should be able to link itself to a module (-option) (something like meta.provides = [ services.xyz.package ])?

The latter needs nix support for catching exceptions (and assertions)

builtins.tryEval catches (some) errors. broken errors for instance is caught. It's not really documented, but this article have some information.

Ping all.

Arseniy Seroka [email protected] writes:

Ping all.

I remember about this, but my hand are currently pretty full with other
stuff. If someone wants to implement this as described above (with new
types) I will be happy to comment on those patches.

Feel free to reopen if there is still interest in this.

Hm. Okay. How do I reopen this? There's no such button.

Well, whatever. Please see #32424.