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

Get access to the path in content merge function #1523

Open
clecat opened this issue Sep 9, 2021 · 2 comments
Open

Get access to the path in content merge function #1523

clecat opened this issue Sep 9, 2021 · 2 comments
Labels
area/api Related to any API type/feature Introduce a new feature

Comments

@clecat
Copy link
Contributor

clecat commented Sep 9, 2021

Fast question:
Could we have access to the current path of the objects to be merged inside the merge function of the store's content.

Long description:
I am currently using Irmin for a project and want to handle merges properly.
From what I have understood, I need to redefine the merge function inside the module I use as content for the store functor:

module Content_string = struct
  include Irmin.Contents.String

  let merge =
    let dt = Irmin.Type.(option string) in
    let equal = Irmin.Type.(unstage (equal dt)) in
    let default = Irmin.Merge.default dt in
    let f ~old x y =
      (* do my own stuff instead of the default idempotent behavior *)
      if equal x y then Irmin.Merge.ok x else Irmin.Merge.f default ~old x y
    in
    Irmin.Merge.v dt f
end

In this bit of code, I would have to replace the content of f copy-pasted from Merge.idempotent with my own custom function.
However, it appeared that I have not access to the current path of the objects that I want to merge.

Indeed, I do not always have the same objects inside the different trees of my store and I will want to have a specific merge for each type of them.

Therefore would it be possible for the signature of the merge function to be something like:
val merge: ~old:t -> ~key:Key.t -> t -> t -> t
Instead of
val merge: ~old:t -> t -> t -> t
It would allow a more specific and refined merge behavior

@samoht
Copy link
Member

samoht commented Sep 9, 2021

That's a great idea and something I wanted to do for a very long time but that API is hard to define properly. as Key.t is (currently) generic and opaque. I am hoping that now that we have introduced schemas, this could be somehow be passed to the user-defined merge function.

@clecat
Copy link
Contributor Author

clecat commented Sep 9, 2021

Cool, thanks a lot, I will waiting for that :D

@maiste maiste added area/api Related to any API type/feature Introduce a new feature labels Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Related to any API type/feature Introduce a new feature
Projects
None yet
Development

No branches or pull requests

3 participants