Re: [PATCH] stash: Fix multiple error messages on create if no HEAD
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 14, 2011, 01:16 UTC
- Message-ID
- <7vsjkorlzp.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20111214001432.GA2959@thinkpad>
Sebastian Morr <sebastian@morr.cc> writes:
Show 12 quoted lines
> This bugged me: In a new, empty repository: > > $ git stash > fatal: bad revision 'HEAD' > fatal: bad revision 'HEAD' > fatal: Needed a single revision > You do not have the initial commit yet > > With this patch: > > $ git stash > You do not have the initial commit yet
Yeah, that looks tidier.
> With the --quiet option, I wouldn't expect diff-index to print error > messages.
Even with --quiet, an error is an error.
Asking for a diff with the current HEAD when you do not have one yet _is_ an error and "diff-index" is correct to report the error. The bug (i.e. "git stash" shows that error message) is in the program that calls diff-index when the operation does not make sense.
"Hiding error" is not desirable, especially if it is only for giving a cleaner error message for a narrow corner case that nobody cares (i.e. running "stash" when you do not have any commit to go back to), with the side effect that it hides _real_ errors that may help users notice unusual situation (e.g. corrupted index file).
Probably the right way to fix it is to check if HEAD is a valid commit before running any command that requires to have a valid HEAD (i.e. create and save would need one; list, show, etc. would not) and give that "You do not have the initial commit yet" message, before passing the control to the implementations of these individual subcommands, without touching the places you touched in this patch.
Show 15 quoted lines
> no_changes () {
> - git diff-index --quiet --cached HEAD --ignore-submodules -- &&
> + git diff-index --quiet --cached HEAD --ignore-submodules -- 2>/dev/null &&
> git diff-files --quiet --ignore-submodules &&
> (test -z "$untracked" || test -z "$(untracked_files)")
> }
> @@ -67,7 +67,7 @@ create_stash () {
> fi
>
> # state of the base commit
> - if b_commit=$(git rev-parse --verify HEAD)
> + if b_commit=$(git rev-parse --quiet --verify HEAD)
> then
> head=$(git rev-list --oneline -n 1 HEAD --)
> else