Le mardi 15 avril 2008, Junio C Hamano a écrit :
quoted text
> Christian Couder <chriscool@tuxfamily.org> writes:
> > This patch refactors rev parsing code in a new "bisect_parse_rev"
> > function, and adds warnings in case a rev with a name that is the
> > same as one of the bisect subcommands is given where a revision
> > is expected.
> >
> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> > ---
> >  git-bisect.sh               |   47
> > ++++++++++++++++++++++++++++++------------ t/t6030-bisect-porcelain.sh
> > |   21 +++++++++++++++++++
> >  2 files changed, 54 insertions(+), 14 deletions(-)
> >
> > diff --git a/git-bisect.sh b/git-bisect.sh
> > index 6b43461..a090b97 100755
> > --- a/git-bisect.sh
> > +++ b/git-bisect.sh
> > @@ -61,6 +61,31 @@ bisect_autostart() {
> >  	}
> >  }
> >
> > +bisect_parse_rev() {
> > +	rev=""
> > +	dont_die_on_wrong_rev=""
> > +	name="$rev^{commit}"
> > +	dubious_rev_name=''
> > +
> > +	case "$rev" in
> > +	HEAD)
> > +		name='HEAD' ;;
> > +	help|start|bad|good|skip|next|reset|visualize|replay|log|run)
> > +		dubious_rev_name='yes' ;;
> > +	esac
>
> I see you listed all the subcommand names here but I somehow doubt this
> is sensible.
>
> The "bisect good $foo bad $bar" example from Ingo's transcript may
> suggest that good/bad may incorrectly appear by copying and pasting from
> an incorrect how-to guide,

Yes, and there are also some "git bisect start good $foo bad $bar" in his 
transcript. So perhaps we should warn only on "bad" good" and maybe "skip".

quoted text
> but on the other hand "bisect bad log" to mark 
> the tip of the "log" branch (or "replay or any other common words) to see
> where in the development trail on that branch things got broken is a
> perfectly valid and plausible thing to do.  Also when you happen to have
> found something broken while you are working on a more important issue,
> you may tentatively do "git tag bad $it", and keep working on that
> important issue, only to come back later and use that lightweight tag to
> feed "git bisect" with.  In either use case, "you named your ref the same
> as bisect subcommand name, bad boy" is an unnecessary noise.

I think Ingo uses git and git bisect very often, and still he made mistakes 
that could have resulted in unwanted behavior with nothing to help 
understand what happened, if he had tags or branches named "good" or "bad".

So it's not for "bad boys" or forcing good naming, but really preventing 
mistakes.

In bisect_autostart we also have:

		echo >&2 'You need to start by "git bisect start"'
		if test -t 0
		then
			echo >&2 -n 'Do you want me to do it for you [Y/n]? '
			read yesno
			case "$yesno" in
			[Nn]*)
				exit ;;
			esac
			bisect_start
		else
			exit 1
		fi

to make sure that the users know what happens, and help them do what they 
want. If you prefer I can add something like the above to make sure that 
the user knows what he is doing.

Thanks,
Christian.




--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html