{"thread":{"id":"27064","subject":"Bug in \"git diff --quiet\" handling.","startedAt":"2011-04-11T21:07:33Z","lastAt":"2011-04-20T14:51:20Z","messageCount":17,"participants":["Paul Gortmaker","Junio C Hamano","Carlos Martín Nieto","=?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?=","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"165650","messageId":"4DA36D95.6060108@windriver.com","threadId":"27064","inReplyTo":null,"subject":"Bug in \"git diff --quiet\" handling.","fromName":"Paul Gortmaker","fromEmail":"paul.gortmaker@windriver.com","sentAt":"2011-04-11T21:07:33Z","receivedAt":"2011-04-11T21:07:33Z","isPatch":false,"sender":{"key":"paul.gortmaker@windriver.com","avatar":null},"body":"I came across this when I was mistakenly thinking I could use \"--quiet\" to\nstop the output of git format-patch giving the 0001-somecommit.patch names as\nit generated them.\n\nIt was then when I read the man page and found it is passed into git diff and\nis supposed to disable all output. The interesting thing, is that it only\nseems to do this on every other commit it format-patches, as shown\nbelow.\n\nI'm assuming this is a bug, since I can't imagine what use case would have\nevery alternate patch being output as useful. If I get a chance, I'll take\na look at the code and see if I can figure out what is going on, but I\nfigured I'd mention it 1st in case it triggered an \"Oh yeah it is XYZ\"\nin someones memory.\n\nPaul.\n\n---\n\nScript started on Mon 11 Apr 2011 04:50:22 PM EDT\npaul@dv2000:~/git/git$ git --version\ngit version 1.7.4.4\npaul@dv2000:~/git/git$ git format-patch --quiet -o foo HEAD~10..HEAD\npaul@dv2000:~/git/git$ ls -l foo\ntotal 60\n-rw-r--r-- 1 paul paul 2960 2011-04-11 16:50 0001-fetch-pull-recurse-into-submodules-when-necessary.patch\n-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\n-rw-r--r-- 1 paul paul 1514 2011-04-11 16:50 0003-config-teach-the-fetch.recurseSubmodules-option-the-.patch\n-rw-r--r-- 1 paul paul 0 2011-04-11 16:50 0004-Submodules-Add-on-demand-value-for-the-fetchRecurseS.patch\n-rw-r--r-- 1 paul paul 1391 2011-04-11 16:50 0005-fetch-pull-Don-t-recurse-into-a-submodule-when-commi.patch\n-rw-r--r-- 1 paul paul 0 2011-04-11 16:50 0006-submodule-update-Don-t-fetch-when-the-submodule-comm.patch\n-rw-r--r-- 1 paul paul 1381 2011-04-11 16:50 0007-fetch-pull-Describe-recurse-submodule-restrictions-i.patch\n-rw-r--r-- 1 paul paul 0 2011-04-11 16:50 0008-remote-disallow-some-nonsensical-option-combinations.patch\n-rw-r--r-- 1 paul paul 3801 2011-04-11 16:50 0009-remote-separate-the-concept-of-push-and-fetch-mirror.patch\n-rw-r--r-- 1 paul paul 0 2011-04-11 16:50 0010-remote-deprecate-mirror.patch\n-rw-r--r-- 1 paul paul 3172 2011-04-11 16:50 0011-submodule-process-conflicting-submodules-only-once.patch\n-rw-r--r-- 1 paul paul 0 2011-04-11 16:50 0012-revisions.txt-consistent-use-of-quotes.patch\n-rw-r--r-- 1 paul paul 9679 2011-04-11 16:50 0013-revisions.txt-structure-with-a-labelled-list.patch\n-rw-r--r-- 1 paul paul 0 2011-04-11 16:50 0014-log-cherry-pick-documentation-regression-fix.patch\n-rw-r--r-- 1 paul paul 1826 2011-04-11 16:50 0015-compat-add-missing-include-sys-resource.h.patch\n-rw-r--r-- 1 paul paul 0 2011-04-11 16:50 0016-pull-do-not-clobber-untracked-files-on-initial-pull.patch\n-rw-r--r-- 1 paul paul 1743 2011-04-11 16:50 0017-Start-preparing-for-1.7.4.4.patch\n-rw-r--r-- 1 paul paul 0 2011-04-11 16:50 0018-gitweb-Fix-parsing-of-negative-fractional-timezones-.patch\n-rw-r--r-- 1 paul paul 1089 2011-04-11 16:50 0019-Documentation-trivial-grammar-fix-in-core.worktree-d.patch\n-rw-r--r-- 1 paul paul 0 2011-04-11 16:50 0020-revisions.txt-language-improvements.patch\n-rw-r--r-- 1 paul paul 1114 2011-04-11 16:50 0021-Git-1.7.4.4.patch\n-rw-r--r-- 1 paul paul 0 2011-04-11 16:50 0022-Git-1.7.5-rc1.patch\n-rw-r--r-- 1 paul paul 4996 2011-04-11 16:50 0023-git-p4-replace-each-tab-with-8-spaces-for-consistenc.patch\npaul@dv2000:~/git/git$ exit\nexit\nScript done on Mon 11 Apr 2011 04:51:34 PM EDT\n"},{"id":"165651","messageId":"7v8vvgv5dm.fsf@alter.siamese.dyndns.org","threadId":"27064","inReplyTo":"4DA36D95.6060108@windriver.com","subject":"Re: Bug in \"git diff --quiet\" handling.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-11T21:35:01Z","receivedAt":"2011-04-11T21:35:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Gortmaker writes:\n\n> I'm assuming this is a bug,...\n\nYeah, it sounds like you found an interesting one.\n\nAs far as I know, whatever \"format-patch\" does in response to \"--quiet\"\noption is not a deliberate and designed behaviour, as squelching the patch\noutput in the context of the command does not make much sense [*1*]; the\ncurrent implementation simply writes anything off as an user error when\n\"format-patch --quiet\" did anything \"interesting\" ;-).\n\nA patch to make --quiet not to squelch the patch output, and instead\nsilence any progress output would be a good addition.\n\nThanks.\n\n[Footnote]\n\n*1* Also note that at least in the original design, the standard output\nfrom \"format-patch\" was never meant to be squelched. It was the only way\nthe calling scripts (and humans) can learn under what filenames the\npatches were output, so that the command line to fire them off as e-mails\ncan be programatically formed without running \"ls\" and filtering non-patch\nfiles manually (if you use \"format-patch -o newdir\" and newdir did not\nhave anythning in it before running the command, of course you can rely on\nthe output from \"ls\").\n"},{"id":"165690","messageId":"1302622538-7535-1-git-send-email-cmn@elego.de","threadId":"27064","inReplyTo":"7v8vvgv5dm.fsf@alter.siamese.dyndns.org","subject":"[PATCH] format-patch: don't pass on the --quiet flag","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-04-12T15:35:38Z","receivedAt":"2011-04-12T15:35:38Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"The --quiet flag is not meant to be passed on to the diff, as the user\nalways wants the patches to be produced so catch it and pass it to\nreopen_stdout which decides whether to print the filename or not.\n\nNoticed by Paul Gortmaker\n\nSigned-off-by: Carlos Martín Nieto \n---\n> A patch to make --quiet not to squelch the patch output, and instead\n> silence any progress output would be a good addition.\n\nSomething like this? I guess the only use case would be together with\n-o.\n\n builtin/log.c | 16 ++++++++++------\n 1 files changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 9a15d69..1ce00ba 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -623,7 +623,7 @@ static FILE *realstdout = NULL;\n static const char *output_directory = NULL;\n static int outdir_offset;\n \n-static int reopen_stdout(struct commit *commit, struct rev_info *rev)\n+static int reopen_stdout(struct commit *commit, struct rev_info *rev, int quiet)\n {\n \tstruct strbuf filename = STRBUF_INIT;\n \tint suffix_len = strlen(fmt_patch_suffix) + 1;\n@@ -639,7 +639,7 @@ static int reopen_stdout(struct commit *commit, struct rev_info *rev)\n \n \tget_patch_filename(commit, rev->nr, fmt_patch_suffix, &filename);\n \n-\tif (!DIFF_OPT_TST(&rev->diffopt, QUICK))\n+\tif (!quiet)\n \t\tfprintf(realstdout, \"%s\\n\", filename.buf + outdir_offset);\n \n \tif (freopen(filename.buf, \"w\", stdout) == NULL)\n@@ -718,7 +718,8 @@ static void print_signature(void)\n static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\t\t int numbered, int numbered_files,\n \t\t\t struct commit *origin,\n-\t\t\t int nr, struct commit **list, struct commit *head)\n+\t\t\t int nr, struct commit **list, struct commit *head,\n+\t\t\t int quiet)\n {\n \tconst char *committer;\n \tconst char *subject_start = NULL;\n@@ -754,7 +755,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\t\tsha1_to_hex(head->object.sha1), committer, committer);\n \t}\n \n-\tif (!use_stdout && reopen_stdout(commit, rev))\n+\tif (!use_stdout && reopen_stdout(commit, rev, quiet))\n \t\treturn;\n \n \tif (commit) {\n@@ -995,6 +996,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tchar *add_signoff = NULL;\n \tstruct strbuf buf = STRBUF_INIT;\n \tint use_patch_format = 0;\n+\tint quiet = 0;\n \tconst struct option builtin_format_patch_options[] = {\n \t\t{ OPTION_CALLBACK, 'n', \"numbered\", &numbered, NULL,\n \t\t\t \"use [PATCH n/m] even with a single patch\",\n@@ -1050,6 +1052,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_OPTARG, thread_callback },\n \t\tOPT_STRING(0, \"signature\", &signature, \"signature\",\n \t\t\t \"add a signature\"),\n+\t\tOPT_BOOLEAN(0, \"quiet\", &quiet,\n+\t\t\t \"don't print the patch filenames\"),\n \t\tOPT_END()\n \t};\n \n@@ -1259,7 +1263,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (thread)\n \t\t\tgen_message_id(&rev, \"cover\");\n \t\tmake_cover_letter(&rev, use_stdout, numbered, numbered_files,\n-\t\t\t\t origin, nr, list, head);\n+\t\t\t\t origin, nr, list, head, quiet);\n \t\ttotal++;\n \t\tstart_number--;\n \t}\n@@ -1305,7 +1309,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\tif (!use_stdout && reopen_stdout(numbered_files ? NULL : commit,\n-\t\t\t\t\t\t &rev))\n+\t\t\t\t\t\t &rev, quiet))\n \t\t\tdie(\"Failed to create output files\");\n \t\tshown = log_tree_commit(&rev, commit);\n \t\tfree(commit->buffer);\n-- \n1.7.4.2.437.g4fc7e.dirty\n"},{"id":"165694","messageId":"1302623497-7658-1-git-send-email-cmn@elego.de","threadId":"27064","inReplyTo":"1302622538-7535-1-git-send-email-cmn@elego.de","subject":"[PATCH] format-patch: document --quiet option","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-04-12T15:51:37Z","receivedAt":"2011-04-12T15:51:37Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Signed-off-by: Carlos Martín Nieto \n---\n\nI guess this should be squashed into the previous one. I forgot it\nwasn't documented, partly because reading the commit log for\nec2956df59 (Nate Case, format-patch: Respect --quiet option) says the\nman page suggests this should work.\n\n Documentation/git-format-patch.txt | 5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 9dcafc6..616726b 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -20,7 +20,7 @@ SYNOPSIS\n \t\t [--ignore-if-in-upstream]\n \t\t [--subject-prefix=Subject-Prefix]\n \t\t [--to=] [--cc=]\n-\t\t [--cover-letter]\n+\t\t [--cover-letter] [--quiet]\n \t\t []\n \t\t [ | ]\n \n@@ -192,6 +192,9 @@ will want to ensure that threading is disabled for `git send-email`.\n \tfilenames, use specified suffix. A common alternative is\n \t`--suffix=.txt`. Leaving this empty will remove the `.patch`\n \tsuffix.\n+\n+--quiet::\n+\tDo not print the patch names to standard output.\n +\n Note that the leading character does not have to be a dot; for example,\n you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.\n-- \n1.7.4.2.437.g4fc7e.dirty\n"},{"id":"165705","messageId":"7vk4ezpacr.fsf@alter.siamese.dyndns.org","threadId":"27064","inReplyTo":"1302622538-7535-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH] format-patch: don't pass on the --quiet flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-12T18:56:20Z","receivedAt":"2011-04-12T18:56:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto writes:\n\n>> A patch to make --quiet not to squelch the patch output, and instead\n>> silence any progress output would be a good addition.\n>\n> Something like this? I guess the only use case would be together with\n> -o.\n\nWhen the user gives -q without giving -o to a new or an empty directory,\nthe user deserves to get what was asked on the command line, so I wouldn't\nworry about this particular case. For a casual user, it is perfectly a\nsensible thing to say \"I'll eyeball; I don't have other files whose names\nbegin with [0-9]{4}- in my working tree\" and I don't think we need safety\nagainst doing that.\n\nI however wonder if we should audit other commands in the \"log\" family to\nsee what they do when \"--quiet\" is given. I know what they do currently\nis whatever they happen to do for a nonsense request, and in no way is a\ndesigned behaviour. We simply did never think about that case.\n\nFor example, what should \"git show master^2 next^2\" do with \"--quiet\"? Of\ncourse the standard way to squelch diff output in the output from \"show\"\nis to use \"-s\" (coming from \"git diff-tree\"), but giving \"--quiet\" should\nat least be a no-op.\n"},{"id":"165706","messageId":"7vfwpnp9uf.fsf@alter.siamese.dyndns.org","threadId":"27064","inReplyTo":"1302623497-7658-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH] format-patch: document --quiet option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-12T19:07:20Z","receivedAt":"2011-04-12T19:07:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto writes:\n\n> Signed-off-by: Carlos Martín Nieto \n> ---\n>\n> I guess this should be squashed into the previous one. I forgot it\n> wasn't documented, partly because reading the commit log for\n> ec2956df59 (Nate Case, format-patch: Respect --quiet option) says the\n> man page suggests this should work.\n\nI don't think the manual page ever meant to say anything like that. It\nused to include generic \"diff\" options from manual pages from diff-tree\nand friends, but the \"output\" the description refers to is the diff\noutput and later dropped by protecting the description in diff-options.txt\nwith \"ifndef::git-format-patch[]\" in the Documentation/ sources.\n\n> @@ -192,6 +192,9 @@ will want to ensure that threading is disabled for `git send-email`.\n> \tfilenames, use specified suffix. A common alternative is\n> \t`--suffix=.txt`. Leaving this empty will remove the `.patch`\n> \tsuffix.\n> +\n> +--quiet::\n> +\tDo not print the patch names to standard output.\n\nI see \"filenames\" in the context and that is \"generated filenames\".\n\nBe consistent and don't introduce an undefined term \"patch names\" here; it\nwill lead to confusing readers to think as if the \"generated filenames\"\nand \"patch names\" are different things.\n"},{"id":"165708","messageId":"7v8vvfp7rd.fsf@alter.siamese.dyndns.org","threadId":"27064","inReplyTo":"7vfwpnp9uf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] format-patch: document --quiet option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-12T19:52:22Z","receivedAt":"2011-04-12T19:52:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> @@ -192,6 +192,9 @@ will want to ensure that threading is disabled for `git send-email`.\n>> \tfilenames, use specified suffix. A common alternative is\n>> \t`--suffix=.txt`. Leaving this empty will remove the `.patch`\n>> \tsuffix.\n>> +\n>> +--quiet::\n>> +\tDo not print the patch names to standard output.\n>\n> I see \"filenames\" in the context and that is \"generated filenames\".\n> ...\n\nAlso the existing \"Note that ...\" that followed your added lines actually\nbelong to the description of the previous item. Thusly....\n\n-- >8 --\nFrom: Carlos Martín Nieto \nSubject: [PATCH] format-patch: document --quiet option\n\nSigned-off-by: Carlos Martín Nieto \nSigned-off-by: Junio C Hamano \n---\n Documentation/git-format-patch.txt | 5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex a5525e9..f4e959d 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -20,7 +20,7 @@ SYNOPSIS\n \t\t [--ignore-if-in-upstream]\n \t\t [--subject-prefix=Subject-Prefix]\n \t\t [--to=] [--cc=]\n-\t\t [--cover-letter]\n+\t\t [--cover-letter] [--quiet]\n \t\t []\n \t\t [ | ]\n \n@@ -196,6 +196,9 @@ will want to ensure that threading is disabled for `git send-email`.\n Note that the leading character does not have to be a dot; for example,\n you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.\n \n+--quiet::\n+\tDo not print the names of the generated files to standard output.\n+\n --no-binary::\n \tDo not output contents of changes in binary files, instead\n \tdisplay a notice that those files changed. Patches generated\n-- \n1.7.5.rc1.16.g9db19\n"},{"id":"165729","messageId":"20110413092620.GA3649@bee.lab.cmartin.tk","threadId":"27064","inReplyTo":"7vk4ezpacr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] format-patch: don't pass on the --quiet flag","fromName":"Carlos Martín Nieto","fromEmail":"carlos@cmartin.tk","sentAt":"2011-04-13T09:26:20Z","receivedAt":"2011-04-13T09:26:20Z","isPatch":true,"sender":{"key":"carlos@cmartin.tk","avatar":"https://gravatar.com/avatar/956bfe8371004f2960febf266a6af789f60cdc01fbae48bb151ad4c9b532c3a2?d=mp&s=160"},"body":"On Tue, Apr 12, 2011 at 11:56:20AM -0700, Junio C Hamano wrote:\n> Carlos Martín Nieto writes:\n> \n> >> A patch to make --quiet not to squelch the patch output, and instead\n> >> silence any progress output would be a good addition.\n> >\n> > Something like this? I guess the only use case would be together with\n> > -o.\n> \n> When the user gives -q without giving -o to a new or an empty directory,\n> the user deserves to get what was asked on the command line, so I wouldn't\n> worry about this particular case. For a casual user, it is perfectly a\n> sensible thing to say \"I'll eyeball; I don't have other files whose names\n> begin with [0-9]{4}- in my working tree\" and I don't think we need safety\n> against doing that.\n> \n\n Agreed.\n\n> I however wonder if we should audit other commands in the \"log\" family to\n> see what they do when \"--quiet\" is given. I know what they do currently\n> is whatever they happen to do for a nonsense request, and in no way is a\n> designed behaviour. We simply did never think about that case.\n> \n> For example, what should \"git show master^2 next^2\" do with \"--quiet\"? Of\n> course the standard way to squelch diff output in the output from \"show\"\n> is to use \"-s\" (coming from \"git diff-tree\"), but giving \"--quiet\" should\n> at least be a no-op.\n> \n\n We get a similar effect to format-patch, and a line disappears\n\n carlos@bee:~/apps/git$ git show --oneline origin/master^2 origin/next^2\n 9973d93 t2021: mark a test as fixed\n diff --git a/t/t2021-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh\n index 27db2ad..5da63e9 100755\n --- a/t/t2021-checkout-overwrite.sh\n +++ b/t/t2021-checkout-overwrite.sh\n @@ -39,7 +39,7 @@ test_expect_success SYMLINKS 'create a commit where dir a/b changed to symlink'\n\t[...] Diff here\n 9db1941 Merge branch 'js/checkout-untracked-symlink'\n\nand\n\n carlos@bee:~/apps/git$ git show --oneline --quiet origin/master^2 origin/next^2\n 9973d93 t2021: mark a test as fixed\n\nso we certainly should catch it.\n\nI'm not so sure what we should do with it, though. We shouldn't\nsquelch all the output, because then it just makes the command useless\n(though I guess the user asked for it in that case). Maybe making it\nbehave like a --no-diff option would make sense (i.e. pretend the user\npassed -s) in order to make it behave like a prettier version of\nrev-parse.\n\nLooking at the other cmd_ functions in builtin/log.c I see:\n - reflog ignores it\n - cherry complains that it doesn't know about the option\n - log -p --quiet is the same as log\n - whatchanged --quiet shows the same as log but skips every second\n commit\n - show --quiet skips one of the commits\n\nand I don't see any others, so whatchanged should be tought not to\nskip the commits. I'll what cmd_log does differently from\ncmd_whatchanged when passing options.\n\n cmn\n"},{"id":"165730","messageId":"20110413092920.GB3649@bee.lab.cmartin.tk","threadId":"27064","inReplyTo":"7v8vvfp7rd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] format-patch: document --quiet option","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-04-13T09:29:20Z","receivedAt":"2011-04-13T09:29:20Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Tue, Apr 12, 2011 at 12:52:22PM -0700, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> >> @@ -192,6 +192,9 @@ will want to ensure that threading is disabled for `git send-email`.\n> >> \tfilenames, use specified suffix. A common alternative is\n> >> \t`--suffix=.txt`. Leaving this empty will remove the `.patch`\n> >> \tsuffix.\n> >> +\n> >> +--quiet::\n> >> +\tDo not print the patch names to standard output.\n> >\n> > I see \"filenames\" in the context and that is \"generated filenames\".\n> > ...\n> \n> Also the existing \"Note that ...\" that followed your added lines actually\n> belong to the description of the previous item. Thusly....\n> \n\n Oops, sorry. I blindly took the indentation change to mean it was\n something unrelated. Thanks for fixing it.\n\n cmn\n"},{"id":"165738","messageId":"1302696644-21809-1-git-send-email-cmn@elego.de","threadId":"27064","inReplyTo":"20110413092620.GA3649@bee.lab.cmartin.tk","subject":"[PATCH] whatchanged: always show the header","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-04-13T12:10:44Z","receivedAt":"2011-04-13T12:10:44Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"If --quiet is passed and there is no patch output, log_tree_commit\nwill not print the log which is certainly not wanted.\n\nSet the always_show_header option to fix this.\n\nSigned-off-by: Carlos Martín Nieto \n---\n\nWith this, \"--quiet\" just means the same as \"-s\" by telling\nlog_tree_commit to output it. I still haven't completely understood\nwhat the relationship between log_tree_commit, log_tree_diff and\nlog_tree_diff_flush is but AFAICS sometimes one function shows the log\nand sometimes the other one shows it, which I guess has to do with the\nQUICK option to diff.\n\nI'm sending this now because it's a one-liner and is probably the\ncorrect behaviour anyway, but a more general solution would be to\nconvert cmd_log_init to use the option parser and catch --quiet there,\nmaybe even making it mean the same as -s.\n\n builtin/log.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 1ce00ba..b24ca8a 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -322,6 +322,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)\n \tinit_revisions(&rev, prefix);\n \trev.diff = 1;\n \trev.simplify_history = 0;\n+\trev.always_show_header = 1;\n \tmemset(&opt, 0, sizeof(opt));\n \topt.def = \"HEAD\";\n \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n-- \n1.7.4.2.437.g4fc7e.dirty\n"},{"id":"165756","messageId":"7vlizem9bx.fsf@alter.siamese.dyndns.org","threadId":"27064","inReplyTo":"1302696644-21809-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH] whatchanged: always show the header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-13T15:58:58Z","receivedAt":"2011-04-13T15:58:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto writes:\n\n> Set the always_show_header option to fix this.\n\nI don't think that is correct. The command should skip empty commits, no?\n\nI'll take a look at this later when I have time. I plan to tag -rc2\ntoday.\n"},{"id":"165772","messageId":"20110413193830.GA8560@bee.lab.cmartin.tk","threadId":"27064","inReplyTo":"7vlizem9bx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] whatchanged: always show the header","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-04-13T19:38:32Z","receivedAt":"2011-04-13T19:38:32Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Wed, Apr 13, 2011 at 08:58:58AM -0700, Junio C Hamano wrote:\n> Carlos Martín Nieto writes:\n> \n> > Set the always_show_header option to fix this.\n> \n> I don't think that is correct. The command should skip empty commits, no?\n\nYes, from the man page, they should be skipped unless \"-m\" is passed.\n\n> \n> I'll take a look at this later when I have time. I plan to tag -rc2\n> today.\n\nMaybe unconditionally unsetting QUICK would give the desired\nresult. Tomorrow I should be done changing cmd_log_init to use the\nparse options API so it's easier to catch \"--quiet\".\n\n cmn\n \n"},{"id":"165813","messageId":"1302791310-19640-1-git-send-email-cmn@elego.de","threadId":"27064","inReplyTo":"7vlizem9bx.fsf@alter.siamese.dyndns.org","subject":"[PATCH] log: convert to parse-options","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-04-14T14:28:30Z","receivedAt":"2011-04-14T14:28:30Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Use parse-options in cmd_log_init instead of manually iterating\nthrough them. This makes the code a bit cleaner but more importantly\nallows us to catch the \"--quiet\" option which causes some of the\nlog-related commands to misbehave as it would otherwise get passed on\nto the diff.\n\nSigned-off-by: Carlos Martín Nieto \n---\n\nThis \"fixes\" the previous --quiet effects by not letting it\nthrough. Later we can decide to make it mean the same as -s or just\nleave it like that.\n\n builtin/log.c | 77 ++++++++++++++++++++++++++++++++++++--------------------\n 1 files changed, 49 insertions(+), 28 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 9a15d69..5316be3 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -25,6 +25,7 @@ static const char *default_date_mode = NULL;\n \n static int default_show_root = 1;\n static int decoration_style;\n+static int decoration_given = 0;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n static const char *fmt_pretty;\n \n@@ -49,12 +50,51 @@ static int parse_decoration_style(const char *var, const char *value)\n \treturn -1;\n }\n \n+static int decorate_callback(const struct option *opt, const char *arg, int unset)\n+{\n+\tif (unset) {\n+\t\tdecoration_style = 0;\n+\t\treturn 0;\n+\t}\n+\n+\tif (arg == NULL) {\n+\t\tdecoration_style = DECORATE_SHORT_REFS;\n+\t\tdecoration_given = 1;\n+\t\treturn 0;\n+\t}\n+\n+\t/* First arg is irrelevant, as it just tries to parse arg */\n+\tdecoration_style = parse_decoration_style(\"decorate\", arg);\n+\tif (decoration_style < 0)\n+\t\tdie(\"invalid --decorate option: %s\", arg);\n+\n+\tdecoration_given = 1;\n+\n+\treturn 0;\n+}\n+\n static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \t\t\t struct rev_info *rev, struct setup_revision_opt *opt)\n {\n-\tint i;\n-\tint decoration_given = 0;\n \tstruct userformat_want w;\n+\tint help, quiet, source;\n+\n+\tconst struct option builtin_log_options[] = {\n+\t\tOPT_BOOLEAN(0, \"h\", &help, \"show help\"),\n+\t\tOPT_BOOLEAN(0, \"quiet\", &quiet, \"supress diff output\"),\n+\t\tOPT_BOOLEAN(0, \"source\", &source, \"show source\"),\n+\t\t{ OPTION_CALLBACK, 0, \"decorate\", NULL, NULL, \"decorate options\",\n+\t\t PARSE_OPT_OPTARG, decorate_callback},\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, builtin_log_options,\n+\t\t\t\t\t\t builtin_log_usage,\n+\t\t\t\t\t\t PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |\n+\t\t\t\t\t\t PARSE_OPT_KEEP_DASHDASH);\n+\n+\tif (help)\n+\t\tusage(builtin_log_usage);\n \n \trev->abbrev = DEFAULT_ABBREV;\n \trev->commit_format = CMIT_FMT_DEFAULT;\n@@ -69,14 +109,12 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \tif (default_date_mode)\n \t\trev->date_mode = parse_date_format(default_date_mode);\n \n-\t/*\n-\t * Check for -h before setup_revisions(), or \"git log -h\" will\n-\t * fail when run without a git directory.\n-\t */\n-\tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n-\t\tusage(builtin_log_usage);\n \targc = setup_revisions(argc, argv, rev, opt);\n \n+\t/* Any arguments at this point are not recognized */\n+\tif (argc > 1)\n+\t\tdie(\"unrecognized argument: %s\", argv[1]);\n+\n \tmemset(&w, 0, sizeof(w));\n \tuserformat_find_requirements(NULL, &w);\n \n@@ -92,26 +130,9 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \t\tif (rev->diffopt.nr_paths != 1)\n \t\t\tusage(\"git logs can only follow renames on one pathname at a time\");\n \t}\n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n-\t\tif (!strcmp(arg, \"--decorate\")) {\n-\t\t\tdecoration_style = DECORATE_SHORT_REFS;\n-\t\t\tdecoration_given = 1;\n-\t\t} else if (!prefixcmp(arg, \"--decorate=\")) {\n-\t\t\tconst char *v = skip_prefix(arg, \"--decorate=\");\n-\t\t\tdecoration_style = parse_decoration_style(arg, v);\n-\t\t\tif (decoration_style < 0)\n-\t\t\t\tdie(\"invalid --decorate option: %s\", arg);\n-\t\t\tdecoration_given = 1;\n-\t\t} else if (!strcmp(arg, \"--no-decorate\")) {\n-\t\t\tdecoration_style = 0;\n-\t\t} else if (!strcmp(arg, \"--source\")) {\n-\t\t\trev->show_source = 1;\n-\t\t} else if (!strcmp(arg, \"-h\")) {\n-\t\t\tusage(builtin_log_usage);\n-\t\t} else\n-\t\t\tdie(\"unrecognized argument: %s\", arg);\n-\t}\n+\n+\tif (source)\n+\t\trev->show_source = 1;\n \n \t/*\n \t * defeat log.decorate configuration interacting with --pretty=raw\n-- \n1.7.4.2.437.g4fc7e.dirty\n"},{"id":"165817","messageId":"7v7hawiww7.fsf@alter.siamese.dyndns.org","threadId":"27064","inReplyTo":"1302791310-19640-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH] log: convert to parse-options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-14T17:08:08Z","receivedAt":"2011-04-14T17:08:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto writes:\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 9a15d69..5316be3 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -25,6 +25,7 @@ static const char *default_date_mode = NULL;\n> \n> static int default_show_root = 1;\n> static int decoration_style;\n> +static int decoration_given = 0;\n\nWe prefer to leave zero-initialization of statics to the linker, similarly\nto how we initialize decoration_style to zero in the line above.\n\n> +static int decorate_callback(const struct option *opt, const char *arg, int unset)\n> +{\n> +\tif (unset) {\n> +\t\tdecoration_style = 0;\n> +\t\treturn 0;\n> +\t}\n\nThis is not a new issue, but I do not think the original code did not mean\nto keep decoration_given unmodified when --no-decorate was given from the\ncommand line. The variable is about \"did we get any --decorate related\noptions from the command line to override whatever log.decorate variable\nsays?\", and when we saw --no-decorate, we did get such an override from\nthe command line. We should consistently set _given variable to 1 here.\n\nIt is immaterial that it happens not to matter to the current user of the\nvariable that sets decoration_style to zero. The next user of _given may\nwant to do other things.\n\n> +\tif (arg == NULL) {\n> +\t\tdecoration_style = DECORATE_SHORT_REFS;\n> +\t\tdecoration_given = 1;\n> +\t\treturn 0;\n> +\t}\n> +\n> +\t/* First arg is irrelevant, as it just tries to parse arg */\n> +\tdecoration_style = parse_decoration_style(\"decorate\", arg);\n\nIt is used in the error message in git_config_long() in the callchain from\nhere, primarily meant to report which configuration variable had a bad\nvalue, so it is in no way irrelevant. We need to say that a bad value\ncomes not from a configuration but from the command line; get_color() in\nbuiltin/config.c passes \"command line\" for this exact reason.\n\n> static void cmd_log_init(int argc, const char **argv, const char *prefix,\n> \t\t\t struct rev_info *rev, struct setup_revision_opt *opt)\n> {\n> -\tint i;\n> -\tint decoration_given = 0;\n> \tstruct userformat_want w;\n> +\tint help, quiet, source;\n> +\n> +\tconst struct option builtin_log_options[] = {\n> +\t\tOPT_BOOLEAN(0, \"h\", &help, \"show help\"),\n> +\t\tOPT_BOOLEAN(0, \"quiet\", &quiet, \"supress diff output\"),\n> +\t\tOPT_BOOLEAN(0, \"source\", &source, \"show source\"),\n> +\t\t{ OPTION_CALLBACK, 0, \"decorate\", NULL, NULL, \"decorate options\",\n> +\t\t PARSE_OPT_OPTARG, decorate_callback},\n> +\t\tOPT_END()\n> +\t};\n> +\n> +\targc = parse_options(argc, argv, prefix, builtin_log_options,\n> +\t\t\t\t\t\t builtin_log_usage,\n> +\t\t\t\t\t\t PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |\n> +\t\t\t\t\t\t PARSE_OPT_KEEP_DASHDASH);\n\nThe 5th parameter is an array of strings terminated with a NULL element.\n\n> +\tif (help)\n> +\t\tusage(builtin_log_usage);\n\nI think parse_options() handles -h and --help itself, so there is no\nlonger need for this.\n\nHow about this fix-up patch on top of your version?\n\n builtin/log.c | 36 ++++++++++++++----------------------\n 1 files changed, 14 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 5316be3..80766a9 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -25,13 +25,15 @@ static const char *default_date_mode = NULL;\n \n static int default_show_root = 1;\n static int decoration_style;\n-static int decoration_given = 0;\n+static int decoration_given;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n static const char *fmt_pretty;\n \n-static const char * const builtin_log_usage =\n+static const char * const builtin_log_usage[] = {\n \t\"git log [] [..] [[--] ...]\\n\"\n-\t\" or: git show [options] ...\";\n+\t\" or: git show [options] ...\",\n+\tNULL\n+};\n \n static int parse_decoration_style(const char *var, const char *value)\n {\n@@ -52,19 +54,13 @@ static int parse_decoration_style(const char *var, const char *value)\n \n static int decorate_callback(const struct option *opt, const char *arg, int unset)\n {\n-\tif (unset) {\n+\tif (unset)\n \t\tdecoration_style = 0;\n-\t\treturn 0;\n-\t}\n-\n-\tif (arg == NULL) {\n+\telse if (arg)\n+\t\tdecoration_style = parse_decoration_style(\"command line\", arg);\n+\telse\n \t\tdecoration_style = DECORATE_SHORT_REFS;\n-\t\tdecoration_given = 1;\n-\t\treturn 0;\n-\t}\n \n-\t/* First arg is irrelevant, as it just tries to parse arg */\n-\tdecoration_style = parse_decoration_style(\"decorate\", arg);\n \tif (decoration_style < 0)\n \t\tdie(\"invalid --decorate option: %s\", arg);\n \n@@ -77,10 +73,9 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \t\t\t struct rev_info *rev, struct setup_revision_opt *opt)\n {\n \tstruct userformat_want w;\n-\tint help, quiet, source;\n+\tint quiet, source;\n \n \tconst struct option builtin_log_options[] = {\n-\t\tOPT_BOOLEAN(0, \"h\", &help, \"show help\"),\n \t\tOPT_BOOLEAN(0, \"quiet\", &quiet, \"supress diff output\"),\n \t\tOPT_BOOLEAN(0, \"source\", &source, \"show source\"),\n \t\t{ OPTION_CALLBACK, 0, \"decorate\", NULL, NULL, \"decorate options\",\n@@ -88,13 +83,10 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \t\tOPT_END()\n \t};\n \n-\targc = parse_options(argc, argv, prefix, builtin_log_options,\n-\t\t\t\t\t\t builtin_log_usage,\n-\t\t\t\t\t\t PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |\n-\t\t\t\t\t\t PARSE_OPT_KEEP_DASHDASH);\n-\n-\tif (help)\n-\t\tusage(builtin_log_usage);\n+\targc = parse_options(argc, argv, prefix,\n+\t\t\t builtin_log_options, builtin_log_usage,\n+\t\t\t PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |\n+\t\t\t PARSE_OPT_KEEP_DASHDASH);\n \n \trev->abbrev = DEFAULT_ABBREV;\n \trev->commit_format = CMIT_FMT_DEFAULT;\n"},{"id":"166082","messageId":"20110419123325.GA10814@bee.lab.cmartin.tk","threadId":"27064","inReplyTo":"7v7hawiww7.fsf@alter.siamese.dyndns.org","subject":"[PATCH] log: convert to parse-options","fromName":"=?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?=","fromEmail":"cmn@elego.de","sentAt":"2011-04-19T12:33:31Z","receivedAt":"2011-04-19T12:33:31Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Use parse-options in cmd_log_init instead of manually iterating\nthrough them. This makes the code a bit cleaner but more importantly\nallows us to catch the \"--quiet\" option which causes some of the\nlog-related commands to misbehave as it would otherwise get passed on\nto the diff.\n\nAlso take this opportinity to add 'whatchanged' to the help output.\n\nSigned-off-by: Carlos Martín Nieto \n---\n\nCarlos Martín Nieto writes:\n\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index 9a15d69..5316be3 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -25,6 +25,7 @@ static const char *default_date_mode =3D3D NULL;\n>>\n>> static int default_show_root =3D3D 1;\n>> static int decoration_style;\n>> +static int decoration_given =3D3D 0;\n>\n>We prefer to leave zero-initialization of statics to the linker, similarly\n>to how we initialize decoration_style to zero in the line above.\n>\n>> +static int decorate_callback(const struct option *opt, const char *arg, int unset)\n>> +{\n>> + if (unset) {\n>> + decoration_style = 0;\n>> + return 0;\n>>+ }\n\n> This is not a new issue, but I do not think the original code did not\n> mean to keep decoration_given unmodified when --no-decorate was given\n> from the command line. The variable is about \"did we get any\n> --decorate related options from the command line to override whatever\n> log.decorate variable says?\", and when we saw --no-decorate, we did\n> get such an override from the command line. We should consistently\n> set _given variable to 1 here.\n\n> It is immaterial that it happens not to matter to the current user of the\n> variable that sets decoration_style to zero. The next user of _given may\n> want to do other things.\n\nOk, I've changed this to use the version in your fixup patch.\n\n> + if (arg == NULL) {\n> + decoration_style = DECORATE_SHORT_REFS;\n> + decoration_given = 1;\n> + return 0;\n> + }\n> +\n> + /* First arg is irrelevant, as it just tries to parse arg */\n> + decoration_style = parse_decoration_style(\"decorate\", arg);\n\n> It is used in the error message in git_config_long() in the callchain from\n> here, primarily meant to report which configuration variable had a bad\n> value, so it is in no way irrelevant. We need to say that a bad value\n> comes not from a configuration but from the command line; get_color() in\n> builtin/config.c passes \"command line\" for this exact reason.\n> \n\ngit_config_long doesn't die with an error (though git_config_int does,\nif git_config_long isn't successful) but returns 0 if it can't parse\nthe value. git_config_maybe_bool notices this and returns -1, which\nparse_decoration_style interprets as \"was not boolean\" and tries to\nmatch \"short\" or \"full\". If it can't, then it returns -1 and\ndecoration_callback dies with the error message.\n\nBe it as it may, gt_config_long doesn't output any error message at\nall, so what we pass is irrelevant, unless we want to future-proof it\nbecause we don't trust the git_config_maybe_bool call to let us handle\nthe error ourselves in the future.\n\n>> static void cmd_log_init(int argc, const char **argv, const char *prefix,\n>> struct rev_info *rev, struct setup_revision_opt *opt)\n>> {\n>> - int i;\n>> - int decoration_given = 0;\n>> struct userformat_want w;\n>> + int help, quiet, source;\n>> +\n>> + const struct option builtin_log_options[] = {\n>> + OPT_BOOLEAN(0, \"h\", &help, \"show help\"),\n>> + OPT_BOOLEAN(0, \"quiet\", &quiet, \"supress diff output\"),\n>> + OPT_BOOLEAN(0, \"source\", &source, \"show source\"),\n>> + { OPTION_CALLBACK, 0, \"decorate\", NULL, NULL, \"decorate options\",\n>> + PARSE_OPT_OPTARG, decorate_callback},\n>> + OPT_END()\n>> + };\n>> +\n>> + argc = parse_options(argc, argv, prefix, builtin_log_options,\n>> + builtin_log_usage,\n>> + PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |\n>> + PARSE_OPT_KEEP_DASHDASH);\n\n> The 5th parameter is an array of strings terminated with a NULL element.\n\nDone.\n\n>> + if (help)\n>> + usage(builtin_log_usage);\n\n> I think parse_options() handles -h and --help itself, so there is no\n> longer need for this.\n\nIndeed, removed.\n\nAs the help is generated from the option struct, I've changed the\ncomment a bit to me more helpful and I've added 'whatchanged' to the\nusage message, as it looks bad if 'git whatchanged -h' doesn't tell\nyou about itself.\n\nI'm not completely sure what \"show source\" is meant to be, I think\nit's the source of merges, which could be added to that explanation, I guess.\n\n Documentation/git-format-patch.txt | 5 ++-\n builtin/log.c | 77 ++++++++++++++++++++++--------------\n 2 files changed, 51 insertions(+), 31 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 9a15d69..d1b0861 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -25,12 +25,16 @@ static const char *default_date_mode = NULL;\n \n static int default_show_root = 1;\n static int decoration_style;\n+static int decoration_given;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n static const char *fmt_pretty;\n \n-static const char * const builtin_log_usage =\n+static const char * const builtin_log_usage[] = {\n \t\"git log [] [..] [[--] ...]\\n\"\n-\t\" or: git show [options] ...\";\n+\t\" or: git show [options] ...\\n\"\n+\t\" or: git whatchanged [options] ...\",\n+\tNULL\n+};\n \n static int parse_decoration_style(const char *var, const char *value)\n {\n@@ -49,12 +53,44 @@ static int parse_decoration_style(const char *var, const char *value)\n \treturn -1;\n }\n \n+static int decorate_callback(const struct option *opt, const char *arg, int unset)\n+{\n+\tif (unset)\n+\t\tdecoration_style = 0;\n+\telse if (arg)\n+\t\tdecoration_style = parse_decoration_style(\"decorate\", arg);\n+\telse\n+\t\tdecoration_style = DECORATE_SHORT_REFS;\n+\n+\tif (decoration_style < 0)\n+\t\tdie(\"invalid --decorate option: %s\", arg);\n+\n+\tdecoration_given = 1;\n+\n+\treturn 0;\n+}\n+\n static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \t\t\t struct rev_info *rev, struct setup_revision_opt *opt)\n {\n-\tint i;\n-\tint decoration_given = 0;\n \tstruct userformat_want w;\n+\tint dummy, source;\n+\n+\t/*\n+\t * The 'quiet' option is a dummy no-op to stop it from propagating\n+\t * to the diff option parsing.\n+\t */\n+\tconst struct option builtin_log_options[] = {\n+\t\tOPT_BOOLEAN(0, \"quiet\", &dummy, \"no-op, provided for compatability\"),\n+\t\tOPT_BOOLEAN(0, \"source\", &source, \"show source\"),\n+\t\t{ OPTION_CALLBACK, 0, \"decorate\", NULL, NULL, \"decoration options (default: short)\",\n+\t\t PARSE_OPT_OPTARG, decorate_callback},\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, builtin_log_options, builtin_log_usage,\n+\t\t\t\t\t\t PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |\n+\t\t\t\t\t\t PARSE_OPT_KEEP_DASHDASH);\n \n \trev->abbrev = DEFAULT_ABBREV;\n \trev->commit_format = CMIT_FMT_DEFAULT;\n@@ -69,14 +105,12 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \tif (default_date_mode)\n \t\trev->date_mode = parse_date_format(default_date_mode);\n \n-\t/*\n-\t * Check for -h before setup_revisions(), or \"git log -h\" will\n-\t * fail when run without a git directory.\n-\t */\n-\tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n-\t\tusage(builtin_log_usage);\n \targc = setup_revisions(argc, argv, rev, opt);\n \n+\t/* Any arguments at this point are not recognized */\n+\tif (argc > 1)\n+\t\tdie(\"unrecognized argument: %s\", argv[1]);\n+\n \tmemset(&w, 0, sizeof(w));\n \tuserformat_find_requirements(NULL, &w);\n \n@@ -92,26 +126,9 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \t\tif (rev->diffopt.nr_paths != 1)\n \t\t\tusage(\"git logs can only follow renames on one pathname at a time\");\n \t}\n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n-\t\tif (!strcmp(arg, \"--decorate\")) {\n-\t\t\tdecoration_style = DECORATE_SHORT_REFS;\n-\t\t\tdecoration_given = 1;\n-\t\t} else if (!prefixcmp(arg, \"--decorate=\")) {\n-\t\t\tconst char *v = skip_prefix(arg, \"--decorate=\");\n-\t\t\tdecoration_style = parse_decoration_style(arg, v);\n-\t\t\tif (decoration_style < 0)\n-\t\t\t\tdie(\"invalid --decorate option: %s\", arg);\n-\t\t\tdecoration_given = 1;\n-\t\t} else if (!strcmp(arg, \"--no-decorate\")) {\n-\t\t\tdecoration_style = 0;\n-\t\t} else if (!strcmp(arg, \"--source\")) {\n-\t\t\trev->show_source = 1;\n-\t\t} else if (!strcmp(arg, \"-h\")) {\n-\t\t\tusage(builtin_log_usage);\n-\t\t} else\n-\t\t\tdie(\"unrecognized argument: %s\", arg);\n-\t}\n+\n+\tif (source)\n+\t\trev->show_source = 1;\n \n \t/*\n \t * defeat log.decorate configuration interacting with --pretty=raw\n-- \n1.7.4.2.437.g4fc7e.dirty\n"},{"id":"166115","messageId":"20110420023817.GA14201@sigill.intra.peff.net","threadId":"27064","inReplyTo":"20110419123325.GA10814@bee.lab.cmartin.tk","subject":"Re: [PATCH] log: convert to parse-options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-04-20T02:38:17Z","receivedAt":"2011-04-20T02:38:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 19, 2011 at 02:33:31PM +0200, =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= wrote:\n\n> Signed-off-by: Carlos Martín Nieto \n\nThis is not about your patch at all, but rather that I notice in your\n\"From\" header that your name is doubly rfc2047-encoded. It looks like\nthis:\n\n From: =?us-ascii?B?PT9VVEYtOD9xP0Nhcmxvcz0yME1hcnQ9QzM9QURuPTIwTmlldG8/?=\n =?us-ascii?Q?=3D?= \n\nwhich decodes to the literal string:\n\n =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?=\n\nwhich in turn decodes again to your proper name.\n\nWe made some changes to format-patch's quoting recently, and I want to\nmake sure this is not a regression. Can you describe your workflow for\nsending these patches? What I think probably happened is:\n\n 1. format-patch encoded your name because of the non-ascii characters\n\n 2. the result was fed literally into mutt via cut-and-paste or\n otherwise pulled into the editor, rather than \"mutt -f patch-file\".\n\nWhich is not a regression, but just an annoying behavior that has been\nthere for a while[1]. But I wanted to double-check.\n\n-Peff\n\n[1] Probably the solution is to let people with a workflow like that\ntell format-patch to give them the literal utf8 instead of encoding the\nheader.\n"},{"id":"166153","messageId":"20110420145120.GC5236@bee.lab.cmartin.tk","threadId":"27064","inReplyTo":"20110420023817.GA14201@sigill.intra.peff.net","subject":"Re: [PATCH] log: convert to parse-options","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-04-20T14:51:20Z","receivedAt":"2011-04-20T14:51:20Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Tue, Apr 19, 2011 at 10:38:17PM -0400, Jeff King wrote:\n> On Tue, Apr 19, 2011 at 02:33:31PM +0200, =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= wrote:\n> \n> > Signed-off-by: Carlos Martín Nieto \n> \n> This is not about your patch at all, but rather that I notice in your\n> \"From\" header that your name is doubly rfc2047-encoded. It looks like\n> this:\n> \n> From: =?us-ascii?B?PT9VVEYtOD9xP0Nhcmxvcz0yME1hcnQ9QzM9QURuPTIwTmlldG8/?=\n> =?us-ascii?Q?=3D?= \n> \n> which decodes to the literal string:\n> \n> =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?=\n> \n> which in turn decodes again to your proper name.\n> \n\nAh, so that's what been going on.\n\n> We made some changes to format-patch's quoting recently, and I want to\n> make sure this is not a regression. Can you describe your workflow for\n> sending these patches? What I think probably happened is:\n> \n> 1. format-patch encoded your name because of the non-ascii characters\n> \n> 2. the result was fed literally into mutt via cut-and-paste or\n> otherwise pulled into the editor, rather than \"mutt -f patch-file\".\n> \n> Which is not a regression, but just an annoying behavior that has been\n> there for a while[1]. But I wanted to double-check.\n\nAs I was only sending one patch, I did what the intertubes suggested\nand used \"mutt -H patch-file\" which I guess that's the problem. I only\nnoticed this because someone mentioned it, after I sent another patch.\n\nSo no regression, and a --leave-utf8-alone option would be useful here.\n\nCheers,\n cmn\n"}]}