# "git revert" message changes... 6 messages from 2007-03-23 to 2007-03-24. Participants: Linus Torvalds, Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/7367 ## Linus Torvalds, 2007-03-23 15:07 Subject: "git revert" message changes... Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0703230805450.6730%40woody.linux-foundation.org ``` Can 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, 2007-03-23 16:06 Subject: [PATCH] git-revert: Revert revert message to old behaviour Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0703231705150.4045%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: ``` 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 --- 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 Torvalds, 2007-03-23 16:27 Subject: Re: [PATCH] git-revert: Revert revert message to old behaviour Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0703230926380.6730%40woody.linux-foundation.org In-Reply-To: ``` 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 Schindelin, 2007-03-23 19:13 Subject: Re: [PATCH] git-revert: Revert revert message to old behaviour Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0703232013330.4045%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: ``` 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 Hamano, 2007-03-24 09:54 Subject: Re: [PATCH] git-revert: Revert revert message to old behaviour Message-ID: <7v648rm0g5.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v648rm0g5.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin 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 Signed-off-by: Junio C Hamano --- 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 Schindelin, 2007-03-24 12:34 Subject: Re: [PATCH] git-revert: Revert revert message to old behaviour Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0703241333580.4045%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7v648rm0g5.fsf@assigned-by-dhcp.cox.net> ``` Hi, On Sat, 24 Mar 2007, Junio C Hamano wrote: > Johannes Schindelin 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 ```