On Tue, Mar 12, 2013 at 5:43 PM, Matt McClure <matthewlmccl...@gmail.com> wrote:
> On Tue, Mar 12, 2013 at 5:06 PM, John Keeping <j...@keeping.me.uk> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to