{"thread":{"id":"34403","subject":"[PATCH v2 2/4] t4203: test check-mailmap command invocation","startedAt":"2013-07-11T14:55:24Z","lastAt":"2013-07-12T06:39:37Z","messageCount":18,"participants":["Eric Sunshine","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"223071","messageId":"1373554528-15775-1-git-send-email-sunshine@sunshineco.com","threadId":"34403","inReplyTo":null,"subject":"[PATCH v2 0/4] add git-check-mailmap command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-11T14:55:24Z","receivedAt":"2013-07-11T14:55:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This is a re-roll of [1] which introduces git-check-mailmap. The\nprimary motivation for this command is to expose git's stable,\nwell-tested C-implementation of .mailmap functionality to scripts and\nporcelains, thus relieving them of the need to reimplement support\nthemselves. The git-contacts [2] script (currently at es/contacts in\n'pu') would be one such client. v2 removes the RFC status and addresses\ncomments from reviewers [1].\n\nChanges since v1:\n\n* Add Documentation/git-check-mailmap.txt.\n\n* Add --null alias for -z.\n\n* Use OPT_BOOL rather than deprecated OPT_BOOLEAN.\n\n* Simplify code which outputs normalized contact.\n\n* Settle on \"stdout\" as argument to maybe_flush_or_die().\n\n* Eliminate diff noise from patch 4/4.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/230068/\n[2]: http://thread.gmane.org/gmane.comp.version-control.git/229533/\n\nEric Sunshine (4):\n  builtin: add git-check-mailmap command\n  t4203: test check-mailmap command invocation\n  t4203: test mailmap functionality directly rather than indirectly\n  t4203: consolidate test-repository setup\n\n .gitignore                             |   1 +\n Documentation/git-check-mailmap.txt    |  55 ++++++++\n Makefile                               |   1 +\n builtin.h                              |   1 +\n builtin/check-mailmap.c                |  69 ++++++++++\n command-list.txt                       |   1 +\n contrib/completion/git-completion.bash |   1 +\n git.c                                  |   1 +\n t/t4203-mailmap.sh                     | 234 +++++++++++++++++----------------\n 9 files changed, 251 insertions(+), 113 deletions(-)\n create mode 100644 Documentation/git-check-mailmap.txt\n create mode 100644 builtin/check-mailmap.c\n\n-- \n1.8.3.2\n"},{"id":"223068","messageId":"1373554528-15775-2-git-send-email-sunshine@sunshineco.com","threadId":"34403","inReplyTo":"1373554528-15775-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH v2 1/4] builtin: add git-check-mailmap command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-11T14:55:25Z","receivedAt":"2013-07-11T14:55:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Introduce command check-mailmap, similar to check-attr and check-ignore,\nwhich allows direct testing of .mailmap configuration.\n\nAs plumbing accessible to scripts and other porcelain, check-mailmap\npublishes the stable, well-tested .mailmap functionality employed by\nbuilt-in Git commands.  Consequently, script authors need not\nre-implement .mailmap functionality manually, thus avoiding potential\nquirks and behavioral differences.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n .gitignore                             |  1 +\n Documentation/git-check-mailmap.txt    | 55 +++++++++++++++++++++++++++\n Makefile                               |  1 +\n builtin.h                              |  1 +\n builtin/check-mailmap.c                | 69 ++++++++++++++++++++++++++++++++++\n command-list.txt                       |  1 +\n contrib/completion/git-completion.bash |  1 +\n git.c                                  |  1 +\n 8 files changed, 130 insertions(+)\n create mode 100644 Documentation/git-check-mailmap.txt\n create mode 100644 builtin/check-mailmap.c\n\ndiff --git a/.gitignore b/.gitignore\nindex efa8db0..6b1fd1b 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -23,6 +23,7 @@\n /git-cat-file\n /git-check-attr\n /git-check-ignore\n+/git-check-mailmap\n /git-check-ref-format\n /git-checkout\n /git-checkout-index\ndiff --git a/Documentation/git-check-mailmap.txt b/Documentation/git-check-mailmap.txt\nnew file mode 100644\nindex 0000000..3040bbc\n--- /dev/null\n+++ b/Documentation/git-check-mailmap.txt\n@@ -0,0 +1,55 @@\n+git-check-mailmap(1)\n+====================\n+\n+NAME\n+----\n+git-check-mailmap - Show canonical mappings of names and email addresses\n+\n+\n+SYNOPSIS\n+--------\n+[verse]\n+'git check-mailmap' [options] <contact>...\n+\n+\n+DESCRIPTION\n+-----------\n+\n+For each ``Name $$<email@address>$$'' or ``$$<email@address>$$'' provided on\n+the command-line or standard input (when using `--stdin`), prints a line with\n+the canonical contact information for that person according to the 'mailmap'\n+(see \"Mapping Authors\" below). If no mapping exists for the person, echoes the\n+provided contact information.\n+\n+\n+OPTIONS\n+-------\n+--stdin::\n+\tRead contacts, one per line, from the standard input after exhausting\n+\tcontacts provided on the command-line.\n+\n+-z::\n+--null::\n+\tTerminate each printed contact line with a null character `\\0` rather\n+\tthan a newline.\n+\n+\n+OUTPUT\n+------\n+\n+For each contact, a single line is printed of the form ``Name\n+$$<email@address>$$'' if the name is provided or known to the 'mailmap'; or of\n+the form ``$$<email@address>$$'' if no name is provided or known. Each line is\n+terminated by a newline, or by a null character `\\0` when `-z` or `--null` is\n+specified.\n+\n+\n+MAPPING AUTHORS\n+---------------\n+\n+include::mailmap.txt[]\n+\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\ndiff --git a/Makefile b/Makefile\nindex 0600eb4..ef442eb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -912,6 +912,7 @@ BUILTIN_OBJS += builtin/bundle.o\n BUILTIN_OBJS += builtin/cat-file.o\n BUILTIN_OBJS += builtin/check-attr.o\n BUILTIN_OBJS += builtin/check-ignore.o\n+BUILTIN_OBJS += builtin/check-mailmap.o\n BUILTIN_OBJS += builtin/check-ref-format.o\n BUILTIN_OBJS += builtin/checkout-index.o\n BUILTIN_OBJS += builtin/checkout.o\ndiff --git a/builtin.h b/builtin.h\nindex 1ed8edb..8afa2de 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -40,6 +40,7 @@ extern int cmd_checkout(int argc, const char **argv, const char *prefix);\n extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);\n extern int cmd_check_attr(int argc, const char **argv, const char *prefix);\n extern int cmd_check_ignore(int argc, const char **argv, const char *prefix);\n+extern int cmd_check_mailmap(int argc, const char **argv, const char *prefix);\n extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c\nnew file mode 100644\nindex 0000000..c36a61c\n--- /dev/null\n+++ b/builtin/check-mailmap.c\n@@ -0,0 +1,69 @@\n+#include \"builtin.h\"\n+#include \"mailmap.h\"\n+#include \"parse-options.h\"\n+#include \"string-list.h\"\n+\n+static int use_stdin;\n+static int null_out;\n+static const char * const check_mailmap_usage[] = {\n+N_(\"git check-mailmap [options] <contact>...\"),\n+NULL\n+};\n+\n+static const struct option check_mailmap_options[] = {\n+\tOPT_BOOL(0, \"stdin\", &use_stdin, N_(\"also read contacts from stdin\")),\n+\tOPT_BOOL('z', \"null\", &null_out, N_(\"null-terminate output lines\")),\n+\tOPT_END()\n+};\n+\n+static void check_mailmap(struct string_list *mailmap, const char *contact)\n+{\n+\tconst char *name, *mail;\n+\tsize_t namelen, maillen;\n+\tstruct ident_split ident;\n+\tchar term = null_out ? '\\0' : '\\n';\n+\n+\tif (split_ident_line(&ident, contact, strlen(contact)))\n+\t\tdie(_(\"unable to parse contact: %s\"), contact);\n+\n+\tname = ident.name_begin;\n+\tnamelen = ident.name_end - ident.name_begin;\n+\tmail = ident.mail_begin;\n+\tmaillen = ident.mail_end - ident.mail_begin;\n+\n+\tmap_user(mailmap, &mail, &maillen, &name, &namelen);\n+\n+\tif (namelen)\n+\t\tprintf(\"%.*s \", (int)namelen, name);\n+\tprintf(\"<%.*s>%c\", (int)maillen, mail, term);\n+}\n+\n+int cmd_check_mailmap(int argc, const char **argv, const char *prefix)\n+{\n+\tint i;\n+\tstruct string_list mailmap = STRING_LIST_INIT_NODUP;\n+\n+\tgit_config(git_default_config, NULL);\n+\targc = parse_options(argc, argv, prefix, check_mailmap_options,\n+\t\tcheck_mailmap_usage, 0);\n+\tif (argc == 0 && !use_stdin)\n+\t\tdie(_(\"no contacts specified\"));\n+\n+\tread_mailmap(&mailmap, NULL);\n+\n+\tfor (i = 0; i < argc; ++i)\n+\t\tcheck_mailmap(&mailmap, argv[i]);\n+\tmaybe_flush_or_die(stdout, \"stdout\");\n+\n+\tif (use_stdin) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\twhile (strbuf_getline(&buf, stdin, '\\n') != EOF) {\n+\t\t\tcheck_mailmap(&mailmap, buf.buf);\n+\t\t\tmaybe_flush_or_die(stdout, \"stdout\");\n+\t\t}\n+\t\tstrbuf_release(&buf);\n+\t}\n+\n+\tclear_mailmap(&mailmap);\n+\treturn 0;\n+}\ndiff --git a/command-list.txt b/command-list.txt\nindex bf83303..08b04e2 100644\n--- a/command-list.txt\n+++ b/command-list.txt\n@@ -13,6 +13,7 @@ git-bundle                              mainporcelain\n git-cat-file                            plumbinginterrogators\n git-check-attr                          purehelpers\n git-check-ignore                        purehelpers\n+git-check-mailmap                       purehelpers\n git-checkout                            mainporcelain common\n git-checkout-index                      plumbingmanipulators\n git-check-ref-format                    purehelpers\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex ebc40d4..c118550 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -648,6 +648,7 @@ __git_list_porcelain_commands ()\n \t\tcat-file)         : plumbing;;\n \t\tcheck-attr)       : plumbing;;\n \t\tcheck-ignore)     : plumbing;;\n+\t\tcheck-mailmap)    : plumbing;;\n \t\tcheck-ref-format) : plumbing;;\n \t\tcheckout-index)   : plumbing;;\n \t\tcommit-tree)      : plumbing;;\ndiff --git a/git.c b/git.c\nindex 4359086..02c3035 100644\n--- a/git.c\n+++ b/git.c\n@@ -324,6 +324,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"cat-file\", cmd_cat_file, RUN_SETUP },\n \t\t{ \"check-attr\", cmd_check_attr, RUN_SETUP },\n \t\t{ \"check-ignore\", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },\n+\t\t{ \"check-mailmap\", cmd_check_mailmap, RUN_SETUP },\n \t\t{ \"check-ref-format\", cmd_check_ref_format },\n \t\t{ \"checkout\", cmd_checkout, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"checkout-index\", cmd_checkout_index,\n-- \n1.8.3.2\n"},{"id":"223067","messageId":"1373554528-15775-3-git-send-email-sunshine@sunshineco.com","threadId":"34403","inReplyTo":"1373554528-15775-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH v2 2/4] t4203: test check-mailmap command invocation","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-11T14:55:26Z","receivedAt":"2013-07-11T14:55:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Test the command-line interface of check-mailmap. (Actual .mailmap\nfunctionality is already covered by existing tests.)\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t4203-mailmap.sh | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 50 insertions(+)\n\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 842b754..8645492 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -13,6 +13,11 @@ fuzz_blame () {\n }\n \n test_expect_success setup '\n+\tcat >contacts <<-\\EOF &&\n+\tA U Thor <author@example.com>\n+\tnick1 <bugs@company.xx>\n+\tEOF\n+\n \techo one >one &&\n \tgit add one &&\n \ttest_tick &&\n@@ -23,6 +28,51 @@ test_expect_success setup '\n \tgit commit --author \"nick1 <bugs@company.xx>\" -m second\n '\n \n+test_expect_success 'check-mailmap no arguments' '\n+\ttest_must_fail git check-mailmap\n+'\n+\n+test_expect_success 'check-mailmap arguments' '\n+\tcat >expect <<-\\EOF &&\n+\tA U Thor <author@example.com>\n+\tnick1 <bugs@company.xx>\n+\tEOF\n+\tgit check-mailmap \\\n+\t\t\"A U Thor <author@example.com>\" \\\n+\t\t\"nick1 <bugs@company.xx>\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'check-mailmap --stdin' '\n+\tcat >expect <<-\\EOF &&\n+\tA U Thor <author@example.com>\n+\tnick1 <bugs@company.xx>\n+\tEOF\n+\tgit check-mailmap --stdin <contacts >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'check-mailmap --stdin arguments' '\n+\tcat >expect <<-\\EOF &&\n+\tInternal Guy <bugs@company.xy>\n+\tEOF\n+\tcat <contacts >>expect &&\n+\tgit check-mailmap --stdin \"Internal Guy <bugs@company.xy>\" \\\n+\t\t<contacts >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'check-mailmap -z' '\n+\tprintf \"A U Thor <author@example.com>\\0\" >expect &&\n+\tprintf \"nick1 <bugs@company.xx>\\0\" >>expect &&\n+\tgit check-mailmap -z --stdin <contacts >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'check-mailmap bogus contact' '\n+\ttest_must_fail git check-mailmap bogus\n+'\n+\n cat >expect <<\\EOF\n A U Thor (1):\n       initial\n-- \n1.8.3.2\n"},{"id":"223070","messageId":"1373554528-15775-4-git-send-email-sunshine@sunshineco.com","threadId":"34403","inReplyTo":"1373554528-15775-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH v2 3/4] t4203: test mailmap functionality directly rather than indirectly","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-11T14:55:27Z","receivedAt":"2013-07-11T14:55:27Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"With the introduction of check-mailmap, it is now possible to check\n.mailmap functionality directly rather than indirectly as a side-effect\nof other commands (such as git-shortlog), therefore, do so.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t4203-mailmap.sh | 133 ++++++++++++++++++-----------------------------------\n 1 file changed, 45 insertions(+), 88 deletions(-)\n\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 8645492..48a000b 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -74,128 +74,96 @@ test_expect_success 'check-mailmap bogus contact' '\n '\n \n cat >expect <<\\EOF\n-A U Thor (1):\n-      initial\n-\n-nick1 (1):\n-      second\n-\n+A U Thor <author@example.com>\n+nick1 <bugs@company.xx>\n EOF\n \n test_expect_success 'No mailmap' '\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin <contacts >actual &&\n \ttest_cmp expect actual\n '\n \n cat >expect <<\\EOF\n-Repo Guy (1):\n-      initial\n-\n-nick1 (1):\n-      second\n-\n+Repo Guy <author@example.com>\n+nick1 <bugs@company.xx>\n EOF\n \n test_expect_success 'default .mailmap' '\n \techo \"Repo Guy <author@example.com>\" > .mailmap &&\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin <contacts >actual &&\n \ttest_cmp expect actual\n '\n \n # Using a mailmap file in a subdirectory of the repo here, but\n # could just as well have been a file outside of the repository\n cat >expect <<\\EOF\n-Internal Guy (1):\n-      second\n-\n-Repo Guy (1):\n-      initial\n-\n+Repo Guy <author@example.com>\n+Internal Guy <bugs@company.xx>\n EOF\n test_expect_success 'mailmap.file set' '\n \tmkdir -p internal_mailmap &&\n \techo \"Internal Guy <bugs@company.xx>\" > internal_mailmap/.mailmap &&\n \tgit config mailmap.file internal_mailmap/.mailmap &&\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin <contacts >actual &&\n \ttest_cmp expect actual\n '\n \n cat >expect <<\\EOF\n-External Guy (1):\n-      initial\n-\n-Internal Guy (1):\n-      second\n-\n+External Guy <author@example.com>\n+Internal Guy <bugs@company.xx>\n EOF\n test_expect_success 'mailmap.file override' '\n \techo \"External Guy <author@example.com>\" >> internal_mailmap/.mailmap &&\n \tgit config mailmap.file internal_mailmap/.mailmap &&\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin <contacts >actual &&\n \ttest_cmp expect actual\n '\n \n cat >expect <<\\EOF\n-Repo Guy (1):\n-      initial\n-\n-nick1 (1):\n-      second\n-\n+Repo Guy <author@example.com>\n+nick1 <bugs@company.xx>\n EOF\n \n test_expect_success 'mailmap.file non-existent' '\n \trm internal_mailmap/.mailmap &&\n \trmdir internal_mailmap &&\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin <contacts >actual &&\n \ttest_cmp expect actual\n '\n \n cat >expect <<\\EOF\n-Internal Guy (1):\n-      second\n-\n-Repo Guy (1):\n-      initial\n-\n+Repo Guy <author@example.com>\n+Internal Guy <bugs@company.xy>\n EOF\n \n test_expect_success 'name entry after email entry' '\n \tmkdir -p internal_mailmap &&\n \techo \"<bugs@company.xy> <bugs@company.xx>\" >internal_mailmap/.mailmap &&\n \techo \"Internal Guy <bugs@company.xx>\" >>internal_mailmap/.mailmap &&\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin <contacts >actual &&\n \ttest_cmp expect actual\n '\n \n cat >expect <<\\EOF\n-Internal Guy (1):\n-      second\n-\n-Repo Guy (1):\n-      initial\n-\n+Repo Guy <author@example.com>\n+Internal Guy <bugs@company.xy>\n EOF\n \n test_expect_success 'name entry after email entry, case-insensitive' '\n \tmkdir -p internal_mailmap &&\n \techo \"<bugs@company.xy> <bugs@company.xx>\" >internal_mailmap/.mailmap &&\n \techo \"Internal Guy <BUGS@Company.xx>\" >>internal_mailmap/.mailmap &&\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin <contacts >actual &&\n \ttest_cmp expect actual\n '\n \n cat >expect <<\\EOF\n-A U Thor (1):\n-      initial\n-\n-nick1 (1):\n-      second\n-\n+A U Thor <author@example.com>\n+nick1 <bugs@company.xx>\n EOF\n test_expect_success 'No mailmap files, but configured' '\n \trm -f .mailmap internal_mailmap/.mailmap &&\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin <contacts >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -217,54 +185,43 @@ test_expect_success 'setup mailmap blob tests' '\n \n test_expect_success 'mailmap.blob set' '\n \tcat >expect <<-\\EOF &&\n-\tBlob Guy (1):\n-\t      second\n-\n-\tRepo Guy (1):\n-\t      initial\n-\n+\tRepo Guy <author@example.com>\n+\tBlob Guy <bugs@company.xx>\n \tEOF\n-\tgit -c mailmap.blob=map:just-bugs shortlog HEAD >actual &&\n+\tgit -c mailmap.blob=map:just-bugs check-mailmap --stdin \\\n+\t\t<contacts >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'mailmap.blob overrides .mailmap' '\n \tcat >expect <<-\\EOF &&\n-\tBlob Guy (2):\n-\t      initial\n-\t      second\n-\n+\tBlob Guy <author@example.com>\n+\tBlob Guy <bugs@company.xx>\n \tEOF\n-\tgit -c mailmap.blob=map:both shortlog HEAD >actual &&\n+\tgit -c mailmap.blob=map:both check-mailmap --stdin \\\n+\t\t<contacts  >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'mailmap.file overrides mailmap.blob' '\n \tcat >expect <<-\\EOF &&\n-\tBlob Guy (1):\n-\t      second\n-\n-\tInternal Guy (1):\n-\t      initial\n-\n+\tInternal Guy <author@example.com>\n+\tBlob Guy <bugs@company.xx>\n \tEOF\n \tgit \\\n \t  -c mailmap.blob=map:both \\\n \t  -c mailmap.file=internal.map \\\n-\t  shortlog HEAD >actual &&\n+\t  check-mailmap --stdin <contacts >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'mailmap.blob can be missing' '\n \tcat >expect <<-\\EOF &&\n-\tRepo Guy (1):\n-\t      initial\n-\n-\tnick1 (1):\n-\t      second\n-\n+\tRepo Guy <author@example.com>\n+\tnick1 <bugs@company.xx>\n \tEOF\n-\tgit -c mailmap.blob=map:nonexistent shortlog HEAD >actual &&\n+\tgit -c mailmap.blob=map:nonexistent check-mailmap --stdin \\\n+\t\t<contacts >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -273,12 +230,12 @@ test_expect_success 'mailmap.blob defaults to off in non-bare repo' '\n \t(\n \t\tcd non-bare &&\n \t\ttest_commit one .mailmap \"Fake Name <author@example.com>\" &&\n-\t\techo \"     1\tFake Name\" >expect &&\n-\t\tgit shortlog -ns HEAD >actual &&\n+\t\techo \"Fake Name <author@example.com>\" >expect &&\n+\t\tgit check-mailmap \"A U Thor <author@example.com>\" >actual &&\n \t\ttest_cmp expect actual &&\n \t\trm .mailmap &&\n-\t\techo \"     1\tA U Thor\" >expect &&\n-\t\tgit shortlog -ns HEAD >actual &&\n+\t\techo \"A U Thor <author@example.com>\" >expect &&\n+\t\tgit check-mailmap \"A U Thor <author@example.com>\" >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n@@ -287,8 +244,8 @@ test_expect_success 'mailmap.blob defaults to HEAD:.mailmap in bare repo' '\n \tgit clone --bare non-bare bare &&\n \t(\n \t\tcd bare &&\n-\t\techo \"     1\tFake Name\" >expect &&\n-\t\tgit shortlog -ns HEAD >actual &&\n+\t\techo \"Fake Name <author@example.com>\" >expect &&\n+\t\tgit check-mailmap \"A U Thor <author@example.com>\" >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n-- \n1.8.3.2\n"},{"id":"223069","messageId":"1373554528-15775-5-git-send-email-sunshine@sunshineco.com","threadId":"34403","inReplyTo":"1373554528-15775-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH v2 4/4] t4203: consolidate test-repository setup","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-11T14:55:28Z","receivedAt":"2013-07-11T14:55:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The simple two-commit test-repository created by 'setup' is no longer\nneeded by any of the tests retrofitted to use check-mailmap. Subsequent,\nmore complex tests of git-shortlog, git-log, and git-blame functionality\nexpand the repository by adding five commits. Consolidate the creation\nof this seven-commit repository into a single setup function.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t4203-mailmap.sh | 53 +++++++++++++++++++++++++++--------------------------\n 1 file changed, 27 insertions(+), 26 deletions(-)\n\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 48a000b..9c3c83a 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -22,10 +22,36 @@ test_expect_success setup '\n \tgit add one &&\n \ttest_tick &&\n \tgit commit -m initial &&\n+\n \techo two >>one &&\n \tgit add one &&\n \ttest_tick &&\n-\tgit commit --author \"nick1 <bugs@company.xx>\" -m second\n+\tgit commit --author \"nick1 <bugs@company.xx>\" -m second &&\n+\n+\techo three >>one &&\n+\tgit add one &&\n+\ttest_tick &&\n+\tgit commit --author \"nick2 <bugs@company.xx>\" -m third &&\n+\n+\techo four >>one &&\n+\tgit add one &&\n+\ttest_tick &&\n+\tgit commit --author \"nick2 <nick2@company.xx>\" -m fourth &&\n+\n+\techo five >>one &&\n+\tgit add one &&\n+\ttest_tick &&\n+\tgit commit --author \"santa <me@company.xx>\" -m fifth &&\n+\n+\techo six >>one &&\n+\tgit add one &&\n+\ttest_tick &&\n+\tgit commit --author \"claus <me@company.xx>\" -m sixth &&\n+\n+\techo seven >>one &&\n+\tgit add one &&\n+\ttest_tick &&\n+\tgit commit --author \"CTO <cto@coompany.xx>\" -m seventh\n '\n \n test_expect_success 'check-mailmap no arguments' '\n@@ -276,31 +302,6 @@ Some Dude <some@dude.xx> (1):\n EOF\n \n test_expect_success 'Shortlog output (complex mapping)' '\n-\techo three >>one &&\n-\tgit add one &&\n-\ttest_tick &&\n-\tgit commit --author \"nick2 <bugs@company.xx>\" -m third &&\n-\n-\techo four >>one &&\n-\tgit add one &&\n-\ttest_tick &&\n-\tgit commit --author \"nick2 <nick2@company.xx>\" -m fourth &&\n-\n-\techo five >>one &&\n-\tgit add one &&\n-\ttest_tick &&\n-\tgit commit --author \"santa <me@company.xx>\" -m fifth &&\n-\n-\techo six >>one &&\n-\tgit add one &&\n-\ttest_tick &&\n-\tgit commit --author \"claus <me@company.xx>\" -m sixth &&\n-\n-\techo seven >>one &&\n-\tgit add one &&\n-\ttest_tick &&\n-\tgit commit --author \"CTO <cto@coompany.xx>\" -m seventh &&\n-\n \tmkdir -p internal_mailmap &&\n \techo \"Committed <committer@example.com>\" > internal_mailmap/.mailmap &&\n \techo \"<cto@company.xx>                       <cto@coompany.xx>\" >> internal_mailmap/.mailmap &&\n-- \n1.8.3.2\n"},{"id":"223091","messageId":"7vli5crk8l.fsf@alter.siamese.dyndns.org","threadId":"34403","inReplyTo":"1373554528-15775-2-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH v2 1/4] builtin: add git-check-mailmap command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-11T19:04:42Z","receivedAt":"2013-07-11T19:04:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> +DESCRIPTION\n> +-----------\n> +\n> +For each ``Name $$<email@address>$$'' or ``$$<email@address>$$'' provided on\n> +the command-line or standard input (when using `--stdin`), prints a line with\n> +the canonical contact information for that person according to the 'mailmap'\n> +(see \"Mapping Authors\" below). If no mapping exists for the person, echoes the\n> +provided contact information.\n\nOK.  The last part needed a reading and a half for me to realize the\n\"echoes the provided contact information\" is the same thing as \"the\n\"input string is printed as-is\", especially because \"contact\ninformation\" is not defined anywhere in the previous part of the\nDESCRIPTION section, though.  I might be worth starting the\nparagraph with:\n\n\tFor each contact information (either in the form of ``Name\n\t<user@host>'' or ...)\n\nin order to clarify that the two forms of input is what you call\n\"contact information\".\n\n> diff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c\n> new file mode 100644\n> index 0000000..c36a61c\n> --- /dev/null\n> +++ b/builtin/check-mailmap.c\n> @@ -0,0 +1,69 @@\n> +#include \"builtin.h\"\n> +#include \"mailmap.h\"\n> +#include \"parse-options.h\"\n> +#include \"string-list.h\"\n> +\n> +static int use_stdin;\n> +static int null_out;\n\nIs there a reason why the variable name should not match that of the\ncorresponding variables in check-ignore and check-attr, which you\ncopied the bulk of the code from?\n\nIf there isn't, use \"null_term_line\" like they do.\n\n> +static const char * const check_mailmap_usage[] = {\n> +N_(\"git check-mailmap [options] <contact>...\"),\n> +NULL\n> +};\n\nThis mis-indentation looks somewhat unfortunate, but they are shared\nwith other check-blah, and they are meant to contain a rather long\nstring, so let's keep it like so.\n\n> +static void check_mailmap(struct string_list *mailmap, const char *contact)\n> +{\n> +\tconst char *name, *mail;\n> +\tsize_t namelen, maillen;\n> +\tstruct ident_split ident;\n> +\tchar term = null_out ? '\\0' : '\\n';\n\nIs there a reason why the variable name \"term\" should not match that\nof the corresponding variables in check-ignore and check-attr, which\nyou copied the bulk of the code from?\n\nIf there isn't, use \"line_termination\" like they do.\n\n> +\tif (split_ident_line(&ident, contact, strlen(contact)))\n> +\t\tdie(_(\"unable to parse contact: %s\"), contact);\n> +\n> +\tname = ident.name_begin;\n> +\tnamelen = ident.name_end - ident.name_begin;\n> +\tmail = ident.mail_begin;\n> +\tmaillen = ident.mail_end - ident.mail_begin;\n> +\n> +\tmap_user(mailmap, &mail, &maillen, &name, &namelen);\n> +\n> +\tif (namelen)\n> +\t\tprintf(\"%.*s \", (int)namelen, name);\n> +\tprintf(\"<%.*s>%c\", (int)maillen, mail, term);\n> +}\n> +\n> +int cmd_check_mailmap(int argc, const char **argv, const char *prefix)\n> +{\n> +\tint i;\n> +\tstruct string_list mailmap = STRING_LIST_INIT_NODUP;\n> +\n> +\tgit_config(git_default_config, NULL);\n> +\targc = parse_options(argc, argv, prefix, check_mailmap_options,\n> +\t\tcheck_mailmap_usage, 0);\n\nIt is a bit distracting that the second line that is not indented\nenough.  Either (1) with enough HT and with minimum number of SP,\nalign \"argc\" and \"check_mailmap_usage\", or (2) with minimum number\nof HT and no SP, push \"check_mailmap_usage\" to the right of opening\nparenthesis of \"(argc\".  Our code tend to prefer (1).\n\n> +\tif (argc == 0 && !use_stdin)\n> +\t\tdie(_(\"no contacts specified\"));\n> +\n> +\tread_mailmap(&mailmap, NULL);\n> +\n> +\tfor (i = 0; i < argc; ++i)\n> +\t\tcheck_mailmap(&mailmap, argv[i]);\n> +\tmaybe_flush_or_die(stdout, \"stdout\");\n> +\n> +\tif (use_stdin) {\n> +\t\tstruct strbuf buf = STRBUF_INIT;\n> +\t\twhile (strbuf_getline(&buf, stdin, '\\n') != EOF) {\n> +\t\t\tcheck_mailmap(&mailmap, buf.buf);\n> +\t\t\tmaybe_flush_or_die(stdout, \"stdout\");\n> +\t\t}\n> +\t\tstrbuf_release(&buf);\n> +\t}\n> +\n> +\tclear_mailmap(&mailmap);\n> +\treturn 0;\n> +}\n> diff --git a/command-list.txt b/command-list.txt\n> index bf83303..08b04e2 100644\n> --- a/command-list.txt\n> +++ b/command-list.txt\n> @@ -13,6 +13,7 @@ git-bundle                              mainporcelain\n>  git-cat-file                            plumbinginterrogators\n>  git-check-attr                          purehelpers\n>  git-check-ignore                        purehelpers\n> +git-check-mailmap                       purehelpers\n>  git-checkout                            mainporcelain common\n>  git-checkout-index                      plumbingmanipulators\n>  git-check-ref-format                    purehelpers\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index ebc40d4..c118550 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -648,6 +648,7 @@ __git_list_porcelain_commands ()\n>  \t\tcat-file)         : plumbing;;\n>  \t\tcheck-attr)       : plumbing;;\n>  \t\tcheck-ignore)     : plumbing;;\n> +\t\tcheck-mailmap)    : plumbing;;\n>  \t\tcheck-ref-format) : plumbing;;\n>  \t\tcheckout-index)   : plumbing;;\n>  \t\tcommit-tree)      : plumbing;;\n> diff --git a/git.c b/git.c\n> index 4359086..02c3035 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -324,6 +324,7 @@ static void handle_internal_command(int argc, const char **argv)\n>  \t\t{ \"cat-file\", cmd_cat_file, RUN_SETUP },\n>  \t\t{ \"check-attr\", cmd_check_attr, RUN_SETUP },\n>  \t\t{ \"check-ignore\", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },\n> +\t\t{ \"check-mailmap\", cmd_check_mailmap, RUN_SETUP },\n\nAs we can read from HEAD:.mailmap these days, the patch is correct\nthat it does not require NEED_WORK_TREE here.\n\nThanks.\n\n>  \t\t{ \"check-ref-format\", cmd_check_ref_format },\n>  \t\t{ \"checkout\", cmd_checkout, RUN_SETUP | NEED_WORK_TREE },\n>  \t\t{ \"checkout-index\", cmd_checkout_index,\n"},{"id":"223092","messageId":"7vhag0rk3u.fsf@alter.siamese.dyndns.org","threadId":"34403","inReplyTo":"1373554528-15775-4-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH v2 3/4] t4203: test mailmap functionality directly rather than indirectly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-11T19:07:33Z","receivedAt":"2013-07-11T19:07:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> With the introduction of check-mailmap, it is now possible to check\n> .mailmap functionality directly rather than indirectly as a side-effect\n> of other commands (such as git-shortlog), therefore, do so.\n\nDoes this patch mean that we will now ignore future breakages in\nshortlog and blame if their mailmap integration becomes buggy?\n\nI am not convinced it is a good idea if that is what is going on.\n\n>\n> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>\n> ---\n>  t/t4203-mailmap.sh | 133 ++++++++++++++++++-----------------------------------\n>  1 file changed, 45 insertions(+), 88 deletions(-)\n>\n> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\n> index 8645492..48a000b 100755\n> --- a/t/t4203-mailmap.sh\n> +++ b/t/t4203-mailmap.sh\n> @@ -74,128 +74,96 @@ test_expect_success 'check-mailmap bogus contact' '\n>  '\n>  \n>  cat >expect <<\\EOF\n> -A U Thor (1):\n> -      initial\n> -\n> -nick1 (1):\n> -      second\n> -\n> +A U Thor <author@example.com>\n> +nick1 <bugs@company.xx>\n>  EOF\n>  \n>  test_expect_success 'No mailmap' '\n> -\tgit shortlog HEAD >actual &&\n> +\tgit check-mailmap --stdin <contacts >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n>  cat >expect <<\\EOF\n> -Repo Guy (1):\n> -      initial\n> -\n> -nick1 (1):\n> -      second\n> -\n> +Repo Guy <author@example.com>\n> +nick1 <bugs@company.xx>\n>  EOF\n>  \n>  test_expect_success 'default .mailmap' '\n>  \techo \"Repo Guy <author@example.com>\" > .mailmap &&\n> -\tgit shortlog HEAD >actual &&\n> +\tgit check-mailmap --stdin <contacts >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n>  # Using a mailmap file in a subdirectory of the repo here, but\n>  # could just as well have been a file outside of the repository\n>  cat >expect <<\\EOF\n> -Internal Guy (1):\n> -      second\n> -\n> -Repo Guy (1):\n> -      initial\n> -\n> +Repo Guy <author@example.com>\n> +Internal Guy <bugs@company.xx>\n>  EOF\n>  test_expect_success 'mailmap.file set' '\n>  \tmkdir -p internal_mailmap &&\n>  \techo \"Internal Guy <bugs@company.xx>\" > internal_mailmap/.mailmap &&\n>  \tgit config mailmap.file internal_mailmap/.mailmap &&\n> -\tgit shortlog HEAD >actual &&\n> +\tgit check-mailmap --stdin <contacts >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n>  cat >expect <<\\EOF\n> -External Guy (1):\n> -      initial\n> -\n> -Internal Guy (1):\n> -      second\n> -\n> +External Guy <author@example.com>\n> +Internal Guy <bugs@company.xx>\n>  EOF\n>  test_expect_success 'mailmap.file override' '\n>  \techo \"External Guy <author@example.com>\" >> internal_mailmap/.mailmap &&\n>  \tgit config mailmap.file internal_mailmap/.mailmap &&\n> -\tgit shortlog HEAD >actual &&\n> +\tgit check-mailmap --stdin <contacts >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n>  cat >expect <<\\EOF\n> -Repo Guy (1):\n> -      initial\n> -\n> -nick1 (1):\n> -      second\n> -\n> +Repo Guy <author@example.com>\n> +nick1 <bugs@company.xx>\n>  EOF\n>  \n>  test_expect_success 'mailmap.file non-existent' '\n>  \trm internal_mailmap/.mailmap &&\n>  \trmdir internal_mailmap &&\n> -\tgit shortlog HEAD >actual &&\n> +\tgit check-mailmap --stdin <contacts >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n>  cat >expect <<\\EOF\n> -Internal Guy (1):\n> -      second\n> -\n> -Repo Guy (1):\n> -      initial\n> -\n> +Repo Guy <author@example.com>\n> +Internal Guy <bugs@company.xy>\n>  EOF\n>  \n>  test_expect_success 'name entry after email entry' '\n>  \tmkdir -p internal_mailmap &&\n>  \techo \"<bugs@company.xy> <bugs@company.xx>\" >internal_mailmap/.mailmap &&\n>  \techo \"Internal Guy <bugs@company.xx>\" >>internal_mailmap/.mailmap &&\n> -\tgit shortlog HEAD >actual &&\n> +\tgit check-mailmap --stdin <contacts >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n>  cat >expect <<\\EOF\n> -Internal Guy (1):\n> -      second\n> -\n> -Repo Guy (1):\n> -      initial\n> -\n> +Repo Guy <author@example.com>\n> +Internal Guy <bugs@company.xy>\n>  EOF\n>  \n>  test_expect_success 'name entry after email entry, case-insensitive' '\n>  \tmkdir -p internal_mailmap &&\n>  \techo \"<bugs@company.xy> <bugs@company.xx>\" >internal_mailmap/.mailmap &&\n>  \techo \"Internal Guy <BUGS@Company.xx>\" >>internal_mailmap/.mailmap &&\n> -\tgit shortlog HEAD >actual &&\n> +\tgit check-mailmap --stdin <contacts >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n>  cat >expect <<\\EOF\n> -A U Thor (1):\n> -      initial\n> -\n> -nick1 (1):\n> -      second\n> -\n> +A U Thor <author@example.com>\n> +nick1 <bugs@company.xx>\n>  EOF\n>  test_expect_success 'No mailmap files, but configured' '\n>  \trm -f .mailmap internal_mailmap/.mailmap &&\n> -\tgit shortlog HEAD >actual &&\n> +\tgit check-mailmap --stdin <contacts >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n> @@ -217,54 +185,43 @@ test_expect_success 'setup mailmap blob tests' '\n>  \n>  test_expect_success 'mailmap.blob set' '\n>  \tcat >expect <<-\\EOF &&\n> -\tBlob Guy (1):\n> -\t      second\n> -\n> -\tRepo Guy (1):\n> -\t      initial\n> -\n> +\tRepo Guy <author@example.com>\n> +\tBlob Guy <bugs@company.xx>\n>  \tEOF\n> -\tgit -c mailmap.blob=map:just-bugs shortlog HEAD >actual &&\n> +\tgit -c mailmap.blob=map:just-bugs check-mailmap --stdin \\\n> +\t\t<contacts >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n>  test_expect_success 'mailmap.blob overrides .mailmap' '\n>  \tcat >expect <<-\\EOF &&\n> -\tBlob Guy (2):\n> -\t      initial\n> -\t      second\n> -\n> +\tBlob Guy <author@example.com>\n> +\tBlob Guy <bugs@company.xx>\n>  \tEOF\n> -\tgit -c mailmap.blob=map:both shortlog HEAD >actual &&\n> +\tgit -c mailmap.blob=map:both check-mailmap --stdin \\\n> +\t\t<contacts  >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n>  test_expect_success 'mailmap.file overrides mailmap.blob' '\n>  \tcat >expect <<-\\EOF &&\n> -\tBlob Guy (1):\n> -\t      second\n> -\n> -\tInternal Guy (1):\n> -\t      initial\n> -\n> +\tInternal Guy <author@example.com>\n> +\tBlob Guy <bugs@company.xx>\n>  \tEOF\n>  \tgit \\\n>  \t  -c mailmap.blob=map:both \\\n>  \t  -c mailmap.file=internal.map \\\n> -\t  shortlog HEAD >actual &&\n> +\t  check-mailmap --stdin <contacts >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n>  test_expect_success 'mailmap.blob can be missing' '\n>  \tcat >expect <<-\\EOF &&\n> -\tRepo Guy (1):\n> -\t      initial\n> -\n> -\tnick1 (1):\n> -\t      second\n> -\n> +\tRepo Guy <author@example.com>\n> +\tnick1 <bugs@company.xx>\n>  \tEOF\n> -\tgit -c mailmap.blob=map:nonexistent shortlog HEAD >actual &&\n> +\tgit -c mailmap.blob=map:nonexistent check-mailmap --stdin \\\n> +\t\t<contacts >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n> @@ -273,12 +230,12 @@ test_expect_success 'mailmap.blob defaults to off in non-bare repo' '\n>  \t(\n>  \t\tcd non-bare &&\n>  \t\ttest_commit one .mailmap \"Fake Name <author@example.com>\" &&\n> -\t\techo \"     1\tFake Name\" >expect &&\n> -\t\tgit shortlog -ns HEAD >actual &&\n> +\t\techo \"Fake Name <author@example.com>\" >expect &&\n> +\t\tgit check-mailmap \"A U Thor <author@example.com>\" >actual &&\n>  \t\ttest_cmp expect actual &&\n>  \t\trm .mailmap &&\n> -\t\techo \"     1\tA U Thor\" >expect &&\n> -\t\tgit shortlog -ns HEAD >actual &&\n> +\t\techo \"A U Thor <author@example.com>\" >expect &&\n> +\t\tgit check-mailmap \"A U Thor <author@example.com>\" >actual &&\n>  \t\ttest_cmp expect actual\n>  \t)\n>  '\n> @@ -287,8 +244,8 @@ test_expect_success 'mailmap.blob defaults to HEAD:.mailmap in bare repo' '\n>  \tgit clone --bare non-bare bare &&\n>  \t(\n>  \t\tcd bare &&\n> -\t\techo \"     1\tFake Name\" >expect &&\n> -\t\tgit shortlog -ns HEAD >actual &&\n> +\t\techo \"Fake Name <author@example.com>\" >expect &&\n> +\t\tgit check-mailmap \"A U Thor <author@example.com>\" >actual &&\n>  \t\ttest_cmp expect actual\n>  \t)\n>  '\n"},{"id":"223121","messageId":"CAPig+cS7rxFzY8Q3gfTtJkggp-K62SVqsjCotbM3Bkm47L44gg@mail.gmail.com","threadId":"34403","inReplyTo":"7vhag0rk3u.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 3/4] t4203: test mailmap functionality directly rather than indirectly","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-12T00:35:51Z","receivedAt":"2013-07-12T00:35:51Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jul 11, 2013 at 3:07 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n>\n>> With the introduction of check-mailmap, it is now possible to check\n>> .mailmap functionality directly rather than indirectly as a side-effect\n>> of other commands (such as git-shortlog), therefore, do so.\n>\n> Does this patch mean that we will now ignore future breakages in\n> shortlog and blame if their mailmap integration becomes buggy?\n>\n> I am not convinced it is a good idea if that is what is going on.\n\nI meant to mention in the cover letter that this patch was open for\ndebate, however, it does not eliminate all testing of these other\ncommands.\n\nThe tests in which git-check-mailmap is substituted for git-shortlog\nall worked against a simplistic two-commit repository. Those tests\nwere checking the low-level mailmap functionality under various\nconditions and configurations; they were not especially checking any\nparticular behavior of git-shortlog.\n\nThere still remain a final eight tests which cover git-shortlog,\ngit-log, and git-blame. These tests do check mailmap-related behavior\nof those commands, and they do so using a more involved seven-commit\nrepository with \"complex\" mappings, so we have not necessarily lost\nany checks of mailmap integration for those commands.\n\nWould this patch become more palatable if I stated the above in the\ncommit message?\n\n-- ES\n"},{"id":"223122","messageId":"20130712005517.GA8482@google.com","threadId":"34403","inReplyTo":"CAPig+cS7rxFzY8Q3gfTtJkggp-K62SVqsjCotbM3Bkm47L44gg@mail.gmail.com","subject":"Re: [PATCH v2 3/4] t4203: test mailmap functionality directly rather than indirectly","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-07-12T00:55:17Z","receivedAt":"2013-07-12T00:55:17Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Eric Sunshine wrote:\n> On Thu, Jul 11, 2013 at 3:07 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>>> With the introduction of check-mailmap, it is now possible to check\n>>> .mailmap functionality directly rather than indirectly as a side-effect\n>>> of other commands (such as git-shortlog), therefore, do so.\n>>\n>> Does this patch mean that we will now ignore future breakages in\n>> shortlog and blame if their mailmap integration becomes buggy?\n>>\n>> I am not convinced it is a good idea if that is what is going on.\n>\n> I meant to mention in the cover letter that this patch was open for\n> debate, however, it does not eliminate all testing of these other\n> commands.\n>\n> The tests in which git-check-mailmap is substituted for git-shortlog\n> all worked against a simplistic two-commit repository. Those tests\n> were checking the low-level mailmap functionality under various\n> conditions and configurations; they were not especially checking any\n> particular behavior of git-shortlog.\n>\n> There still remain a final eight tests which cover git-shortlog,\n> git-log, and git-blame. These tests do check mailmap-related behavior\n> of those commands, and they do so using a more involved seven-commit\n> repository with \"complex\" mappings, so we have not necessarily lost\n> any checks of mailmap integration for those commands.\n>\n> Would this patch become more palatable if I stated the above in the\n> commit message?\n\nMy current thinking is \"no\" --- the patch has as a justification \"Now\nwe can test these aspects of .mailmap handling directly with a\nlow-level tool instead of using the tool most people will use, so do\nso\", which sounds an awful lot like \"Reduce test coverage of commonly\nused tools, because we can\".\n\nBut I imagine the actual motivation was something other than \"because\nwe can\".  For example, maybe the idea is that after this patch, it\nshould be easier to make cosmetic improvements to the shortlog, log,\nand blame output and only have to change those final 8 tests that are\nadequately covering the output?  If you have such plans and this patch\nmakes them easier, it could sound like a good patch as a stepping\nstone toward that.\n\nThanks and hope that helps,\nJonathan\n"},{"id":"223123","messageId":"CAPig+cRtX=DHGK0D8zK2V9vZyS5-sGKmvEysLcG99De7RLGBOw@mail.gmail.com","threadId":"34403","inReplyTo":"20130712005517.GA8482@google.com","subject":"Re: [PATCH v2 3/4] t4203: test mailmap functionality directly rather than indirectly","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-12T02:37:15Z","receivedAt":"2013-07-12T02:37:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jul 11, 2013 at 8:55 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Eric Sunshine wrote:\n>> On Thu, Jul 11, 2013 at 3:07 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Eric Sunshine <sunshine@sunshineco.com> writes:\n>\n>>>> With the introduction of check-mailmap, it is now possible to check\n>>>> .mailmap functionality directly rather than indirectly as a side-effect\n>>>> of other commands (such as git-shortlog), therefore, do so.\n>>>\n>>> Does this patch mean that we will now ignore future breakages in\n>>> shortlog and blame if their mailmap integration becomes buggy?\n>>>\n>>> I am not convinced it is a good idea if that is what is going on.\n>>\n>> I meant to mention in the cover letter that this patch was open for\n>> debate, however, it does not eliminate all testing of these other\n>> commands.\n>>\n>> The tests in which git-check-mailmap is substituted for git-shortlog\n>> all worked against a simplistic two-commit repository. Those tests\n>> were checking the low-level mailmap functionality under various\n>> conditions and configurations; they were not especially checking any\n>> particular behavior of git-shortlog.\n>>\n>> There still remain a final eight tests which cover git-shortlog,\n>> git-log, and git-blame. These tests do check mailmap-related behavior\n>> of those commands, and they do so using a more involved seven-commit\n>> repository with \"complex\" mappings, so we have not necessarily lost\n>> any checks of mailmap integration for those commands.\n>>\n>> Would this patch become more palatable if I stated the above in the\n>> commit message?\n>\n> My current thinking is \"no\" --- the patch has as a justification \"Now\n> we can test these aspects of .mailmap handling directly with a\n> low-level tool instead of using the tool most people will use, so do\n> so\", which sounds an awful lot like \"Reduce test coverage of commonly\n> used tools, because we can\".\n>\n> But I imagine the actual motivation was something other than \"because\n> we can\".\n\nThe motivation is as stated in the subject: Direct rather than\nindirect testing of mailmap functionality. The tests in question are\nproperly crafted to check only low-level mailmap (not git-shortlog)\nfunctionality under various conditions and configurations, yet the\nmental load on the reader is high because he has to keep in mind the\nauthors and commits in the repository underlying git-shortlog's\noutput. When testing via git-check-mailmap, mental load is low: there\nis a direct correlation between the \"Name <email@address>\" which goes\nin and that which comes out. What is being tested is the same, but\nit's easier to reason about and understand the the tests when done\ndirectly.\n\nWhen converting the tests, I also discovered an additional, perhaps\nmore important motivation. Most of the tests check only that name\nremapping functions correctly, however, a couple also attempt to check\naddress remapping (bugs@company.xx => bugs@company.xy). Even though\nthe tests succeed, they don't actually prove that address remapping\nwas correct (or occurred at all) since git-shortlog does not display\nthe email address. git-check-mailmap always displays the email\naddress, thus the check actually tests the intended email address\nremapping.\n\n> For example, maybe the idea is that after this patch, it\n> should be easier to make cosmetic improvements to the shortlog, log,\n> and blame output and only have to change those final 8 tests that are\n> adequately covering the output?  If you have such plans and this patch\n> makes them easier, it could sound like a good patch as a stepping\n> stone toward that.\n"},{"id":"223124","messageId":"CAPig+cQvnKSxZ5M6zW_r6Y9xwZ0Ch99JDq4SccYXCRof9svsOg@mail.gmail.com","threadId":"34403","inReplyTo":"7vli5crk8l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/4] builtin: add git-check-mailmap command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-12T03:28:09Z","receivedAt":"2013-07-12T03:28:09Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jul 11, 2013 at 3:04 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n>\n>> +DESCRIPTION\n>> +-----------\n>> +\n>> +For each ``Name $$<email@address>$$'' or ``$$<email@address>$$'' provided on\n>> +the command-line or standard input (when using `--stdin`), prints a line with\n>> +the canonical contact information for that person according to the 'mailmap'\n>> +(see \"Mapping Authors\" below). If no mapping exists for the person, echoes the\n>> +provided contact information.\n>\n> OK.  The last part needed a reading and a half for me to realize the\n> \"echoes the provided contact information\" is the same thing as \"the\n> \"input string is printed as-is\", especially because \"contact\n> information\" is not defined anywhere in the previous part of the\n> DESCRIPTION section, though.  I might be worth starting the\n> paragraph with:\n>\n>         For each contact information (either in the form of ``Name\n>         <user@host>'' or ...)\n>\n> in order to clarify that the two forms of input is what you call\n> \"contact information\".\n\nIs this easier to read?\n\n    For each ``Name $$<user@host>$$'' or ``$$<user@host>$$'' from the\n    command-line or standard input (when using `--stdin`), print a line\n    showing either the canonical name and email address (see \"Mapping\n    Authors\" below), or the input ``Name $$<user@host>$$'' or\n    ``$$<user@host>$$'' if there is no mapping for that person.\n\n>> diff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c\n>> new file mode 100644\n>> index 0000000..c36a61c\n>> --- /dev/null\n>> +++ b/builtin/check-mailmap.c\n>> @@ -0,0 +1,69 @@\n>> +#include \"builtin.h\"\n>> +#include \"mailmap.h\"\n>> +#include \"parse-options.h\"\n>> +#include \"string-list.h\"\n>> +\n>> +static int use_stdin;\n>> +static int null_out;\n>\n> Is there a reason why the variable name should not match that of the\n> corresponding variables in check-ignore and check-attr, which you\n> copied the bulk of the code from?\n>\n> If there isn't, use \"null_term_line\" like they do.\n\nIn check-attr, null_term_line indicates that _input_ lines are\nnull-terminated. In check-ignore, null_term_lines is overloaded (and\nperhaps abused) to mean that both _input_ and _output_ lines are\nnull-terminated. In check-mailmap, -z affects only _output_ lines. A\nreader of the code can easily be misled into thinking that the\nvariable controls the same function in all three programs, hence\nnull_out was chosen to make it clear that it applies only to output. A\nlesser reason is that, in the future, someone might add an option to\nnull terminate input lines (distinct from output), and null_in would\nbe a reasonable name for that variable.\n\nOther than the above reasoning, I don't feel strongly about it and can\nrevert the name if you prefer.\n\n>> +static void check_mailmap(struct string_list *mailmap, const char *contact)\n>> +{\n>> +     const char *name, *mail;\n>> +     size_t namelen, maillen;\n>> +     struct ident_split ident;\n>> +     char term = null_out ? '\\0' : '\\n';\n>\n> Is there a reason why the variable name \"term\" should not match that\n> of the corresponding variables in check-ignore and check-attr, which\n> you copied the bulk of the code from?\n>\n> If there isn't, use \"line_termination\" like they do.\n\nNo strong justification. As with 'buf' vs. 'buffer',\n'line_termination' required more reading effort without conveying any\nmore information than 'term'.\n\n>> +     if (split_ident_line(&ident, contact, strlen(contact)))\n>> +             die(_(\"unable to parse contact: %s\"), contact);\n>> +\n>> +     name = ident.name_begin;\n>> +     namelen = ident.name_end - ident.name_begin;\n>> +     mail = ident.mail_begin;\n>> +     maillen = ident.mail_end - ident.mail_begin;\n>> +\n>> +     map_user(mailmap, &mail, &maillen, &name, &namelen);\n>> +\n>> +     if (namelen)\n>> +             printf(\"%.*s \", (int)namelen, name);\n>> +     printf(\"<%.*s>%c\", (int)maillen, mail, term);\n>> +}\n>> +\n>> +int cmd_check_mailmap(int argc, const char **argv, const char *prefix)\n>> +{\n>> +     int i;\n>> +     struct string_list mailmap = STRING_LIST_INIT_NODUP;\n>> +\n>> +     git_config(git_default_config, NULL);\n>> +     argc = parse_options(argc, argv, prefix, check_mailmap_options,\n>> +             check_mailmap_usage, 0);\n>\n> It is a bit distracting that the second line that is not indented\n> enough.  Either (1) with enough HT and with minimum number of SP,\n> align \"argc\" and \"check_mailmap_usage\", or (2) with minimum number\n> of HT and no SP, push \"check_mailmap_usage\" to the right of opening\n> parenthesis of \"(argc\".  Our code tend to prefer (1).\n\nOkay.\n"},{"id":"223126","messageId":"7vbo68pbwm.fsf@alter.siamese.dyndns.org","threadId":"34403","inReplyTo":"CAPig+cQvnKSxZ5M6zW_r6Y9xwZ0Ch99JDq4SccYXCRof9svsOg@mail.gmail.com","subject":"Re: [PATCH v2 1/4] builtin: add git-check-mailmap command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T05:47:37Z","receivedAt":"2013-07-12T05:47:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>>         For each contact information (either in the form of ``Name\n>>         <user@host>'' or ...)\n>>\n>> in order to clarify that the two forms of input is what you call\n>> \"contact information\".\n>\n> Is this easier to read?\n>\n>     For each ``Name $$<user@host>$$'' or ``$$<user@host>$$'' from the\n>     command-line or standard input (when using `--stdin`), print a line\n>     showing either the canonical name and email address (see \"Mapping\n>     Authors\" below), or the input ``Name $$<user@host>$$'' or\n>     ``$$<user@host>$$'' if there is no mapping for that person.\n\nI find it easier than your original, but I do not know if you would\nwant to repeat the \"Name... or <user@host>\" at the end.  It does not\nseem to add much useful information and is distracting.\n\n>> If there isn't, use \"null_term_line\" like they do.\n>\n> In check-attr, null_term_line indicates that _input_ lines are\n> null-terminated. In check-ignore, null_term_lines is overloaded (and\n> perhaps abused) to mean that both _input_ and _output_ lines are\n> null-terminated.\n\nThat is unfortunate but it is good that you found the breakage.  As\nwe do not have --nul-terminated-input and --nul-terminated-output\noptions separtely, -z should apply to both input and output.  What\nb4666852 (check-attr: Add --stdin option, 2008-10-07) did is broken.\nWhat check-ignore does \n\nWe should find a way to fix it.  I have a feeling that silently\nfixing it and seeing if anybody screams might be the best course of\naction ;-).\n\nAlso \"git check-ignore -h\" advertises \"-z\" as only affecting \"--stdin\",\nwhich is also wrong.  It does affect both input and output as it should,\nso it should be described as such, I think.\n\nThanks for noticing.\n"},{"id":"223127","messageId":"7v7ggwpbut.fsf@alter.siamese.dyndns.org","threadId":"34403","inReplyTo":"20130712005517.GA8482@google.com","subject":"Re: [PATCH v2 3/4] t4203: test mailmap functionality directly rather than indirectly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T05:48:42Z","receivedAt":"2013-07-12T05:48:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> My current thinking is \"no\" --- the patch has as a justification \"Now\n> we can test these aspects of .mailmap handling directly with a\n> low-level tool instead of using the tool most people will use, so do\n> so\", which sounds an awful lot like \"Reduce test coverage of commonly\n> used tools, because we can\".\n\nYes, that was exactly my reaction that prompted my response.\n"},{"id":"223130","messageId":"CAPig+cSfqONOFmmXX=s+HWKnLr5eG27jU9xgE7Dki0sycPPz1g@mail.gmail.com","threadId":"34403","inReplyTo":"7v7ggwpbut.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 3/4] t4203: test mailmap functionality directly rather than indirectly","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-12T06:05:18Z","receivedAt":"2013-07-12T06:05:18Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 12, 2013 at 1:48 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n>\n>> My current thinking is \"no\" --- the patch has as a justification \"Now\n>> we can test these aspects of .mailmap handling directly with a\n>> low-level tool instead of using the tool most people will use, so do\n>> so\", which sounds an awful lot like \"Reduce test coverage of commonly\n>> used tools, because we can\".\n>\n> Yes, that was exactly my reaction that prompted my response.\n\nDoes any of my follow-up commentary result in a different reaction? If\nnot, I'll drop patches 3 &4 in the re-roll.\n"},{"id":"223139","messageId":"CAPig+cTwXe7rSKUgJPVHbAvrxYbmL77xNXtLyUeRnd8Taxtrdw@mail.gmail.com","threadId":"34403","inReplyTo":"7vbo68pbwm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/4] builtin: add git-check-mailmap command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-12T06:24:43Z","receivedAt":"2013-07-12T06:24:43Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 12, 2013 at 1:47 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n>\n>>>         For each contact information (either in the form of ``Name\n>>>         <user@host>'' or ...)\n>>>\n>>> in order to clarify that the two forms of input is what you call\n>>> \"contact information\".\n>>\n>> Is this easier to read?\n>>\n>>     For each ``Name $$<user@host>$$'' or ``$$<user@host>$$'' from the\n>>     command-line or standard input (when using `--stdin`), print a line\n>>     showing either the canonical name and email address (see \"Mapping\n>>     Authors\" below), or the input ``Name $$<user@host>$$'' or\n>>     ``$$<user@host>$$'' if there is no mapping for that person.\n>\n> I find it easier than your original, but I do not know if you would\n> want to repeat the \"Name... or <user@host>\" at the end.  It does not\n> seem to add much useful information and is distracting.\n\nNext attempt:\n\n    For each ``Name $$<user@host>$$'' or ``$$<user@host>$$'' from the\n    command-line or standard input (when using `--stdin`) look up the\n    person's canonical name and email address (see \"Mapping Authors\"\n    below). If found, print them; otherwise print the input as-is.\n\n>> In check-attr, null_term_line indicates that _input_ lines are\n>> null-terminated. In check-ignore, null_term_lines is overloaded (and\n>> perhaps abused) to mean that both _input_ and _output_ lines are\n>> null-terminated.\n>\n> That is unfortunate but it is good that you found the breakage.  As\n> we do not have --nul-terminated-input and --nul-terminated-output\n> options separtely, -z should apply to both input and output.  What\n> b4666852 (check-attr: Add --stdin option, 2008-10-07) did is broken.\n\nI can make git-check-mailmap behave this way, however, other than\ngit-check-ignore (which is quite new), there doesn't seem to be any\nprecedence (that I can find) anywhere else in git which ties input and\noutput null-termination to a single switch. Is it desirable to do so\nor should the user have more fine-grained control? (\"xargs -0\" comes\nto mind when thinking of a null-termination input switch.)\n\n> Also \"git check-ignore -h\" advertises \"-z\" as only affecting \"--stdin\",\n> which is also wrong.  It does affect both input and output as it should,\n> so it should be described as such, I think.\n\nI also noticed this. (It was copied from check-attr.c).\n"},{"id":"223140","messageId":"7vr4f4nvkw.fsf@alter.siamese.dyndns.org","threadId":"34403","inReplyTo":"CAPig+cSfqONOFmmXX=s+HWKnLr5eG27jU9xgE7Dki0sycPPz1g@mail.gmail.com","subject":"Re: [PATCH v2 3/4] t4203: test mailmap functionality directly rather than indirectly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T06:25:35Z","receivedAt":"2013-07-12T06:25:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Fri, Jul 12, 2013 at 1:48 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Jonathan Nieder <jrnieder@gmail.com> writes:\n>>\n>>> My current thinking is \"no\" --- the patch has as a justification \"Now\n>>> we can test these aspects of .mailmap handling directly with a\n>>> low-level tool instead of using the tool most people will use, so do\n>>> so\", which sounds an awful lot like \"Reduce test coverage of commonly\n>>> used tools, because we can\".\n>>\n>> Yes, that was exactly my reaction that prompted my response.\n>\n> Does any of my follow-up commentary result in a different\n> reaction?\n\nNot really.  While I _do_ think direct testing has merits, I think\nthat should be done by adding direct tests, not by removing the\ntests that are meant to protect higher level _users_ of the\nunderlying mechanism from breakages.  After all, their breakages may\nnot come from new breakages of the lower level mechanism (i.e. the\nmailmap.c code) but the way these higher level code makes calls into\nthe mechanism, and direct test of the lower level mechanism will not\nprotect them from the latter kind of breakages.\n"},{"id":"223144","messageId":"7vmwpsnv5l.fsf@alter.siamese.dyndns.org","threadId":"34403","inReplyTo":"CAPig+cTwXe7rSKUgJPVHbAvrxYbmL77xNXtLyUeRnd8Taxtrdw@mail.gmail.com","subject":"Re: [PATCH v2 1/4] builtin: add git-check-mailmap command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T06:34:46Z","receivedAt":"2013-07-12T06:34:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> I find it easier than your original, but I do not know if you would\n>> want to repeat the \"Name... or <user@host>\" at the end.  It does not\n>> seem to add much useful information and is distracting.\n>\n> Next attempt:\n>\n>     For each ``Name $$<user@host>$$'' or ``$$<user@host>$$'' from the\n>     command-line or standard input (when using `--stdin`) look up the\n>     person's canonical name and email address (see \"Mapping Authors\"\n>     below). If found, print them; otherwise print the input as-is.\n\nNice.\n\n> ... Is it desirable to do so\n> or should the user have more fine-grained control? (\"xargs -0\" comes\n> to mind when thinking of a null-termination input switch.)\n\nFor the purposes of check-attr and check-ignore, a single \"-z\"\ngoverning both is sufficient.  I think you already got that from my\n4-patch series, but the core reason for that is :\n\n - when \"-z\" is used, the user knows the input paths may need\n   protection against LF.\n\n - our output contains these same paths.\n\n - which means our output cannot be expressed unambiguously using LF\n   as record separator.\n\nFor the purpose of check-mailmap, I actually do not see much point\nin supporting \"-z\" format.  We do not even handle names or addresses\nwith LF in it.  The mailmap format would not let you express such\nrecords in the first place, no?\n"},{"id":"223147","messageId":"CAPig+cQz8jAHw3p4Fi30-KGZoVcb1vkgS9yA23abJVJLpDrvUA@mail.gmail.com","threadId":"34403","inReplyTo":"7vmwpsnv5l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/4] builtin: add git-check-mailmap command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-12T06:39:37Z","receivedAt":"2013-07-12T06:39:37Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 12, 2013 at 2:34 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n>\n>> ... Is it desirable to do so\n>> or should the user have more fine-grained control? (\"xargs -0\" comes\n>> to mind when thinking of a null-termination input switch.)\n>\n> For the purposes of check-attr and check-ignore, a single \"-z\"\n> governing both is sufficient.  I think you already got that from my\n> 4-patch series, but the core reason for that is :\n\nI'm reading it right now.\n\n>  - when \"-z\" is used, the user knows the input paths may need\n>    protection against LF.\n>\n>  - our output contains these same paths.\n>\n>  - which means our output cannot be expressed unambiguously using LF\n>    as record separator.\n>\n> For the purpose of check-mailmap, I actually do not see much point\n> in supporting \"-z\" format.  We do not even handle names or addresses\n> with LF in it.  The mailmap format would not let you express such\n> records in the first place, no?\n\nYou're right. I had this exact argument in mind for why\nnull-termination was not needed on the input side of check-mailmap,\nbut for some reason I had a blind spot concerning the output side.\nI'll drop this option in the next re-roll.\n"}]}