[PATCH 1/5] Add more tests for format-patch

[PATCH 1/5] Add more tests for format-patch

[Daniel Barkalow]

Tests -o, and an excessively long subject, and --thread, with and
without --in-reply-to=

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 t/t4014-format-patch.sh |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 0a6fe53..6e8b5f4 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -88,4 +88,49 @@ test_expect_success 'replay did not screw up the log message' '
 
 '
 
+test_expect_success 'multiple files' '
+
+	rm -rf patches/ &&
+	git checkout side &&
+	git format-patch -o patches/ master &&
+	ls patches/0001-Side-changes-1.patch patches/0002-Side-changes-2.patch patches/0003-Side-changes-3-with-n-backslash-n-in-it.patch
+'
+
+test_expect_success 'thread' '
+
+	rm -rf patches/ &&
+	git checkout side &&
+	git format-patch --thread -o patches/ master &&
+	FIRST_MID=$(grep "Message-Id:" patches/0001-* | sed "s/^[^<]*\(<[^>]*>\).*$/\1/") &&
+	for i in patches/0002-* patches/0003-*
+	do
+	  grep "References: $FIRST_MID" $i &&
+	  grep "In-Reply-To: $FIRST_MID" $i
+	done
+'
+
+test_expect_success 'thread in-reply-to' '
+
+	rm -rf patches/ &&
+	git checkout side &&
+	git format-patch --in-reply-to="<test.message>" --thread -o patches/ master &&
+	FIRST_MID="<test.message>" &&
+	for i in patches/*
+	do
+	  grep "References: $FIRST_MID" $i &&
+	  grep "In-Reply-To: $FIRST_MID" $i
+	done
+'
+
+test_expect_success 'excessive subject' '
+
+	rm -rf patches/ &&
+	git checkout side &&
+	for i in 5 6 1 2 3 A 4 B C 7 8 9 10 D E F; do echo "$i"; done >>file &&
+	git update-index file &&
+	git commit -m "This is an excessively long subject line for a message due to the habit some projects have of not having a short, one-line subject at the start of the commit message, but rather sticking a whole paragraph right at the start as the only thing in the commit message. It had better not become the filename for the patch." &&
+	git format-patch -o patches/ master..side &&
+	ls patches/0004-This-is-an-excessively-long-subject-line-for-a-messa.patch
+'
+
 test_done
-- 
1.5.4.1.191.gfbf10

[PATCH 2/5] Improve message-id generation flow control for format-patch

[Daniel Barkalow]

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 builtin-log.c |   29 ++++++++++++++---------------
 revision.h    |    2 +-
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 99d69f0..4f08ca4 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -575,16 +575,19 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const cha
 	o2->flags = flags2;
 }
 
-static void gen_message_id(char *dest, unsigned int length, char *base)
+static void gen_message_id(struct rev_info *info, char *base)
 {
 	const char *committer = git_committer_info(IDENT_WARN_ON_NO_NAME);
 	const char *email_start = strrchr(committer, '<');
 	const char *email_end = strrchr(committer, '>');
-	if(!email_start || !email_end || email_start > email_end - 1)
+	struct strbuf buf;
+	if (!email_start || !email_end || email_start > email_end - 1)
 		die("Could not extract email from committer identity.");
-	snprintf(dest, length, "%s.%lu.git.%.*s", base,
-		 (unsigned long) time(NULL),
-		 (int)(email_end - email_start - 1), email_start + 1);
+	strbuf_init(&buf, 0);
+	strbuf_addf(&buf, "%s.%lu.git.%.*s", base,
+		    (unsigned long) time(NULL),
+		    (int)(email_end - email_start - 1), email_start + 1);
+	info->message_id = strbuf_detach(&buf, NULL);
 }
 
 static const char *clean_message_id(const char *msg_id)
@@ -625,8 +628,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	const char *in_reply_to = NULL;
 	struct patch_ids ids;
 	char *add_signoff = NULL;
-	char message_id[1024];
-	char ref_message_id[1024];
 
 	git_config(git_format_config);
 	init_revisions(&rev, prefix);
@@ -809,15 +810,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		rev.nr = total - nr + (start_number - 1);
 		/* Make the second and subsequent mails replies to the first */
 		if (thread) {
-			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;
+			if (rev.message_id) {
+				if (rev.ref_message_id)
+					free(rev.message_id);
+				else
+					rev.ref_message_id = rev.message_id;
 			}
-			gen_message_id(message_id, sizeof(message_id),
-				       sha1_to_hex(commit->object.sha1));
-			rev.message_id = message_id;
+			gen_message_id(&rev, sha1_to_hex(commit->object.sha1));
 		}
 		if (!use_stdout)
 			if (reopen_stdout(commit, rev.nr, keep_subject,
diff --git a/revision.h b/revision.h
index 8572315..e3559d0 100644
--- a/revision.h
+++ b/revision.h
@@ -74,7 +74,7 @@ struct rev_info {
 	struct log_info *loginfo;
 	int		nr, total;
 	const char	*mime_boundary;
-	const char	*message_id;
+	char		*message_id;
 	const char	*ref_message_id;
 	const char	*add_signoff;
 	const char	*extra_headers;
-- 
1.5.4.1.191.gfbf10

[PATCH 3/5] Export some email and pretty-printing functions

[Daniel Barkalow]

These will be used for generating the cover letter in addition to the
patch emails.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 commit.h   |   15 +++++++
 log-tree.c |  126 ++++++++++++++++++++++++++++++++---------------------------
 log-tree.h |    2 +
 pretty.c   |   34 ++++++++--------
 4 files changed, 102 insertions(+), 75 deletions(-)

diff --git a/commit.h b/commit.h
index 10e2b5d..80d65b9 100644
--- a/commit.h
+++ b/commit.h
@@ -71,6 +71,21 @@ extern void pretty_print_commit(enum cmit_fmt fmt, const struct commit*,
                                 int abbrev, const char *subject,
                                 const char *after_subject, enum date_mode,
 				int non_ascii_present);
+void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
+		   const char *line, enum date_mode dmode,
+		   const char *encoding);
+void pp_title_line(enum cmit_fmt fmt,
+		   const char **msg_p,
+		   struct strbuf *sb,
+		   const char *subject,
+		   const char *after_subject,
+		   const char *encoding,
+		   int plain_non_ascii);
+void pp_remainder(enum cmit_fmt fmt,
+		  const char **msg_p,
+		  struct strbuf *sb,
+		  int indent);
+
 
 /** Removes the first commit from a list sorted by date, and adds all
  * of its parents.
diff --git a/log-tree.c b/log-tree.c
index 1f3fcf1..1b084dc 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -137,6 +137,72 @@ static int has_non_ascii(const char *s)
 	return 0;
 }
 
+void log_write_email_headers(struct rev_info *opt, const char *name,
+			     const char **subject_p, const char **extra_headers_p)
+{
+	const char *subject = NULL;
+	const char *extra_headers = opt->extra_headers;
+	if (opt->total > 0) {
+		static char buffer[64];
+		snprintf(buffer, sizeof(buffer),
+			 "Subject: [%s %0*d/%d] ",
+			 opt->subject_prefix,
+			 digits_in_number(opt->total),
+			 opt->nr, opt->total);
+		subject = buffer;
+	} else if (opt->total == 0 && opt->subject_prefix && *opt->subject_prefix) {
+		static char buffer[256];
+		snprintf(buffer, sizeof(buffer),
+			 "Subject: [%s] ",
+			 opt->subject_prefix);
+		subject = buffer;
+	} else {
+		subject = "Subject: ";
+	}
+
+	printf("From %s Mon Sep 17 00:00:00 2001\n", name);
+	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];
+		snprintf(subject_buffer, sizeof(subject_buffer) - 1,
+			 "%s"
+			 "MIME-Version: 1.0\n"
+			 "Content-Type: multipart/mixed;"
+			 " boundary=\"%s%s\"\n"
+			 "\n"
+			 "This is a multi-part message in MIME "
+			 "format.\n"
+			 "--%s%s\n"
+			 "Content-Type: text/plain; "
+			 "charset=UTF-8; format=fixed\n"
+			 "Content-Transfer-Encoding: 8bit\n\n",
+			 extra_headers ? extra_headers : "",
+			 mime_boundary_leader, opt->mime_boundary,
+			 mime_boundary_leader, opt->mime_boundary);
+		extra_headers = subject_buffer;
+
+		snprintf(buffer, sizeof(buffer) - 1,
+			 "--%s%s\n"
+			 "Content-Type: text/x-patch;"
+			 " name=\"%s.diff\"\n"
+			 "Content-Transfer-Encoding: 8bit\n"
+			 "Content-Disposition: %s;"
+			 " filename=\"%s.diff\"\n\n",
+			 mime_boundary_leader, opt->mime_boundary,
+			 name,
+			 opt->no_inline ? "attachment" : "inline",
+			 name);
+		opt->diffopt.stat_sep = buffer;
+	}
+	*subject_p = subject;
+	*extra_headers_p = extra_headers;
+}
+
 void show_log(struct rev_info *opt, const char *sep)
 {
 	struct strbuf msgbuf;
@@ -186,64 +252,8 @@ void show_log(struct rev_info *opt, const char *sep)
 	 */
 
 	if (opt->commit_format == CMIT_FMT_EMAIL) {
-		char *sha1 = sha1_to_hex(commit->object.sha1);
-		if (opt->total > 0) {
-			static char buffer[64];
-			snprintf(buffer, sizeof(buffer),
-					"Subject: [%s %0*d/%d] ",
-					opt->subject_prefix,
-					digits_in_number(opt->total),
-					opt->nr, opt->total);
-			subject = buffer;
-		} else if (opt->total == 0 && opt->subject_prefix && *opt->subject_prefix) {
-			static char buffer[256];
-			snprintf(buffer, sizeof(buffer),
-					"Subject: [%s] ",
-					opt->subject_prefix);
-			subject = buffer;
-		} else {
-			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];
-			snprintf(subject_buffer, sizeof(subject_buffer) - 1,
-				 "%s"
-				 "MIME-Version: 1.0\n"
-				 "Content-Type: multipart/mixed;"
-				 " boundary=\"%s%s\"\n"
-				 "\n"
-				 "This is a multi-part message in MIME "
-				 "format.\n"
-				 "--%s%s\n"
-				 "Content-Type: text/plain; "
-				 "charset=UTF-8; format=fixed\n"
-				 "Content-Transfer-Encoding: 8bit\n\n",
-				 extra_headers ? extra_headers : "",
-				 mime_boundary_leader, opt->mime_boundary,
-				 mime_boundary_leader, opt->mime_boundary);
-			extra_headers = subject_buffer;
-
-			snprintf(buffer, sizeof(buffer) - 1,
-				 "--%s%s\n"
-				 "Content-Type: text/x-patch;"
-				 " name=\"%s.diff\"\n"
-				 "Content-Transfer-Encoding: 8bit\n"
-				 "Content-Disposition: %s;"
-				 " filename=\"%s.diff\"\n\n",
-				 mime_boundary_leader, opt->mime_boundary,
-				 sha1,
-				 opt->no_inline ? "attachment" : "inline",
-				 sha1);
-			opt->diffopt.stat_sep = buffer;
-		}
+		log_write_email_headers(opt, sha1_to_hex(commit->object.sha1),
+					&subject, &extra_headers);
 	} else if (opt->commit_format != CMIT_FMT_USERFORMAT) {
 		fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), stdout);
 		if (opt->commit_format != CMIT_FMT_ONELINE)
diff --git a/log-tree.h b/log-tree.h
index b33f7cd..0cc9344 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -13,5 +13,7 @@ int log_tree_commit(struct rev_info *, struct commit *);
 int log_tree_opt_parse(struct rev_info *, const char **, int);
 void show_log(struct rev_info *opt, const char *sep);
 void show_decorations(struct commit *commit);
+void log_write_email_headers(struct rev_info *opt, const char *name,
+			     const char **subject_p, const char **extra_headers_p);
 
 #endif
diff --git a/pretty.c b/pretty.c
index b987ff2..d5db1bd 100644
--- a/pretty.c
+++ b/pretty.c
@@ -110,9 +110,9 @@ needquote:
 	strbuf_addstr(sb, "?=");
 }
 
-static void add_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
-			 const char *line, enum date_mode dmode,
-			 const char *encoding)
+void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
+		  const char *line, enum date_mode dmode,
+		  const char *encoding)
 {
 	char *date;
 	int namelen;
@@ -295,7 +295,7 @@ static void format_person_part(struct strbuf *sb, char part,
 	/*
 	 * If it does not even have a '<' and '>', that is
 	 * quite a bogus commit author and we discard it;
-	 * this is in line with add_user_info() that is used
+	 * this is in line with pp_user_info() that is used
 	 * in the normal codepath.  When end points at the '<'
 	 * that we found, it should have matching '>' later,
 	 * which means start (beginning of email address) must
@@ -643,23 +643,23 @@ static void pp_header(enum cmit_fmt fmt,
 		 */
 		if (!memcmp(line, "author ", 7)) {
 			strbuf_grow(sb, linelen + 80);
-			add_user_info("Author", fmt, sb, line + 7, dmode, encoding);
+			pp_user_info("Author", fmt, sb, line + 7, dmode, encoding);
 		}
 		if (!memcmp(line, "committer ", 10) &&
 		    (fmt == CMIT_FMT_FULL || fmt == CMIT_FMT_FULLER)) {
 			strbuf_grow(sb, linelen + 80);
-			add_user_info("Commit", fmt, sb, line + 10, dmode, encoding);
+			pp_user_info("Commit", fmt, sb, line + 10, dmode, encoding);
 		}
 	}
 }
 
-static void pp_title_line(enum cmit_fmt fmt,
-			  const char **msg_p,
-			  struct strbuf *sb,
-			  const char *subject,
-			  const char *after_subject,
-			  const char *encoding,
-			  int plain_non_ascii)
+void pp_title_line(enum cmit_fmt fmt,
+		   const char **msg_p,
+		   struct strbuf *sb,
+		   const char *subject,
+		   const char *after_subject,
+		   const char *encoding,
+		   int plain_non_ascii)
 {
 	struct strbuf title;
 
@@ -708,10 +708,10 @@ static void pp_title_line(enum cmit_fmt fmt,
 	strbuf_release(&title);
 }
 
-static void pp_remainder(enum cmit_fmt fmt,
-			 const char **msg_p,
-			 struct strbuf *sb,
-			 int indent)
+void pp_remainder(enum cmit_fmt fmt,
+		  const char **msg_p,
+		  struct strbuf *sb,
+		  int indent)
 {
 	int first = 1;
 	for (;;) {
-- 
1.5.4.1.191.gfbf10

[PATCH 4/5] Retain extra blank lines between the summary and the body

[Daniel Barkalow]

This is so that it's possible to have a message that will generate an
email with the first line blank. If the email you're generating is
actually a template, you may have a single line filling in for a
~3-line paragraph, meaning that the space starts with a blank line,
which was impossible before as output from the message-splitting code.

Furthermore, extra blank lines that aren't at the beginning of the
body portion aren't stripped, so this is more consistant overall.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 pretty.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/pretty.c b/pretty.c
index d5db1bd..1c57920 100644
--- a/pretty.c
+++ b/pretty.c
@@ -713,7 +713,6 @@ void pp_remainder(enum cmit_fmt fmt,
 		  struct strbuf *sb,
 		  int indent)
 {
-	int first = 1;
 	for (;;) {
 		const char *line = *msg_p;
 		int linelen = get_one_line(line);
@@ -723,12 +722,9 @@ void pp_remainder(enum cmit_fmt fmt,
 			break;
 
 		if (is_empty_line(line, &linelen)) {
-			if (first)
-				continue;
 			if (fmt == CMIT_FMT_SHORT)
 				break;
 		}
-		first = 0;
 
 		strbuf_grow(sb, linelen + indent + 20);
 		if (indent) {
-- 
1.5.4.1.191.gfbf10

[PATCH 5/5] Add a --cover-letter option to format-patch

[Daniel Barkalow]

If --cover-letter is provided, generate a cover letter message before
the patches, numbered 0.

Original patch thanks to Johannes Schindelin

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 Documentation/git-format-patch.txt                 |   20 +-
 builtin-log.c                                      |  232 +++++++++++++++-----
 t/t4013-diff-various.sh                            |    1 +
 ...tch_--stdout_--cover-letter_-n_initial..master^ |  101 +++++++++
 t/t4014-format-patch.sh                            |   26 +++
 5 files changed, 315 insertions(+), 65 deletions(-)
 create mode 100644 t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 651efe6..b27bb94 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -10,13 +10,14 @@ SYNOPSIS
 --------
 [verse]
 'git-format-patch' [-k] [-o <dir> | --stdout] [--thread]
-                   [--attach[=<boundary>] | --inline[=<boundary>]]
-                   [-s | --signoff] [<common diff options>]
-                   [-n | --numbered | -N | --no-numbered]
-                   [--start-number <n>] [--numbered-files]
-                   [--in-reply-to=Message-Id] [--suffix=.<sfx>]
-                   [--ignore-if-in-upstream]
-                   [--subject-prefix=Subject-Prefix]
+		   [--attach[=<boundary>] | --inline[=<boundary>]]
+		   [-s | --signoff] [<common diff options>]
+		   [-n | --numbered | -N | --no-numbered]
+		   [--start-number <n>] [--numbered-files]
+		   [--in-reply-to=Message-Id] [--suffix=.<sfx>]
+		   [--ignore-if-in-upstream]
+		   [--subject-prefix=Subject-Prefix]
+		   [--cover-letter]
 		   [ <since> | <revision range> ]
 
 DESCRIPTION
@@ -135,6 +136,11 @@ include::diff-options.txt[]
 	allows for useful naming of a patch series, and can be
 	combined with the --numbered option.
 
+--cover-letter::
+	Generate a cover letter template.  You still have to fill in
+	a description, but the shortlog and the diffstat will be
+	generated for you.
+
 --suffix=.<sfx>::
 	Instead of using `.patch` as the suffix for generated
 	filenames, use specified suffix.  A common alternative is
diff --git a/builtin-log.c b/builtin-log.c
index 4f08ca4..a67b463 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -14,6 +14,7 @@
 #include "reflog-walk.h"
 #include "patch-ids.h"
 #include "refs.h"
+#include "run-command.h"
 
 static int default_show_root = 1;
 static const char *fmt_patch_subject_prefix = "PATCH";
@@ -452,74 +453,81 @@ static int git_format_config(const char *var, const char *value)
 }
 
 
+static const char *get_oneline_for_filename(struct commit *commit,
+					    int keep_subject)
+{
+	static char filename[PATH_MAX];
+	char *sol;
+	int len = 0;
+	int suffix_len = strlen(fmt_patch_suffix) + 1;
+
+	sol = strstr(commit->buffer, "\n\n");
+	if (!sol)
+		filename[0] = '\0';
+	else {
+		int j, space = 0;
+
+		sol += 2;
+		/* strip [PATCH] or [PATCH blabla] */
+		if (!keep_subject && !prefixcmp(sol, "[PATCH")) {
+			char *eos = strchr(sol + 6, ']');
+			if (eos) {
+				while (isspace(*eos))
+					eos++;
+				sol = eos;
+			}
+		}
+
+		for (j = 0;
+		     j < FORMAT_PATCH_NAME_MAX - suffix_len - 5 &&
+			     len < sizeof(filename) - suffix_len &&
+			     sol[j] && sol[j] != '\n';
+		     j++) {
+			if (istitlechar(sol[j])) {
+				if (space) {
+					filename[len++] = '-';
+					space = 0;
+				}
+				filename[len++] = sol[j];
+				if (sol[j] == '.')
+					while (sol[j + 1] == '.')
+						j++;
+			} else
+				space = 1;
+		}
+		while (filename[len - 1] == '.'
+		       || filename[len - 1] == '-')
+			len--;
+		filename[len] = '\0';
+	}
+	return filename;
+}
+
 static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
 
-static int reopen_stdout(struct commit *commit, int nr, int keep_subject,
-			 int numbered_files)
+static int reopen_stdout(const char *oneline, int nr, int total)
 {
 	char filename[PATH_MAX];
-	char *sol;
 	int len = 0;
 	int suffix_len = strlen(fmt_patch_suffix) + 1;
 
 	if (output_directory) {
-		if (strlen(output_directory) >=
+		len = snprintf(filename, sizeof(filename), "%s",
+				output_directory);
+		if (len >=
 		    sizeof(filename) - FORMAT_PATCH_NAME_MAX - suffix_len)
 			return error("name of output directory is too long");
-		strlcpy(filename, output_directory, sizeof(filename) - suffix_len);
-		len = strlen(filename);
 		if (filename[len - 1] != '/')
 			filename[len++] = '/';
 	}
 
-	if (numbered_files) {
-		sprintf(filename + len, "%d", nr);
-		len = strlen(filename);
-
-	} else {
-		sprintf(filename + len, "%04d", nr);
-		len = strlen(filename);
-
-		sol = strstr(commit->buffer, "\n\n");
-		if (sol) {
-			int j, space = 1;
-
-			sol += 2;
-			/* strip [PATCH] or [PATCH blabla] */
-			if (!keep_subject && !prefixcmp(sol, "[PATCH")) {
-				char *eos = strchr(sol + 6, ']');
-				if (eos) {
-					while (isspace(*eos))
-						eos++;
-					sol = eos;
-				}
-			}
-
-			for (j = 0;
-			     j < FORMAT_PATCH_NAME_MAX - suffix_len - 5 &&
-				     len < sizeof(filename) - suffix_len &&
-				     sol[j] && sol[j] != '\n';
-			     j++) {
-				if (istitlechar(sol[j])) {
-					if (space) {
-						filename[len++] = '-';
-						space = 0;
-					}
-					filename[len++] = sol[j];
-					if (sol[j] == '.')
-						while (sol[j + 1] == '.')
-							j++;
-				} else
-					space = 1;
-			}
-			while (filename[len - 1] == '.'
-			       || filename[len - 1] == '-')
-				len--;
-			filename[len] = 0;
-		}
-		if (len + suffix_len >= sizeof(filename))
-			return error("Patch pathname too long");
+	if (!oneline)
+		len += sprintf(filename + len, "%d", nr);
+	else {
+		len += sprintf(filename + len, "%04d-", nr);
+		len += snprintf(filename + len, sizeof(filename) - len - 1
+				- suffix_len, "%s", oneline);
 		strcpy(filename + len, fmt_patch_suffix);
 	}
 
@@ -590,6 +598,76 @@ static void gen_message_id(struct rev_info *info, char *base)
 	info->message_id = strbuf_detach(&buf, NULL);
 }
 
+static void make_cover_letter(struct rev_info *rev,
+		int use_stdout, int numbered, int numbered_files,
+			      struct commit *origin, struct commit *head)
+{
+	const char *committer;
+	const char *origin_sha1, *head_sha1;
+	const char *argv[7];
+	const char *subject_start = NULL;
+	const char *body = "*** SUBJECT HERE ***\n\n\n*** BLURB HERE ***\n";
+	const char *msg;
+	const char *extra_headers = rev->extra_headers;
+	struct strbuf sb;
+	const char *encoding = "utf-8";
+
+	if (rev->commit_format != CMIT_FMT_EMAIL)
+		die("Cover letter needs email format");
+
+	if (!use_stdout && reopen_stdout(numbered_files ?
+				NULL : "cover-letter", 0, rev->total))
+		return;
+
+	origin_sha1 = sha1_to_hex(origin ? origin->object.sha1 : null_sha1);
+	head_sha1 = sha1_to_hex(head->object.sha1);
+
+	log_write_email_headers(rev, head_sha1, &subject_start, &extra_headers);
+
+	committer = git_committer_info(0);
+
+	msg = body;
+	strbuf_init(&sb, 0);
+	pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,
+		     encoding);
+	pp_title_line(CMIT_FMT_EMAIL, &msg, &sb, subject_start, extra_headers,
+		      encoding, 0);
+	pp_remainder(CMIT_FMT_EMAIL, &msg, &sb, 0);
+	printf("%s\n", sb.buf);
+
+	strbuf_release(&sb);
+
+	/*
+	 * We can only do diffstat with a unique reference point, and
+	 * log is a bit tricky, so just skip it.
+	 */
+	if (!origin)
+		return;
+
+	argv[0] = "shortlog";
+	argv[1] = head_sha1;
+	argv[2] = "--not";
+	argv[3] = origin_sha1;
+	argv[4] = "--";
+	argv[5] = NULL;
+	fflush(stdout);
+	run_command_v_opt(argv, RUN_GIT_CMD);
+
+	argv[0] = "diff";
+	argv[1] = "--stat";
+	argv[2] = "--summary";
+	argv[3] = head_sha1;
+	argv[4] = "--not";
+	argv[5] = origin_sha1;
+	argv[6] = "--";
+	argv[7] = NULL;
+	fflush(stdout);
+	run_command_v_opt(argv, RUN_GIT_CMD);
+
+	fflush(stdout);
+	printf("\n");
+}
+
 static const char *clean_message_id(const char *msg_id)
 {
 	char ch;
@@ -625,6 +703,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	int subject_prefix = 0;
 	int ignore_if_in_upstream = 0;
 	int thread = 0;
+	int cover_letter = 0;
+	struct commit *origin = NULL, *head = NULL;
 	const char *in_reply_to = NULL;
 	struct patch_ids ids;
 	char *add_signoff = NULL;
@@ -724,6 +804,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			rev.subject_prefix = argv[i] + 17;
 		} else if (!prefixcmp(argv[i], "--suffix="))
 			fmt_patch_suffix = argv[i] + 9;
+		else if (!strcmp(argv[i], "--cover-letter"))
+			cover_letter = 1;
 		else
 			argv[j++] = argv[i];
 	}
@@ -775,6 +857,25 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		 * get_revision() to do the usual traversal.
 		 */
 	}
+	if (cover_letter) {
+		/* remember the range */
+		int negative_count = 0;
+		int i;
+		for (i = 0; i < rev.pending.nr; i++) {
+			struct object *o = rev.pending.objects[i].item;
+			if (o->flags & UNINTERESTING) {
+				origin = (struct commit *)o;
+				negative_count++;
+			} else
+				head = (struct commit *)o;
+		}
+		/* Multiple origins don't work for diffstat. */
+		if (negative_count > 1)
+			origin = NULL;
+		/* We can't generate a cover letter without any patches */
+		if (!head)
+			return 0;
+	}
 
 	if (ignore_if_in_upstream)
 		get_patch_ids(&rev, &ids, prefix);
@@ -801,16 +902,31 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		numbered = 1;
 	if (numbered)
 		rev.total = total + start_number - 1;
-	rev.add_signoff = add_signoff;
 	if (in_reply_to)
 		rev.ref_message_id = clean_message_id(in_reply_to);
+	if (cover_letter) {
+		if (thread)
+			gen_message_id(&rev, "cover");
+		make_cover_letter(&rev, use_stdout, numbered, numbered_files,
+				  origin, head);
+		total++;
+		start_number--;
+	}
+	rev.add_signoff = add_signoff;
 	while (0 <= --nr) {
 		int shown;
 		commit = list[nr];
 		rev.nr = total - nr + (start_number - 1);
 		/* Make the second and subsequent mails replies to the first */
 		if (thread) {
+			/* Have we already had a message ID? */
 			if (rev.message_id) {
+				/*
+				 * If we've got the ID to be a reply
+				 * to, discard the current ID;
+				 * otherwise, make everything a reply
+				 * to that.
+				 */
 				if (rev.ref_message_id)
 					free(rev.message_id);
 				else
@@ -818,10 +934,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			}
 			gen_message_id(&rev, sha1_to_hex(commit->object.sha1));
 		}
-		if (!use_stdout)
-			if (reopen_stdout(commit, rev.nr, keep_subject,
-					  numbered_files))
-				die("Failed to create output files");
+		if (!use_stdout && reopen_stdout(numbered_files ? NULL :
+				get_oneline_for_filename(commit, keep_subject),
+				rev.nr, rev.total))
+			die("Failed to create output files");
 		shown = log_tree_commit(&rev, commit);
 		free(commit->buffer);
 		commit->buffer = NULL;
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 9eec754..6b4d1c5 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -245,6 +245,7 @@ format-patch --inline --stdout initial..master
 format-patch --inline --stdout --subject-prefix=TESTCASE initial..master
 config format.subjectprefix DIFFERENT_PREFIX
 format-patch --inline --stdout initial..master^^
+format-patch --stdout --cover-letter -n initial..master^
 
 diff --abbrev initial..side
 diff -r initial..side
diff --git a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
new file mode 100644
index 0000000..311e207
--- /dev/null
+++ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
@@ -0,0 +1,101 @@
+$ git format-patch --stdout --cover-letter -n initial..master^
+From 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 Mon Sep 17 00:00:00 2001
+From: C O Mitter <committer@example.com>
+Date: Mon, 26 Jun 2006 00:05:00 +0000
+Subject: [DIFFERENT_PREFIX 0/2] *** SUBJECT HERE ***
+
+
+*** BLURB HERE ***
+
+A U Thor (2):
+      Second
+      Third
+
+ dir/sub |    4 ++++
+ file0   |    3 +++
+ file1   |    3 +++
+ file2   |    3 ---
+ 4 files changed, 10 insertions(+), 3 deletions(-)
+ create mode 100644 file1
+ delete mode 100644 file2
+
+From 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 Mon Sep 17 00:00:00 2001
+From: A U Thor <author@example.com>
+Date: Mon, 26 Jun 2006 00:01:00 +0000
+Subject: [DIFFERENT_PREFIX 1/2] Second
+
+This is the second commit.
+---
+ dir/sub |    2 ++
+ file0   |    3 +++
+ file2   |    3 ---
+ 3 files changed, 5 insertions(+), 3 deletions(-)
+ delete mode 100644 file2
+
+diff --git a/dir/sub b/dir/sub
+index 35d242b..8422d40 100644
+--- a/dir/sub
++++ b/dir/sub
+@@ -1,2 +1,4 @@
+ A
+ B
++C
++D
+diff --git a/file0 b/file0
+index 01e79c3..b414108 100644
+--- a/file0
++++ b/file0
+@@ -1,3 +1,6 @@
+ 1
+ 2
+ 3
++4
++5
++6
+diff --git a/file2 b/file2
+deleted file mode 100644
+index 01e79c3..0000000
+--- a/file2
++++ /dev/null
+@@ -1,3 +0,0 @@
+-1
+-2
+-3
+-- 
+g-i-t--v-e-r-s-i-o-n
+
+
+From 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 Mon Sep 17 00:00:00 2001
+From: A U Thor <author@example.com>
+Date: Mon, 26 Jun 2006 00:02:00 +0000
+Subject: [DIFFERENT_PREFIX 2/2] Third
+
+---
+ dir/sub |    2 ++
+ file1   |    3 +++
+ 2 files changed, 5 insertions(+), 0 deletions(-)
+ create mode 100644 file1
+
+diff --git a/dir/sub b/dir/sub
+index 8422d40..cead32e 100644
+--- a/dir/sub
++++ b/dir/sub
+@@ -2,3 +2,5 @@ A
+ B
+ C
+ D
++E
++F
+diff --git a/file1 b/file1
+new file mode 100644
+index 0000000..b1e6722
+--- /dev/null
++++ b/file1
+@@ -0,0 +1,3 @@
++A
++B
++C
+-- 
+g-i-t--v-e-r-s-i-o-n
+
+$
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 6e8b5f4..ac78752 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -122,6 +122,32 @@ test_expect_success 'thread in-reply-to' '
 	done
 '
 
+test_expect_success 'thread cover-letter' '
+
+	rm -rf patches/ &&
+	git checkout side &&
+	git format-patch --cover-letter --thread -o patches/ master &&
+	FIRST_MID=$(grep "Message-Id:" patches/0000-* | sed "s/^[^<]*\(<[^>]*>\).*$/\1/") &&
+	for i in patches/0001-* patches/0002-* patches/0003-* 
+	do
+	  grep "References: $FIRST_MID" $i &&
+	  grep "In-Reply-To: $FIRST_MID" $i
+	done
+'
+
+test_expect_success 'thread cover-letter in-reply-to' '
+
+	rm -rf patches/ &&
+	git checkout side &&
+	git format-patch --cover-letter --in-reply-to="<test.message>" --thread -o patches/ master &&
+	FIRST_MID="<test.message>" &&
+	for i in patches/*
+	do
+	  grep "References: $FIRST_MID" $i &&
+	  grep "In-Reply-To: $FIRST_MID" $i
+	done
+'
+
 test_expect_success 'excessive subject' '
 
 	rm -rf patches/ &&
-- 
1.5.4.1.191.gfbf10

Re: [PATCH 2/5] Improve message-id generation flow control for format-patch

[Johannes Schindelin]

Hi,

On Mon, 18 Feb 2008, Daniel Barkalow wrote:

> diff --git a/revision.h b/revision.h
> index 8572315..e3559d0 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -74,7 +74,7 @@ struct rev_info {
>  	struct log_info *loginfo;
>  	int		nr, total;
>  	const char	*mime_boundary;
> -	const char	*message_id;
> +	char		*message_id;
>  	const char	*ref_message_id;
>  	const char	*add_signoff;
>  	const char	*extra_headers;

Mini-nit: technically, ref_message_id should be "char *", too.

Ciao,
Dscho

Re: [PATCH 3/5] Export some email and pretty-printing functions

[Johannes Schindelin]

Hi,

On Mon, 18 Feb 2008, Daniel Barkalow wrote:

> These will be used for generating the cover letter in addition to the 
> patch emails.

I copied the hunks of log_write_email_headers() and used "git diff -w" to 
verify that only one thing changed: "sha1" was renamed to "name".  It 
might make sense to rename it to "from", but then, I do not really care.

Ciao,
Dscho

Re: [PATCH 3/5] Export some email and pretty-printing functions

[Daniel Barkalow]

On Tue, 19 Feb 2008, Johannes Schindelin wrote:

> Hi,
> 
> On Mon, 18 Feb 2008, Daniel Barkalow wrote:
> 
> > These will be used for generating the cover letter in addition to the 
> > patch emails.
> 
> I copied the hunks of log_write_email_headers() and used "git diff -w" to 
> verify that only one thing changed: "sha1" was renamed to "name".  It 
> might make sense to rename it to "from", but then, I do not really care.

No, it's actually supposed to be the filename, I think. Beats me as to why 
it's used as the envelope from, but I'm not changing the logic there.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 2/5] Improve message-id generation flow control for format-patch

[Daniel Barkalow]

On Tue, 19 Feb 2008, Johannes Schindelin wrote:

> Hi,
> 
> On Mon, 18 Feb 2008, Daniel Barkalow wrote:
> 
> > diff --git a/revision.h b/revision.h
> > index 8572315..e3559d0 100644
> > --- a/revision.h
> > +++ b/revision.h
> > @@ -74,7 +74,7 @@ struct rev_info {
> >  	struct log_info *loginfo;
> >  	int		nr, total;
> >  	const char	*mime_boundary;
> > -	const char	*message_id;
> > +	char		*message_id;
> >  	const char	*ref_message_id;
> >  	const char	*add_signoff;
> >  	const char	*extra_headers;
> 
> Mini-nit: technically, ref_message_id should be "char *", too.

I'd intended ref_message_id to just reference the string owned by a 
different message's message_id, but that doesn't really work with struct 
rev_info. I bet you'd like me to not leak memory out through 
ref_message_id, too. I think I want to see how the code to collect the set 
of commits in the series (for more capable generation of cover letter 
contents) goes before changing this either way.

(It's incidentally kind of amusing that you managed to convey the way you 
thought the code should be designed, different from how I'd designed it, 
through a note on whether a struct field should be const or not.)

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 2/5] Improve message-id generation flow control for format-patch

[Johannes Schindelin]

Hi,

On Tue, 19 Feb 2008, Daniel Barkalow wrote:

> On Tue, 19 Feb 2008, Johannes Schindelin wrote:
> 
> > On Mon, 18 Feb 2008, Daniel Barkalow wrote:
> > 
> > > diff --git a/revision.h b/revision.h
> > > index 8572315..e3559d0 100644
> > > --- a/revision.h
> > > +++ b/revision.h
> > > @@ -74,7 +74,7 @@ struct rev_info {
> > >  	struct log_info *loginfo;
> > >  	int		nr, total;
> > >  	const char	*mime_boundary;
> > > -	const char	*message_id;
> > > +	char		*message_id;
> > >  	const char	*ref_message_id;
> > >  	const char	*add_signoff;
> > >  	const char	*extra_headers;
> > 
> > Mini-nit: technically, ref_message_id should be "char *", too.
> 
> I'd intended ref_message_id to just reference the string owned by a 
> different message's message_id, but that doesn't really work with struct 
> rev_info. I bet you'd like me to not leak memory out through 
> ref_message_id, too. I think I want to see how the code to collect the 
> set of commits in the series (for more capable generation of cover 
> letter contents) goes before changing this either way.
> 
> (It's incidentally kind of amusing that you managed to convey the way 
> you thought the code should be designed, different from how I'd designed 
> it, through a note on whether a struct field should be const or not.)

Hehe!

You're right in that the revision machinery needs some cleaning up.  AFAIK 
there is no method yet to release the memory allocated during one revision 
walk (including the preparation).

And you're right that this should be done separately, so please kindly 
accept my apologies while I retract my comment.

Ciao,
Dscho

Re: [PATCH 5/5] Add a --cover-letter option to format-patch

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> +	const char *body = "*** SUBJECT HERE ***\n\n\n*** BLURB HERE ***\n";
> ...
> diff --git a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
> new file mode 100644
> index 0000000..311e207
> --- /dev/null
> +++ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
> @@ -0,0 +1,101 @@
> +$ git format-patch --stdout --cover-letter -n initial..master^
> +From 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 Mon Sep 17 00:00:00 2001
> +From: C O Mitter <committer@example.com>
> +Date: Mon, 26 Jun 2006 00:05:00 +0000
> +Subject: [DIFFERENT_PREFIX 0/2] *** SUBJECT HERE ***
> +
> +
> +*** BLURB HERE ***
> +
> +A U Thor (2):
> +      Second
> +      Third

I still disagree with your earlier point on the extra blank line
here.

When we give commit log template, we do leave an extra blank line
before we start the template material "# Please enter ...", to
ease typing, but it is different.  The template material does
not have to be removed by the end-user, so giving a blank line
upfront and starting the editor at that line is a usability
improvement, compared to the case where there is none.

But this "*** BLURB HERE ***" needs to be removed when the user
edits the cover-letter, and I really do not see any reason to
have an extra blank line in front of it.

Incidentally, I strongly suspect that if you do not have that
extra blank line, we would not even need to have the [PATCH 4/5]
(which is a slight regression) in the series.

So how about this squashed in, with [PATCH 4/5] skipped?


 builtin-log.c                                      |    2 +-
 ...tch_--stdout_--cover-letter_-n_initial..master^ |    1 -
 2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index e26c986..0b348eb 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -629,7 +629,7 @@ static void make_cover_letter(struct rev_info *rev,
 	const char *origin_sha1, *head_sha1;
 	const char *argv[7];
 	const char *subject_start = NULL;
-	const char *body = "*** SUBJECT HERE ***\n\n\n*** BLURB HERE ***\n";
+	const char *body = "*** SUBJECT HERE ***\n\n*** BLURB HERE ***\n";
 	const char *msg;
 	const char *extra_headers = rev->extra_headers;
 	struct strbuf sb;
diff --git a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
index 311e207..0151453 100644
--- a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
+++ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
@@ -4,7 +4,6 @@ From: C O Mitter <committer@example.com>
 Date: Mon, 26 Jun 2006 00:05:00 +0000
 Subject: [DIFFERENT_PREFIX 0/2] *** SUBJECT HERE ***
 
-
 *** BLURB HERE ***
 
 A U Thor (2):

Re: [PATCH 5/5] Add a --cover-letter option to format-patch

[Daniel Barkalow]

On Tue, 19 Feb 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > +	const char *body = "*** SUBJECT HERE ***\n\n\n*** BLURB HERE ***\n";
> > ...
> > diff --git a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
> > new file mode 100644
> > index 0000000..311e207
> > --- /dev/null
> > +++ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
> > @@ -0,0 +1,101 @@
> > +$ git format-patch --stdout --cover-letter -n initial..master^
> > +From 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 Mon Sep 17 00:00:00 2001
> > +From: C O Mitter <committer@example.com>
> > +Date: Mon, 26 Jun 2006 00:05:00 +0000
> > +Subject: [DIFFERENT_PREFIX 0/2] *** SUBJECT HERE ***
> > +
> > +
> > +*** BLURB HERE ***
> > +
> > +A U Thor (2):
> > +      Second
> > +      Third
> 
> I still disagree with your earlier point on the extra blank line
> here.
> 
> When we give commit log template, we do leave an extra blank line
> before we start the template material "# Please enter ...", to
> ease typing, but it is different.  The template material does
> not have to be removed by the end-user, so giving a blank line
> upfront and starting the editor at that line is a usability
> improvement, compared to the case where there is none.
> 
> But this "*** BLURB HERE ***" needs to be removed when the user
> edits the cover-letter, and I really do not see any reason to
> have an extra blank line in front of it.
> 
> Incidentally, I strongly suspect that if you do not have that
> extra blank line, we would not even need to have the [PATCH 4/5]
> (which is a slight regression) in the series.
> 
> So how about this squashed in, with [PATCH 4/5] skipped?

Sure. I still like the extra line, but it's not that big a deal. (And I 
still think it's odd that the code removes extra blank lines from that 
spot but not elsewhere, but likewise not a big deal)

	-Daniel
*This .sig left intentionally blank*