From: Jon Seymour <jon.seymour@gmail.com>
Date: Sun, 31 Jul 2011 19:48:40 GMT
Subject: Re: [PATCH v8 4/7] bisect: introduce --no-checkout, --update-ref=<ref> support into porcelain.
Message-ID: <CAH3Anrp+Jynon0O8mQJnKvCNtp0v6oR9pLetmg0B6GnoP2QWMg@mail.gmail.com>
In-Reply-To: <CAP8UFD2uWRS4KznUAQCCF8sShGy6+sGWgNsLj8mmO7Ur1CDrCQ@mail.gmail.com>

On Mon, Aug 1, 2011 at 4:35 AM, Christian Couder
<christian.couder@gmail.com> wrote:
> On Sun, Jul 31, 2011 at 1:55 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
>>
>> @@ -69,13 +71,18 @@ bisect_start() {
>>        orig_args=$(git rev-parse --sq-quote "$@")
>>        bad_seen=0
>>        eval=''
>> +       BISECT_UPDATE_REF=
>>        while [ $# -gt 0 ]; do
>>            arg="$1"
>>            case "$arg" in
>>            --)
>> -               shift
>> -               break
>> -               ;;
>> +               shift; break ;;
>
> Please don't change this. We try to avoid having many instructions on
> the same line like this.

Sure.

FWIW: there are two places in git-am.sh and git-repack.sh that put shift on
the same line as another statement,. Also this:

     case "$arg" in --) has_double_dash=1; break ;; esac

However, 'fixing' those can be another change, assessed on its own merits.

>
>> +           --no-checkout)
>> +               BISECT_UPDATE_REF=HEAD; shift ;;
>
> Perhaps:
>
>                   test -z "$BISECT_UPDATE_REF" && BISECT_UPDATE_REF=HEAD
>
> so that "--update-ref=someref --no-checkout" works the same as
> "--no-checkout --update-ref=someref".
>

Got the intent, thanks.

 How about BISECT_UPDATE_REF=${BISECT_UPDATE_REF:-HEAD}?

jon.