Skip to content

PatienceAlgorithm flag in CompareOptions #1039

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
May 7, 2015

Conversation

dmalikov
Copy link
Contributor

@dmalikov dmalikov commented May 5, 2015

Fix #1037

@dmalikov
Copy link
Contributor Author

dmalikov commented May 5, 2015

Do we need some tests here? I'm not quite sure what is the best approach to verify whether particular diff is patience or not.

@nulltoken
Copy link
Member

Do we need some tests here?

Indeed, we tend to prefer covering features with tests. The easiest way to make this happen would be to

  • find the smallest use case in git.git where the diff differs based on the use of "patience",
  • reproduce the outcome by building two Trees (cf. the TreeDefinition type and the repo.ObjectDatabase.CreateTree() method and how they're leveraged in the test suites) and diff them.

@dmalikov
Copy link
Contributor Author

dmalikov commented May 5, 2015

find the smallest use case in git.git where the diff differs based on the use of "patience",

What do you mean by git.git?

@Therzok
Copy link
Member

Therzok commented May 5, 2015

git.git being the git commandline tool.

@dmalikov
Copy link
Contributor Author

dmalikov commented May 5, 2015

@nulltoken is there some helper method to parse TreeChanges from a string containing actual git diff?

@nulltoken
Copy link
Member

@dmalikov No. you'll have to build one Tree with one version of the file to compare and another Tree with the second version. The TreeChanges will be generated by the call to Diff.Compare().

If you've found a way to identify a small case where --patience shines please share it here and we'll help you build the Trees out of it.

@nulltoken
Copy link
Member

FWIW, I've found a short example that should be sufficient from a test coverage perspective

http://bl.ocks.org/roryokane/79b8ebcb3813ebd934c4

@nulltoken
Copy link
Member

@dmalikov

The following code will create a Tree, containing one entry file.txt which content is My content.

var content = "My content";
var path = "file.txt";

var td = new TreeDefinition();
td.Add(path, OdbHelper.CreateBlob(repo, content), Mode.NonExecutableFile);

var t = repo.ObjectDatabase.CreateTree(td);

@dmalikov
Copy link
Contributor Author

dmalikov commented May 5, 2015

Well, the problem is not to build 2 trees and calculate a diff between
them. Problem is to assert that resulted diff is equal to the expected one.

Regular diff and patience diff are almost always identical in terms of
number of added and removed lines. Thus we should match diff fully line by line

On 5 May 2015 at 21:09, nulltoken [email protected] wrote:

@dmalikov https://github.com/dmalikov

The following code will create a Tree, containing one entry file.txt
which content is My content.

var content = "My content";var path = "file.txt";
var td = new TreeDefinition();
td.Add(path, OdbHelper.CreateBlob(repo, content), Mode.NonExecutableFile);
var t = repo.ObjectDatabase.CreateTree(td);


Reply to this email directly or view it on GitHub
#1039 (comment).

@nulltoken
Copy link
Member

@dmalikov Oh, sorry! I misunderstood your question.

You may be willing to actually leverage repo.Diff.Comapre<Patch>() then. See an example in DiffTreeToTargetFixture.cs

@dmalikov
Copy link
Contributor Author

dmalikov commented May 6, 2015

Thanks for examples, test is added

string repoPath = InitNewRepository();
using (var repo = new Repository(repoPath))
{
Func<string, Tree> fromString = s => repo.ObjectDatabase.CreateTree(new TreeDefinition().Add("file.txt", OdbHelper.CreateBlob(repo, s), Mode.NonExecutableFile));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please wrap this line so that it reads better on GitHub (without having to scroll horizontally)?

@dmalikov
Copy link
Contributor Author

dmalikov commented May 6, 2015

@nulltoken updated

@nulltoken
Copy link
Member

@dmalikov Awesome!

Last request: Could you please squash those commits together into a single cohesive one?

@dmalikov dmalikov force-pushed the feature/1037-patience-diff branch from 6747b5e to 70d1439 Compare May 7, 2015 07:50
@dmalikov
Copy link
Contributor Author

dmalikov commented May 7, 2015

@nulltoken squashed

nulltoken added a commit that referenced this pull request May 7, 2015
PatienceAlgorithm flag in CompareOptions
@nulltoken nulltoken merged commit 296aa06 into libgit2:vNext May 7, 2015
@nulltoken
Copy link
Member

✨ ✨ ✨ ✨ ✨ ✨ Very cool contribution!

Thanks a lot!

@dmalikov
Copy link
Contributor Author

dmalikov commented May 7, 2015

@nulltoken could you please build a new version of the NuGet package and upload it?

@nulltoken
Copy link
Member

@dmalikov Sir! Done, Sir! 😉

Published as NuGet pre-release package LibGit2Sharp.0.22.0-pre20150507080442

@dmalikov
Copy link
Contributor Author

dmalikov commented May 9, 2015

Thanks!

On 9 May 2015 at 13:18, nulltoken [email protected] wrote:

@dmalikov https://github.com/dmalikov Sir! Done, Sir! [image: 😉]

Published as NuGet pre-release package
LibGit2Sharp.0.22.0-pre20150507080442


Reply to this email directly or view it on GitHub
#1039 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants