On Tue, Mar 12, 2013 at 5:43 PM, Matt McClure <matthewlmcclure@xxxxxxxxx> wrote:
> On Tue, Mar 12, 2013 at 5:06 PM, John Keeping <john@xxxxxxxxxxxxx> wrote:
>>
>> is it sufficient to say
>> "there is no more than one non-option to the left of '--' and '--cached'
>> is not among the options"?
>
> An alternative approach would be to reuse git-diff's option parsing
> and make it tell git-difftool when git-diff sees the working tree
> case. At this point, I haven't seen an obvious place in the source
> where git-diff makes that choice, but if someone could point me in the
> right direction, I think I'd actually prefer that approach. What do
> you think?
There's an interesting comment in cmd_diff:
/*
* We could get N tree-ish in the rev.pending_objects list.
* Also there could be M blobs there, and P pathspecs.
*
* N=0, M=0:
* cache vs files (diff-files)
* N=0, M=2:
* compare two random blobs. P must be zero.
* N=0, M=1, P=1:
* compare a blob with a working tree file.
*
* N=1, M=0:
* tree vs cache (diff-index --cached)
*
* N=2, M=0:
* tree vs tree (diff-tree)
*
* N=0, M=0, P=2:
* compare two filesystem entities (aka --no-index).
*
* Other cases are errors.
*/
whereas inspecting rev.pending in the "compare against working tree"
case, I see:
(gdb) p rev.pending
$3 = {
nr = 1,
alloc = 64,
objects = 0x100807a00
}
(gdb) p *rev.pending.objects
$4 = {
item = 0x100831a48,
name = 0x7fff5fbff8f8 "HEAD^",
mode = 12288
}
Given the cases listed in the comment, I assume cmd_diff must
interpret this case as:
* N=1, M=0:
* tree vs cache (diff-index --cached)
The description of that case is confusing or wrong given that
git-diff-index(1) says:
--cached
do not consider the on-disk file at all
***
cmd_diff executes this case:
else if (ents == 1)
result = builtin_diff_index(&rev, argc, argv);
So it looks like I could short-circuit in builtin_diff_index or
something it calls -- e.g., run_diff_index -- to get git-diff to tell
git-difftool that it's the working tree case. I see that
run_diff_index does:
diff_set_mnemonic_prefix(&revs->diffopt, "c/", cached ? "i/" : "w/");
So that looks like a good place where the code is already deciding
that it's the working tree case -- "w/", though surprisingly to me:
(gdb) p revs->diffopt
$12 = {
...
a_prefix = 0x1001c25aa "a/",
b_prefix = 0x1001c25ad "b/",
...
So diff_set_mnemonic_prefix doesn't actually use the "w/" value passed
to it because:
if (!options->b_prefix)
options->b_prefix = b;
Maybe if I could prevent b_prefix from getting set earlier, I could
get some variant of git-diff to emit the "w/" for git-difftool.
--
Matt McClure
http://www.matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html