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

List item diff should preserve order #59

Open
philpennock opened this issue Aug 25, 2023 · 4 comments
Open

List item diff should preserve order #59

philpennock opened this issue Aug 25, 2023 · 4 comments

Comments

@philpennock
Copy link

(First, thank you for this tool, it's very helpful in double-checking that correct fields have been changed! Made my day.)

Given a top-level list, each member of which is an object, when generating a diff the diff is emitted in reverse order. This is contrary to my expectations.

Eg, given old.json of

[{"id":"fred","value":42,"extra":"a"},{"id":"barney","value":35}]

and new.json of:

[{"id":"fred","value":44},{"id":"barney","value":37,"extra":"b"}]

then at present (jd version 1.7.1), we see:

% jd old.json new.json
@ [1,"value"]
- 35
+ 37
@ [1,"extra"]
+ "b"
@ [0,"extra"]
- "a"
@ [0,"value"]
- 42
+ 44

I would expect the items to be shown in increasing index order, normally.

@josephburnett
Copy link
Owner

Yeah, that is a little odd. I was iterating backward through lists so that when a diff is adding or removing something from the end of a list, it can still be applied linearly. An important invariant is that the diff can be applied with the patch operation to get the original value. And diffs are always applied one hunk at a time.

I've just finished refactoring the diff format on the master branch to address this and a couple other oddities. In the next version (which will be a major version bump) iteration through lists will forwards and make more sense. I'll also be calculating the minimum common sub-sequence in order to produce a minimum diff. And I'll be able to stitch in a whole series of values instead of adding them each in a separate hunk. I'll post back here when I have a working prototype, in case you're interested in trying it out.

And I'm glad this tool was helpful! :)

@dudicoco
Copy link

Hi @josephburnett,

I'm running into perhaps a similar issue: when applying a patch the original structure, newlines and comments are not preserved.
Example:

file.yaml

deployment:
  replicaCount: 1
  strategy:
    type: RollingUpdate
    maxUnavailable: 0
    maxSurge: 12%

# comment 
podTemplate:
  containers:
    amazing-app:
      image:
        tag: 6.1.4

      env:
        FOO: bah 

      resources: {}

hpa:
  enabled: false
  maxReplicas: 10

ingress:
  enabled: false
  scheme: internal
$ cat patch
{"podTemplate":{"containers":{"amazing-app":{"env":{"FOO":"bar"}}}}}%

jd -yaml -f merge -p patch file.yaml

deployment:
  replicaCount: 1
  strategy:
    maxSurge: 12%
    maxUnavailable: 0
    type: RollingUpdate
hpa:
  enabled: false
  maxReplicas: 10
ingress:
  enabled: false
  scheme: internal
podTemplate:
  containers:
    amazing-app:
      env:
        FOO: bar
      image:
        tag: 6.1.4
      resources: {}

Is there a way to preserve the original file's formatting?

@josephburnett
Copy link
Owner

@philpennock I've finally merged my v2 branch to master. The tool has a new flag -v2 which will use the v2 library. The biggest difference is how lists are handled. It calculates the longest common subsequence in order to produce a minimum diff. It also traverses the list from the beginning to the end.

go run . -v2 ~/a.json ~/b.json:

@ [0,"extra"]
- "a"
@ [0,"value"]
- 42
+ 44
@ [1,"value"]
- 35
+ 37
@ [1,"extra"]
+ "b"

It's not published in a version yet, but you can play around with it by running jd from source. I'll let you know when I publish a new version. It will be in the 1.y.z series because -v2 defaults to false. Once I change that, the major version will change to 2.y.z because the new format is not backward compatible. It adds some neat features like diff context.

@josephburnett
Copy link
Owner

@dudicoco unfortunately jd only uses the subset of YAML which is compatible with JSON. Because this is primarily a JSON diff tool. JSON doesn't support comments :( so this tool doesn't either.

This isn't related to this reported issue. But you are welcome to open a feature request if you like. That will help me understand how many people want preservation of YAML comments.

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

3 participants