feat(wrapperModules.niri): change settings types for better merging#531
Conversation
| default = { }; | ||
| type = lib.types.submodule { | ||
| freeformType = lib.types.attrs; | ||
| freeformType = lib.types.anything; |
There was a problem hiding this comment.
| freeformType = lib.types.anything; | |
| freeformType = wlib.types.attrsRecursive; |
^ lib.types.anything but also merges lists like normal too rather than overwriting with no choice in the matter? (like, you could lib.mkForce to overwrite the list still with this too, but also lib.mkBefore and lib.mkAfter and stuff)
There was a problem hiding this comment.
I didnt notice the lib.types.attrs in the original PR, and then I noticed it but I just haven't really felt the need to change it, but if it is surprising to people, happy to do so!
There was a problem hiding this comment.
I would also be open to changing layouts and maybe the other ones.
Also it does actually merge functions as far as I am aware? Especially functions that return sets. If it doesn't we should see why not!
nix-wrapper-modules/lib/types.nix
Lines 41 to 48 in 616a22a
^ If it encounters a function, as specified by builtins.typeOf, it returns a function that calls the merge function on the results of the functions. lib.types.anything also works like this.
Also the function recieves self as an arg, it is called via lib.fix so it would know about the extras too, if you had use for that.
However, once you used a function or a set, all future ones would have to be of that type. It can't merge a set with a function, you would need a custom type for that.
On that note, we can also see about maybe making a custom kdl type to match our generator one day which is better than wlib.types.attrsRecursive for that usecase, i.e. allowing us to merge a list or set kdl value into its function variant, so that all the things you could provide as a value can merge according to the format created by wlib.toKdl regardless of what was first provided there.
There was a problem hiding this comment.
Also it does actually merge functions as far as I am aware? Especially functions that return sets. If it doesn't we should see why not!
Oh that's neat.
wlib.types.attrsRecursive seems like a good choice too. I'll go with that.
There was a problem hiding this comment.
I would also be open to changing layouts and maybe the other ones.
Yeah, give the ability of wlib.types.attrsRecursive to merge functionTo I can also add changes for layout, binds and outputs.
I don't think adding to window-rules and layer-rules would help much but it probably wouldn't hurt either?
There was a problem hiding this comment.
Lets leave those alone in the interest of not making the module system do more checking than we care about.
This is fine.
Issue
Currently it is not possible to merge
settingstogether in theniriwrapper module that are part of the freeform configuration, this leads to unexpected overriding of config instead of merging config options together.Example
An example of an option that wouldn't merge prior to this minor change is
inputThe above snippet means that only one of the
inputconfigs is selected, there is not even an error warning that multiple options have been set.Additionally, previously you could not set config with an override level using
mkDefaultinside nested attributes (againinputis an example) as themkDefaultwould be interpreted in its unwrapped attrSet.Other extensions
It may also be worth changing more of the defined options in the freeform settings type to use
anythinginstead ofattrssince anything correctly recursively merges attribute sets.layout: is a good candidate as nested attribute sets can occur herebinds: it depends, it would allow partial overriding of binds set elsewhere, but this may not necessarily be wanted behaviour, and since you can't merge afunctionTotype anyway, it appears less useful here as you may always want to set all config of a binding in one place.outputs: for similar reasons tobindswindow-rulesandlayer-rules: are lists of attrs, and so merging also won't happen here either so is not needed.Just changing the toplevel to
anythingmostly solves my issues porting myniriconfig properly tonix-wrapper-modulesfromniri-flake.