2010/10/29 Jonathan Nieder <jrnieder@xxxxxxxxx>:
>> --- a/builtin/diff.c
>> +++ b/builtin/diff.c
>> @@ -330,8 +330,13 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
>> Â Â Â Â Â Â Â Â Â Â Â else if (!strcmp(arg, "--cached") ||
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â!strcmp(arg, "--staged")) {
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â add_head_to_pending(&rev);
>> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (!rev.pending.nr)
>> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â die("No HEAD commit to compare with (yet)");
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (!rev.pending.nr) {
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct object *obj;
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (!rev.show_root_diff)
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â die("No HEAD commit to compare with (yet)");
>
> How does this condition get tripped? ÂThe code allowing "[log]
> showroot" to be set to false is only invoked by the log family of
> commands.
>
> Using --root as the backward-compatibility option seems like
> an abuse of language, anyway.
Hmm.. I thought --root was supported by all diff command family. Now I
think of it, only "diff-tree --root" makes sense.
> Â"git diff --cached" has two meanings:
>
> Â1. show changes to be committed
>
> Â Â1b. show what git show --format=" " would say after a commit
>
> Â2. show differences between the index and the commit named by the
> Â Â(implicit) HEAD argument
>
> With interpretation (1b), --root should be respected, and the output
> should be empty (!), not an error, when "[log] showroot" is false.
>
> With interpretation (2), --root should not be respected, and an
> attempt to diff --cached in an unborn branch should be an error.
If you commit to an unborn branch, it would become the first commit of
that branch. So by (1a), it should show what is to be commited, isn't
it?
>
> (1a) and (1b) are the only useful interpretations. ÂSo for simplicity,
> would it make sense to drop the "if ()" for --root and make
>
>> +test_expect_success 'diff --cached' '
>> + Â Â test_must_fail git diff --cached
>> +'
>
> fail?
All for simpler, yes.
>
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â obj = (struct object*)lookup_tree((unsigned char*)EMPTY_TREE_SHA1_BIN);
>
> Â Â Â Âstruct tree *tree = lookup_tree((const unsigned char *) ...
> Â Â Â Âobj = &tree->object;
>
> might be more clear (and robust against future layout changes).
>
OK
--
Duy
--
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