Bug in "git diff --quiet" handling.

I came across this when I was mistakenly thinking I could use "--quiet" to stop the output of git format-patch giving the 0001-somecommit.patch names as it generated them.

It was then when I read the man page and found it is passed into git diff and is supposed to disable all output. The interesting thing, is that it only seems to do this on every other commit it format-patches, as shown below.

I'm assuming this is a bug, since I can't imagine what use case would have every alternate patch being output as useful. If I get a chance, I'll take a look at the code and see if I can figure out what is going on, but I figured I'd mention it 1st in case it triggered an "Oh yeah it is XYZ" in someones memory.

Paul.

---

Script started on Mon 11 Apr 2011 04:50:22 PM EDT

paul@dv2000:~/git/git$ git --version
git version 1.7.4.4
paul@dv2000:~/git/git$ git format-patch --quiet -o foo HEAD~10..HEAD
paul@dv2000:~/git/git$ ls -l foo
total 60
-rw-r--r-- 1 paul paul 2960 2011-04-11 16:50 0001-fetch-pull-recurse-into-submodules-when-necessary.patch
-rw-r--r-- 1 paul paul    0 2011-04-11 16:50 0002-fetch-pull-Add-the-on-demand-value-to-the-recurse-su.patch
-rw-r--r-- 1 paul paul 1514 2011-04-11 16:50 0003-config-teach-the-fetch.recurseSubmodules-option-the-.patch
-rw-r--r-- 1 paul paul    0 2011-04-11 16:50 0004-Submodules-Add-on-demand-value-for-the-fetchRecurseS.patch
-rw-r--r-- 1 paul paul 1391 2011-04-11 16:50 0005-fetch-pull-Don-t-recurse-into-a-submodule-when-commi.patch
-rw-r--r-- 1 paul paul    0 2011-04-11 16:50 0006-submodule-update-Don-t-fetch-when-the-submodule-comm.patch
-rw-r--r-- 1 paul paul 1381 2011-04-11 16:50 0007-fetch-pull-Describe-recurse-submodule-restrictions-i.patch
-rw-r--r-- 1 paul paul    0 2011-04-11 16:50 0008-remote-disallow-some-nonsensical-option-combinations.patch
-rw-r--r-- 1 paul paul 3801 2011-04-11 16:50 0009-remote-separate-the-concept-of-push-and-fetch-mirror.patch
-rw-r--r-- 1 paul paul    0 2011-04-11 16:50 0010-remote-deprecate-mirror.patch
-rw-r--r-- 1 paul paul 3172 2011-04-11 16:50 0011-submodule-process-conflicting-submodules-only-once.patch
-rw-r--r-- 1 paul paul    0 2011-04-11 16:50 0012-revisions.txt-consistent-use-of-quotes.patch
-rw-r--r-- 1 paul paul 9679 2011-04-11 16:50 0013-revisions.txt-structure-with-a-labelled-list.patch
-rw-r--r-- 1 paul paul    0 2011-04-11 16:50 0014-log-cherry-pick-documentation-regression-fix.patch
-rw-r--r-- 1 paul paul 1826 2011-04-11 16:50 0015-compat-add-missing-include-sys-resource.h.patch
-rw-r--r-- 1 paul paul    0 2011-04-11 16:50 0016-pull-do-not-clobber-untracked-files-on-initial-pull.patch
-rw-r--r-- 1 paul paul 1743 2011-04-11 16:50 0017-Start-preparing-for-1.7.4.4.patch
-rw-r--r-- 1 paul paul    0 2011-04-11 16:50 0018-gitweb-Fix-parsing-of-negative-fractional-timezones-.patch
-rw-r--r-- 1 paul paul 1089 2011-04-11 16:50 0019-Documentation-trivial-grammar-fix-in-core.worktree-d.patch
-rw-r--r-- 1 paul paul    0 2011-04-11 16:50 0020-revisions.txt-language-improvements.patch
-rw-r--r-- 1 paul paul 1114 2011-04-11 16:50 0021-Git-1.7.4.4.patch
-rw-r--r-- 1 paul paul    0 2011-04-11 16:50 0022-Git-1.7.5-rc1.patch
-rw-r--r-- 1 paul paul 4996 2011-04-11 16:50 0023-git-p4-replace-each-tab-with-8-spaces-for-consistenc.patch
paul@dv2000:~/git/git$ exit
exit
Script done on Mon 11 Apr 2011 04:51:34 PM EDT

Yeah, it sounds like you found an interesting one.

As far as I know, whatever "format-patch" does in response to "--quiet" option is not a deliberate and designed behaviour, as squelching the patch output in the context of the command does not make much sense [*1*]; the current implementation simply writes anything off as an user error when "format-patch --quiet" did anything "interesting" ;-).

A patch to make --quiet not to squelch the patch output, and instead silence any progress output would be a good addition.

Thanks.

[Footnote]

*1* Also note that at least in the original design, the standard output from "format-patch" was never meant to be squelched. It was the only way the calling scripts (and humans) can learn under what filenames the patches were output, so that the command line to fire them off as e-mails can be programatically formed without running "ls" and filtering non-patch files manually (if you use "format-patch -o newdir" and newdir did not have anythning in it before running the command, of course you can rely on the output from "ls").

The --quiet flag is not meant to be passed on to the diff, as the user always wants the patches to be produced so catch it and pass it to reopen_stdout which decides whether to print the filename or not.

Noticed by Paul Gortmaker

---

Something like this? I guess the only use case would be together with -o.

 builtin/log.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

index 9a15d69..1ce00ba 100644
@@ -623,7 +623,7 @@ static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
 static int outdir_offset;
 
-static int reopen_stdout(struct commit *commit, struct rev_info *rev)
+static int reopen_stdout(struct commit *commit, struct rev_info *rev, int quiet)
 {
 	struct strbuf filename = STRBUF_INIT;
 	int suffix_len = strlen(fmt_patch_suffix) + 1;

@@ -639,7 +639,7 @@ static int reopen_stdout(struct commit *commit, struct rev_info *rev)
 
 	get_patch_filename(commit, rev->nr, fmt_patch_suffix, &filename);
 
-	if (!DIFF_OPT_TST(&rev->diffopt, QUICK))
+	if (!quiet)
 		fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
 
 	if (freopen(filename.buf, "w", stdout) == NULL)

@@ -718,7 +718,8 @@ static void print_signature(void)
 static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			      int numbered, int numbered_files,
 			      struct commit *origin,
-			      int nr, struct commit **list, struct commit *head)
+			      int nr, struct commit **list, struct commit *head,
+			      int quiet)
 {
 	const char *committer;
 	const char *subject_start = NULL;

@@ -754,7 +755,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			sha1_to_hex(head->object.sha1), committer, committer);
 	}
 
-	if (!use_stdout && reopen_stdout(commit, rev))
+	if (!use_stdout && reopen_stdout(commit, rev, quiet))
 		return;
 
 	if (commit) {

@@ -995,6 +996,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	char *add_signoff = NULL;
 	struct strbuf buf = STRBUF_INIT;
 	int use_patch_format = 0;
+	int quiet = 0;
 	const struct option builtin_format_patch_options[] = {
 		{ OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
 			    "use [PATCH n/m] even with a single patch",

@@ -1050,6 +1052,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			    PARSE_OPT_OPTARG, thread_callback },
 		OPT_STRING(0, "signature", &signature, "signature",
 			    "add a signature"),
+		OPT_BOOLEAN(0, "quiet", &quiet,
+			    "don't print the patch filenames"),
 		OPT_END()
 	};
 

@@ -1259,7 +1263,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (thread)
 			gen_message_id(&rev, "cover");
 		make_cover_letter(&rev, use_stdout, numbered, numbered_files,
-				  origin, nr, list, head);
+				  origin, nr, list, head, quiet);
 		total++;
 		start_number--;
 	}

@@ -1305,7 +1309,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		}
 
 		if (!use_stdout && reopen_stdout(numbered_files ? NULL : commit,
-						 &rev))
+						 &rev, quiet))
 			die("Failed to create output files");
 		shown = log_tree_commit(&rev, commit);
 		free(commit->buffer);

1.7.4.2.437.g4fc7e.dirty

---

I guess this should be squashed into the previous one. I forgot it wasn't documented, partly because reading the commit log for ec2956df59 (Nate Case, format-patch: Respect --quiet option) says the man page suggests this should work.

 Documentation/git-format-patch.txt |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

index 9dcafc6..616726b 100644
@@ -20,7 +20,7 @@ SYNOPSIS
 		   [--ignore-if-in-upstream]
 		   [--subject-prefix=Subject-Prefix]
 		   [--to=<email>] [--cc=<email>]
-		   [--cover-letter]
+		   [--cover-letter] [--quiet]
 		   [<common diff options>]
 		   [ <since> | <revision range> ]
 

@@ -192,6 +192,9 @@ will want to ensure that threading is disabled for `git send-email`.
 	filenames, use specified suffix.  A common alternative is
 	`--suffix=.txt`.  Leaving this empty will remove the `.patch`
 	suffix.
+
+--quiet::
+	Do not print the patch names to standard output.
 +
 Note that the leading character does not have to be a dot; for example,
 you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.

1.7.4.2.437.g4fc7e.dirty

I don't think the manual page ever meant to say anything like that. It used to include generic "diff" options from manual pages from diff-tree and friends, but the "output" the description refers to is the diff output and later dropped by protecting the description in diff-options.txt with "ifndef::git-format-patch[]" in the Documentation/ sources.

Also the existing "Note that ..." that followed your added lines actually belong to the description of the previous item. Thusly....

-- >8 --
From: Carlos Martín Nieto <[email protected]>
Subject: [PATCH] format-patch: document --quiet option

---
 Documentation/git-format-patch.txt |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

index a5525e9..f4e959d 100644
@@ -20,7 +20,7 @@ SYNOPSIS
 		   [--ignore-if-in-upstream]
 		   [--subject-prefix=Subject-Prefix]
 		   [--to=<email>] [--cc=<email>]
-		   [--cover-letter]
+		   [--cover-letter] [--quiet]
 		   [<common diff options>]
 		   [ <since> | <revision range> ]
 

@@ -196,6 +196,9 @@ will want to ensure that threading is disabled for `git send-email`.
 Note that the leading character does not have to be a dot; for example,
 you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.
 
+--quiet::
+	Do not print the names of the generated files to standard output.
+
 --no-binary::
 	Do not output contents of changes in binary files, instead
 	display a notice that those files changed.  Patches generated

1.7.5.rc1.16.g9db19

Oops, sorry. I blindly took the indentation change to mean it was something unrelated. Thanks for fixing it.

cmn

When the user gives -q without giving -o to a new or an empty directory, the user deserves to get what was asked on the command line, so I wouldn't worry about this particular case. For a casual user, it is perfectly a sensible thing to say "I'll eyeball; I don't have other files whose names begin with [0-9]{4}- in my working tree" and I don't think we need safety against doing that.

I however wonder if we should audit other commands in the "log" family to see what they do when "--quiet" is given. I know what they do currently is whatever they happen to do for a nonsense request, and in no way is a designed behaviour. We simply did never think about that case.

For example, what should "git show master^2 next^2" do with "--quiet"? Of course the standard way to squelch diff output in the output from "show" is to use "-s" (coming from "git diff-tree"), but giving "--quiet" should at least be a no-op.

Agreed.

We get a similar effect to format-patch, and a line disappears

    carlos@bee:~/apps/git$ git show --oneline origin/master^2 origin/next^2
    9973d93 t2021: mark a test as fixed
    diff --git a/t/t2021-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh
    index 27db2ad..5da63e9 100755
    --- a/t/t2021-checkout-overwrite.sh
    +++ b/t/t2021-checkout-overwrite.sh
    @@ -39,7 +39,7 @@ test_expect_success SYMLINKS 'create a commit where dir a/b changed to symlink'
	[...] Diff here
    9db1941 Merge branch 'js/checkout-untracked-symlink'

and

    carlos@bee:~/apps/git$ git show --oneline --quiet origin/master^2 origin/next^2
    9973d93 t2021: mark a test as fixed

so we certainly should catch it.

I'm not so sure what we should do with it, though. We shouldn't squelch all the output, because then it just makes the command useless (though I guess the user asked for it in that case). Maybe making it behave like a --no-diff option would make sense (i.e. pretend the user passed -s) in order to make it behave like a prettier version of rev-parse.

Looking at the other cmd_ functions in builtin/log.c I see:
- reflog ignores it
- cherry complains that it doesn't know about the option
- log -p --quiet is the same as log
- whatchanged --quiet shows the same as log but skips every second commit
- show --quiet skips one of the commits

and I don't see any others, so whatchanged should be tought not to skip the commits. I'll what cmd_log does differently from cmd_whatchanged when passing options.

cmn

If --quiet is passed and there is no patch output, log_tree_commit will not print the log which is certainly not wanted.

Set the always_show_header option to fix this.

---

With this, "--quiet" just means the same as "-s" by telling log_tree_commit to output it. I still haven't completely understood what the relationship between log_tree_commit, log_tree_diff and log_tree_diff_flush is but AFAICS sometimes one function shows the log and sometimes the other one shows it, which I guess has to do with the QUICK option to diff.

I'm sending this now because it's a one-liner and is probably the correct behaviour anyway, but a more general solution would be to convert cmd_log_init to use the option parser and catch --quiet there, maybe even making it mean the same as -s.

 builtin/log.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

index 1ce00ba..b24ca8a 100644
@@ -322,6 +322,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
 	init_revisions(&rev, prefix);
 	rev.diff = 1;
 	rev.simplify_history = 0;
+	rev.always_show_header = 1;
 	memset(&opt, 0, sizeof(opt));
 	opt.def = "HEAD";
 	cmd_log_init(argc, argv, prefix, &rev, &opt);

1.7.4.2.437.g4fc7e.dirty

I don't think that is correct. The command should skip empty commits, no?

I'll take a look at this later when I have time. I plan to tag -rc2 today.

Use parse-options in cmd_log_init instead of manually iterating through them. This makes the code a bit cleaner but more importantly allows us to catch the "--quiet" option which causes some of the log-related commands to misbehave as it would otherwise get passed on to the diff.

---

This "fixes" the previous --quiet effects by not letting it through. Later we can decide to make it mean the same as -s or just leave it like that.

 builtin/log.c |   77 ++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 49 insertions(+), 28 deletions(-)

index 9a15d69..5316be3 100644
@@ -25,6 +25,7 @@ static const char *default_date_mode = NULL;
 
 static int default_show_root = 1;
 static int decoration_style;
+static int decoration_given = 0;
 static const char *fmt_patch_subject_prefix = "PATCH";
 static const char *fmt_pretty;
 

@@ -49,12 +50,51 @@ static int parse_decoration_style(const char *var, const char *value)
 	return -1;
 }
 
+static int decorate_callback(const struct option *opt, const char *arg, int unset)
+{
+	if (unset) {
+		decoration_style = 0;
+		return 0;
+	}
+
+	if (arg == NULL) {
+		decoration_style = DECORATE_SHORT_REFS;
+		decoration_given = 1;
+		return 0;
+	}
+
+	/* First arg is irrelevant, as it just tries to parse arg */
+	decoration_style = parse_decoration_style("decorate", arg);
+	if (decoration_style < 0)
+		die("invalid --decorate option: %s", arg);
+
+	decoration_given = 1;
+
+	return 0;
+}
+
 static void cmd_log_init(int argc, const char **argv, const char *prefix,
 			 struct rev_info *rev, struct setup_revision_opt *opt)
 {
-	int i;
-	int decoration_given = 0;
 	struct userformat_want w;
+	int help, quiet, source;
+
+	const struct option builtin_log_options[] = {
+		OPT_BOOLEAN(0, "h", &help, "show help"),
+		OPT_BOOLEAN(0, "quiet", &quiet, "supress diff output"),
+		OPT_BOOLEAN(0, "source", &source, "show source"),
+		{ OPTION_CALLBACK, 0, "decorate", NULL, NULL, "decorate options",
+		  PARSE_OPT_OPTARG, decorate_callback},
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, builtin_log_options,
+						 builtin_log_usage,
+						 PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
+						 PARSE_OPT_KEEP_DASHDASH);
+
+	if (help)
+		usage(builtin_log_usage);
 
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;

@@ -69,14 +109,12 @@ 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, opt);
 
+	/* Any arguments at this point are not recognized */
+	if (argc > 1)
+		die("unrecognized argument: %s", argv[1]);
+
 	memset(&w, 0, sizeof(w));
 	userformat_find_requirements(NULL, &w);
 

@@ -92,26 +130,9 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		if (rev->diffopt.nr_paths != 1)
 			usage("git logs can only follow renames on one pathname at a time");
 	}
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-		if (!strcmp(arg, "--decorate")) {
-			decoration_style = DECORATE_SHORT_REFS;
-			decoration_given = 1;
-		} else if (!prefixcmp(arg, "--decorate=")) {
-			const char *v = skip_prefix(arg, "--decorate=");
-			decoration_style = parse_decoration_style(arg, v);
-			if (decoration_style < 0)
-				die("invalid --decorate option: %s", arg);
-			decoration_given = 1;
-		} else if (!strcmp(arg, "--no-decorate")) {
-			decoration_style = 0;
-		} 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 (source)
+		rev->show_source = 1;
 
 	/*
 	 * defeat log.decorate configuration interacting with --pretty=raw

1.7.4.2.437.g4fc7e.dirty

Use parse-options in cmd_log_init instead of manually iterating through them. This makes the code a bit cleaner but more importantly allows us to catch the "--quiet" option which causes some of the log-related commands to misbehave as it would otherwise get passed on to the diff.

Also take this opportinity to add 'whatchanged' to the help output.

---

Ok, I've changed this to use the version in your fixup patch.

git_config_long doesn't die with an error (though git_config_int does, if git_config_long isn't successful) but returns 0 if it can't parse the value. git_config_maybe_bool notices this and returns -1, which parse_decoration_style interprets as "was not boolean" and tries to match "short" or "full". If it can't, then it returns -1 and decoration_callback dies with the error message.

Be it as it may, gt_config_long doesn't output any error message at all, so what we pass is irrelevant, unless we want to future-proof it because we don't trust the git_config_maybe_bool call to let us handle the error ourselves in the future.

Done.

Indeed, removed.

As the help is generated from the option struct, I've changed the comment a bit to me more helpful and I've added 'whatchanged' to the usage message, as it looks bad if 'git whatchanged -h' doesn't tell you about itself.

I'm not completely sure what "show source" is meant to be, I think it's the source of merges, which could be added to that explanation, I guess.

 Documentation/git-format-patch.txt |    5 ++-
 builtin/log.c                      |   77 ++++++++++++++++++++++--------------
 2 files changed, 51 insertions(+), 31 deletions(-)

index 9a15d69..d1b0861 100644
@@ -25,12 +25,16 @@ static const char *default_date_mode = NULL;
 
 static int default_show_root = 1;
 static int decoration_style;
+static int decoration_given;
 static const char *fmt_patch_subject_prefix = "PATCH";
 static const char *fmt_pretty;
 
-static const char * const builtin_log_usage =
+static const char * const builtin_log_usage[] = {
 	"git log [<options>] [<since>..<until>] [[--] <path>...]\n"
-	"   or: git show [options] <object>...";
+	"   or: git show [options] <object>...\n"
+	"   or: git whatchanged [options] <object>...",
+	NULL
+};
 
 static int parse_decoration_style(const char *var, const char *value)
 {

@@ -49,12 +53,44 @@ static int parse_decoration_style(const char *var, const char *value)
 	return -1;
 }
 
+static int decorate_callback(const struct option *opt, const char *arg, int unset)
+{
+	if (unset)
+		decoration_style = 0;
+	else if (arg)
+		decoration_style = parse_decoration_style("decorate", arg);
+	else
+		decoration_style = DECORATE_SHORT_REFS;
+
+	if (decoration_style < 0)
+		die("invalid --decorate option: %s", arg);
+
+	decoration_given = 1;
+
+	return 0;
+}
+
 static void cmd_log_init(int argc, const char **argv, const char *prefix,
 			 struct rev_info *rev, struct setup_revision_opt *opt)
 {
-	int i;
-	int decoration_given = 0;
 	struct userformat_want w;
+	int dummy, source;
+
+	/*
+	 * The 'quiet' option is a dummy no-op to stop it from propagating
+	 * to the diff option parsing.
+	 */
+	const struct option builtin_log_options[] = {
+		OPT_BOOLEAN(0, "quiet", &dummy, "no-op, provided for compatability"),
+		OPT_BOOLEAN(0, "source", &source, "show source"),
+		{ OPTION_CALLBACK, 0, "decorate", NULL, NULL, "decoration options (default: short)",
+		  PARSE_OPT_OPTARG, decorate_callback},
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, builtin_log_options, builtin_log_usage,
+						 PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
+						 PARSE_OPT_KEEP_DASHDASH);
 
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;

@@ -69,14 +105,12 @@ 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, opt);
 
+	/* Any arguments at this point are not recognized */
+	if (argc > 1)
+		die("unrecognized argument: %s", argv[1]);
+
 	memset(&w, 0, sizeof(w));
 	userformat_find_requirements(NULL, &w);
 

@@ -92,26 +126,9 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		if (rev->diffopt.nr_paths != 1)
 			usage("git logs can only follow renames on one pathname at a time");
 	}
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-		if (!strcmp(arg, "--decorate")) {
-			decoration_style = DECORATE_SHORT_REFS;
-			decoration_given = 1;
-		} else if (!prefixcmp(arg, "--decorate=")) {
-			const char *v = skip_prefix(arg, "--decorate=");
-			decoration_style = parse_decoration_style(arg, v);
-			if (decoration_style < 0)
-				die("invalid --decorate option: %s", arg);
-			decoration_given = 1;
-		} else if (!strcmp(arg, "--no-decorate")) {
-			decoration_style = 0;
-		} 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 (source)
+		rev->show_source = 1;
 
 	/*
 	 * defeat log.decorate configuration interacting with --pretty=raw

1.7.4.2.437.g4fc7e.dirty

This is not about your patch at all, but rather that I notice in your "From" header that your name is doubly rfc2047-encoded. It looks like this:

From: =?us-ascii?B?PT9VVEYtOD9xP0Nhcmxvcz0yME1hcnQ9QzM9QURuPTIwTmlldG8/?=

    =?us-ascii?Q?=3D?= <[email protected]>

which decodes to the literal string:

=?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?=

which in turn decodes again to your proper name.

We made some changes to format-patch's quoting recently, and I want to make sure this is not a regression. Can you describe your workflow for sending these patches? What I think probably happened is:

1. format-patch encoded your name because of the non-ascii characters

2. the result was fed literally into mutt via cut-and-paste or otherwise pulled into the editor, rather than "mutt -f patch-file".

Which is not a regression, but just an annoying behavior that has been there for a while[1]. But I wanted to double-check.

-Peff

[1] Probably the solution is to let people with a workflow like that tell format-patch to give them the literal utf8 instead of encoding the header.

Ah, so that's what been going on.

As I was only sending one patch, I did what the intertubes suggested and used "mutt -H patch-file" which I guess that's the problem. I only noticed this because someone mentioned it, after I sent another patch.

So no regression, and a --leave-utf8-alone option would be useful here.

Cheers,
cmn

Yes, from the man page, they should be skipped unless "-m" is passed.

Maybe unconditionally unsetting QUICK would give the desired result. Tomorrow I should be done changing cmd_log_init to use the parse options API so it's easier to catch "--quiet".

cmn