Correct index passed to DiffInternal for modification detection in LS…#45
Open
barry-bvl wants to merge 2 commits intoweichch:mainfrom
Open
Correct index passed to DiffInternal for modification detection in LS…#45barry-bvl wants to merge 2 commits intoweichch:mainfrom
barry-bvl wants to merge 2 commits intoweichch:mainfrom
Conversation
weichch
reviewed
Apr 14, 2024
| // We have two objects equal by position or other criteria | ||
| var itemDiff = new JsonDiffDelta(); | ||
| DiffInternal(ref itemDiff, left[entry.LeftIndex], right[entry.RightIndex], options); | ||
| DiffInternal(ref itemDiff, left[commonHead + entry.LeftIndex], right[commonHead + entry.RightIndex], options); |
Owner
There was a problem hiding this comment.
Thanks for digging.
I think this change makes sense. I realized I couldn't reproduce this issue without a custom key finder. Is that what you use? Without a key finder the diff result is correct but not optimal (no modification).
Do you want to add a test for this?
Author
There was a problem hiding this comment.
Indeed using an array objectkeyfinder, added a specific unittest that covers this usecase.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Noticed that the diff of arrays was not always correct when items were removed, modified and added.
The modifications where not identified.
example:
Lets assume I want to diff the following
{ "id": "644fe678c3541f5485b760d2", "myArray": [ { "id": "644fe6b2c3541f5485b7639f", "comment": "bogus" }, { "id": "65fa21a122020709ac83b0ca", "comment": "willberemoved" }, { "id": "644febdbc3541f5485b79026", "comment": "foobar" }, { "id": "645001bfc3541f5485b7e189", "comment": "example" }, { "id": "645003fac3541f5485b7f2ab", "comment": "ok" } ] }with
{ "id": "644fe678c3541f5485b760d2", "myArray": [ { "id": "644fe6b2c3541f5485b7639f", "comment": "bogus" }, { "id": "644febdbc3541f5485b79026", "comment": "foobar" }, { "id": "645001bfc3541f5485b7e189", "comment": "example adapted" }, { "id": "645003fac3541f5485b7f2ab", "comment": "ok" }, { "id": "myid", "comment": "isadded" }, { "id": "myid2", "comment": "isadded2" }[example, example modified] modification is not found by the diff (delta format).
After some investigation it seems to just pass the wrong index to the DiffInternal in the LCS when checking for modifications in the right side.
After the modification the delta includes the modification as well.