[PATCHv2 0/3] format-patch --attach/--inline use filename instead of SHA1

[PATCHv2 0/3] format-patch --attach/--inline use filename instead of SHA1

[Stephen Boyd]

I'm resending this series because it seems it wasn't picked up probably because
the patches were mangled by my mailer.

This patch series modifies the behavior of format-patch when used with the
--attach or --inline command line settings. Current behavior names the attached
or inlined patches with the SHA1 of the commit, which isn't very friendly or
easy for a human to use when downloading the attachments. This series replaces
the SHA1 values with the filename used by format-patch when it outputs the
patches to files.

Stephen Boyd (3):
  format-patch: create patch filename in one function
  format-patch: --attach/inline uses filename instead of SHA1
  format-patch: --numbered-files and --stdout aren't mutually exclusive

 Documentation/git-format-patch.txt                 |    1 -
 builtin-log.c                                      |   51 ++++++++--------
 log-tree.c                                         |    8 +-
 revision.h                                         |    1 +
 t/t4013-diff-various.sh                            |    1 +
 ..._--attach_--stdout_--suffix=.diff_initial..side |   61 ++++++++++++++++++++
 ....format-patch_--attach_--stdout_initial..master |   12 ++--
 ...format-patch_--attach_--stdout_initial..master^ |    8 +-
 ...ff.format-patch_--attach_--stdout_initial..side |    4 +-
 ...tdout_--subject-prefix=TESTCASE_initial..master |   12 ++--
 ....format-patch_--inline_--stdout_initial..master |   12 ++--
 ...format-patch_--inline_--stdout_initial..master^ |    8 +-
 ...ormat-patch_--inline_--stdout_initial..master^^ |    4 +-
 ...ff.format-patch_--inline_--stdout_initial..side |    4 +-
 14 files changed, 124 insertions(+), 63 deletions(-)
 create mode 100644 t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side

[PATCHv2 1/3] format-patch: create patch filename in one function

[Stephen Boyd]

reopen_stdout() usually takes the oneline description of a commit,
appends the patch suffix, prepends the output directory (if any) and
then reopens stdout as the resulting file. Now the patch filename (the
oneline description and the patch suffix) is created in
get_patch_filename() and passed to reopen_stdout() which prepends the
output directory and reopens stdout as that file.

The original function to get the oneline description,
get_oneline_for_filename(), has been renamed to get_patch_filename() to
reflect its new functionality.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 builtin-log.c |   49 +++++++++++++++++++++++++------------------------
 1 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 8af55d2..cc6d663 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -519,21 +519,17 @@ static int git_format_config(const char *var, const char *value, void *cb)
 }
 
 
-static const char *get_oneline_for_filename(struct commit *commit,
-					    int keep_subject)
+static const char *get_patch_filename(char* sol, int keep_subject, int nr)
 {
 	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 {
+	if (sol)
+	{
 		int j, space = 0;
 
-		sol += 2;
+		len += sprintf(filename, "%04d-", nr);
 		/* strip [PATCH] or [PATCH blabla] */
 		if (!keep_subject && !prefixcmp(sol, "[PATCH")) {
 			char *eos = strchr(sol + 6, ']');
@@ -564,8 +560,11 @@ static const char *get_oneline_for_filename(struct commit *commit,
 		while (filename[len - 1] == '.'
 		       || filename[len - 1] == '-')
 			len--;
-		filename[len] = '\0';
+		strcpy(filename + len, fmt_patch_suffix);
 	}
+	else
+		sprintf(filename, "%d", nr);
+
 	return filename;
 }
 
@@ -573,7 +572,7 @@ static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
 static int outdir_offset;
 
-static int reopen_stdout(const char *oneline, int nr, struct rev_info *rev)
+static int reopen_stdout(const char *oneline, struct rev_info *rev)
 {
 	char filename[PATH_MAX];
 	int len = 0;
@@ -588,15 +587,7 @@ static int reopen_stdout(const char *oneline, int nr, struct rev_info *rev)
 		if (filename[len - 1] != '/')
 			filename[len++] = '/';
 	}
-
-	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);
-	}
+	strncpy(filename+len, oneline, PATH_MAX-len-1);
 
 	if (!DIFF_OPT_TST(&rev->diffopt, QUIET))
 		fprintf(realstdout, "%s\n", filename + outdir_offset);
@@ -688,8 +679,9 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	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))
+	if (!use_stdout && reopen_stdout(get_patch_filename(numbered_files ?
+							NULL : "cover-letter",
+							0, 0), rev))
 		return;
 
 	head_sha1 = sha1_to_hex(head->object.sha1);
@@ -802,6 +794,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct patch_ids ids;
 	char *add_signoff = NULL;
 	struct strbuf buf = STRBUF_INIT;
+	char *sol = NULL;
 
 	git_config(git_format_config, NULL);
 	init_revisions(&rev, prefix);
@@ -1106,9 +1099,17 @@ 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 && reopen_stdout(numbered_files ? NULL :
-				get_oneline_for_filename(commit, keep_subject),
-				rev.nr, &rev))
+		/*
+		 * If we're outputting numbered files we don't need a subject
+		 * line. Otherwise we want the subject line of the commit
+		 * message which starts after the first double newline
+		 * occurence in the commit buffer.
+		 */
+		if (!numbered_files && (sol = strstr(commit->buffer, "\n\n")))
+			sol += 2;
+		if (!use_stdout && reopen_stdout(get_patch_filename(sol,
+								keep_subject,
+								rev.nr), &rev))
 			die("Failed to create output files");
 		shown = log_tree_commit(&rev, commit);
 		free(commit->buffer);
-- 
1.6.2

[PATCHv2 2/3] format-patch: --attach/inline uses filename instead of SHA1

[Stephen Boyd]

Currently when format-patch is used with --attach or --inline the patch
attachment has the SHA1 of the commit for its filename. This replaces
the SHA1 with the filename used by format-patch when outputting to files.

Fix tests relying on the SHA1 output and add a test showing how the
--suffix option affects the attachment filename output.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
Is modifying the rev_info struct correct?

 builtin-log.c                                      |    6 +-
 log-tree.c                                         |    8 +-
 revision.h                                         |    1 +
 t/t4013-diff-various.sh                            |    1 +
 ..._--attach_--stdout_--suffix=.diff_initial..side |   61 ++++++++++++++++++++
 ....format-patch_--attach_--stdout_initial..master |   12 ++--
 ...format-patch_--attach_--stdout_initial..master^ |    8 +-
 ...ff.format-patch_--attach_--stdout_initial..side |    4 +-
 ...tdout_--subject-prefix=TESTCASE_initial..master |   12 ++--
 ....format-patch_--inline_--stdout_initial..master |   12 ++--
 ...format-patch_--inline_--stdout_initial..master^ |    8 +-
 ...ormat-patch_--inline_--stdout_initial..master^^ |    4 +-
 ...ff.format-patch_--inline_--stdout_initial..side |    4 +-
 13 files changed, 102 insertions(+), 39 deletions(-)
 create mode 100644 t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side

diff --git a/builtin-log.c b/builtin-log.c
index cc6d663..8ea25e0 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1107,9 +1107,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		 */
 		if (!numbered_files && (sol = strstr(commit->buffer, "\n\n")))
 			sol += 2;
-		if (!use_stdout && reopen_stdout(get_patch_filename(sol,
-								keep_subject,
-								rev.nr), &rev))
+		rev.fmt_patch_filename = get_patch_filename(sol, keep_subject,
+								rev.nr);
+		if (!use_stdout && reopen_stdout(rev.fmt_patch_filename, &rev))
 			die("Failed to create output files");
 		shown = log_tree_commit(&rev, commit);
 		free(commit->buffer);
diff --git a/log-tree.c b/log-tree.c
index 9565c18..dc37d16 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -245,14 +245,14 @@ void log_write_email_headers(struct rev_info *opt, const char *name,
 		snprintf(buffer, sizeof(buffer) - 1,
 			 "\n--%s%s\n"
 			 "Content-Type: text/x-patch;"
-			 " name=\"%s.diff\"\n"
+			 " name=\"%s\"\n"
 			 "Content-Transfer-Encoding: 8bit\n"
 			 "Content-Disposition: %s;"
-			 " filename=\"%s.diff\"\n\n",
+			 " filename=\"%s\"\n\n",
 			 mime_boundary_leader, opt->mime_boundary,
-			 name,
+			 opt->fmt_patch_filename,
 			 opt->no_inline ? "attachment" : "inline",
-			 name);
+			 opt->fmt_patch_filename);
 		opt->diffopt.stat_sep = buffer;
 	}
 	*subject_p = subject;
diff --git a/revision.h b/revision.h
index ad123d7..c34eac9 100644
--- a/revision.h
+++ b/revision.h
@@ -86,6 +86,7 @@ struct rev_info {
 	struct log_info *loginfo;
 	int		nr, total;
 	const char	*mime_boundary;
+	const char	*fmt_patch_filename;
 	char		*message_id;
 	struct string_list *ref_message_ids;
 	const char	*add_signoff;
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 426e64e..6592a4f 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -246,6 +246,7 @@ format-patch --stdout initial..master
 format-patch --stdout --no-numbered initial..master
 format-patch --stdout --numbered initial..master
 format-patch --attach --stdout initial..side
+format-patch --attach --stdout --suffix=.diff initial..side
 format-patch --attach --stdout initial..master^
 format-patch --attach --stdout initial..master
 format-patch --inline --stdout initial..side
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side b/t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side
new file mode 100644
index 0000000..52116d3
--- /dev/null
+++ b/t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side
@@ -0,0 +1,61 @@
+$ git format-patch --attach --stdout --suffix=.diff initial..side
+From c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a Mon Sep 17 00:00:00 2001
+From: A U Thor <author@example.com>
+Date: Mon, 26 Jun 2006 00:03:00 +0000
+Subject: [PATCH] Side
+MIME-Version: 1.0
+Content-Type: multipart/mixed; boundary="------------g-i-t--v-e-r-s-i-o-n"
+
+This is a multi-part message in MIME format.
+--------------g-i-t--v-e-r-s-i-o-n
+Content-Type: text/plain; charset=UTF-8; format=fixed
+Content-Transfer-Encoding: 8bit
+
+---
+ dir/sub |    2 ++
+ file0   |    3 +++
+ file3   |    4 ++++
+ 3 files changed, 9 insertions(+), 0 deletions(-)
+ create mode 100644 file3
+
+
+--------------g-i-t--v-e-r-s-i-o-n
+Content-Type: text/x-patch; name="0001-Side.diff"
+Content-Transfer-Encoding: 8bit
+Content-Disposition: attachment; filename="0001-Side.diff"
+
+diff --git a/dir/sub b/dir/sub
+index 35d242b..7289e35 100644
+--- a/dir/sub
++++ b/dir/sub
+@@ -1,2 +1,4 @@
+ A
+ B
++1
++2
+diff --git a/file0 b/file0
+index 01e79c3..f4615da 100644
+--- a/file0
++++ b/file0
+@@ -1,3 +1,6 @@
+ 1
+ 2
+ 3
++A
++B
++C
+diff --git a/file3 b/file3
+new file mode 100644
+index 0000000..7289e35
+--- /dev/null
++++ b/file3
+@@ -0,0 +1,4 @@
++A
++B
++1
++2
+
+--------------g-i-t--v-e-r-s-i-o-n--
+
+
+$
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_initial..master b/t/t4013/diff.format-patch_--attach_--stdout_initial..master
index e5ab744..ce49bd6 100644
--- a/t/t4013/diff.format-patch_--attach_--stdout_initial..master
+++ b/t/t4013/diff.format-patch_--attach_--stdout_initial..master
@@ -22,9 +22,9 @@ This is the second commit.
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Type: text/x-patch; name="0001-Second.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: attachment; filename="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Disposition: attachment; filename="0001-Second.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..8422d40 100644
@@ -80,9 +80,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Type: text/x-patch; name="0002-Third.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: attachment; filename="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Disposition: attachment; filename="0002-Third.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 8422d40..cead32e 100644
@@ -129,9 +129,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Type: text/x-patch; name="0003-Side.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: attachment; filename="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Disposition: attachment; filename="0003-Side.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..7289e35 100644
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_initial..master^ b/t/t4013/diff.format-patch_--attach_--stdout_initial..master^
index 2c71d20..5f1b238 100644
--- a/t/t4013/diff.format-patch_--attach_--stdout_initial..master^
+++ b/t/t4013/diff.format-patch_--attach_--stdout_initial..master^
@@ -22,9 +22,9 @@ This is the second commit.
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Type: text/x-patch; name="0001-Second.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: attachment; filename="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Disposition: attachment; filename="0001-Second.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..8422d40 100644
@@ -80,9 +80,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Type: text/x-patch; name="0002-Third.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: attachment; filename="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Disposition: attachment; filename="0002-Third.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 8422d40..cead32e 100644
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_initial..side b/t/t4013/diff.format-patch_--attach_--stdout_initial..side
index 38f7902..4a2364a 100644
--- a/t/t4013/diff.format-patch_--attach_--stdout_initial..side
+++ b/t/t4013/diff.format-patch_--attach_--stdout_initial..side
@@ -20,9 +20,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Type: text/x-patch; name="0001-Side.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: attachment; filename="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Disposition: attachment; filename="0001-Side.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..7289e35 100644
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master b/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master
index 58f8a7b..ca3f60b 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master
+++ b/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master
@@ -22,9 +22,9 @@ This is the second commit.
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Type: text/x-patch; name="0001-Second.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Disposition: inline; filename="0001-Second.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..8422d40 100644
@@ -80,9 +80,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Type: text/x-patch; name="0002-Third.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Disposition: inline; filename="0002-Third.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 8422d40..cead32e 100644
@@ -129,9 +129,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Type: text/x-patch; name="0003-Side.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Disposition: inline; filename="0003-Side.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..7289e35 100644
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_initial..master b/t/t4013/diff.format-patch_--inline_--stdout_initial..master
index 9e7bbdf..08f2301 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_initial..master
+++ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master
@@ -22,9 +22,9 @@ This is the second commit.
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Type: text/x-patch; name="0001-Second.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Disposition: inline; filename="0001-Second.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..8422d40 100644
@@ -80,9 +80,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Type: text/x-patch; name="0002-Third.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Disposition: inline; filename="0002-Third.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 8422d40..cead32e 100644
@@ -129,9 +129,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Type: text/x-patch; name="0003-Side.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Disposition: inline; filename="0003-Side.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..7289e35 100644
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_initial..master^ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master^
index f881f64..07f1230 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_initial..master^
+++ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master^
@@ -22,9 +22,9 @@ This is the second commit.
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Type: text/x-patch; name="0001-Second.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Disposition: inline; filename="0001-Second.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..8422d40 100644
@@ -80,9 +80,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Type: text/x-patch; name="0002-Third.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Disposition: inline; filename="0002-Third.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 8422d40..cead32e 100644
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_initial..master^^ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master^^
index 4f258b8..29e00ab 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_initial..master^^
+++ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master^^
@@ -22,9 +22,9 @@ This is the second commit.
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Type: text/x-patch; name="0001-Second.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Disposition: inline; filename="0001-Second.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..8422d40 100644
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_initial..side b/t/t4013/diff.format-patch_--inline_--stdout_initial..side
index e86dce6..67633d4 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_initial..side
+++ b/t/t4013/diff.format-patch_--inline_--stdout_initial..side
@@ -20,9 +20,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Type: text/x-patch; name="0001-Side.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Disposition: inline; filename="0001-Side.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..7289e35 100644
-- 
1.6.2

[PATCHv2 3/3] format-patch: --numbered-files and --stdout aren't mutually exclusive

[Stephen Boyd]

For example:

    git format-patch --numbered-files --stdout --attach HEAD~~

will create two messages with files 1 and 2 attached respectively.
There is no effect when using --numbered-files and --stdout together
without an --attach or --inline, the files simply aren't created.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 Documentation/git-format-patch.txt |    1 -
 builtin-log.c                      |    2 --
 2 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index c14e3ee..c2eb5fa 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -97,7 +97,6 @@ include::diff-options.txt[]
 --numbered-files::
 	Output file names will be a simple number sequence
 	without the default first line of the commit appended.
-	Mutually exclusive with the --stdout option.
 
 -k::
 --keep-subject::
diff --git a/builtin-log.c b/builtin-log.c
index 8ea25e0..f81d3a3 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -951,8 +951,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		die ("-n and -k are mutually exclusive.");
 	if (keep_subject && subject_prefix)
 		die ("--subject-prefix and -k are mutually exclusive.");
-	if (numbered_files && use_stdout)
-		die ("--numbered-files and --stdout are mutually exclusive.");
 
 	argc = setup_revisions(argc, argv, &rev, "HEAD");
 	if (argc > 1)
-- 
1.6.2

Re: [PATCHv2 1/3] format-patch: create patch filename in one function

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

> reopen_stdout() usually takes the oneline description of a commit,
> appends the patch suffix, prepends the output directory (if any) and
> then reopens stdout as the resulting file. Now the patch filename (the
> oneline description and the patch suffix) is created in
> get_patch_filename() and passed to reopen_stdout() which prepends the
> output directory and reopens stdout as that file.

The renaming is a good idea even without any change in the feature.
Naming functions after what their result is used _for_ is never a good
idea, and we should name them after what they do.

Does it still make sense to pass "keep_subject" to the function?  After
all what it does is to retain "[PATCH.." prefix that is useless for the
purpose of making each patch easily identifiable.  Because people almost
always use patch acceptance tools in non-keep mode to strip the
"[PATCH..]"  prefix when creating the commits these days anyway, it may
make more sense to lose the parameter altogether and simplify the
processing.

> -static const char *get_oneline_for_filename(struct commit *commit,
> -					    int keep_subject)
> +static const char *get_patch_filename(char* sol, int keep_subject, int nr)

Asterisk sticks to the variable name, not type name.

I also wonder if it makes sense to move what this function does into a
user format; especially the logic that sanitizes the oneline string into
filename friendly one may be something Porcelains may want an access to
from outside.

IOW, you can introduce a new format specifier (say, "%f") to
format_commit_message() and the implemention of get_patch_filename() would
just prepare a strbuf and call format_commit_message() on it, no?

Re: [PATCHv2 2/3] format-patch: --attach/inline uses filename instead of SHA1

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

> Currently when format-patch is used with --attach or --inline the patch
> attachment has the SHA1 of the commit for its filename. This replaces
> the SHA1 with the filename used by format-patch when outputting to files.
>
> Fix tests relying on the SHA1 output and add a test showing how the
> --suffix option affects the attachment filename output.
>
> Signed-off-by: Stephen Boyd <bebarino@gmail.com>
> ---
> Is modifying the rev_info struct correct?

Most of the rev_info is about the overall traversal not about individual
commits, and I think the new field you are adding is about a single commit
you will update every time you switch to a new commit to process, so in
that sense it may violate the general idea of what rev-info is, but I do
not think it matters too much.

Re: [PATCHv2 3/3] format-patch: --numbered-files and --stdout aren't mutually exclusive

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

> For example:
>
>     git format-patch --numbered-files --stdout --attach HEAD~~
>
> will create two messages with files 1 and 2 attached respectively.
> There is no effect when using --numbered-files and --stdout together
> without an --attach or --inline, the files simply aren't created.

Hmm, "the files aren't created" is true, but I had to scratch my head if
you are describing a bug or justifying the change by saying "so filename
does not matter".

I think the change makes sense.  It could be argued that combination of
these two options should give a warning("ignoring --numbered"), but I do
not think it is even worth it.

Re: [PATCHv2 1/3] format-patch: create patch filename in one function

[Stephen Boyd]

Junio C Hamano wrote:
> The renaming is a good idea even without any change in the feature.
> Naming functions after what their result is used _for_ is never a good
> idea, and we should name them after what they do.
>
> Does it still make sense to pass "keep_subject" to the function?  After
> all what it does is to retain "[PATCH.." prefix that is useless for the
> purpose of making each patch easily identifiable.  Because people almost
> always use patch acceptance tools in non-keep mode to strip the
> "[PATCH..]"  prefix when creating the commits these days anyway, it may
> make more sense to lose the parameter altogether and simplify the
> processing.

I originally removed "keep_subject" but I took it out because I couldn't
figure out what it was being used for and I thought it might be
important. When it's removed all the tests pass. It's not even used by
other code besides a few if cases regarding numbering logic so I think
the entire command line switch could be removed.

>
> I also wonder if it makes sense to move what this function does into a
> user format; especially the logic that sanitizes the oneline string into
> filename friendly one may be something Porcelains may want an access to
> from outside.
>
> IOW, you can introduce a new format specifier (say, "%f") to
> format_commit_message() and the implemention of get_patch_filename() would
> just prepare a strbuf and call format_commit_message() on it, no?

This sounds great! I'm new so I don't know where to look for something
like this.

Re: [PATCHv2 1/3] format-patch: create patch filename in one function

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

> Junio C Hamano wrote:
> ...
>> IOW, you can introduce a new format specifier (say, "%f") to
>> format_commit_message() and the implemention of get_patch_filename() would
>> just prepare a strbuf and call format_commit_message() on it, no?
>
> This sounds great! I'm new so I don't know where to look for something
> like this.

I suspect you may not even have to pass the generated string around if you
did so.  Instead, you could pass the commit to log_write_email_headers()
instead of sha1_to_hex(commit->object.sha1) from show_log(), and use the
sha-1on the unix "From " line, and inside "if (opt->mime_boundar)", you
can ask format_commit_message("%f") to come up with a filename.

Re: [PATCHv2 1/3] format-patch: create patch filename in one function

[Stephen Boyd]

Junio C Hamano wrote:
> Stephen Boyd <bebarino@gmail.com> writes:
>
>> Junio C Hamano wrote:
>> ...
>>> IOW, you can introduce a new format specifier (say, "%f") to
>>> format_commit_message() and the implemention of get_patch_filename() would
>>> just prepare a strbuf and call format_commit_message() on it, no?
>> This sounds great! I'm new so I don't know where to look for something
>> like this.
>
> I suspect you may not even have to pass the generated string around if you
> did so.  Instead, you could pass the commit to log_write_email_headers()
> instead of sha1_to_hex(commit->object.sha1) from show_log(), and use the
> sha-1on the unix "From " line, and inside "if (opt->mime_boundar)", you
> can ask format_commit_message("%f") to come up with a filename.

I believe I won't be able to get the patch suffix at that point in the
code. Unless I decide to add that to the rev_info instead?

Re: [PATCHv2 1/3] format-patch: create patch filename in one function

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

> Junio C Hamano wrote:
>> Stephen Boyd <bebarino@gmail.com> writes:
>>
>>> Junio C Hamano wrote:
>>> ...
>>>> IOW, you can introduce a new format specifier (say, "%f") to
>>>> format_commit_message() and the implemention of get_patch_filename() would
>>>> just prepare a strbuf and call format_commit_message() on it, no?
>>> This sounds great! I'm new so I don't know where to look for something
>>> like this.
>>
>> I suspect you may not even have to pass the generated string around if you
>> did so.  Instead, you could pass the commit to log_write_email_headers()
>> instead of sha1_to_hex(commit->object.sha1) from show_log(), and use the
>> sha-1on the unix "From " line, and inside "if (opt->mime_boundar)", you
>> can ask format_commit_message("%f") to come up with a filename.
>
> I believe I won't be able to get the patch suffix at that point in the
> code. Unless I decide to add that to the rev_info instead?

Yeah, and I think that won't be "per commit" but "the same across the
traversal controlled by that rev_info", which is more in line with what
rev_info is about ;-)

[PATCHv3 0/6] format-patch --attach/--inline uses filename not SHA1

[Stephen Boyd]

This patch series modifies the behavior of format-patch when used with the
--attach or --inline command line settings. Current behavior names the attached
or inlined patches with the SHA1 of the commit, which isn't very friendly or
easy for a human to use when downloading the attachments. This series replaces
the SHA1 values with the filename used by format-patch when it outputs the
patches to files.

New features in this version: 
    - %f format specifier exposing sanitized filenames to porcelain
    - get_patch_filename() moved to log-tree

Stephen Boyd (6):
  pretty.c: add %f format specifier to format_commit_message()
  format-patch: construct patch filename in one function
  format-patch: pass a commit to reopen_stdout()
  format-patch: move get_patch_filename() into log-tree
  format-patch: --attach/inline uses filename instead of SHA1
  format-patch: --numbered-files and --stdout aren't mutually exclusive

 Documentation/pretty-formats.txt                   |    1 +
 builtin-log.c                                      |  132 +++++----------
 log-tree.c                                         |   41 ++++-
 log-tree.h                                         |    6 +-
 pretty.c                                           |   38 +++++
 revision.h                                         |    2 +
 t/t4013-diff-various.sh                            |    3 +-
 ..._--attach_--stdout_--suffix=.diff_initial..side |   61 +++++++
 ....format-patch_--attach_--stdout_initial..master |   12 +-
 ...format-patch_--attach_--stdout_initial..master^ |    8 +-
 ...ff.format-patch_--attach_--stdout_initial..side |    4 +-
 ...nline_--stdout_--numbered-files_initial..master |  170 ++++++++++++++++++++
 ...tdout_--subject-prefix=TESTCASE_initial..master |   12 +-
 ....format-patch_--inline_--stdout_initial..master |   12 +-
 ...format-patch_--inline_--stdout_initial..master^ |    8 +-
 ...ormat-patch_--inline_--stdout_initial..master^^ |    4 +-
 ...ff.format-patch_--inline_--stdout_initial..side |    4 +-
 17 files changed, 389 insertions(+), 129 deletions(-)
 create mode 100644 t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side
 create mode 100644 t/t4013/diff.format-patch_--inline_--stdout_--numbered-files_initial..master

[PATCHv3 1/6] pretty.c: add %f format specifier to format_commit_message()

[Stephen Boyd]

This specifier represents the sanitized and filename friendly subject
line of a commit. No checks are made against the length of the string,
so users may need to trim the result to the desired length if using as a
filename. This is commonly used by format-patch to massage commit
subjects into filenames and output patches to files.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 Documentation/pretty-formats.txt |    1 +
 pretty.c                         |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 5c6e678..2a845b1 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -121,6 +121,7 @@ The placeholders are:
 - '%d': ref names, like the --decorate option of linkgit:git-log[1]
 - '%e': encoding
 - '%s': subject
+- '%f': sanitized subject line, suitable for a filename
 - '%b': body
 - '%Cred': switch color to red
 - '%Cgreen': switch color to green
diff --git a/pretty.c b/pretty.c
index efa7024..97de415 100644
--- a/pretty.c
+++ b/pretty.c
@@ -493,6 +493,41 @@ static void parse_commit_header(struct format_commit_context *context)
 	context->commit_header_parsed = 1;
 }
 
+static int istitlechar(char c)
+{
+	return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') ||
+		(c >= '0' && c <= '9') || c == '.' || c == '_';
+}
+
+static void format_sanitized_subject(struct strbuf *sb, const char *msg)
+{
+	size_t trimlen;
+	int space = 0;
+
+	for (; *msg && *msg != '\n'; msg++) {
+		if (istitlechar(*msg))
+		{
+		    if (space) {
+			strbuf_addch(sb, '-');
+			space = 0;
+		    }
+		    strbuf_addch(sb, *msg);
+		    if (*msg == '.')
+			while (*(msg+1) == '.')
+				msg++;
+		}
+		else
+			space = 1;
+	}
+
+	// trim any trailing '.' or '-' characters
+	trimlen = 0;
+	while (sb->buf[sb->len - 1 - trimlen] == '.'
+		|| sb->buf[sb->len - 1 - trimlen] == '-')
+		trimlen++;
+	strbuf_remove(sb, sb->len - trimlen, trimlen);
+}
+
 const char *format_subject(struct strbuf *sb, const char *msg,
 			   const char *line_separator)
 {
@@ -683,6 +718,9 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 	case 's':	/* subject */
 		format_subject(sb, msg + c->subject_off, " ");
 		return 1;
+	case 'f':	/* sanitized subject */
+		format_sanitized_subject(sb, msg + c->subject_off);
+		return 1;
 	case 'b':	/* body */
 		strbuf_addstr(sb, msg + c->body_off);
 		return 1;
-- 
1.6.2

[PATCHv3 2/6] format-patch: construct patch filename in one function

[Stephen Boyd]

reopen_stdout() usually takes the oneline subject of a commit,
appends the patch suffix, prepends the output directory (if any) and
then reopens stdout as the resulting file. Now the patch filename (the
oneline subject and the patch suffix) is created in
get_patch_filename() and passed to reopen_stdout() which prepends the
output directory and reopens stdout as that file.

The original function to get the oneline description,
get_oneline_for_filename(), has been renamed to get_patch_filename() to
reflect its new functionality.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 builtin-log.c |   99 +++++++++++++++++++-------------------------------------
 1 files changed, 34 insertions(+), 65 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index c7a5772..54ef248 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -419,12 +419,6 @@ int cmd_log(int argc, const char **argv, const char *prefix)
 /* format-patch */
 #define FORMAT_PATCH_NAME_MAX 64
 
-static int istitlechar(char c)
-{
-	return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') ||
-		(c >= '0' && c <= '9') || c == '.' || c == '_';
-}
-
 static const char *fmt_patch_suffix = ".patch";
 static int numbered = 0;
 static int auto_number = 1;
@@ -519,61 +513,34 @@ static int git_format_config(const char *var, const char *value, void *cb)
 }
 
 
-static const char *get_oneline_for_filename(struct commit *commit,
-					    int keep_subject)
+static void get_patch_filename(struct commit *commit, int nr,
+			       const char *suffix, struct strbuf *buf)
 {
-	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;
-			}
-		}
+	int suffix_len = strlen(suffix) + 1;
+	int start_len = buf->len;
 
-		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';
+	strbuf_addf(buf, commit ? "%04d-" : "%d", nr);
+	if (commit)
+	{
+		format_commit_message(commit, "%f", buf, DATE_NORMAL);
+		/*
+		 * Replace characters at the end with the suffix if the the
+		 * filename is too long
+		 */
+		if (buf->len + suffix_len > FORMAT_PATCH_NAME_MAX + start_len)
+			strbuf_splice(buf,
+				start_len + FORMAT_PATCH_NAME_MAX - suffix_len,
+				suffix_len, suffix, suffix_len);
+		else
+			strbuf_addstr(buf, suffix);
 	}
-	return filename;
 }
 
 static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
 static int outdir_offset;
 
-static int reopen_stdout(const char *oneline, int nr, struct rev_info *rev)
+static int reopen_stdout(const char *oneline, struct rev_info *rev)
 {
 	char filename[PATH_MAX];
 	int len = 0;
@@ -589,14 +556,7 @@ static int reopen_stdout(const char *oneline, int nr, struct rev_info *rev)
 			filename[len++] = '/';
 	}
 
-	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);
-	}
+	strncpy(filename + len, oneline, PATH_MAX - len);
 
 	if (!DIFF_OPT_TST(&rev->diffopt, QUIET))
 		fprintf(realstdout, "%s\n", filename + outdir_offset);
@@ -684,12 +644,17 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	const char *encoding = "utf-8";
 	struct diff_options opts;
 	int need_8bit_cte = 0;
+	char filename[PATH_MAX];
 
 	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))
+	if (numbered_files)
+	    sprintf(filename, "0");
+	else
+	    sprintf(filename, "%04d-cover-letter%s", 0, fmt_patch_suffix);
+
+	if (!use_stdout && reopen_stdout(filename, rev))
 		return;
 
 	head_sha1 = sha1_to_hex(head->object.sha1);
@@ -802,6 +767,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct patch_ids ids;
 	char *add_signoff = NULL;
 	struct strbuf buf = STRBUF_INIT;
+	struct strbuf patch_filename = STRBUF_INIT;
 
 	git_config(git_format_config, NULL);
 	init_revisions(&rev, prefix);
@@ -1104,10 +1070,12 @@ 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 && reopen_stdout(numbered_files ? NULL :
-				get_oneline_for_filename(commit, keep_subject),
-				rev.nr, &rev))
+
+		get_patch_filename(numbered_files ? NULL : commit, rev.nr,
+				    fmt_patch_suffix, &patch_filename);
+		if (!use_stdout && reopen_stdout(patch_filename.buf, &rev))
 			die("Failed to create output files");
+		strbuf_setlen(&patch_filename, 0);
 		shown = log_tree_commit(&rev, commit);
 		free(commit->buffer);
 		commit->buffer = NULL;
@@ -1131,6 +1099,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (!use_stdout)
 			fclose(stdout);
 	}
+	strbuf_release(&patch_filename);
 	free(list);
 	if (ignore_if_in_upstream)
 		free_patch_ids(&ids);
-- 
1.6.2

[PATCHv3 3/6] format-patch: pass a commit to reopen_stdout()

[Stephen Boyd]

We use the commit to generate the patch filename in reopen_stdout()
before we redirect stdout. The cover letter codepath creates a dummy
commit with the desired subject line 'cover letter'.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 builtin-log.c |   68 +++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 54ef248..392b16a 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -540,30 +540,29 @@ static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
 static int outdir_offset;
 
-static int reopen_stdout(const char *oneline, struct rev_info *rev)
+static int reopen_stdout(struct commit *commit, struct rev_info *rev)
 {
-	char filename[PATH_MAX];
-	int len = 0;
+	struct strbuf filename = STRBUF_INIT;
 	int suffix_len = strlen(fmt_patch_suffix) + 1;
 
 	if (output_directory) {
-		len = snprintf(filename, sizeof(filename), "%s",
-				output_directory);
-		if (len >=
-		    sizeof(filename) - FORMAT_PATCH_NAME_MAX - suffix_len)
+		strbuf_addstr(&filename, output_directory);
+		if (filename.len >=
+		    PATH_MAX - FORMAT_PATCH_NAME_MAX - suffix_len)
 			return error("name of output directory is too long");
-		if (filename[len - 1] != '/')
-			filename[len++] = '/';
+		if (filename.buf[filename.len - 1] != '/')
+			strbuf_addch(&filename, '/');
 	}
 
-	strncpy(filename + len, oneline, PATH_MAX - len);
+	get_patch_filename(commit, rev->nr, fmt_patch_suffix, &filename);
 
 	if (!DIFF_OPT_TST(&rev->diffopt, QUIET))
-		fprintf(realstdout, "%s\n", filename + outdir_offset);
+		fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
 
-	if (freopen(filename, "w", stdout) == NULL)
-		return error("Cannot open patch file %s",filename);
+	if (freopen(filename.buf, "w", stdout) == NULL)
+		return error("Cannot open patch file %s", filename.buf);
 
+	strbuf_release(&filename);
 	return 0;
 }
 
@@ -644,26 +643,43 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	const char *encoding = "utf-8";
 	struct diff_options opts;
 	int need_8bit_cte = 0;
-	char filename[PATH_MAX];
+	struct commit *commit = NULL;
 
 	if (rev->commit_format != CMIT_FMT_EMAIL)
 		die("Cover letter needs email format");
 
-	if (numbered_files)
-	    sprintf(filename, "0");
-	else
-	    sprintf(filename, "%04d-cover-letter%s", 0, fmt_patch_suffix);
+	committer = git_committer_info(0);
+	head_sha1 = sha1_to_hex(head->object.sha1);
+
+	if (!numbered_files)
+	{
+		/*
+		 * We fake a commit for the cover letter so we get the filename
+		 * desired.
+		 */
+		commit = xcalloc(1, sizeof(*commit));
+		commit->buffer = xmalloc(400);
+		snprintf(commit->buffer, 400,
+			"tree 0000000000000000000000000000000000000000\n"
+			"parent %s\n"
+			"author %s\n"
+			"committer %s\n\n"
+			"cover letter\n",
+			head_sha1, committer, committer);
+	}
 
-	if (!use_stdout && reopen_stdout(filename, rev))
+	if (!use_stdout && reopen_stdout(commit, rev))
 		return;
 
-	head_sha1 = sha1_to_hex(head->object.sha1);
+	if (commit)
+	{
+		free(commit->buffer);
+		free(commit);
+	}
 
 	log_write_email_headers(rev, head_sha1, &subject_start, &extra_headers,
 				&need_8bit_cte);
 
-	committer = git_committer_info(0);
-
 	msg = body;
 	pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,
 		     encoding);
@@ -767,7 +783,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct patch_ids ids;
 	char *add_signoff = NULL;
 	struct strbuf buf = STRBUF_INIT;
-	struct strbuf patch_filename = STRBUF_INIT;
 
 	git_config(git_format_config, NULL);
 	init_revisions(&rev, prefix);
@@ -1071,11 +1086,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			gen_message_id(&rev, sha1_to_hex(commit->object.sha1));
 		}
 
-		get_patch_filename(numbered_files ? NULL : commit, rev.nr,
-				    fmt_patch_suffix, &patch_filename);
-		if (!use_stdout && reopen_stdout(patch_filename.buf, &rev))
+		if (!use_stdout && reopen_stdout(numbered_files ? NULL : commit,
+						 &rev))
 			die("Failed to create output files");
-		strbuf_setlen(&patch_filename, 0);
 		shown = log_tree_commit(&rev, commit);
 		free(commit->buffer);
 		commit->buffer = NULL;
@@ -1099,7 +1112,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (!use_stdout)
 			fclose(stdout);
 	}
-	strbuf_release(&patch_filename);
 	free(list);
 	if (ignore_if_in_upstream)
 		free_patch_ids(&ids);
-- 
1.6.2

[PATCHv3 4/6] format-patch: move get_patch_filename() into log-tree

[Stephen Boyd]

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 builtin-log.c |   25 -------------------------
 log-tree.c    |   23 +++++++++++++++++++++++
 log-tree.h    |    4 ++++
 3 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 392b16a..a8d6037 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -417,7 +417,6 @@ int cmd_log(int argc, const char **argv, const char *prefix)
 }
 
 /* format-patch */
-#define FORMAT_PATCH_NAME_MAX 64
 
 static const char *fmt_patch_suffix = ".patch";
 static int numbered = 0;
@@ -512,30 +511,6 @@ static int git_format_config(const char *var, const char *value, void *cb)
 	return git_log_config(var, value, cb);
 }
 
-
-static void get_patch_filename(struct commit *commit, int nr,
-			       const char *suffix, struct strbuf *buf)
-{
-	int suffix_len = strlen(suffix) + 1;
-	int start_len = buf->len;
-
-	strbuf_addf(buf, commit ? "%04d-" : "%d", nr);
-	if (commit)
-	{
-		format_commit_message(commit, "%f", buf, DATE_NORMAL);
-		/*
-		 * Replace characters at the end with the suffix if the the
-		 * filename is too long
-		 */
-		if (buf->len + suffix_len > FORMAT_PATCH_NAME_MAX + start_len)
-			strbuf_splice(buf,
-				start_len + FORMAT_PATCH_NAME_MAX - suffix_len,
-				suffix_len, suffix, suffix_len);
-		else
-			strbuf_addstr(buf, suffix);
-	}
-}
-
 static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
 static int outdir_offset;
diff --git a/log-tree.c b/log-tree.c
index 9565c18..9a790ab 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -179,6 +179,29 @@ static int has_non_ascii(const char *s)
 	return 0;
 }
 
+void get_patch_filename(struct commit *commit, int nr, const char *suffix,
+			struct strbuf *buf)
+{
+	int suffix_len = strlen(suffix) + 1;
+	int start_len = buf->len;
+
+	strbuf_addf(buf, commit ? "%04d-" : "%d", nr);
+	if (commit)
+	{
+		format_commit_message(commit, "%f", buf, DATE_NORMAL);
+		/*
+		 * Replace characters at the end with the suffix if the the
+		 * filename is too long
+		 */
+		if (buf->len + suffix_len > FORMAT_PATCH_NAME_MAX + start_len)
+			strbuf_splice(buf,
+				start_len + FORMAT_PATCH_NAME_MAX - suffix_len,
+				suffix_len, suffix, suffix_len);
+		else
+			strbuf_addstr(buf, suffix);
+	}
+}
+
 void log_write_email_headers(struct rev_info *opt, const char *name,
 			     const char **subject_p,
 			     const char **extra_headers_p,
diff --git a/log-tree.h b/log-tree.h
index f2a9008..78dc5be 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -19,4 +19,8 @@ void log_write_email_headers(struct rev_info *opt, const char *name,
 			     int *need_8bit_cte_p);
 void load_ref_decorations(void);
 
+#define FORMAT_PATCH_NAME_MAX 64
+void get_patch_filename(struct commit *commit, int nr, const char *suffix,
+			struct strbuf *buf);
+
 #endif
-- 
1.6.2

[PATCHv3 5/6] format-patch: --attach/inline uses filename instead of SHA1

[Stephen Boyd]

Currently when format-patch is used with --attach or --inline the patch
attachment has the SHA1 of the commit for its filename.  This replaces
the SHA1 with the filename used by format-patch when outputting to
files.

Fix tests relying on the SHA1 output and add a test showing how the
--suffix option affects the attachment filename output.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 builtin-log.c                                      |    8 +-
 log-tree.c                                         |   18 ++++--
 log-tree.h                                         |    2 +-
 revision.h                                         |    2 +
 t/t4013-diff-various.sh                            |    1 +
 ..._--attach_--stdout_--suffix=.diff_initial..side |   61 ++++++++++++++++++++
 ....format-patch_--attach_--stdout_initial..master |   12 ++--
 ...format-patch_--attach_--stdout_initial..master^ |    8 +-
 ...ff.format-patch_--attach_--stdout_initial..side |    4 +-
 ...tdout_--subject-prefix=TESTCASE_initial..master |   12 ++--
 ....format-patch_--inline_--stdout_initial..master |   12 ++--
 ...format-patch_--inline_--stdout_initial..master^ |    8 +-
 ...ormat-patch_--inline_--stdout_initial..master^^ |    4 +-
 ...ff.format-patch_--inline_--stdout_initial..side |    4 +-
 14 files changed, 112 insertions(+), 44 deletions(-)
 create mode 100644 t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side

diff --git a/builtin-log.c b/builtin-log.c
index a8d6037..83d8c03 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -607,7 +607,6 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			      int nr, struct commit **list, struct commit *head)
 {
 	const char *committer;
-	char *head_sha1;
 	const char *subject_start = NULL;
 	const char *body = "*** SUBJECT HERE ***\n\n*** BLURB HERE ***\n";
 	const char *msg;
@@ -624,7 +623,6 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 		die("Cover letter needs email format");
 
 	committer = git_committer_info(0);
-	head_sha1 = sha1_to_hex(head->object.sha1);
 
 	if (!numbered_files)
 	{
@@ -640,7 +638,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			"author %s\n"
 			"committer %s\n\n"
 			"cover letter\n",
-			head_sha1, committer, committer);
+			sha1_to_hex(head->object.sha1), committer, committer);
 	}
 
 	if (!use_stdout && reopen_stdout(commit, rev))
@@ -652,7 +650,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 		free(commit);
 	}
 
-	log_write_email_headers(rev, head_sha1, &subject_start, &extra_headers,
+	log_write_email_headers(rev, head, &subject_start, &extra_headers,
 				&need_8bit_cte);
 
 	msg = body;
@@ -1012,6 +1010,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		const char *msgid = clean_message_id(in_reply_to);
 		string_list_append(msgid, rev.ref_message_ids);
 	}
+	rev.numbered_files = numbered_files;
+	rev.patch_suffix = fmt_patch_suffix;
 	if (cover_letter) {
 		if (thread)
 			gen_message_id(&rev, "cover");
diff --git a/log-tree.c b/log-tree.c
index 9a790ab..5c9e2cf 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -202,13 +202,14 @@ void get_patch_filename(struct commit *commit, int nr, const char *suffix,
 	}
 }
 
-void log_write_email_headers(struct rev_info *opt, const char *name,
+void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			     const char **subject_p,
 			     const char **extra_headers_p,
 			     int *need_8bit_cte_p)
 {
 	const char *subject = NULL;
 	const char *extra_headers = opt->extra_headers;
+	const char *name = sha1_to_hex(commit->object.sha1);
 
 	*need_8bit_cte_p = 0; /* unknown */
 	if (opt->total > 0) {
@@ -247,6 +248,7 @@ void log_write_email_headers(struct rev_info *opt, const char *name,
 	if (opt->mime_boundary) {
 		static char subject_buffer[1024];
 		static char buffer[1024];
+		struct strbuf filename =  STRBUF_INIT;
 		*need_8bit_cte_p = -1; /* NEVER */
 		snprintf(subject_buffer, sizeof(subject_buffer) - 1,
 			 "%s"
@@ -265,18 +267,21 @@ void log_write_email_headers(struct rev_info *opt, const char *name,
 			 mime_boundary_leader, opt->mime_boundary);
 		extra_headers = subject_buffer;
 
+		get_patch_filename(opt->numbered_files ? NULL : commit, opt->nr,
+				    opt->patch_suffix, &filename);
 		snprintf(buffer, sizeof(buffer) - 1,
 			 "\n--%s%s\n"
 			 "Content-Type: text/x-patch;"
-			 " name=\"%s.diff\"\n"
+			 " name=\"%s\"\n"
 			 "Content-Transfer-Encoding: 8bit\n"
 			 "Content-Disposition: %s;"
-			 " filename=\"%s.diff\"\n\n",
+			 " filename=\"%s\"\n\n",
 			 mime_boundary_leader, opt->mime_boundary,
-			 name,
+			 filename.buf,
 			 opt->no_inline ? "attachment" : "inline",
-			 name);
+			 filename.buf);
 		opt->diffopt.stat_sep = buffer;
+		strbuf_release(&filename);
 	}
 	*subject_p = subject;
 	*extra_headers_p = extra_headers;
@@ -356,8 +361,7 @@ void show_log(struct rev_info *opt)
 	 */
 
 	if (opt->commit_format == CMIT_FMT_EMAIL) {
-		log_write_email_headers(opt, sha1_to_hex(commit->object.sha1),
-					&subject, &extra_headers,
+		log_write_email_headers(opt, commit, &subject, &extra_headers,
 					&need_8bit_cte);
 	} else if (opt->commit_format != CMIT_FMT_USERFORMAT) {
 		fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), stdout);
diff --git a/log-tree.h b/log-tree.h
index 78dc5be..20b5caf 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -13,7 +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);
 void show_decorations(struct rev_info *opt, struct commit *commit);
-void log_write_email_headers(struct rev_info *opt, const char *name,
+void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			     const char **subject_p,
 			     const char **extra_headers_p,
 			     int *need_8bit_cte_p);
diff --git a/revision.h b/revision.h
index ad123d7..5259ed5 100644
--- a/revision.h
+++ b/revision.h
@@ -86,6 +86,8 @@ struct rev_info {
 	struct log_info *loginfo;
 	int		nr, total;
 	const char	*mime_boundary;
+	const char	*patch_suffix;
+	int		numbered_files;
 	char		*message_id;
 	struct string_list *ref_message_ids;
 	const char	*add_signoff;
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 426e64e..6592a4f 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -246,6 +246,7 @@ format-patch --stdout initial..master
 format-patch --stdout --no-numbered initial..master
 format-patch --stdout --numbered initial..master
 format-patch --attach --stdout initial..side
+format-patch --attach --stdout --suffix=.diff initial..side
 format-patch --attach --stdout initial..master^
 format-patch --attach --stdout initial..master
 format-patch --inline --stdout initial..side
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side b/t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side
new file mode 100644
index 0000000..52116d3
--- /dev/null
+++ b/t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side
@@ -0,0 +1,61 @@
+$ git format-patch --attach --stdout --suffix=.diff initial..side
+From c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a Mon Sep 17 00:00:00 2001
+From: A U Thor <author@example.com>
+Date: Mon, 26 Jun 2006 00:03:00 +0000
+Subject: [PATCH] Side
+MIME-Version: 1.0
+Content-Type: multipart/mixed; boundary="------------g-i-t--v-e-r-s-i-o-n"
+
+This is a multi-part message in MIME format.
+--------------g-i-t--v-e-r-s-i-o-n
+Content-Type: text/plain; charset=UTF-8; format=fixed
+Content-Transfer-Encoding: 8bit
+
+---
+ dir/sub |    2 ++
+ file0   |    3 +++
+ file3   |    4 ++++
+ 3 files changed, 9 insertions(+), 0 deletions(-)
+ create mode 100644 file3
+
+
+--------------g-i-t--v-e-r-s-i-o-n
+Content-Type: text/x-patch; name="0001-Side.diff"
+Content-Transfer-Encoding: 8bit
+Content-Disposition: attachment; filename="0001-Side.diff"
+
+diff --git a/dir/sub b/dir/sub
+index 35d242b..7289e35 100644
+--- a/dir/sub
++++ b/dir/sub
+@@ -1,2 +1,4 @@
+ A
+ B
++1
++2
+diff --git a/file0 b/file0
+index 01e79c3..f4615da 100644
+--- a/file0
++++ b/file0
+@@ -1,3 +1,6 @@
+ 1
+ 2
+ 3
++A
++B
++C
+diff --git a/file3 b/file3
+new file mode 100644
+index 0000000..7289e35
+--- /dev/null
++++ b/file3
+@@ -0,0 +1,4 @@
++A
++B
++1
++2
+
+--------------g-i-t--v-e-r-s-i-o-n--
+
+
+$
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_initial..master b/t/t4013/diff.format-patch_--attach_--stdout_initial..master
index e5ab744..ce49bd6 100644
--- a/t/t4013/diff.format-patch_--attach_--stdout_initial..master
+++ b/t/t4013/diff.format-patch_--attach_--stdout_initial..master
@@ -22,9 +22,9 @@ This is the second commit.
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Type: text/x-patch; name="0001-Second.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: attachment; filename="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Disposition: attachment; filename="0001-Second.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..8422d40 100644
@@ -80,9 +80,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Type: text/x-patch; name="0002-Third.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: attachment; filename="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Disposition: attachment; filename="0002-Third.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 8422d40..cead32e 100644
@@ -129,9 +129,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Type: text/x-patch; name="0003-Side.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: attachment; filename="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Disposition: attachment; filename="0003-Side.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..7289e35 100644
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_initial..master^ b/t/t4013/diff.format-patch_--attach_--stdout_initial..master^
index 2c71d20..5f1b238 100644
--- a/t/t4013/diff.format-patch_--attach_--stdout_initial..master^
+++ b/t/t4013/diff.format-patch_--attach_--stdout_initial..master^
@@ -22,9 +22,9 @@ This is the second commit.
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Type: text/x-patch; name="0001-Second.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: attachment; filename="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Disposition: attachment; filename="0001-Second.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..8422d40 100644
@@ -80,9 +80,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Type: text/x-patch; name="0002-Third.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: attachment; filename="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Disposition: attachment; filename="0002-Third.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 8422d40..cead32e 100644
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_initial..side b/t/t4013/diff.format-patch_--attach_--stdout_initial..side
index 38f7902..4a2364a 100644
--- a/t/t4013/diff.format-patch_--attach_--stdout_initial..side
+++ b/t/t4013/diff.format-patch_--attach_--stdout_initial..side
@@ -20,9 +20,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Type: text/x-patch; name="0001-Side.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: attachment; filename="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Disposition: attachment; filename="0001-Side.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..7289e35 100644
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master b/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master
index 58f8a7b..ca3f60b 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master
+++ b/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master
@@ -22,9 +22,9 @@ This is the second commit.
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Type: text/x-patch; name="0001-Second.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Disposition: inline; filename="0001-Second.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..8422d40 100644
@@ -80,9 +80,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Type: text/x-patch; name="0002-Third.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Disposition: inline; filename="0002-Third.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 8422d40..cead32e 100644
@@ -129,9 +129,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Type: text/x-patch; name="0003-Side.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Disposition: inline; filename="0003-Side.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..7289e35 100644
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_initial..master b/t/t4013/diff.format-patch_--inline_--stdout_initial..master
index 9e7bbdf..08f2301 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_initial..master
+++ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master
@@ -22,9 +22,9 @@ This is the second commit.
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Type: text/x-patch; name="0001-Second.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Disposition: inline; filename="0001-Second.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..8422d40 100644
@@ -80,9 +80,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Type: text/x-patch; name="0002-Third.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Disposition: inline; filename="0002-Third.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 8422d40..cead32e 100644
@@ -129,9 +129,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Type: text/x-patch; name="0003-Side.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Disposition: inline; filename="0003-Side.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..7289e35 100644
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_initial..master^ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master^
index f881f64..07f1230 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_initial..master^
+++ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master^
@@ -22,9 +22,9 @@ This is the second commit.
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Type: text/x-patch; name="0001-Second.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Disposition: inline; filename="0001-Second.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..8422d40 100644
@@ -80,9 +80,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Type: text/x-patch; name="0002-Third.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
+Content-Disposition: inline; filename="0002-Third.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 8422d40..cead32e 100644
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_initial..master^^ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master^^
index 4f258b8..29e00ab 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_initial..master^^
+++ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master^^
@@ -22,9 +22,9 @@ This is the second commit.
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Type: text/x-patch; name="0001-Second.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
+Content-Disposition: inline; filename="0001-Second.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..8422d40 100644
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_initial..side b/t/t4013/diff.format-patch_--inline_--stdout_initial..side
index e86dce6..67633d4 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_initial..side
+++ b/t/t4013/diff.format-patch_--inline_--stdout_initial..side
@@ -20,9 +20,9 @@ Content-Transfer-Encoding: 8bit
 
 
 --------------g-i-t--v-e-r-s-i-o-n
-Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Type: text/x-patch; name="0001-Side.patch"
 Content-Transfer-Encoding: 8bit
-Content-Disposition: inline; filename="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
+Content-Disposition: inline; filename="0001-Side.patch"
 
 diff --git a/dir/sub b/dir/sub
 index 35d242b..7289e35 100644
-- 
1.6.2

[PATCHv3 6/6] format-patch: --numbered-files and --stdout aren't mutually exclusive

[Stephen Boyd]

For example:

    git format-patch --numbered-files --stdout --attach HEAD~~

will create two messages with files 1 and 2 attached respectively.
There is no effect when using --numbered-files and --stdout together
without an --attach or --inline, the --numbered-files option will be
ignored. Add a test to show this.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 t/t4013-diff-various.sh                            |    2 +-
 ...nline_--stdout_--numbered-files_initial..master |  170 ++++++++++++++++++++
 2 files changed, 171 insertions(+), 1 deletions(-)
 create mode 100644 t/t4013/diff.format-patch_--inline_--stdout_--numbered-files_initial..master

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 6592a4f..8b33321 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -251,7 +251,7 @@ format-patch --attach --stdout initial..master^
 format-patch --attach --stdout initial..master
 format-patch --inline --stdout initial..side
 format-patch --inline --stdout initial..master^
-format-patch --inline --stdout initial..master
+format-patch --inline --stdout --numbered-files initial..master
 format-patch --inline --stdout initial..master
 format-patch --inline --stdout --subject-prefix=TESTCASE initial..master
 config format.subjectprefix DIFFERENT_PREFIX
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_--numbered-files_initial..master b/t/t4013/diff.format-patch_--inline_--stdout_--numbered-files_initial..master
new file mode 100644
index 0000000..43b81eb
--- /dev/null
+++ b/t/t4013/diff.format-patch_--inline_--stdout_--numbered-files_initial..master
@@ -0,0 +1,170 @@
+$ git format-patch --inline --stdout --numbered-files initial..master
+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: [PATCH 1/3] Second
+MIME-Version: 1.0
+Content-Type: multipart/mixed; boundary="------------g-i-t--v-e-r-s-i-o-n"
+
+This is a multi-part message in MIME format.
+--------------g-i-t--v-e-r-s-i-o-n
+Content-Type: text/plain; charset=UTF-8; format=fixed
+Content-Transfer-Encoding: 8bit
+
+
+This is the second commit.
+---
+ dir/sub |    2 ++
+ file0   |    3 +++
+ file2   |    3 ---
+ 3 files changed, 5 insertions(+), 3 deletions(-)
+ delete mode 100644 file2
+
+
+--------------g-i-t--v-e-r-s-i-o-n
+Content-Type: text/x-patch; name="1"
+Content-Transfer-Encoding: 8bit
+Content-Disposition: inline; filename="1"
+
+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: [PATCH 2/3] Third
+MIME-Version: 1.0
+Content-Type: multipart/mixed; boundary="------------g-i-t--v-e-r-s-i-o-n"
+
+This is a multi-part message in MIME format.
+--------------g-i-t--v-e-r-s-i-o-n
+Content-Type: text/plain; charset=UTF-8; format=fixed
+Content-Transfer-Encoding: 8bit
+
+---
+ dir/sub |    2 ++
+ file1   |    3 +++
+ 2 files changed, 5 insertions(+), 0 deletions(-)
+ create mode 100644 file1
+
+
+--------------g-i-t--v-e-r-s-i-o-n
+Content-Type: text/x-patch; name="2"
+Content-Transfer-Encoding: 8bit
+Content-Disposition: inline; filename="2"
+
+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--
+
+
+
+From c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a Mon Sep 17 00:00:00 2001
+From: A U Thor <author@example.com>
+Date: Mon, 26 Jun 2006 00:03:00 +0000
+Subject: [PATCH 3/3] Side
+MIME-Version: 1.0
+Content-Type: multipart/mixed; boundary="------------g-i-t--v-e-r-s-i-o-n"
+
+This is a multi-part message in MIME format.
+--------------g-i-t--v-e-r-s-i-o-n
+Content-Type: text/plain; charset=UTF-8; format=fixed
+Content-Transfer-Encoding: 8bit
+
+---
+ dir/sub |    2 ++
+ file0   |    3 +++
+ file3   |    4 ++++
+ 3 files changed, 9 insertions(+), 0 deletions(-)
+ create mode 100644 file3
+
+
+--------------g-i-t--v-e-r-s-i-o-n
+Content-Type: text/x-patch; name="3"
+Content-Transfer-Encoding: 8bit
+Content-Disposition: inline; filename="3"
+
+diff --git a/dir/sub b/dir/sub
+index 35d242b..7289e35 100644
+--- a/dir/sub
++++ b/dir/sub
+@@ -1,2 +1,4 @@
+ A
+ B
++1
++2
+diff --git a/file0 b/file0
+index 01e79c3..f4615da 100644
+--- a/file0
++++ b/file0
+@@ -1,3 +1,6 @@
+ 1
+ 2
+ 3
++A
++B
++C
+diff --git a/file3 b/file3
+new file mode 100644
+index 0000000..7289e35
+--- /dev/null
++++ b/file3
+@@ -0,0 +1,4 @@
++A
++B
++1
++2
+
+--------------g-i-t--v-e-r-s-i-o-n--
+
+
+$
-- 
1.6.2

Re: [PATCHv3 1/6] pretty.c: add %f format specifier to format_commit_message()

[René Scharfe]

Stephen Boyd schrieb:
> This specifier represents the sanitized and filename friendly subject
> line of a commit. No checks are made against the length of the string,
> so users may need to trim the result to the desired length if using as a
> filename. This is commonly used by format-patch to massage commit
> subjects into filenames and output patches to files.
> 
> Signed-off-by: Stephen Boyd <bebarino@gmail.com>
> ---
>  Documentation/pretty-formats.txt |    1 +
>  pretty.c                         |   38 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> index 5c6e678..2a845b1 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -121,6 +121,7 @@ The placeholders are:
>  - '%d': ref names, like the --decorate option of linkgit:git-log[1]
>  - '%e': encoding
>  - '%s': subject
> +- '%f': sanitized subject line, suitable for a filename
>  - '%b': body
>  - '%Cred': switch color to red
>  - '%Cgreen': switch color to green
> diff --git a/pretty.c b/pretty.c
> index efa7024..97de415 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -493,6 +493,41 @@ static void parse_commit_header(struct format_commit_context *context)
>  	context->commit_header_parsed = 1;
>  }
>  
> +static int istitlechar(char c)
> +{
> +	return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') ||
> +		(c >= '0' && c <= '9') || c == '.' || c == '_';

How about this?

	return isalnum(c) || c == '.' || c == '_';

> +}
> +
> +static void format_sanitized_subject(struct strbuf *sb, const char *msg)
> +{
> +	size_t trimlen;
> +	int space = 0;
> +
> +	for (; *msg && *msg != '\n'; msg++) {
> +		if (istitlechar(*msg))
> +		{
> +		    if (space) {
> +			strbuf_addch(sb, '-');
> +			space = 0;
> +		    }
> +		    strbuf_addch(sb, *msg);
> +		    if (*msg == '.')
> +			while (*(msg+1) == '.')
> +				msg++;
> +		}
> +		else
> +			space = 1;
> +	}
> +

> +	// trim any trailing '.' or '-' characters
> +	trimlen = 0;
> +	while (sb->buf[sb->len - 1 - trimlen] == '.'
> +		|| sb->buf[sb->len - 1 - trimlen] == '-')
> +		trimlen++;
> +	strbuf_remove(sb, sb->len - trimlen, trimlen);

You need to make sure that trimming stops as soon as the strbuf has been
shortened to its original length.  E.g. for a subject line of "..."
you'd access the char before the first dot currently, or sb->buf[-1] if
the strbuf was empty initially.

(One could also check for sb->len > 0 to just prevent the buffer
underrun, but %f sometimes eating preceding dots and dashes is
counter-intuitive to me.)

> +}
> +
>  const char *format_subject(struct strbuf *sb, const char *msg,
>  			   const char *line_separator)
>  {
> @@ -683,6 +718,9 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
>  	case 's':	/* subject */
>  		format_subject(sb, msg + c->subject_off, " ");
>  		return 1;
> +	case 'f':	/* sanitized subject */
> +		format_sanitized_subject(sb, msg + c->subject_off);
> +		return 1;
>  	case 'b':	/* body */
>  		strbuf_addstr(sb, msg + c->body_off);
>  		return 1;

[PATCH] format_sanitized_subject: Don't trim past initial length of strbuf

[Stephen Boyd]

If the subject line is '...' the strbuf will be accessed before the
first dot is added; potentially changing the strbuf passed into the
function or accessing sb->buf[-1] if it was originally empty.

Reported-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>
---
I was thinking about this today actually. Thanks.

With regards to the isalnum(), I kept the original code because I wasn't sure
if the functionality would be different.

 pretty.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/pretty.c b/pretty.c
index c57cef4..a0ef356 100644
--- a/pretty.c
+++ b/pretty.c
@@ -502,6 +502,7 @@ static int istitlechar(char c)
 static void format_sanitized_subject(struct strbuf *sb, const char *msg)
 {
 	size_t trimlen;
+	size_t start_len = sb->len;
 	int space = 2;
 
 	for (; *msg && *msg != '\n'; msg++) {
@@ -519,8 +520,9 @@ static void format_sanitized_subject(struct strbuf *sb, const char *msg)
 
 	/* trim any trailing '.' or '-' characters */
 	trimlen = 0;
-	while (sb->buf[sb->len - 1 - trimlen] == '.'
-		|| sb->buf[sb->len - 1 - trimlen] == '-')
+	while (sb->len - trimlen > start_len &&
+		(sb->buf[sb->len - 1 - trimlen] == '.'
+		|| sb->buf[sb->len - 1 - trimlen] == '-'))
 		trimlen++;
 	strbuf_remove(sb, sb->len - trimlen, trimlen);
 }
-- 
1.6.2

Re: [PATCH] format_sanitized_subject: Don't trim past initial length of strbuf

[Stephen Boyd]

Forgot to say this is based on next.