Re: [PATCH/RFC 0/2] custom format for interactive rebase todo

From: William Clifford <mr.william.clifford@gmail.com>
Date: Sep 10, 2014, 14:48 UTC
Message-ID: <CAFY06=jqNEo=G7HFjHn-8i8tLMdHwg3yZdwidW_j=_Tw90tnhQ@mail.gmail.com>
In-Reply-To: <xmqqha0glo3j.fsf@gitster.dls.corp.google.com>

Thanks! I've set it up just this way and it seems to work fine.

It turns out the formatted string is processed by the shell `read` later on, which happens to ignore everything after the first line, so it seems like it should be safe. But doing something explicit about it seems like a good idea in this case. I will see about actually reading in additional lines and raising an error, but it occurs to me that if I can read in the additional lines, I could also insert them as comments, and then it should be as safe as any of the other comments inserted in the rebase todo. In any case I will look into it and prepare another patch.

On Tue, Sep 9, 2014 at 10:07 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 30 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>> William Clifford <mr.william.clifford@gmail.com> writes:
>>
>>> A couple of examples:
>>>
>>> - `git config sequence.format "%<(12,trunc)%ae %s"`
>>> - `git config sequence.format "%s <%aN %aE>"`
>>> - `git config sequence.format "%s%n%%n%b"`
>>> ... I'm unsure what would happen if I tried to rebase with the
>>> third style unedited or uncommented.
>>
>> It should be simply forbidden.  The body part may have a line that
>> is similar enough (i.e. starting with one of the command words and
>> then a hexadecimal string) to confuse the sequencing machinery.
>>
>> Other than that safety issue, I am not fundamentally opposed to the
>> idea.
>>
>> As to the implementation in 1/2, your unconditional use of ">%h" is
>> wrong (you would end up including the commits from the left side).
>>
>> Use '%m' instead of a hardcoded '>', perhaps?
>
> Also, I do not think you want to make the prepending of "%m%h "
> conditional.  If the user for whatever silly reason asks to use a
> format "%m%h %m%h %m%h", let her have that _after_ the "%m%h " the
> machinery needs to operate, i.e. "%m%h %m%h %m%h %m%h".  It is far
> easier to explain to the users and you can lose three lines from the
> second patch.

-- 
William Clifford