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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:02:28

Junio C Hamano [off-list ref] writes:
William Clifford [off-list ref] writes:
quoted
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

From: William Clifford <hidden>
Date: 2016-06-15 23:02:28

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 [off-list ref] wrote:
Junio C Hamano [off-list ref] writes:
quoted
William Clifford [off-list ref] writes:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help