Re: [PATCH 2/2] Bisect: implement "bisect dunno" to mark untestable revisions.

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Mon, 8 Oct 2007, Christian Couder wrote:

> diff --git a/git-bisect.sh b/git-bisect.sh
> index 388887a..c556318 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -143,7 +145,7 @@ bisect_write_bad() {
>  
>  bisect_good() {
>  	bisect_autostart
> -        case "$#" in
> +	case "$#" in

White space breakage.

> @@ -153,7 +155,6 @@ bisect_good() {
>  		rev=$(git rev-parse --verify "$rev^{commit}") || exit
>  		bisect_write_good "$rev"
>  		echo "git-bisect good $rev" >>"$GIT_DIR/BISECT_LOG"
> -

?

> @@ -164,6 +165,28 @@ bisect_write_good() {
>  	echo "# good: "$(git show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
>  }
>  
> +bisect_dunno() {
> +	bisect_autostart
> +	case "$#" in
> +	0)    revs=$(git rev-parse --verify HEAD) || exit ;;
> +	*)    revs=$(git rev-parse --revs-only --no-flags "$@") &&
> +		test '' != "$revs" || die "Bad rev input: $@" ;;
> +	esac
> +	for rev in $revs
> +	do
> +		rev=$(git rev-parse --verify "$rev^{commit}") || exit
> +		bisect_write_dunno "$rev"
> +		echo "git-bisect dunno $rev" >>"$GIT_DIR/BISECT_LOG"

Should the last line not be put into bisect_write_dunno?  OTOH this is the 
only call site of that function, so I strongly doubt that the function 
(consisting of 3 lines, where the first is 'rev="$1"') is necessary at 
all.

> @@ -206,17 +229,104 @@ bisect_auto_next() {
>  	bisect_next_check && bisect_next || :
>  }
>  
> +search_dunno() {
> +	_hash="$1"
> +	_dunno="$2"
> +
> +	for _val in $_dunno ; do
> +		case $_hash in $_val) return 1 ;; esac
> +	done

This would be faster as

	case " $1" in " $2") return 1 ;; esac

I guess.

But as I said in the other reply, I think this logic belongs into the C 
core, instead of generating mostly useless information, passing it down to 
the script, and filtering it out again.

Thanks,
Dscho