A few thoughts on this PR:

• I could split this up into several separate PRs, since the option documentation fixes don't depend on the either change (they just don't affect anything without it)
• Because this change really only affects documentation (and nixos-option) maybe it is safe enough to go ahead and do the naive recursion and just fix nixpkgs's recursive types, since any third parties that define their own recursive types will only be affected if they also build documentation (though they would still hit the issue if they use nixos-option to inspect any option using their recursive type). I haven't tested to see if this would pick up any more options though, so maybe I should find that out.
• home-manager and nix-darwin might want to test their own documentation against this PR to see if they have any options that need documentation fixes.

I tested out the "make either fully recursive" option and while it does add more options, they're entirely within services.tor, such as services.tor.settings.ControlPort.*.GroupWritable, and they're all ultimately of type oneOf [ ... (listOf (oneOf [ ... submodule ]))]. The downside is I had to update 12 recursive types in formats plus another 14 modules that declared their own custom recursive types. That's a lot more recursive types than I expected to find.

Alternatively this also suggests we could just update the 2 affected types in services.tor (by overriding getSubOptions/getSubModules/substSubModules to defer directly to the submodule) to get the same effect without making either fully recursive. There may be options in home-manager or nix-darwin that would also benefit from the fully-recursive either, but the fact that in all of nixpkgs it's just 2 of the types in services.tor that are affected suggests that a construct like either (listOf submodule) … (where the submodule isn't also at the top level of the either) is sufficiently rare that it may not show up anywhere else at all.

Edit: actually, the fully recursive change hurts some of the tor documentation because it modifies services.tor.settings.DNSPort sub-options so that e.g. services.tor.settings.DNSPort.IsolateClientAddr is now services.tor.settings.DNSPort.*.IsolateClientAddr, which happens because the either submodule (listOf submodule) now overrides the left's options with the right. So if we do want fully recursive we need to be smarter about merging options from both sides, though also this suggests that my getSubOptions impl might want to do t2.getSubOptions prefix // t1.getSubOptions prefix just to prioritize options from the left side.

Also we can't just trivially update those two types in services.tor as an alternative, because of how substSubModules works it's a lot more complicated to modify those types to defer to the submodule.

Another thing to consider here is either module (listOf module) such as services.tor.settings.TransPort, which really should generate option documentation for both the direct sub-options and the listOf sub-options, e.g. both services.tor.settings.TransPort.IsolateClientAddr and services.tor.settings.TransPort.*.IsolateClientAddr. But right now this can't happen because even though listOf adds "*" to the prefix, this isn't reflected in the returned attrset, so there's no way to merge the sub-options from the module and the listOf module, unless I want to change either to return something like { left = t1.getSubOptions prefix; right = t2.getSubOptions prefix; } and that has negative consequences for nixos-option.

With that context, we could consider changing listOf and attrsOf to put the placeholder into the returned attrset, e.g. having listOf return { "*" = elemType.getSubOptions (prefix ++ [ "*" ]); }. That way this can be merged with the non-list version of the same module. The downside is nixos-option would have to be updated to expect this.

The changes to either I just pushed are:

• getSubOptions now uses lib.recursiveUpdateUntil so either moduleA moduleB where the two submodules define distinct options nested within the same key will work, e.g. either (submodule [{options.foo.bar = mkOption { … }; }]) (submodule [{options.foo.qux = mkOption { … }; }])
• getSubOptions now resolves conflicts in favor of the left type if both types define options
• getSubModules now returns the left type's submodules if both types define submodules (this affects services.postgrey)
• substSubModules similarly now substitutes the left type's submodules if both types define submodules

Rebased to fix merge conflict (conflict was due to the tor module being formatted)

Mixing isn't so bad by itself.
The reason we usually ask this is that lib changes tend to require very thorough scrutiny, which shouldn't block other improvements even if they're somewhat related, which seems to be the case here.

I certainly can split this up, though the non-lib changes here are all fairly meaningless without the lib changes, as those documentations simply aren't rendered without the lib changes. So there's no problem with "blocking" those changes on the lib changes.

If I split this up, would it be better to do one PR for all the non-lib stuff, or a separate PR for each module (e.g. turning each commit into its own PR)?

I fear that the changes in nixos might be due to a breaking change in the either type.

To clarify on this point, the changes in nixos are to fix code that is currently broken but that brokenness wasn't noticed because the options aren't rendered into documentation until the lib change. Some of it is documentation where the nix code is simply wrong (e.g. the dconf module said example = literalExpression ''…'' without importing that, so the fix is to say example = lib.literalExpression ''…''), some of it fixes the documentation to conform to the restrictions in place when building the manual (pay-respects used a deprecated function and had a default key that referenced something from another module without using defaultText, rspamd also did the latter), and some was just options that were missing descriptions (movim and tor).

All of this stuff could be detected by using the repl to poke around at the module structure, but it went unnoticed because none of these options ended up in the documentation manual before. So technically this either change could be a breaking change just by exposing buggy code to documentation rendering, but getting those options in the rendered documentation is the whole point of this change.