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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 12, 2015, 22:05 UTC
Message-ID: <xmqqzj44bn1l.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CANoM8SW-N6_yJ0kgGDuGWB+RS-0d54D4FtaRbKqhsf0_fSeMdw@mail.gmail.com>

Mike Rappazzo <rappazzo@gmail.com> writes:

Show 15 quoted lines

> On Fri, Jun 12, 2015 at 4:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> The autosquash part somehow makes me feel uneasy, though.  The
>> feature fundamentally has to have %s as the first thing in the
>> format to work, but by making the format overridable, you are
>> potentially breaking that feature, aren't you?
>
> It only needs the '%s' for the autosquash when the todo/instruction
> list order is determined.  For this, in the rearrange_squash function,
> it will re-calculate the message:
>
> +               test -z "${format}" || message=$(git log -n 1
> --format="%s" ${sha1})
>
> Additionally, it may also rerun the log command when preparing the final list.

Yeah, I noticed that when I took a diff between v3 and v4. I didn't mean by "break" that you may end up reorder lines incorrectly.

But because you overwrite the $message variable you read from the original insn sheet (which uses the custom format) and compute $rest based on the default "%s" and store that in "$1.sq", lines in "$1.sq" do not know anything about the custom format, do they?

And then they are injected to appropriate places in "$1.rearranged". Moved lines in the the rearranged result would end up written in the default "%s" format, no?

That was the part that made me uneasy.

I do not think that is a bug worth fixing, but I view it as a sign that fundamentally the autosquash and the idea of configurable format do not mesh well with each other.