Add: Failing test case.

[cadesalaberry]

Your tools came in pretty handy in some of my recent projects.

However I did not realise, but when dealing with arrays, it could lead to data loss.

How do you think it should be resolved ?

[jonatanpedersen]

Thanks :)

I am not sure. It is just using xdiff underneath for the actual merge, and xdiff deals with arrays like this https://github.com/dominictarr/xdiff#arrays

This is one of the reasons I almost always use maps instead of arrays. But I realise that this is not always an option.

I will think about it.

[cadesalaberry]

I also tend to prefer maps for that particular reason as well.

I am using your tool to try and solve my painful merges in the json files generated by l10ns.

It might be pretty specific, but if your plugin were to deduce uniqueness from an "id" field, it would solve most cases.

Other than that, in order to avoid data loss, I feel like concatenating the arrays by default would make sense.

[jonatanpedersen]

It actually talks about that specific issue in the xdiff README.

if you don't don't use id's xdiff won't know that an object that has changed is actually the same object. this would cause it to reinsert a new copy of that object.

id's are this is really useful when you need to do 3-way-merges to merge together concurrent changes.

in a future version, xdiff will allow changing the id key. currently it uses only the id property.

I think these problems are best solved at the source (xdiff) rather than trying to fix this github merge driver.

If you submit a pull-request to xdiff for changing the id key, and get it released, then I would be happy to include the new version of xdiff in git-json-merge.

[cadesalaberry]

Alright, I'll check it out, thanks !

[jonatanpedersen]

I guess https://github.com/dominictarr/xdiff is pretty much dead though. No commits since 2012. We could try to find a different merge engine.

[cadesalaberry]

I think https://github.com/carlos8f/xdiff should do the job

[jonatanpedersen]

I made a pull-request https://github.com/jonatanpedersen/git-json-merge/pull/4 with the @carlos8f fork and added object id tests:

They still fail though:

22 passing (23ms)
  2 failing

  1) gitJsonMerge mergeJsonPrimitivesArray given arguments of "[\"foo\",\"bar\"]" as ours, "[\"foo\"]" as base and "[\"bar\"]" as theirs should return "[\"bar\"]":

      AssertionError: expected '["bar","bar"]' to deeply equal '["bar"]'
      + expected - actual

      -["bar","bar"]
      +["bar"]

      at Assertion.assertEqual (node_modules/chai/lib/chai/core/assertions.js:485:19)
      at Assertion.ctx.(anonymous function) [as equal] (node_modules/chai/lib/chai/utils/addMethod.js:41:25)
      at Context.<anonymous> (test/git-json-merge.spec.js:77:27)

  2) gitJsonMerge mergeJsonObjectsArray given arguments of "[{\"id\":\"foo\"},{\"id\":\"bar\"}]" as ours, "[{\"id\":\"foo\"}]" as base and "[{\"id\":\"bar\"}]" as theirs should return "[{\"id\":\"bar\"}]":

      AssertionError: expected '[{"id":"bar"},{"id":"bar"}]' to deeply equal '[{"id":"bar"}]'
      + expected - actual

      -[{"id":"bar"},{"id":"bar"}]
      +[{"id":"bar"}]

      at Assertion.assertEqual (node_modules/chai/lib/chai/core/assertions.js:485:19)
      at Assertion.ctx.(anonymous function) [as equal] (node_modules/chai/lib/chai/utils/addMethod.js:41:25)
      at Context.<anonymous> (test/git-json-merge.spec.js:77:27)

[cadesalaberry]

Crap, obviously it could not be easy...

I'll see what I can do