Skip to content
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

Inconsistent behavior on new keys in RHS #1

Closed
dermetfan opened this issue Dec 3, 2021 · 1 comment
Closed

Inconsistent behavior on new keys in RHS #1

dermetfan opened this issue Dec 3, 2021 · 1 comment

Comments

@dermetfan
Copy link

dermetfan commented Dec 3, 2021

Defining new keys works as expected if you don't use merge functions in the RHS:

merge {} {
  new = [];
  nested.new = [];
}
{
  "nested": {
    "new": []
  },
  "new": []
}

But if you use a merge function it fails:

merge {} {
  new = append [];
}
error: evaluation aborted with the following error message: '
       a fresh right-hand-side cannot be an array merge function
       at 'new':
         - rhs: lambda @ …

Ok, understandable. Unexpectedly it works if the new key is nested, although the result is the append lambda itself. This is probably not what the user wants:

merge {} {
  nested.new = append [];
}
{ nested = { new = <LAMBDA>; }; }

This seem inconsistent. Defining a new key with a merge function in the RHS should either always fail with a helpful message or always succeed.

I tend to prefer the latter because that would be consistent with the behavior we observe if no merge functions are used in the RHS (see first snippet).

The UX benefit would be that it always just works™.

One drawback could be that you can make less guarantees about the outcome from looking at the LHS. However, we can already not make any guarantees about which new keys will exist in the result since that depends on whether the new keys in the RHS use merge functions or not, and whether they are nested or not, which the LHS does not know. But the benefits of the data spine would still hold if data-merge accepted both scenarios in the RHS.

The real life problem is that you may not always know which keys exist in the LHS if you get it from elsewhere, like when writing a library. Currently it is necessary to pre-create the new keys in the LHS first to allow merging in possibly new keys with merge functions from the RHS (runnable example):

{
  inputs.data-merge.url = "github:divnix/data-merge";

  outputs = { nixpkgs, data-merge, ... }: {
    result = with data-merge; let
      lhs = {}; # we may not know this value
      rhs = { new = append []; };
      # defeats the purpose of data-merge
      prepopulatedLhs = nixpkgs.lib.recursiveUpdate lhs {
        new = lhs.new or [];
      };
    in merge prepopulatedLhs rhs; # ok
    # in merge lhs rhs; # fails
  };
}
@blaggacao
Copy link
Collaborator

Since update [ <idx1> <idx2> ] [ { } { } ] can't be reasonably used without intimate knowledge of the lhs, I decided to keep the usage semantics of both decorators the same.

Thanks for the bug report!! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants