Re: [PATCH] diff: support --root --cached combination
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:49:56
Nguyễn Thái Ngọc Duy wrote:
I have a ritual of doing "git dic" (short for diff --cached) before committing and does not want to break it, even on new repos. Looks like a good thing and no harm to the rest of the world.
This explanation belongs in the commit message, methinks.
quoted hunk ↗ jump to hunk
--- 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. "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.
(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?
+ 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).