josephburnett / jd

JSON diff and patch
MIT License
826 stars 38 forks source link

List item diff should preserve order #59

Open philpennock opened 1 year ago

philpennock commented 1 year ago

(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 commented 1 year ago

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 commented 2 months ago

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 commented 1 month ago

@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 commented 1 month ago

@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.