basepi / libgit2

The Library
http://libgit2.github.com
Other
0 stars 0 forks source link

Patience diff #19

Open trane opened 13 years ago

trane commented 13 years ago

Do we want patience diff in a separate file? I don't think it would hurt to do so. We could model it after JGit, where the both algos are in different files.

hausdorf commented 13 years ago

How we organize this will probably depend on what functions are called. If it calls all the same functions, we may just want to put them in one file.

hausdorf commented 13 years ago

I know for sure that patience calls at least 3 of the same functions in xdiff. My guess is that the pipeline is really similar. For example, creating the context used in recs_cmp(), patience requires that we compact it. Implying that at least a significant part of the pipeline is similar.

trane commented 13 years ago

Alright, I'll write it at the bottom of libdiff.c. I will need some new structs declared but they are only libdiff.c specific, so I won't declare them outside of it.

hausdorf commented 13 years ago

FYI, I'm compacting the functions I've already written and combining the structs. This will yield invasive changes. I'll refactor patience into these changes, so do what you're doing, but do let me know if patience requires substantially different infrastructure from Myers.

trane commented 13 years ago

It assumes that the environment has already been prepared by xdl_prepare_env(), which I assume you are refactoring. Other than that I don't see anything that would cause big problems.

trane commented 13 years ago

What is your new refactored version of the new xpparam_t ?

hausdorf commented 13 years ago

xpparam_t holds only a long whose bit pattern holds flags. I hold the flags in git_diffresults_conf.

Note that currently this depends on them being initialized before calling the diff() function. This will eventually be resolved by helper methods that build the results_conf for users, especially for common-path functions like printing to stdout.

hausdorf commented 13 years ago

One of us needs to branch off if we're working in the same file, or every push gives us a recursive merge. So I've branched to dev-algo-core. The caveat is, if you alter anything in my code, I need to know so that I don't build around something that's wrong. Also the methods that you see up there are very likely to change.

So.

trane commented 13 years ago

Sounds like a good idea. I will actually branch to dev-algo-patience. We can use dev-diff-algo to merge our branches. Sound good?

hausdorf commented 13 years ago

Yes, that sounds great.

trane commented 13 years ago

This is implemented and will be tested when the unit tests are completed.

hausdorf commented 13 years ago

How are the tests going, by the way?

trane commented 13 years ago

@crakdmirror ^^

basepi commented 13 years ago

Weird, I wasn't getting the notifications! Gotta use those mentions I guess. I suppose you get notifications on anything you've commented on, been mentioned on, are assigned to, or on the initial creation of an issue.

To answer your question, working on them. Won't probably be completely done tonight, but I'll pump through anything I don't have done tomorrow morning. In terms of the one we wanted to run during our presentation, I submit that we don't want a classic unit test -- we should create a demonstration, such that it will actually put out the diff and so forth, so we can prove that it works beyond just getting an "Everything's OK!" line. Or if we do normal unit tests in the presentation, we should also do a visible demonstration. Just sayin'.

trane commented 13 years ago

My impression was that at least some of the unit tests could/would output data that could be displayed if needed.

basepi commented 13 years ago

I'll look into that. You might be right, I know the git unit test framework I was looking at had the ability to pass in a -v for verbose execution. Didn't look in detail as to what that meant, but I will look for something equivalent in the libgit2 testing framework.

trane commented 13 years ago

imho, at least 1 working test to demo is better than 0. So, even if it is impossible to get fully featured testing in place by Tuesday, getting at least 1 will allow us to demo.

basepi commented 13 years ago

Right. Once I get one test the rest will be easy. Still learning the testing framework. Had to spend more time on algorithms last night than planned.

On Mon, Apr 25, 2011 at 2:18 PM, trane < reply@reply.github.com>wrote:

imho, at least 1 working test to demo is better than 0. So, even if it is impossible to get fully featured testing in place by Tuesday, getting at least 1 will allow us to demo.

Reply to this email directly or view it on GitHub: https://github.com/crakdmirror/libgit2/issues/19#comment_1054824