Linus TorvaldsCan we please revert the "revert" changes to the default message?
Not only did it make the one-line description much less readable by putting the shortened SHA1 in there (and removing the quotes), it also missed the empty line between the header and the body.
IOW, the old scripted message was much better. The built-in is nice, but the new message format is inferior.
Dscho?
Linus
Johannes Schindelin[PATCH] git-revert: Revert revert message to old behaviour
When converting from the shell script, based on a misreading of the sed invocation, the builtin included the abbreviated commit name, and did _not_ include the quotes around the oneline message.
This fixes it.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> ---
On Fri, 23 Mar 2007, Linus Torvalds wrote: > > Can we please revert the "revert" changes to the default message? > > Dscho?
Yeah?
builtin-revert.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/builtin-revert.c b/builtin-revert.c index f3f3f5c..f4e1e22 100644 --- a/builtin-revert.c +++ b/builtin-revert.c @@ -296,11 +296,9 @@ static int revert_or_cherry_pick(int argc, const char **argv) if (action == REVERT) { base = commit; next = commit->parents->item; - add_to_msg("Revert "); - add_to_msg(find_unique_abbrev(commit->object.sha1, - DEFAULT_ABBREV)); + add_to_msg("Revert \""); add_to_msg(oneline); - add_to_msg("\nThis reverts commit "); + add_to_msg("\"\nThis reverts commit "); add_to_msg(sha1_to_hex(commit->object.sha1)); add_to_msg(".\n"); } else {
-- 1.5.1.rc1.2356.g2054
Linus TorvaldsRe: [PATCH] git-revert: Revert revert message to old behaviour
On Fri, 23 Mar 2007, Johannes Schindelin wrote:
> > When converting from the shell script, based on a misreading of the > sed invocation, the builtin included the abbreviated commit name, > and did _not_ include the quotes around the oneline message. > > This fixes it.
How about the empty line in between the message and the "This reverts commit "..
> + add_to_msg("Revert \"");
> add_to_msg(oneline);
> - add_to_msg("\nThis reverts commit ");
> + add_to_msg("\"\nThis reverts commit ");This should probably be
add_to_msg("\"\n\nThis reverts commit ");with *two* \n's, no?
Linus
Johannes SchindelinRe: [PATCH] git-revert: Revert revert message to old behaviour
Hi,
On Fri, 23 Mar 2007, Linus Torvalds wrote:
> On Fri, 23 Mar 2007, Johannes Schindelin wrote:
> >
> > When converting from the shell script, based on a misreading of the
> > sed invocation, the builtin included the abbreviated commit name,
> > and did _not_ include the quotes around the oneline message.
> >
> > This fixes it.
>
> How about the empty line in between the message and the "This reverts
> commit "..
>
> > + add_to_msg("Revert \"");
> > add_to_msg(oneline);
> > - add_to_msg("\nThis reverts commit ");
> > + add_to_msg("\"\nThis reverts commit ");
>
> This should probably be
>
> add_to_msg("\"\n\nThis reverts commit ");
>
> with *two* \n's, no?Yes. Sorry.
Ciao, Dscho
Junio C HamanoRe: [PATCH] git-revert: Revert revert message to old behaviour
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> How about the empty line in between the message and the "This reverts
>> commit "..
>>
>> > + add_to_msg("Revert \"");
>> > add_to_msg(oneline);
>> > - add_to_msg("\nThis reverts commit ");
>> > + add_to_msg("\"\nThis reverts commit ");
>>
>> This should probably be
>>
>> add_to_msg("\"\n\nThis reverts commit ");
>>
>> with *two* \n's, no?
>
> Yes. Sorry.Not only that.
The older one had two, duplicated abbrev, and your fix reduces it to once, but we do not want any abbrev there.
-- >8 -- [PATCH] git-revert: Revert revert message to old behaviour
When converting from the shell script, based on a misreading of the sed invocation, the builtin included the abbreviated commit name, and did _not_ include the quotes around the oneline message.
This fixes it.
[jc: with a fix for the typo/thinko spotted by Linus, and also removing the unwanted abbrev at the beginning.]
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Junio C Hamano <junkio@cox.net> --- builtin-revert.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin-revert.c b/builtin-revert.c index f3f3f5c..4ba0ee6 100644 --- a/builtin-revert.c +++ b/builtin-revert.c @@ -294,13 +294,13 @@ static int revert_or_cherry_pick(int argc, const char **argv) oneline = get_oneline(message); if (action == REVERT) { + char *oneline_body = strchr(oneline, ' '); + base = commit; next = commit->parents->item; - add_to_msg("Revert "); - add_to_msg(find_unique_abbrev(commit->object.sha1, - DEFAULT_ABBREV)); - add_to_msg(oneline); - add_to_msg("\nThis reverts commit "); + add_to_msg("Revert \""); + add_to_msg(oneline_body + 1); + add_to_msg("\"\n\nThis reverts commit "); add_to_msg(sha1_to_hex(commit->object.sha1)); add_to_msg(".\n"); } else {
-- 1.5.1.rc1.666.g2823
Johannes SchindelinRe: [PATCH] git-revert: Revert revert message to old behaviour
Hi,
On Sat, 24 Mar 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> How about the empty line in between the message and the "This reverts
> >> commit "..
> >>
> >> > + add_to_msg("Revert \"");
> >> > add_to_msg(oneline);
> >> > - add_to_msg("\nThis reverts commit ");
> >> > + add_to_msg("\"\nThis reverts commit ");
> >>
> >> This should probably be
> >>
> >> add_to_msg("\"\n\nThis reverts commit ");
> >>
> >> with *two* \n's, no?
> >
> > Yes. Sorry.
>
> Not only that.
>
> The older one had two, duplicated abbrev, and your fix reduces
> it to once, but we do not want any abbrev there.D'oh.
Thanks, Dscho