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/recursiveUpdateUntil: fix code to match documentation #41604
lib/recursiveUpdateUntil: fix code to match documentation #41604
Conversation
$ nix repl lib Welcome to Nix version 2.0.2. Type :? for help. Loading 'lib'... Added 350 variables. -- this is the exact example from the function's documentation: nix-repl> recursiveUpdateUntil (path: l: r: path == ["foo"]) { # first attribute set foo.bar = 1; foo.baz = 2; bar = 3; } { #second attribute set foo.bar = 1; foo.quz = 2; baz = 4; } { bar = 3; baz = 4; foo = { bar = 1; baz = 2; quz = 2; }; } -- although the documentation says: { foo.bar = 1; # 'foo.*' from the second set foo.quz = 2; # bar = 3; # 'bar' from the first set baz = 4; # 'baz' from the second set }
Ping @Profpatsch I'd rather have the documentation changed instead of the behaviour of the function. These functions might be widely used by other projects, which we can't know of whether it would break anything. |
tbh that wouldn’t have happened if the original author spared five minutes to add the example to the library tests. 😞 I’m in favour of changing this function to match its documentation, because I don’t think we really need to be bug compatible in this case. @bluescreen303 can you add a section to the release note file in |
Ping? |
@infinisil added the test & release note, what do you say? |
@GrahamcOfBorg eval |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Profpatsch Looking good
Motivation for this change
The function
recursiveUpdateUntil
did not produce the expected result for the exact example given in the documentation.This fixes that and makes the function useable for any predicate that actually looks at the path argument. In current nixpkgs, there is only 1 location that touches this functionality:
nixpkgs/nixos/maintainers/option-usages.nix
Lines 128 to 131 in 75601e1
I'm not fully sure what that is all about, but since it's maintainer-only I do not expect issues will arise because of this fix, as they probably just assumed this function worked as advertised as well.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)