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

Merging 2 files with the -x/--overwrite option in version 3 does not behave consistently with version 2 in a list or array. #455

Closed
jak74 opened this issue Jun 6, 2020 · 7 comments
Labels
Milestone

Comments

@jak74
Copy link

jak74 commented Jun 6, 2020

Describe the bug
Merge with the -x/--overwrite option only replaces select values and not the entire list or array. This seems to be true for multiple (or all?) releases of version 3 including 3.01 and 3.3.0. The expected behavior is representative of how version 2 handles overwrite in a merge.

Input Yaml
Concise yaml document(s) (as simple as possible to show the bug)
data1.yml:

a: value
b:
  - orig1
  - orig2
  - orig3
  - orig4
  - orig5
  - orig6
c: true

data2.yml:

b:
  - new1
  - new2
  - new3

Command
The command you ran:

yq merge -x data1.yml data2.yml

Actual behavior

a: value
b:
- new1
- new2
- new3
- orig4
- orig5
- orig6
c: true

Expected behavior

a: value
b:
- new1
- new2
- new3
c: true

Additional context

@jak74 jak74 added the bug label Jun 6, 2020
@DonBower
Copy link

DonBower commented Jun 8, 2020

It seems this broke between 2.4.1 and 3.0.1:

yq/3.0.1/yq_linux_amd64 merge -x data1.yml data2.yml
a: value
b:
- new1
- new2
- new3
- orig4
- orig5
- orig6
c: true
yq/2.4.1/yq_linux_amd64  merge -x data1.yml data2.yml
a: value
b:
- new1
- new2
- new3
c: true

@mikefarah
Copy link
Owner

Yeah I see. To preserve comments and formatting, I had to rewrite merge in 3.0 - I tried to keep as much functionality and behavior as I could.

This is a result of attempting to deeply merge the arrays, that is each item of the array, rather than the array as a whole. This is done deliberately, for scenarios where there is an object or sequence as an array item that should be merged.

Have to think about this one.

If you merge an object that's missing keys from the original, I don't delete the original keys.

@artem-nefedov
Copy link

Even disregarding backwards compatibility, the current behaviour just doesn't make sense from user perspective. I think all similar tools and libraries (e.g. python's anyconfig.merge) will overwrite arrays completely, unless you explicitly want to merge them (which is already covered by -a).

For cases where the array consists of map objects, I guess there could be some rationale why you would sometimes want to merge them recursively, but I would argue that's:

  • it's very rare case, and definitely not worth sacrificing the ability to overwrite array completely
  • it would only make sense to merge those objects without --overwrite

@mikefarah
Copy link
Owner

mikefarah commented Jun 18, 2020

Ok - to keep backwards compatibility, I'll add a new flag called overwriteArrays which will not deeply traverse arrays and simply overwrite them (where as overwrite will deeply traverse and overwrite array elements, and append will ...well append).

@mikefarah mikefarah modified the milestones: 3.3.2, 3.3.3 Jun 18, 2020
@artem-nefedov
Copy link

artem-nefedov commented Jun 18, 2020

Great. It's important to be able to overwrite arrays and other elements as well, so I assume that you should be able to specify both flags together (unless there's a better way to handle this).

@mikefarah
Copy link
Owner

mikefarah commented Jun 18, 2020 via email

@mikefarah
Copy link
Owner

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

No branches or pull requests

4 participants