benjamine / jsondiffpatch

Diff & patch JavaScript objects
MIT License
4.78k stars 466 forks source link

Not all unchanged elements are presented in html formatter's diff #72

Open Keniamin opened 9 years ago

Keniamin commented 9 years ago

Let us compare json

[
    [{"id": 1, "diff": "none"}, {"id": 2, "diff": "none"}],
    [{"id": 1, "diff": "none"}, {"id": 2, "diff": "was"}]
]

with

[
    [{"id": 2, "diff": "none"}, {"id": 1, "diff": "none"}],
    [{"id": 2, "diff": "now"}, {"id": 1, "diff": "none"}]
]

That is, in first array we just swapped elements, while in the second one we also changed one of the elements. And now look to the screenshot how html formatter draws it when "show unchanged" option enabled. jsondifflib formatter bug In the first case it's ok, we can see unchanged element with id=1 and moved id=2. In the second case unchanged element is not present.

P.S. By the way, if one changes element with id=1 rather than id=2 diff would look even stranger (unexisting changes still related to id=2, like above): jsondifflib formatter bug 2

benjamine commented 9 years ago

interesting, I see nested changes in moved items don't play well with this representation, specially when using "show unchanged values". the problem in this case is: the element that changed took the position of the one that's unchanged, and both cannot be shown in the same place. I'm not sure of to solve this, how would you visually represent your example?

Keniamin commented 9 years ago

Well, it's not very obvious indeed, but I think something like this would be more acceptable: no1 Grey parts, as it was, must not be shown if "Show unchanged" mode is disabled.

I suppose here we have exactly what we want: unchanged parts stay in their real places, changes are shown near the element, in which they occurred, and movements are shown by arrows and "=> <new_index>" marks.

By the way, there are also some differences between visualization of changed and unchanged elements, at least alignment and quotes: no2 It seems to be more beautiful if they look identical. Is it a bug or was it made consciously?

benjamine commented 9 years ago

That's nice, for the item above (without a nested change) looks good, and it's probably not a difficult change. but for the one below, I'm not sure I like changing the location of the nested diff, it seems easier to find if it's in their final position, as other nested changes are. maybe the unchanged item could appear right below, as that's where it actually ends up after moving the other item above. I think I'll make some experiments considering other situations.

regarding the unchanged JSON syntax differences, that's because it's rendered using native JSON.stringify (which causes that indentation and properties in double quotes), the only alternatives I can think of are using a custom json stringifier for unchanged parts (slower and potentially brittle), or using double quotes in the changed parts (but I'd prefer to prioritize readability there, as unchanged values are most commonly hidden), Thanks for the detailed feedback!

Keniamin commented 9 years ago

maybe the unchanged item could appear right below, as that's where it actually ends up after moving the other item above

Of course, it's another possible solution. I think it depends on numeration which we take as a "basic": initial one (which was in the original array) of final one (as in array for which we've calculated diff). I'm not sure that I understand, how your formatter behaves here, so decision is up to you.

that's because it's rendered using native JSON.stringify

Hm... Well, you also can "post-parse" the result of stringify function (for example by a regex) in purpose to remove "excess" quotes... But yes, it's ugly, I know it :)

Thanks for your responses too.