Skip to content

Expose the Minimal diff algorithm #1227

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

Merged
merged 1 commit into from
Nov 4, 2015

Conversation

tomcsmith1990
Copy link
Contributor

This adds the Minimal diff algorithm and uses it if specified when doing a diff, fixing #1057.

Unfortunately it's difficult to find an example which produces a different diff output for the Meyers and Minimal algorithm, so I don't have a unit test to check 1dd11be. It looks like the libgit code suffered the same problem and only test that output from patience differs from the default, which is already checked in UsingPatienceAlgorithmCompareOptionProducesPatienceDiff.

The change could be tested by extracting a class from Diff.BuildOptions and checking that the GitDiffOptions returned have the correct flags set, but that does break the encapsulation slightly. I'm happy to make that change if you want though.

@nulltoken
Copy link
Member

@tomcsmith1990 Neat! One nitpick though. Would you mind melding those two commits together into an atomic/cohesive one?

Unfortunately it's difficult to find an example which produces a different diff output for the Meyers and Minimal algorithm, so I don't have a unit test to check 1dd11be. It looks like the libgit code suffered the same problem and only test that output from patience differs from the default, which is already checked in UsingPatienceAlgorithmCompareOptionProducesPatienceDiff.

Duh. If I remember correctly I also searched for a simple example, in vain.

@peff Would you know more about this?

@nulltoken
Copy link
Member

Fix #1057

@peff
Copy link
Member

peff commented Nov 3, 2015

When in doubt, throw CPU at the problem:

$ cd git
$ git log -p --diff-algorithm=myers >myers
$ git log -p --diff-algorithm=minimal >minimal
$ diff myers minimal | less

This comes up with lots of examples in the translation files, which are quite hard to read (for instance, git/git@82aa9b7). We can try it restricted only to *.c. That turns up only a single commit, git/git@d18f76d, which is pretty much a giant unreadable diff no matter how you slice it.

So no, I guess I don't really have any good examples.

@nulltoken
Copy link
Member

@peff ❤️ for this amazing answer!

@tomcsmith1990 tomcsmith1990 force-pushed the minimal-diff-algorithm branch from 1dd11be to 6e6d08d Compare November 4, 2015 09:12
@tomcsmith1990
Copy link
Contributor Author

@nulltoken thanks, I've squashed the commits down into one.

@tomcsmith1990 tomcsmith1990 force-pushed the minimal-diff-algorithm branch from 6e6d08d to 6cad2ee Compare November 4, 2015 09:40
@nulltoken nulltoken added this to the v0.22 milestone Nov 4, 2015
nulltoken added a commit that referenced this pull request Nov 4, 2015
@nulltoken nulltoken merged commit e90dc97 into libgit2:vNext Nov 4, 2015
@nulltoken
Copy link
Member

💥

Cheers!

@tomcsmith1990 tomcsmith1990 deleted the minimal-diff-algorithm branch November 4, 2015 12:41
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

Successfully merging this pull request may close these issues.

3 participants