Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 10, 2015, 00:44 UTC
Message-ID: <xmqqwpzcv1c0.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CANoM8SVXmYJn7wbxcPuBSUa-X8v0nggpv_vC8LtPgaBwMtP0HQ@mail.gmail.com>

Mike Rappazzo <rappazzo@gmail.com> writes:

> I have since reworked this script to support the short hash in the
> custom format as a special case:

I thought that we always give short form when presenting it to the end user to edit, but for internal bookkeeping purposes we make sure that we use the full SHA-1, so that new objects created during the rebase session will not cause "used to be unique but not anymore" ambiguity in the commit object names in the instruction sheet.

I have been assuming that the "rev-list" we have been mucking with was to prepare the latter, the internal bookkeeping data, which must always spell 40-hex (that is why the default "oneline" is not "--oneline" but "--pretty=oneline).

Why is it necessary to make %m%H part customizable in the first place?

Puzzled....

Show 9 quoted lines

>
> -git rev-list $merges_option --pretty=oneline --reverse --left-right
> --topo-order \
> +format=$(git config --get rebase.instructionFormat)
> +no_format=$?
> +if test ${no_format} -ne 0
> +then
> + format="%H %s"
> +elif test "${format:0:3}" != "%H " && test "${format:0:3}" != "%h "

Do not use bash-ism "substring".

Show 22 quoted lines

> +then
> + format="%H ${format}"
> +fi
> +# the 'rev-list .. | sed' requires %m to parse; the instruction
> requires %H to parse
> +git rev-list $merges_option --format="%m${format}" \
> + --reverse --left-right --topo-order \
>
> I also use the $no_format variable later on in the autosquash
> re-ordering, and have the tests passing.  I want to add some new tests
> on the custom format, and will send a new patch when that is complete.
>
> On Tue, Jun 9, 2015 at 3:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>>
>>> Besides, are you sure you don't want to substitute an empty
>>> rebase.instructionFormat' by '%s'? I would have expected to read
>>> ${format:-%s}` (note the colon), but then, this was Junio's
>>> suggestion...
>>
>> That was me simply being sloppy myself, expecting people not to copy
>> and paste literally without thinking.  Thanks for noticing.