1 files changed, 357 insertions, 199 deletions

diff --git a/builtin-log.c b/builtin-log.c
index 0f0adf2bab..e0d5caa61b 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -17,6 +17,8 @@
 #include "run-command.h"
 #include "shortlog.h"
 #include "remote.h"
+#include "string-list.h"
+#include "parse-options.h"
 
 /* Set a default date-time format for git log ("log.date" config variable) */
 static const char *default_date_mode = NULL;
@@ -25,10 +27,15 @@ static int default_show_root = 1;
 static const char *fmt_patch_subject_prefix = "PATCH";
 static const char *fmt_pretty;
 
+static const char * const builtin_log_usage =
+	"git log [<options>] [<since>..<until>] [[--] <path>...]\n"
+	"   or: git show [options] <object>...";
+
 static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		      struct rev_info *rev)
 {
 	int i;
+	int decoration_style = 0;
 
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
@@ -43,8 +50,17 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	if (default_date_mode)
 		rev->date_mode = parse_date_format(default_date_mode);
 
+	/*
+	 * Check for -h before setup_revisions(), or "git log -h" will
+	 * fail when run without a git directory.
+	 */
+	if (argc == 2 && !strcmp(argv[1], "-h"))
+		usage(builtin_log_usage);
 	argc = setup_revisions(argc, argv, rev, "HEAD");
 
+	if (!rev->show_notes_given && !rev->pretty_given)
+		rev->show_notes = 1;
+
 	if (rev->diffopt.pickaxe || rev->diffopt.filter)
 		rev->always_show_header = 0;
 	if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
@@ -55,13 +71,26 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 		if (!strcmp(arg, "--decorate")) {
-			load_ref_decorations();
-			rev->show_decorations = 1;
+			decoration_style = DECORATE_SHORT_REFS;
+		} else if (!prefixcmp(arg, "--decorate=")) {
+			const char *v = skip_prefix(arg, "--decorate=");
+			if (!strcmp(v, "full"))
+				decoration_style = DECORATE_FULL_REFS;
+			else if (!strcmp(v, "short"))
+				decoration_style = DECORATE_SHORT_REFS;
+			else
+				die("invalid --decorate option: %s", arg);
 		} else if (!strcmp(arg, "--source")) {
 			rev->show_source = 1;
+		} else if (!strcmp(arg, "-h")) {
+			usage(builtin_log_usage);
 		} else
 			die("unrecognized argument: %s", arg);
 	}
+	if (decoration_style) {
+		rev->show_decorations = 1;
+		load_ref_decorations(decoration_style);
+	}
 }
 
 /*
@@ -92,7 +121,7 @@ static void show_early_header(struct rev_info *rev, const char *stage, int nr)
 	printf("Final output: %d %s\n", nr, stage);
 }
 
-struct itimerval early_output_timer;
+static struct itimerval early_output_timer;
 
 static void log_show_early(struct rev_info *revs, struct commit_list *list)
 {
@@ -255,7 +284,7 @@ static void show_tagger(char *buf, int len, struct rev_info *rev)
 	pp_user_info("Tagger", rev->commit_format, &out, buf, rev->date_mode,
 		git_log_output_encoding ?
 		git_log_output_encoding: git_commit_encoding);
-	printf("%s\n", out.buf);
+	printf("%s", out.buf);
 	strbuf_release(&out);
 }
 
@@ -327,11 +356,14 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 		case OBJ_TAG: {
 			struct tag *t = (struct tag *)o;
 
+			if (rev.shown_one)
+				putchar('\n');
 			printf("%stag %s%s\n",
 					diff_get_color_opt(&rev.diffopt, DIFF_COMMIT),
 					t->tag,
 					diff_get_color_opt(&rev.diffopt, DIFF_RESET));
 			ret = show_object(o->sha1, 1, &rev);
+			rev.shown_one = 1;
 			if (ret)
 				break;
 			o = parse_object(t->tagged->sha1);
@@ -343,12 +375,15 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 			break;
 		}
 		case OBJ_TREE:
+			if (rev.shown_one)
+				putchar('\n');
 			printf("%stree %s%s\n\n",
 					diff_get_color_opt(&rev.diffopt, DIFF_COMMIT),
 					name,
 					diff_get_color_opt(&rev.diffopt, DIFF_RESET));
 			read_tree_recursive((struct tree *)o, "", 0, 0, NULL,
 					show_tree_object, NULL);
+			rev.shown_one = 1;
 			break;
 		case OBJ_COMMIT:
 			rev.pending.nr = rev.pending.alloc = 0;
@@ -416,18 +451,13 @@ 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;
 
+static char *default_attach = NULL;
+
 static char **extra_hdr;
 static int extra_hdr_nr;
 static int extra_hdr_alloc;
@@ -459,6 +489,11 @@ static void add_header(const char *value)
 	extra_hdr[extra_hdr_nr++] = xstrndup(value, len);
 }
 
+#define THREAD_SHALLOW 1
+#define THREAD_DEEP 2
+static int thread = 0;
+static int do_signoff = 0;
+
 static int git_format_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "format.headers")) {
@@ -488,94 +523,60 @@ static int git_format_config(const char *var, const char *value, void *cb)
 		auto_number = auto_number && numbered;
 		return 0;
 	}
-
-	return git_log_config(var, value, cb);
-}
-
-
-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;
-			}
+	if (!strcmp(var, "format.attach")) {
+		if (value && *value)
+			default_attach = xstrdup(value);
+		else
+			default_attach = xstrdup(git_version_string);
+		return 0;
+	}
+	if (!strcmp(var, "format.thread")) {
+		if (value && !strcasecmp(value, "deep")) {
+			thread = THREAD_DEEP;
+			return 0;
 		}
-
-		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;
+		if (value && !strcasecmp(value, "shallow")) {
+			thread = THREAD_SHALLOW;
+			return 0;
 		}
-		while (filename[len - 1] == '.'
-		       || filename[len - 1] == '-')
-			len--;
-		filename[len] = '\0';
+		thread = git_config_bool(var, value) && THREAD_SHALLOW;
+		return 0;
 	}
-	return filename;
+	if (!strcmp(var, "format.signoff")) {
+		do_signoff = git_config_bool(var, value);
+		return 0;
+	}
+
+	return git_log_config(var, value, cb);
 }
 
 static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
 static int outdir_offset;
 
-static int reopen_stdout(const char *oneline, int nr, int total)
+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, '/');
 	}
 
-	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);
-	}
+	get_patch_filename(commit, rev->nr, fmt_patch_suffix, &filename);
+
+	if (!DIFF_OPT_TST(&rev->diffopt, QUICK))
+		fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
 
-	fprintf(realstdout, "%s\n", filename + 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;
 }
 
@@ -645,7 +646,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;
@@ -653,23 +653,47 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	struct shortlog log;
 	struct strbuf sb = STRBUF_INIT;
 	int i;
-	const char *encoding = "utf-8";
+	const char *encoding = "UTF-8";
 	struct diff_options opts;
 	int need_8bit_cte = 0;
+	struct commit *commit = NULL;
 
 	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))
+	committer = git_committer_info(0);
+
+	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",
+			sha1_to_hex(head->object.sha1), committer, committer);
+	}
+
+	if (!use_stdout && reopen_stdout(commit, rev))
 		return;
 
-	head_sha1 = sha1_to_hex(head->object.sha1);
+	if (commit) {
 
-	log_write_email_headers(rev, head_sha1, &subject_start, &extra_headers,
+		free(commit->buffer);
+		free(commit);
+	}
+
+	log_write_email_headers(rev, head, &subject_start, &extra_headers,
 				&need_8bit_cte);
 
-	committer = git_committer_info(0);
+	for (i = 0; !need_8bit_cte && i < nr; i++)
+		if (has_non_ascii(list[i]->buffer))
+			need_8bit_cte = 1;
 
 	msg = body;
 	pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,
@@ -754,19 +778,120 @@ static const char *set_outdir(const char *prefix, const char *output_directory)
 				       output_directory));
 }
 
+static const char * const builtin_format_patch_usage[] = {
+	"git format-patch [options] [<since> | <revision range>]",
+	NULL
+};
+
+static int keep_subject = 0;
+
+static int keep_callback(const struct option *opt, const char *arg, int unset)
+{
+	((struct rev_info *)opt->value)->total = -1;
+	keep_subject = 1;
+	return 0;
+}
+
+static int subject_prefix = 0;
+
+static int subject_prefix_callback(const struct option *opt, const char *arg,
+			    int unset)
+{
+	subject_prefix = 1;
+	((struct rev_info *)opt->value)->subject_prefix = arg;
+	return 0;
+}
+
+static int numbered_cmdline_opt = 0;
+
+static int numbered_callback(const struct option *opt, const char *arg,
+			     int unset)
+{
+	*(int *)opt->value = numbered_cmdline_opt = unset ? 0 : 1;
+	if (unset)
+		auto_number =  0;
+	return 0;
+}
+
+static int no_numbered_callback(const struct option *opt, const char *arg,
+				int unset)
+{
+	return numbered_callback(opt, arg, 1);
+}
+
+static int output_directory_callback(const struct option *opt, const char *arg,
+			      int unset)
+{
+	const char **dir = (const char **)opt->value;
+	if (*dir)
+		die("Two output directories?");
+	*dir = arg;
+	return 0;
+}
+
+static int thread_callback(const struct option *opt, const char *arg, int unset)
+{
+	int *thread = (int *)opt->value;
+	if (unset)
+		*thread = 0;
+	else if (!arg || !strcmp(arg, "shallow"))
+		*thread = THREAD_SHALLOW;
+	else if (!strcmp(arg, "deep"))
+		*thread = THREAD_DEEP;
+	else
+		return 1;
+	return 0;
+}
+
+static int attach_callback(const struct option *opt, const char *arg, int unset)
+{
+	struct rev_info *rev = (struct rev_info *)opt->value;
+	if (unset)
+		rev->mime_boundary = NULL;
+	else if (arg)
+		rev->mime_boundary = arg;
+	else
+		rev->mime_boundary = git_version_string;
+	rev->no_inline = unset ? 0 : 1;
+	return 0;
+}
+
+static int inline_callback(const struct option *opt, const char *arg, int unset)
+{
+	struct rev_info *rev = (struct rev_info *)opt->value;
+	if (unset)
+		rev->mime_boundary = NULL;
+	else if (arg)
+		rev->mime_boundary = arg;
+	else
+		rev->mime_boundary = git_version_string;
+	rev->no_inline = 0;
+	return 0;
+}
+
+static int header_callback(const struct option *opt, const char *arg, int unset)
+{
+	add_header(arg);
+	return 0;
+}
+
+static int cc_callback(const struct option *opt, const char *arg, int unset)
+{
+	ALLOC_GROW(extra_cc, extra_cc_nr + 1, extra_cc_alloc);
+	extra_cc[extra_cc_nr++] = xstrdup(arg);
+	return 0;
+}
+
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
 	struct commit *commit;
 	struct commit **list = NULL;
 	struct rev_info rev;
-	int nr = 0, total, i, j;
+	int nr = 0, total, i;
 	int use_stdout = 0;
 	int start_number = -1;
-	int keep_subject = 0;
 	int numbered_files = 0;		/* _just_ numbers */
-	int subject_prefix = 0;
 	int ignore_if_in_upstream = 0;
-	int thread = 0;
 	int cover_letter = 0;
 	int boundary_count = 0;
 	int no_binary_diff = 0;
@@ -775,6 +900,61 @@ 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;
+	int use_patch_format = 0;
+	const struct option builtin_format_patch_options[] = {
+		{ OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
+			    "use [PATCH n/m] even with a single patch",
+			    PARSE_OPT_NOARG, numbered_callback },
+		{ OPTION_CALLBACK, 'N', "no-numbered", &numbered, NULL,
+			    "use [PATCH] even with multiple patches",
+			    PARSE_OPT_NOARG, no_numbered_callback },
+		OPT_BOOLEAN('s', "signoff", &do_signoff, "add Signed-off-by:"),
+		OPT_BOOLEAN(0, "stdout", &use_stdout,
+			    "print patches to standard out"),
+		OPT_BOOLEAN(0, "cover-letter", &cover_letter,
+			    "generate a cover letter"),
+		OPT_BOOLEAN(0, "numbered-files", &numbered_files,
+			    "use simple number sequence for output file names"),
+		OPT_STRING(0, "suffix", &fmt_patch_suffix, "sfx",
+			    "use <sfx> instead of '.patch'"),
+		OPT_INTEGER(0, "start-number", &start_number,
+			    "start numbering patches at <n> instead of 1"),
+		{ OPTION_CALLBACK, 0, "subject-prefix", &rev, "prefix",
+			    "Use [<prefix>] instead of [PATCH]",
+			    PARSE_OPT_NONEG, subject_prefix_callback },
+		{ OPTION_CALLBACK, 'o', "output-directory", &output_directory,
+			    "dir", "store resulting files in <dir>",
+			    PARSE_OPT_NONEG, output_directory_callback },
+		{ OPTION_CALLBACK, 'k', "keep-subject", &rev, NULL,
+			    "don't strip/add [PATCH]",
+			    PARSE_OPT_NOARG | PARSE_OPT_NONEG, keep_callback },
+		OPT_BOOLEAN(0, "no-binary", &no_binary_diff,
+			    "don't output binary diffs"),
+		OPT_BOOLEAN(0, "ignore-if-in-upstream", &ignore_if_in_upstream,
+			    "don't include a patch matching a commit upstream"),
+		{ OPTION_BOOLEAN, 'p', "no-stat", &use_patch_format, NULL,
+		  "show patch format instead of default (patch + stat)",
+		  PARSE_OPT_NONEG | PARSE_OPT_NOARG },
+		OPT_GROUP("Messaging"),
+		{ OPTION_CALLBACK, 0, "add-header", NULL, "header",
+			    "add email header", PARSE_OPT_NONEG,
+			    header_callback },
+		{ OPTION_CALLBACK, 0, "cc", NULL, "email", "add Cc: header",
+			    PARSE_OPT_NONEG, cc_callback },
+		OPT_STRING(0, "in-reply-to", &in_reply_to, "message-id",
+			    "make first mail a reply to <message-id>"),
+		{ OPTION_CALLBACK, 0, "attach", &rev, "boundary",
+			    "attach the patch", PARSE_OPT_OPTARG,
+			    attach_callback },
+		{ OPTION_CALLBACK, 0, "inline", &rev, "boundary",
+			    "inline the patch",
+			    PARSE_OPT_OPTARG | PARSE_OPT_NONEG,
+			    inline_callback },
+		{ OPTION_CALLBACK, 0, "thread", &thread, "style",
+			    "enable message threading, styles: shallow, deep",
+			    PARSE_OPT_OPTARG, thread_callback },
+		OPT_END()
+	};
 
 	git_config(git_format_config, NULL);
 	init_revisions(&rev, prefix);
@@ -787,100 +967,30 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 	rev.subject_prefix = fmt_patch_subject_prefix;
 
+	if (default_attach) {
+		rev.mime_boundary = default_attach;
+		rev.no_inline = 1;
+	}
+
 	/*
 	 * Parse the arguments before setup_revisions(), or something
 	 * like "git format-patch -o a123 HEAD^.." may fail; a123 is
 	 * possibly a valid SHA1.
 	 */
-	for (i = 1, j = 1; i < argc; i++) {
-		if (!strcmp(argv[i], "--stdout"))
-			use_stdout = 1;
-		else if (!strcmp(argv[i], "-n") ||
-				!strcmp(argv[i], "--numbered"))
-			numbered = 1;
-		else if (!strcmp(argv[i], "-N") ||
-				!strcmp(argv[i], "--no-numbered")) {
-			numbered = 0;
-			auto_number = 0;
-		}
-		else if (!prefixcmp(argv[i], "--start-number="))
-			start_number = strtol(argv[i] + 15, NULL, 10);
-		else if (!strcmp(argv[i], "--numbered-files"))
-			numbered_files = 1;
-		else if (!strcmp(argv[i], "--start-number")) {
-			i++;
-			if (i == argc)
-				die("Need a number for --start-number");
-			start_number = strtol(argv[i], NULL, 10);
-		}
-		else if (!prefixcmp(argv[i], "--cc=")) {
-			ALLOC_GROW(extra_cc, extra_cc_nr + 1, extra_cc_alloc);
-			extra_cc[extra_cc_nr++] = xstrdup(argv[i] + 5);
-		}
-		else if (!strcmp(argv[i], "-k") ||
-				!strcmp(argv[i], "--keep-subject")) {
-			keep_subject = 1;
-			rev.total = -1;
-		}
-		else if (!strcmp(argv[i], "--output-directory") ||
-			 !strcmp(argv[i], "-o")) {
-			i++;
-			if (argc <= i)
-				die("Which directory?");
-			if (output_directory)
-				die("Two output directories?");
-			output_directory = argv[i];
-		}
-		else if (!strcmp(argv[i], "--signoff") ||
-			 !strcmp(argv[i], "-s")) {
-			const char *committer;
-			const char *endpos;
-			committer = git_committer_info(IDENT_ERROR_ON_NO_NAME);
-			endpos = strchr(committer, '>');
-			if (!endpos)
-				die("bogus committer info %s", committer);
-			add_signoff = xmemdupz(committer, endpos - committer + 1);
-		}
-		else if (!strcmp(argv[i], "--attach")) {
-			rev.mime_boundary = git_version_string;
-			rev.no_inline = 1;
-		}
-		else if (!prefixcmp(argv[i], "--attach=")) {
-			rev.mime_boundary = argv[i] + 9;
-			rev.no_inline = 1;
-		}
-		else if (!strcmp(argv[i], "--inline")) {
-			rev.mime_boundary = git_version_string;
-			rev.no_inline = 0;
-		}
-		else if (!prefixcmp(argv[i], "--inline=")) {
-			rev.mime_boundary = argv[i] + 9;
-			rev.no_inline = 0;
-		}
-		else if (!strcmp(argv[i], "--ignore-if-in-upstream"))
-			ignore_if_in_upstream = 1;
-		else if (!strcmp(argv[i], "--thread"))
-			thread = 1;
-		else if (!prefixcmp(argv[i], "--in-reply-to="))
-			in_reply_to = argv[i] + 14;
-		else if (!strcmp(argv[i], "--in-reply-to")) {
-			i++;
-			if (i == argc)
-				die("Need a Message-Id for --in-reply-to");
-			in_reply_to = argv[i];
-		} else if (!prefixcmp(argv[i], "--subject-prefix=")) {
-			subject_prefix = 1;
-			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 if (!strcmp(argv[i], "--no-binary"))
-			no_binary_diff = 1;
-		else
-			argv[j++] = argv[i];
+	argc = parse_options(argc, argv, prefix, builtin_format_patch_options,
+			     builtin_format_patch_usage,
+			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
+			     PARSE_OPT_KEEP_DASHDASH);
+
+	if (do_signoff) {
+		const char *committer;
+		const char *endpos;
+		committer = git_committer_info(IDENT_ERROR_ON_NO_NAME);
+		endpos = strchr(committer, '>');
+		if (!endpos)
+			die("bogus committer info %s", committer);
+		add_signoff = xmemdupz(committer, endpos - committer + 1);
 	}
-	argc = j;
 
 	for (i = 0; i < extra_hdr_nr; i++) {
 		strbuf_addstr(&buf, extra_hdr[i]);
@@ -909,10 +1019,19 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		strbuf_addch(&buf, '\n');
 	}
 
-	rev.extra_headers = strbuf_detach(&buf, 0);
+	rev.extra_headers = strbuf_detach(&buf, NULL);
 
 	if (start_number < 0)
 		start_number = 1;
+
+	/*
+	 * If numbered is set solely due to format.numbered in config,
+	 * and it would conflict with --keep-subject (-k) from the
+	 * command line, reset "numbered".
+	 */
+	if (numbered && keep_subject && !numbered_cmdline_opt)
+		numbered = 0;
+
 	if (numbered && keep_subject)
 		die ("-n and -k are mutually exclusive.");
 	if (keep_subject && subject_prefix)
@@ -922,9 +1041,20 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (argc > 1)
 		die ("unrecognized argument: %s", argv[1]);
 
-	if (!rev.diffopt.output_format
-		|| rev.diffopt.output_format == DIFF_FORMAT_PATCH)
-		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY | DIFF_FORMAT_PATCH;
+	if (rev.diffopt.output_format & DIFF_FORMAT_NAME)
+		die("--name-only does not make sense");
+	if (rev.diffopt.output_format & DIFF_FORMAT_NAME_STATUS)
+		die("--name-status does not make sense");
+	if (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+		die("--check does not make sense");
+
+	if (!use_patch_format &&
+		(!rev.diffopt.output_format ||
+		 rev.diffopt.output_format == DIFF_FORMAT_PATCH))
+		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY;
+
+	/* Always generate a patch */
+	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
 
 	if (!DIFF_OPT_TST(&rev.diffopt, TEXT) && !no_binary_diff)
 		DIFF_OPT_SET(&rev.diffopt, BINARY);
@@ -936,8 +1066,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (use_stdout)
 			die("standard output, or directory, which one?");
 		if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
-			die("Could not create directory %s",
-			    output_directory);
+			die_errno("Could not create directory '%s'",
+				  output_directory);
 	}
 
 	if (rev.pending.nr == 1) {
@@ -959,7 +1089,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 	/*
 	 * We cannot move this anywhere earlier because we do want to
-	 * know if --root was given explicitly from the comand line.
+	 * know if --root was given explicitly from the command line.
 	 */
 	rev.show_root_diff = 1;
 
@@ -1009,8 +1139,14 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		numbered = 1;
 	if (numbered)
 		rev.total = total + start_number - 1;
-	if (in_reply_to)
-		rev.ref_message_id = clean_message_id(in_reply_to);
+	if (in_reply_to || thread || cover_letter)
+		rev.ref_message_ids = xcalloc(1, sizeof(struct string_list));
+	if (in_reply_to) {
+		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");
@@ -1029,21 +1165,39 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			/* 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.
+				 * For deep threading: make every mail
+				 * a reply to the previous one, no
+				 * matter what other options are set.
+				 *
+				 * For shallow threading:
+				 *
+				 * Without --cover-letter and
+				 * --in-reply-to, make every mail a
+				 * reply to the one before.
+				 *
+				 * With --in-reply-to but no
+				 * --cover-letter, make every mail a
+				 * reply to the <reply-to>.
+				 *
+				 * With --cover-letter, make every
+				 * mail but the cover letter a reply
+				 * to the cover letter.  The cover
+				 * letter is a reply to the
+				 * --in-reply-to, if specified.
 				 */
-				if (rev.ref_message_id)
+				if (thread == THREAD_SHALLOW
+				    && rev.ref_message_ids->nr > 0
+				    && (!cover_letter || rev.nr > 1))
 					free(rev.message_id);
 				else
-					rev.ref_message_id = rev.message_id;
+					string_list_append(rev.message_id,
+							   rev.ref_message_ids);
 			}
 			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.total))
+
+		if (!use_stdout && reopen_stdout(numbered_files ? NULL : commit,
+						 &rev))
 			die("Failed to create output files");
 		shown = log_tree_commit(&rev, commit);
 		free(commit->buffer);
@@ -1108,6 +1262,9 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 		argv++;
 	}
 
+	if (argc > 1 && !strcmp(argv[1], "-h"))
+		usage(cherry_usage);
+
 	switch (argc) {
 	case 4:
 		limit = argv[3];
@@ -1175,8 +1332,9 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 
 		if (verbose) {
 			struct strbuf buf = STRBUF_INIT;
+			struct pretty_print_context ctx = {0};
 			pretty_print_commit(CMIT_FMT_ONELINE, commit,
-			                    &buf, 0, NULL, NULL, 0, 0);
+					    &buf, &ctx);
 			printf("%c %s %s\n", sign,
 			       sha1_to_hex(commit->object.sha1), buf.buf);
 			strbuf_release(&buf);