if the number of option definitions is less than or equal to 1, the function simply “merges” (i.e., keeps) the value.
otherwise, it throws an error due to conflicting definitions, avoiding the complexity and potential risks of merging multiple lists, which might lead to unwanted or unsafe behavior.
What are the benefits of this approach compared to removing those lines entirely (so that lib.types.mergeEqualOption would be called instead)? This alternative seems to retain the behavior of rejecting the merge of different lists (i.e., lists that don’t match the != operator) while also handling options that contain the same values, which, as far as I can see, wouldn’t introduce any unsafe or unwanted behavior.
Thank you for your super fast and clear answer, here is the PR.
It goes straight to the point of removing the few lines I mentioned above.
Do I need to do other things ? Its my first contribution to nixpkgs other than packages .
I didn’t write no tests, nor executes anything to run existing ones.
@jakehamilton The preference should always be to define a specific type that matches what the module expects. types.anything’s main purpose is to be a “safe” fallback when it’s unclear what the type should be, with behavior that’s as generic (accepting all values) and predictable (having expected merge results) as possible. I intended for types.anything to eventually replace types.unspecified (which has terrible merge behavior!) as the default option type.
The problem with listOf anything’s merging behavior is that it merges by concatenating values in somewhat non-deterministic order (it depends on imports order), which can cause problems when the order is important (and it has before!). Furthermore, merging by concatenation can break stuff, such as when lists should have a fixed length (e.g. a transformation Matrix), or if the list represents one “thing” by itself (e.g. interpolation points for a gradient). That’s why it’s not fit as a safe default for types.anything, even though it would work in most cases.
Gotcha, thanks for the insight! I think I have just been surprised in the past when the two look like they should operate the same, but don’t.
I feel that, for list types, order is unimportant and it is fine to merge things via concatenation. Assuming, of course, that an ordered alternative exists. I don’t think nixpkgs has something like that outside of ordered strings. Perhaps Home-Manager’s DAG type should be used in these places instead? I’ve had some great success working with it to implement much more complex behaviors where order is important.
.