# [PATCH] git-format-patch: Make the second and subsequent mails replies to the first 13 messages from 2006-07-10 to 2006-07-15. Participants: Josh Triplett, Johannes Schindelin, Erik Mouw, Jakub Narebski, Junio C Hamano, Petr Baudis. Thread: https://gitlist.dev/t/4832 ## Josh Triplett, 2006-07-10 15:44 Subject: [PATCH] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: <1152546266.8890.19.camel@josh-work.beaverton.ibm.com> URL: https://gitlist.dev/e/1152546266.8890.19.camel%40josh-work.beaverton.ibm.com ``` Add message_id and ref_message_id fields to struct rev_info, used in show_log with CMIT_FMT_EMAIL to set Message-Id and In-Reply-To/References respectively. Use these in git-format-patch to make the second and subsequent patch mails replies to the first patch mail. Signed-off-by: Josh Triplett --- git-send-email already does this, but this change helps people who use things like git-imap-send or similar to send the patch emails by other means. builtin-log.c | 23 +++++++++++++++++++++++ log-tree.c | 5 +++++ revision.h | 2 ++ 3 files changed, 30 insertions(+), 0 deletions(-) diff --git a/builtin-log.c b/builtin-log.c index 864c6cd..9d0cae1 100644 --- a/builtin-log.c +++ b/builtin-log.c @@ -220,6 +220,17 @@ static void get_patch_ids(struct rev_inf o2->flags = flags2; } +static void gen_message_id(char *dest, unsigned int length, char *base) +{ + const char *committer = git_committer_info(1); + const char *email_start = strrchr(committer, '<'); + const char *email_end = strrchr(committer, '>'); + if(!email_start || !email_end || email_start > email_end - 1) + die("Could not extract email from committer identity."); + snprintf(dest, length, "%s.%u.git.%.*s", base, time(NULL), + email_end - email_start - 1, email_start + 1); +} + int cmd_format_patch(int argc, const char **argv, char **envp) { struct commit *commit; @@ -233,6 +244,8 @@ int cmd_format_patch(int argc, const cha int ignore_if_in_upstream = 0; struct diff_options patch_id_opts; char *add_signoff = NULL; + char message_id[1024]; + char ref_message_id[1024]; init_revisions(&rev); rev.commit_format = CMIT_FMT_EMAIL; @@ -359,6 +372,16 @@ int cmd_format_patch(int argc, const cha int shown; commit = list[nr]; rev.nr = total - nr + (start_number - 1); + /* Make the second and subsequent mails replies to the first */ + if (nr == (total - 2)) { + strncpy(ref_message_id, message_id, + sizeof(ref_message_id)); + ref_message_id[sizeof(ref_message_id)-1] = '\0'; + rev.ref_message_id = ref_message_id; + } + gen_message_id(message_id, sizeof(message_id), + sha1_to_hex(commit->object.sha1)); + rev.message_id = message_id; if (!use_stdout) reopen_stdout(commit, rev.nr, keep_subject); shown = log_tree_commit(&rev, commit); diff --git a/log-tree.c b/log-tree.c index 9d8d46f..4971988 100644 --- a/log-tree.c +++ b/log-tree.c @@ -97,6 +97,11 @@ void show_log(struct rev_info *opt, cons subject = "Subject: "; printf("From %s Mon Sep 17 00:00:00 2001\n", sha1); + if (opt->message_id) + printf("Message-Id: <%s>\n", opt->message_id); + if (opt->ref_message_id) + printf("In-Reply-To: <%s>\nReferences: <%s>\n", + opt->ref_message_id, opt->ref_message_id); if (opt->mime_boundary) { static char subject_buffer[1024]; static char buffer[1024]; diff --git a/revision.h b/revision.h index c010a08..e23ec8f 100644 --- a/revision.h +++ b/revision.h @@ -61,6 +61,8 @@ struct rev_info { struct log_info *loginfo; int nr, total; const char *mime_boundary; + const char *message_id; + const char *ref_message_id; const char *add_signoff; const char *extra_headers; -- 1.4.1.gbe4c7-dirty ``` ## Johannes Schindelin, 2006-07-10 16:01 Subject: Re: [PATCH] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0607101801360.29667%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <1152546266.8890.19.camel@josh-work.beaverton.ibm.com> ``` Hi, please make that behaviour optional. Ciao, Dscho ``` ## Erik Mouw, 2006-07-10 16:29 Subject: Re: [PATCH] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: <20060710162920.GR20191@harddisk-recovery.com> URL: https://gitlist.dev/e/20060710162920.GR20191%40harddisk-recovery.com In-Reply-To: ``` On Mon, Jul 10, 2006 at 06:01:48PM +0200, Johannes Schindelin wrote: > please make that behaviour optional. Rather make it consistent with git-send-email. Principle of least surprise. Erik -- +-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 -- | Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands ``` ## Josh Triplett, 2006-07-10 16:43 Subject: Re: [PATCH] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: <1152549787.8890.36.camel@josh-work.beaverton.ibm.com> URL: https://gitlist.dev/e/1152549787.8890.36.camel%40josh-work.beaverton.ibm.com In-Reply-To: <20060710162920.GR20191@harddisk-recovery.com> ``` On Mon, 2006-07-10 at 18:29 +0200, Erik Mouw wrote: > On Mon, Jul 10, 2006 at 06:01:48PM +0200, Johannes Schindelin wrote: > > please make that behaviour optional. > > Rather make it consistent with git-send-email. Principle of least > surprise. Well, git-send-email does not include an option to disable the threading headers, so consistency with git-send-email would imply not including any such option. I can, however, implement a --no-thread option to omit the headers, as well as git-send-email's --in-reply-to option to set an initial In-Reply-To/References. New patch series shortly. - Josh Triplett ``` ## Josh Triplett, 2006-07-10 18:41 Subject: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: <1152556878.8890.45.camel@josh-work.beaverton.ibm.com> URL: https://gitlist.dev/e/1152556878.8890.45.camel%40josh-work.beaverton.ibm.com In-Reply-To: <20060710162920.GR20191@harddisk-recovery.com> ``` Add message_id and ref_message_id fields to struct rev_info, used in show_log with CMIT_FMT_EMAIL to set Message-Id and In-Reply-To/References respectively. Use these in git-format-patch to make the second and subsequent patch mails replies to the first patch mail. Signed-off-by: Josh Triplett --- Resend of previous patch as part of new patch series. builtin-log.c | 23 +++++++++++++++++++++++ log-tree.c | 5 +++++ revision.h | 2 ++ 3 files changed, 30 insertions(+), 0 deletions(-) diff --git a/builtin-log.c b/builtin-log.c index 864c6cd..9d0cae1 100644 --- a/builtin-log.c +++ b/builtin-log.c @@ -220,6 +220,17 @@ static void get_patch_ids(struct rev_inf o2->flags = flags2; } +static void gen_message_id(char *dest, unsigned int length, char *base) +{ + const char *committer = git_committer_info(1); + const char *email_start = strrchr(committer, '<'); + const char *email_end = strrchr(committer, '>'); + if(!email_start || !email_end || email_start > email_end - 1) + die("Could not extract email from committer identity."); + snprintf(dest, length, "%s.%u.git.%.*s", base, time(NULL), + email_end - email_start - 1, email_start + 1); +} + int cmd_format_patch(int argc, const char **argv, char **envp) { struct commit *commit; @@ -233,6 +244,8 @@ int cmd_format_patch(int argc, const cha int ignore_if_in_upstream = 0; struct diff_options patch_id_opts; char *add_signoff = NULL; + char message_id[1024]; + char ref_message_id[1024]; init_revisions(&rev); rev.commit_format = CMIT_FMT_EMAIL; @@ -359,6 +372,16 @@ int cmd_format_patch(int argc, const cha int shown; commit = list[nr]; rev.nr = total - nr + (start_number - 1); + /* Make the second and subsequent mails replies to the first */ + if (nr == (total - 2)) { + strncpy(ref_message_id, message_id, + sizeof(ref_message_id)); + ref_message_id[sizeof(ref_message_id)-1] = '\0'; + rev.ref_message_id = ref_message_id; + } + gen_message_id(message_id, sizeof(message_id), + sha1_to_hex(commit->object.sha1)); + rev.message_id = message_id; if (!use_stdout) reopen_stdout(commit, rev.nr, keep_subject); shown = log_tree_commit(&rev, commit); diff --git a/log-tree.c b/log-tree.c index 9d8d46f..4971988 100644 --- a/log-tree.c +++ b/log-tree.c @@ -97,6 +97,11 @@ void show_log(struct rev_info *opt, cons subject = "Subject: "; printf("From %s Mon Sep 17 00:00:00 2001\n", sha1); + if (opt->message_id) + printf("Message-Id: <%s>\n", opt->message_id); + if (opt->ref_message_id) + printf("In-Reply-To: <%s>\nReferences: <%s>\n", + opt->ref_message_id, opt->ref_message_id); if (opt->mime_boundary) { static char subject_buffer[1024]; static char buffer[1024]; diff --git a/revision.h b/revision.h index c010a08..e23ec8f 100644 --- a/revision.h +++ b/revision.h @@ -61,6 +61,8 @@ struct rev_info { struct log_info *loginfo; int nr, total; const char *mime_boundary; + const char *message_id; + const char *ref_message_id; const char *add_signoff; const char *extra_headers; -- 1.4.1.gf029 ``` ## Jakub Narebski, 2006-07-10 20:25 Subject: Re: [PATCH] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: URL: https://gitlist.dev/e/e8ud22%24jo1%241%40sea.gmane.org In-Reply-To: <1152549787.8890.36.camel@josh-work.beaverton.ibm.com> ``` Josh Triplett wrote: > On Mon, 2006-07-10 at 18:29 +0200, Erik Mouw wrote: >> On Mon, Jul 10, 2006 at 06:01:48PM +0200, Johannes Schindelin wrote: >> > please make that behaviour optional. >> >> Rather make it consistent with git-send-email. Principle of least >> surprise. > > Well, git-send-email does not include an option to disable the threading > headers, so consistency with git-send-email would imply not including > any such option. I can, however, implement a --no-thread option to omit > the headers, as well as git-send-email's --in-reply-to option to set an > initial In-Reply-To/References. New patch series shortly. git-send-email has three ways of sending files: 1. Chain Reply-To:, where every patch refers to earlier in series. Ugly in threaded mail/news readers, harder to comment, but there is no way to loose the order (e.g. if patches are not numbered *blush*) 2. No chain reply-to, with cover letter introducing patch series. IMHO nicest format... provided there are no errors nor mistakes. 3. No chain reply-to, without cover letter. I presonally don't like this format, YMMV. -- Jakub Narebski Warsaw, Poland ShadeHawk on #git ``` ## Junio C Hamano, 2006-07-10 21:44 Subject: Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: <7vwtal9lu1.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vwtal9lu1.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <1152556878.8890.45.camel@josh-work.beaverton.ibm.com> ``` Josh Triplett writes: > Add message_id and ref_message_id fields to struct rev_info, used in show_log > with CMIT_FMT_EMAIL to set Message-Id and In-Reply-To/References respectively. > Use these in git-format-patch to make the second and subsequent patch mails > replies to the first patch mail. > > Signed-off-by: Josh Triplett > --- > Resend of previous patch as part of new patch series. While I understand what you said about imap-send, I really would feel better if this was optional. Do not change the default output format, please. ``` ## Josh Triplett, 2006-07-14 17:16 Subject: Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: <1152897407.5177.8.camel@josh-work.beaverton.ibm.com> URL: https://gitlist.dev/e/1152897407.5177.8.camel%40josh-work.beaverton.ibm.com In-Reply-To: <7vwtal9lu1.fsf@assigned-by-dhcp.cox.net> ``` On Mon, 2006-07-10 at 14:44 -0700, Junio C Hamano wrote: > Josh Triplett writes: > > > Add message_id and ref_message_id fields to struct rev_info, used in show_log > > with CMIT_FMT_EMAIL to set Message-Id and In-Reply-To/References respectively. > > Use these in git-format-patch to make the second and subsequent patch mails > > replies to the first patch mail. > > > > Signed-off-by: Josh Triplett > > --- > > Resend of previous patch as part of new patch series. > > While I understand what you said about imap-send, I really would > feel better if this was optional. Do not change the default > output format, please. So rather than the --no-thread option provided in the second patch of this series, you'd prefer a --thread option to enable setting the In-Reply-To/References headers? Note that I based the direction of the --no-thread switch on git-send-email's inclusion of these headers by default with no way to turn them off, figuring that having an option to do so gave it an advantage over git-send-email while remaining consistent with it. I also figured that most people would not mess with the defaults, and thus the default should make a patch series more readable on mailing lists. - Josh Triplett ``` ## Junio C Hamano, 2006-07-14 18:23 Subject: Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: <7v4pxkoxjp.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v4pxkoxjp.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <1152897407.5177.8.camel@josh-work.beaverton.ibm.com> ``` Josh Triplett writes: >> While I understand what you said about imap-send, I really would >> feel better if this was optional. Do not change the default >> output format, please. > > So rather than the --no-thread option provided in the second patch of > this series, you'd prefer a --thread option to enable setting the > In-Reply-To/References headers? Eh, that's not what I meant. I do not mind the code you added to log-tree.c and revision.h, and honestly I do not care which of threading or non-threading mode is the default, although I think your explanation that it parallels what send-email does makes a lot of sense. But I do mind that the code added by the first patch to cmd_format_patch runs by default, and worse yet, there is no option turn it off. Setting message_id and ref_message_id in rev_info struct should be something the end user should ask for explicitly by invoking the command with an option, perhaps --with-message-id, which you probably would also want to turn on when any of --no-thread, --thread or --initial-reply-to options are given. - ``` ## Josh Triplett, 2006-07-14 19:20 Subject: Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: <1152904829.5177.19.camel@josh-work.beaverton.ibm.com> URL: https://gitlist.dev/e/1152904829.5177.19.camel%40josh-work.beaverton.ibm.com In-Reply-To: <7v4pxkoxjp.fsf@assigned-by-dhcp.cox.net> ``` On Fri, 2006-07-14 at 11:23 -0700, Junio C Hamano wrote: > Josh Triplett writes: > > >> While I understand what you said about imap-send, I really would > >> feel better if this was optional. Do not change the default > >> output format, please. > > > > So rather than the --no-thread option provided in the second patch of > > this series, you'd prefer a --thread option to enable setting the > > In-Reply-To/References headers? > > Eh, that's not what I meant. > > I do not mind the code you added to log-tree.c and revision.h, > and honestly I do not care which of threading or non-threading > mode is the default, although I think your explanation that it > parallels what send-email does makes a lot of sense. > > But I do mind that the code added by the first patch to > cmd_format_patch runs by default, and worse yet, there is no > option turn it off. Setting message_id and ref_message_id in > rev_info struct should be something the end user should ask for > explicitly by invoking the command with an option, perhaps > --with-message-id, which you probably would also want to turn on > when any of --no-thread, --thread or --initial-reply-to options > are given. How would that work though? Threading requires a Message-Id on at least the first message, so to avoid Message-IDs by default would require turning off threading by default; I can do that if you like, but you suggested that you didn't mind having threading as the default. I could, however, avoid generating Message-Id on the subsequent messages, and avoid generating that Message-Id if you give --no-thread. Would that work? - Josh Triplett ``` ## Junio C Hamano, 2006-07-14 19:32 Subject: Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: <7vwtagnfsk.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vwtagnfsk.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <1152904829.5177.19.camel@josh-work.beaverton.ibm.com> ``` Josh Triplett writes: > ..., but you > suggested that you didn't mind having threading as the default. Did I? ... then that was either a mistake or miscommunication. I do mind changing the default output. I do not mind threading as the default ONLY IF user asks for output with these extra headers. ``` ## Petr Baudis, 2006-07-15 07:45 Subject: Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: <20060715074532.GF13776@pasky.or.cz> URL: https://gitlist.dev/e/20060715074532.GF13776%40pasky.or.cz In-Reply-To: <7vwtagnfsk.fsf@assigned-by-dhcp.cox.net> ``` Dear diary, on Fri, Jul 14, 2006 at 09:32:27PM CEST, I got a letter where Junio C Hamano said that... > Josh Triplett writes: > > > ..., but you > > suggested that you didn't mind having threading as the default. > > Did I? ... then that was either a mistake or miscommunication. > > I do mind changing the default output. I do not mind threading > as the default ONLY IF user asks for output with these extra > headers. What's the big deal? It's not like we didn't change those things in the past if it doesn't horribly break everything and the new behaviour is clearly more sensible. It would be good to know what the general policy on this is nowadays. -- Petr "Pasky" Baudis Stuff: http://pasky.or.cz/ Snow falling on Perl. White noise covering line noise. Hides all the bugs too. -- J. Putnam ``` ## Junio C Hamano, 2006-07-15 08:10 Subject: Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first Message-ID: <7virlzmgoo.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7virlzmgoo.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20060715074532.GF13776@pasky.or.cz> ``` Petr Baudis writes: > Dear diary, on Fri, Jul 14, 2006 at 09:32:27PM CEST, I got a letter > where Junio C Hamano said that... >> Josh Triplett writes: >> >> > ..., but you >> > suggested that you didn't mind having threading as the default. >> >> Did I? ... then that was either a mistake or miscommunication. >> >> I do mind changing the default output. I do not mind threading >> as the default ONLY IF user asks for output with these extra >> headers. > > What's the big deal? It's not like we didn't change those things in the > past if it doesn't horribly break everything and the new behaviour is > clearly more sensible. While I agree to the whole three lines, I do not think adding the Message-Id and In-Reply-To header lines by default is more sensible at all. Adding phoney Message-Id to format-patch output makes some sense only when you are sending messages, and if I recall original "motive" message correctly only with git-imap-send. We do not need this for git-send-email, since it can do its own threading. Although I've already accepted the series to "next", now after you brought up the issue, I started to suspect that it might even make sense not to do this in format-patch but make it a responsibility for MUA-looking commands instead. More importantly, format-patch is used to extract patches into separate files (I do that myself often, and I think Andrew Morten uses it to extract stuff from git-maintained trees). In such a case having phoney Message-Id is simply a waste. Running "head -n X 0*.txt" now needs one or two larger X to view the same information, and fewer patches fit on the screen than before. So the new behaviour, if it were not optional, is clearly less useful for such purpose. It could even be confusing and inviting mistakes. When quoting a change from somebody that was sent in an e-mail to the list, giving its Message-Id is often helpful to others who want to go to the source themselves, but if a file that was generated by format-patch by default carries a phoney Message-Id, it can be mistakenly used in such a quote. ```