Re: [RFC/PATCH] Fix git-diff --cached to not error out if HEAD points to a nonexistant branch
- From
- Peter Baumann <waste.manager@gmx.de>
- Date
- Feb 24, 2007, 22:16 UTC
- Message-ID
- <20070224221622.GA3897@xp.machine.xx>
- In-Reply-To
- <7vvehrw9mz.fsf@assigned-by-dhcp.cox.net>
On Sat, Feb 24, 2007 at 01:03:48PM -0800, Junio C Hamano wrote:
Show 24 quoted lines
> Peter Baumann <waste.manager@gmx.de> writes: > > > The documentation mentions "git-diff --cached" to see what is staged for > > the next commit. But this failes if you haven't done any commits yet. > > So lets fix it. > > ... > > ... I am > > not sure if this is the right fix and/or if git-diff-index > > should also be fixed. I decided against it and let the core > > cmd git-diff-index stay as it is now. > > I think you decision here is a correct one. The plumbing level > command git-diff-index should error out if you do not give a > tree to compare against. > > My preference for 'git-diff --cached' issue is to fix the > explanation. Clearly document that --cached is to review the > difference between any commit (we could even be more precise to > say any tree, but I think we should say commit here, as the > description is at the end-user level) and what is staged for the > commit that will be created with your next 'git-commit'. For > convenience it defaults to 'HEAD', the latest commit on your > current branch, because that is what people would do most often. >
I tend to agree, but I'd like to also have somethin in the spirit of "log.showroot = true" which handles the diff of the first commit like diffing against an empty tree. Why should diff --cached differ from this? At least it is easier to explain, just mention that diff --cached shows everything which would become the next commit.
> Until you have a commit at HEAD, there really is nothing to diff > against. I think "foo is a new entry, no comparison available." > is one of the very few things that CVS got right. >
Hm. But a diff against nothing should show you what you have added :-)
Show 7 quoted lines
> The bug in the current code is that we do not check if that HEAD > is sensible when we add it as the default commit to compare > with. The error message coming out of the low-level diff-index > code might be sensible if that 'HEAD' were what the user > actually gave us, but clearly not the right error message in > this case. >
Here I agree with you. But I guess you know what I would prefere.
-Peter
Show 19 quoted lines
> ---
>
> builtin-diff.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-diff.c b/builtin-diff.c
> index c387ebb..67f4932 100644
> --- a/builtin-diff.c
> +++ b/builtin-diff.c
> @@ -261,6 +261,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
> break;
> else if (!strcmp(arg, "--cached")) {
> add_head(&rev);
> + if (!rev.pending.nr)
> + die("No HEAD commit to compare with (yet)");
> break;
> }
> }
>