From: Christian Couder <chriscool@tuxfamily.org>
Date: Tue, 13 Oct 2009 20:06:18 GMT
Subject: Re: [PATCH v2] bisect reset: Allow resetting to any commit, not just a branch
Message-ID: <200910132206.18460.chriscool@tuxfamily.org>
In-Reply-To: <alpine.DEB.2.00.0910131116300.5105@dr-wily.mit.edu>

On Tuesday 13 October 2009, Anders Kaseorg wrote:
> @@ -311,8 +311,7 @@ bisect_reset() {
>  	}
>  	case "$#" in
>  	0) branch=$(cat "$GIT_DIR/BISECT_START") ;;
> -	1) git show-ref --verify --quiet -- "refs/heads/$1" ||
> -	       die "$1 does not seem to be a valid branch"
> +	1) git rev-parse --verify "$1^{commit}" || exit
>  	   branch="$1" ;;
>  	*)
>  	    usage ;;

I agree with the purpose of the patch but I think something like the 
following would be better:

@@ -311,8 +311,8 @@ bisect_reset() {
        }
        case "$#" in
        0) branch=$(cat "$GIT_DIR/BISECT_START") ;;
-       1) git show-ref --verify --quiet -- "refs/heads/$1" ||
-              die "$1 does not seem to be a valid branch"
+       1) git rev-parse --quiet --verify "$1^{commit}" > /dev/null ||
+               die "'$1' does not seem to point to a valid commit"
           branch="$1" ;;
        *)
            usage ;;

It would give a better error message when "git rev-parse" fails instead of:

fatal: Needed a single revision

and it would not print the SHA1 from "$1^{commit}" when "git rev-parse" 
succeeds.

Best regards,
Christian.