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

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

[William Clifford]

I'm sorry if this shows up twice. I messed up the first time.

I was recently in a situation of rebasing a topic branch with many
commits by several colleagues and I thought it would be useful if I
could see the author of the commit in the commit table in the rebase
todo file. I wanted try to squash commits by the same author where
appropriate. With the help of Matt Boeh, we figured out how to patch in
custom formatting of this line, and I have been trying it out for the
last couple of weeks and it seems to work pretty well.

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"`

The first one tries to conserve linespace, which is a good idea for
these formats. The second one could be really long but otherwise
harmless. I'm unsure what would happen if I tried to rebase with the
third style unedited or uncommented. I'm wary of casually testing it. It
does not seem like a good idea, and should perhaps be discouraged.

Please let me know what you think.

William Clifford (2):
  Add sequence.format to interactive rebase
  Add documentation of sequence.format to interactive rebase

 Documentation/config.txt   |  9 +++++++++
 git-rebase--interactive.sh | 14 +++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

-- 
William Clifford

Message-ID: <867g1dia81.fsf@gmail.com>
X-Draft-From: ("INBOX")

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

[Junio C Hamano]

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?

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

[Junio C Hamano]

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.

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

[William Clifford]

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:
> 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

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

[Junio C Hamano]

William Clifford <mr.william.clifford@gmail.com> writes:

> 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.

;-)