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

bug in replace array in json #18

Open
mohamadk opened this issue Aug 4, 2016 · 3 comments
Open

bug in replace array in json #18

mohamadk opened this issue Aug 4, 2016 · 3 comments

Comments

@mohamadk
Copy link

mohamadk commented Aug 4, 2016

hi, thanks for this great library.
when I'm getting different between this two jsons

   "{\"b\": [1,2,4,9]}"

and

  "{\"b\": [6,2,4,9]}"

the difference value currectly return operation replace in location 0.

but if I get diff this two jsons

 "{\"b\": [2,2,5,6]}"

and

  "{\"b\": [4,2,5,6]}"

it returns add and remove operation in 0 and 2 positions.
I think the reason of this bug is because of two number 2 .

@vishwakarma
Copy link
Member

Agreed, thanks for pointing out the issue. Clearly, the diffs generated are not optimized in this case.
I will fix this once i get some space to work on this project, Moreover, i would be glad to accept PR for same.

@nebevay
Copy link

nebevay commented Oct 26, 2016

Hi, I've looked at the code a bit and I think the underlying issue is that REPLACE operations are not collapsed. Take the following arrays:

source: [1, 2, 3, 4, 5]
target: [5, 1, 4, 3, 2]
lcs: [1, 2] OR [1, 4] or [1, 3] would also be valid

You will get, REMOVE 1, REPLACE 2 5, ADD 1, REPLACE 3, 4, REPLACE 4, 3, REMOVE 5, ADD 2

To reduce this into moves you would need the following:
REMOVE 1, ADD 1 -> MOVE 1
REMOVE 5, REPLACE 2 5, ADD, 2 -> MOVE 5
REPLACE 3, 4, REPLACE 4, 3 -> MOVE 3

Obviously you need to keep track of coordinates as well. The optimization that is currently in place only collapses REMOVE and ADD into MOVE, it doesn't take into account these chains of replacements.

Sadly I won't have enough time to code this myself right now, but thought it might help if someone else wanted to tackle it.

@aantoniuk-gl
Copy link

What about this bug? It's been created more than 3 years ago

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

4 participants