Re: [PATCH v10 7/7] bisect: allow any terms set by user

From: Christian Couder <christian.couder@gmail.com>
Date: Jun 26, 2015, 20:29 UTC
Message-ID: <CAP8UFD1Donr4v3Z8bpawv0_heDUQ+-EkepZF6UvuXOed7TQBXA@mail.gmail.com>
In-Reply-To: <1435337896-20709-8-git-send-email-Matthieu.Moy@imag.fr>

On Fri, Jun 26, 2015 at 6:58 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:

Show 5 quoted lines

> From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
>
> Introduction of the git bisect terms command. The user can set his own
> terms. It will work exactly like before. The terms must be set before the
> start.

After looking a bit at the code, I think that for now existing predefined terms ("bad", "good", "new" and "old") as well as some other terms that look like bisect subcommands like "skip", "start" and "terms" should be disallowed as arguments to "git bisect terms", and this should be stated in the commit message and in the documentation as well as checked and tested.

For example a user might want to search for a fix by using "git bisect terms good bad" (which should swap "good" and "bad"), but we should not at least for now allow that.

Show 11 quoted lines

> @@ -185,7 +197,12 @@ bisect_start() {
>         eval "$eval true" &&
>         if test $must_write_terms -eq 1
>         then
> -               write_terms "$NAME_BAD" "$NAME_GOOD"
> +               write_terms "$NAME_BAD" "$NAME_GOOD" &&
> +               if test $must_log_terms -eq 1
> +               then
> +                       echo "git bisect terms $NAME_BAD $NAME_GOOD" \
> +                           >>"$GIT_DIR/BISECT_LOG"
> +               fi

Maybe you could move appending to the log into write_terms() though you might need to pass an additional argument to enable or disable logging.