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

TextDiffMode.Efficient produces unexpected results #26

Closed
majdisorder opened this issue May 24, 2018 · 3 comments
Closed

TextDiffMode.Efficient produces unexpected results #26

majdisorder opened this issue May 24, 2018 · 3 comments

Comments

@majdisorder
Copy link

majdisorder commented May 24, 2018

In ran into some weird results when comparing string values longer than 50 characters. A quick look at the code reveals that under the default settings TextDiffMode.Efficient is used for these particular strings. Switching to TextDiffMode.Simple solves the problem, but it seems like there is an issue in Efficient mode.

I diffed the following objects:
{ "Title": "SPRINT 2: Koppeling Afas naar Nop 2 + functionaliteiten" }
and
{ "Title": "SPRINT 2: Koppeling Afas naar Nop 2 + functionaliteiten - SYNCED" }
and got this result:
{ "Title": [ "@@ -48,8 +48,17 @@\n liteiten\n+ - SYNCED\n", 0, 2 ] }
where I expected:
{ "Title": [ "SPRINT 2: Koppeling Afas naar Nop 2 + functionaliteiten", "SPRINT 2: Koppeling Afas naar Nop 2 + functionaliteiten - SYNCED" ] }

Is this by design? If so, how can I get the actual diff? I want the actual diff to create patch requests for objects stored in a NoSql database.
Will ArrayDiffMode.Efficient produce similar results?

@wbish
Copy link
Owner

wbish commented May 29, 2018

Hey @dimitritron, yes that is by design. The efficient mode produces a diff that can be used by Google's diffmatchpatch (https://github.com/google/diff-match-patch). Patching and unpatching should work just fine.

@majdisorder
Copy link
Author

majdisorder commented Jun 2, 2018

Okay, understood.

I guess it makes sense when using the patch/unpatch functionality, but not when you are only interested in the diff, as is the case in my usecase.

Maybe this setting deserves a little more documentation. On carefully reading the readme I indeed discovered this point. However, from the documentation, it isn't clear that this strategy will automatically be selected for strings longer than 50 chars. That's what caused my confusion in the first place.

Wouldn't it make more sense to use the simple strategy per default?

Also, I was thinking that it would be less confusing if the results were strongly typed, instead of JTokens which are either an array of 2, or 3 items, depending on the strategy. Especially if the strategy can change for each field of a single input.
There should at least be some metadata available, don't you think?

@wbish
Copy link
Owner

wbish commented Jun 2, 2018

@dimi3tron this library is a port of benjamine/jsondiffpatch to c#. The format of the output is dictated by the schema that has been developed by that library over time. The goal, at least for the default diff output, is to remain fully compatible with the original library and consistent with regards to default behaviors.

That being said, the request for custom diff outputs has come up over and over again and I've documented it in #23 and it is something that I think would be a great way to extend this library.

To your original point on the documentation about efficient/simple, yes I think better documentation here is a good idea. I was going through the existing docs and its not clear at all what either simple or efficient even mean.

@wbish wbish closed this as completed Nov 28, 2020
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

2 participants