{"thread":{"id":"34398","subject":"[RFC/PATCH 1/4] builtin: add git-check-mailmap command","startedAt":"2013-07-10T19:03:57Z","lastAt":"2013-07-11T10:28:18Z","messageCount":10,"participants":["Eric Sunshine","Duy Nguyen","Antoine Pelisse"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"223035","messageId":"1373483041-27901-1-git-send-email-sunshine@sunshineco.com","threadId":"34398","inReplyTo":null,"subject":"[RFC/PATCH 0/4] add git-check-mailmap command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-10T19:03:57Z","receivedAt":"2013-07-10T19:03:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This patch series adds builtin command check-mailmap, similar to\ncheck-attr and check-ignore, which allows direct testing of .mailmap\nconfiguration. More importantly, as plumbing accessible to scripts\nand other porcelain, check-mailmap publishes the stable, well-tested\n.mailmap functionality employed by built-in Git commands.\n\nIt is RFC because it lacks documentation; because its utility as a\ncheck-foo command is minimal compared to check-attr and check-ignore\n(although its utility as plumbing for scripts and porcelain is more\nsignificant); and because I want to make sure that the project is\nwilling to accept yet another check-foo command.\n\nThe idea and motivation for git-check-mailmap arose from the effort to\nimplement .mailmap support for git-contacts [1] (presently at\nes/contacts in 'pu').\n\nFelipe's Ruby implementation of .mailmap support for his git-related\nscript is introduced in patch 9/15 [2] of v5, and is augmented in patch\n10/15 [3] to support invocation from within a subdirectory. His version\nsupports configuration variable mailmap.file, but not mailmap.blob.\n\nRather than rewriting the functionality yet again, this time in Perl,\nalong with all the details and nuances of the C version employed by\nGit builtins, I decided that it made more sense to expose the\nwell-tested and polished C implementation as plumbing, thus allowing\nany script or porcelain to take advantage of it.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/229533/\n[2]: http://article.gmane.org/gmane.comp.version-control.git/224782/\n[3]: http://article.gmane.org/gmane.comp.version-control.git/224783/\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 Makefile | 1 +\n builtin.h | 1 +\n builtin/check-mailmap.c | 73 ++++++++++\n command-list.txt | 1 +\n contrib/completion/git-completion.bash | 1 +\n git.c | 1 +\n t/t4203-mailmap.sh | 246 +++++++++++++++++----------------\n 8 files changed, 209 insertions(+), 116 deletions(-)\n create mode 100644 builtin/check-mailmap.c\n\n-- \n1.8.3.2\n"},{"id":"223032","messageId":"1373483041-27901-2-git-send-email-sunshine@sunshineco.com","threadId":"34398","inReplyTo":"1373483041-27901-1-git-send-email-sunshine@sunshineco.com","subject":"[RFC/PATCH 1/4] builtin: add git-check-mailmap command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-10T19:03:58Z","receivedAt":"2013-07-10T19:03:58Z","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 \n---\n .gitignore | 1 +\n Makefile | 1 +\n builtin.h | 1 +\n builtin/check-mailmap.c | 73 ++++++++++++++++++++++++++++++++++\n command-list.txt | 1 +\n contrib/completion/git-completion.bash | 1 +\n git.c | 1 +\n 7 files changed, 79 insertions(+)\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/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..cbcc6db\n--- /dev/null\n+++ b/builtin/check-mailmap.c\n@@ -0,0 +1,73 @@\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] ...\"),\n+NULL\n+};\n+\n+static const struct option check_mailmap_options[] = {\n+\tOPT_BOOLEAN(0, \"stdin\", &use_stdin,\n+\t\t N_(\"also read contacts from stdin\")),\n+\tOPT_BOOLEAN('z', NULL, &null_out,\n+\t\t 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 <%.*s>%c\",\n+\t\t\t(int)namelen, name, (int)maillen, mail, term);\n+\telse\n+\t\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, \"contact to 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, \"contact to 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":"223034","messageId":"1373483041-27901-3-git-send-email-sunshine@sunshineco.com","threadId":"34398","inReplyTo":"1373483041-27901-1-git-send-email-sunshine@sunshineco.com","subject":"[RFC/PATCH 2/4] t4203: test check-mailmap command invocation","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-10T19:03:59Z","receivedAt":"2013-07-10T19:03:59Z","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 \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 \n+\tnick1 \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 \" -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 \n+\tnick1 \n+\tEOF\n+\tgit check-mailmap \\\n+\t\t\"A U Thor \" \\\n+\t\t\"nick1 \" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'check-mailmap --stdin' '\n+\tcat >expect <<-\\EOF &&\n+\tA U Thor \n+\tnick1 \n+\tEOF\n+\tgit check-mailmap --stdin actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'check-mailmap --stdin arguments' '\n+\tcat >expect <<-\\EOF &&\n+\tInternal Guy \n+\tEOF\n+\tcat >expect &&\n+\tgit check-mailmap --stdin \"Internal Guy \" \\\n+\t\tactual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'check-mailmap -z' '\n+\tprintf \"A U Thor \\0\" >expect &&\n+\tprintf \"nick1 \\0\" >>expect &&\n+\tgit check-mailmap -z --stdin 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":"223033","messageId":"1373483041-27901-4-git-send-email-sunshine@sunshineco.com","threadId":"34398","inReplyTo":"1373483041-27901-1-git-send-email-sunshine@sunshineco.com","subject":"[RFC/PATCH 3/4] t4203: test mailmap functionality directly rather than indirectly","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-10T19:04:00Z","receivedAt":"2013-07-10T19:04:00Z","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 \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 \n+nick1 \n EOF\n \n test_expect_success 'No mailmap' '\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin 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 \n+nick1 \n EOF\n \n test_expect_success 'default .mailmap' '\n \techo \"Repo Guy \" > .mailmap &&\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin 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 \n+Internal Guy \n EOF\n test_expect_success 'mailmap.file set' '\n \tmkdir -p internal_mailmap &&\n \techo \"Internal Guy \" > internal_mailmap/.mailmap &&\n \tgit config mailmap.file internal_mailmap/.mailmap &&\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin 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 \n+Internal Guy \n EOF\n test_expect_success 'mailmap.file override' '\n \techo \"External Guy \" >> internal_mailmap/.mailmap &&\n \tgit config mailmap.file internal_mailmap/.mailmap &&\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin 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 \n+nick1 \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 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 \n+Internal Guy \n EOF\n \n test_expect_success 'name entry after email entry' '\n \tmkdir -p internal_mailmap &&\n \techo \" \" >internal_mailmap/.mailmap &&\n \techo \"Internal Guy \" >>internal_mailmap/.mailmap &&\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin 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 \n+Internal Guy \n EOF\n \n test_expect_success 'name entry after email entry, case-insensitive' '\n \tmkdir -p internal_mailmap &&\n \techo \" \" >internal_mailmap/.mailmap &&\n \techo \"Internal Guy \" >>internal_mailmap/.mailmap &&\n-\tgit shortlog HEAD >actual &&\n+\tgit check-mailmap --stdin 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 \n+nick1 \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 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 \n+\tBlob Guy \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\tactual &&\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 \n+\tBlob Guy \n \tEOF\n-\tgit -c mailmap.blob=map:both shortlog HEAD >actual &&\n+\tgit -c mailmap.blob=map:both check-mailmap --stdin \\\n+\t\tactual &&\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 \n+\tBlob Guy \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 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 \n+\tnick1 \n \tEOF\n-\tgit -c mailmap.blob=map:nonexistent shortlog HEAD >actual &&\n+\tgit -c mailmap.blob=map:nonexistent check-mailmap --stdin \\\n+\t\tactual &&\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 \" &&\n-\t\techo \" 1\tFake Name\" >expect &&\n-\t\tgit shortlog -ns HEAD >actual &&\n+\t\techo \"Fake Name \" >expect &&\n+\t\tgit check-mailmap \"A U Thor \" >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 \" >expect &&\n+\t\tgit check-mailmap \"A U Thor \" >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 \" >expect &&\n+\t\tgit check-mailmap \"A U Thor \" >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n-- \n1.8.3.2\n"},{"id":"223036","messageId":"1373483041-27901-5-git-send-email-sunshine@sunshineco.com","threadId":"34398","inReplyTo":"1373483041-27901-1-git-send-email-sunshine@sunshineco.com","subject":"[RFC/PATCH 4/4] t4203: consolidate test-repository setup","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-10T19:04:01Z","receivedAt":"2013-07-10T19:04:01Z","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 repository-setup function.\n\nSigned-off-by: Eric Sunshine \n---\n\nThe code near the top of the file which creates two commits, and the\ncode later which creates five commits have been consolidated into a\nsingle repository-setup function. The patch is primarily code movement,\nbut the noisy diff makes it look more busy than it actually is.\n\n\n t/t4203-mailmap.sh | 69 ++++++++++++++++++++++++++++++------------------------\n 1 file changed, 38 insertions(+), 31 deletions(-)\n\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 48a000b..7e8b3cc 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -13,19 +13,10 @@ fuzz_blame () {\n }\n \n test_expect_success setup '\n-\tcat >contacts <<-\\EOF &&\n+\tcat >contacts <<-\\EOF\n \tA U Thor \n \tnick1 \n \tEOF\n-\n-\techo one >one &&\n-\tgit add one &&\n-\ttest_tick &&\n-\tgit commit -m initial &&\n-\techo two >>one &&\n-\tgit add one &&\n-\ttest_tick &&\n-\tgit commit --author \"nick1 \" -m second\n '\n \n test_expect_success 'check-mailmap no arguments' '\n@@ -168,8 +159,10 @@ test_expect_success 'No mailmap files, but configured' '\n '\n \n test_expect_success 'setup mailmap blob tests' '\n+\tgit checkout --orphan empty &&\n+\tgit commit --allow-empty --allow-empty-message &&\n \tgit checkout -b map &&\n-\ttest_when_finished \"git checkout master\" &&\n+\ttest_when_finished \"git checkout empty\" &&\n \tcat >just-bugs <<-\\EOF &&\n \tBlob Guy \n \tEOF\n@@ -254,28 +247,19 @@ test_expect_success 'cleanup after mailmap.blob tests' '\n \trm -f .mailmap\n '\n \n-# Extended mailmap configurations should give us the following output for shortlog\n-cat >expect <<\\EOF\n-A U Thor (1):\n- initial\n-\n-CTO (1):\n- seventh\n+test_expect_success 'setup mailmap test repo' '\n+\tgit checkout --orphan master &&\n \n-Other Author (2):\n- third\n- fourth\n-\n-Santa Claus (2):\n- fifth\n- sixth\n-\n-Some Dude (1):\n- second\n+\techo one >one &&\n+\tgit add one &&\n+\ttest_tick &&\n+\tgit commit -m initial &&\n \n-EOF\n+\techo two >>one &&\n+\tgit add one &&\n+\ttest_tick &&\n+\tgit commit --author \"nick1 \" -m second &&\n \n-test_expect_success 'Shortlog output (complex mapping)' '\n \techo three >>one &&\n \tgit add one &&\n \ttest_tick &&\n@@ -299,8 +283,31 @@ test_expect_success 'Shortlog output (complex mapping)' '\n \techo seven >>one &&\n \tgit add one &&\n \ttest_tick &&\n-\tgit commit --author \"CTO \" -m seventh &&\n+\tgit commit --author \"CTO \" -m seventh\n+'\n+\n+# Extended mailmap configurations should give us the following output for shortlog\n+cat >expect <<\\EOF\n+A U Thor (1):\n+ initial\n+\n+CTO (1):\n+ seventh\n \n+Other Author (2):\n+ third\n+ fourth\n+\n+Santa Claus (2):\n+ fifth\n+ sixth\n+\n+Some Dude (1):\n+ second\n+\n+EOF\n+\n+test_expect_success 'Shortlog output (complex mapping)' '\n \tmkdir -p internal_mailmap &&\n \techo \"Committed \" > internal_mailmap/.mailmap &&\n \techo \" \" >> internal_mailmap/.mailmap &&\n-- \n1.8.3.2\n"},{"id":"223041","messageId":"CACsJy8C6_ycum9RfxgfB0Xa2U7Fk02NyED5VvH6o1pExkcV6vA@mail.gmail.com","threadId":"34398","inReplyTo":"1373483041-27901-2-git-send-email-sunshine@sunshineco.com","subject":"Re: [RFC/PATCH 1/4] builtin: add git-check-mailmap command","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-07-11T02:31:25Z","receivedAt":"2013-07-11T02:31:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jul 11, 2013 at 2:03 AM, Eric Sunshine wrote:\n> +static const struct option check_mailmap_options[] = {\n> + OPT_BOOLEAN(0, \"stdin\", &use_stdin,\n> + N_(\"also read contacts from stdin\")),\n> + OPT_BOOLEAN('z', NULL, &null_out,\n> + N_(\"null-terminate output lines\")),\n\nI think OPT_BOOLEAN is deprecated in favor of OPT_BOOL (or OPT_COUNTUP\nif you really want -z -z -z to mean differently than -z)\n\n> + maybe_flush_or_die(stdout, \"contact to stdout\");\n\nOn error this function will print\n\nwrite failure on 'contact to stdout'\n\nmaybe maybe_flush_or_die(stdout, \"write contact to stdout\") or\nsomething? From i18n point of view, maybe_flush_or_die should not\ncompose a sentence this way. Let the second argument be a complete\nsentence so that translators have more freedom. But that's a different\nissue.\n--\nDuy\n"},{"id":"223042","messageId":"CAPig+cRxuqOvRZQ5WZmUWruvEQytYM+RduYdkkN4_f8UdcuQnQ@mail.gmail.com","threadId":"34398","inReplyTo":"CACsJy8C6_ycum9RfxgfB0Xa2U7Fk02NyED5VvH6o1pExkcV6vA@mail.gmail.com","subject":"Re: [RFC/PATCH 1/4] builtin: add git-check-mailmap command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-11T05:50:46Z","receivedAt":"2013-07-11T05:50:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jul 10, 2013 at 10:31 PM, Duy Nguyen wrote:\n> On Thu, Jul 11, 2013 at 2:03 AM, Eric Sunshine wrote:\n>> +static const struct option check_mailmap_options[] = {\n>> + OPT_BOOLEAN(0, \"stdin\", &use_stdin,\n>> + N_(\"also read contacts from stdin\")),\n>> + OPT_BOOLEAN('z', NULL, &null_out,\n>> + N_(\"null-terminate output lines\")),\n>\n> I think OPT_BOOLEAN is deprecated in favor of OPT_BOOL (or OPT_COUNTUP\n> if you really want -z -z -z to mean differently than -z)\n\nThanks, I knew this and intended to change it to OPT_BOOL but forgot.\n(The OPT_BOOLEAN was inherited from check-attr.c and check-ignore.c\nwhich I used as templates for check-mailmap.c.)\n\n>> + maybe_flush_or_die(stdout, \"contact to stdout\");\n>\n> On error this function will print\n>\n> write failure on 'contact to stdout'\n>\n> maybe maybe_flush_or_die(stdout, \"write contact to stdout\") or\n> something? From i18n point of view, maybe_flush_or_die should not\n> compose a sentence this way. Let the second argument be a complete\n> sentence so that translators have more freedom. But that's a different\n> issue.\n\nIndeed, it's not ideal. I chose \"contact to stdout\" for consistency\nwith other callers, not because of a fondness for it.\n\n % git grep maybe_flush_or_die\n builtin/blame.c: maybe_flush_or_die(stdout, \"stdout\");\n builtin/check-attr.c: maybe_flush_or_die(stdout, \"attribute to stdout\");\n builtin/check-attr.c: maybe_flush_or_die(stdout, \"attribute to stdout\");\n builtin/check-ignore.c: maybe_flush_or_die(stdout, \"check-ignore to stdout\");\n builtin/check-ignore.c: maybe_flush_or_die(stdout, \"ignore to stdout\");\n builtin/hash-object.c: maybe_flush_or_die(stdout, \"hash to stdout\");\n builtin/rev-list.c: maybe_flush_or_die(stdout, \"stdout\");\n log-tree.c: maybe_flush_or_die(stdout, \"stdout\");\n\nThey seem pretty evenly split between just \"stdout\" and \"foo to\nstdout\". (I actually prefer plain \"stdout\" and will happily change it\nto that.)\n\n-- ES\n"},{"id":"223044","messageId":"CALWbr2zJTBSptGsOr6tqrr4KcVd2GOWCkgy4GgdZ2+0Vz7DU4w@mail.gmail.com","threadId":"34398","inReplyTo":"1373483041-27901-2-git-send-email-sunshine@sunshineco.com","subject":"Re: [RFC/PATCH 1/4] builtin: add git-check-mailmap command","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-07-11T06:45:42Z","receivedAt":"2013-07-11T06:45:42Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Wed, Jul 10, 2013 at 9:03 PM, Eric Sunshine wrote:\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> + 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\nWould it be useful to check the return value of this function, to\ndisplay a message when the name can't mapped ?\n\n> + if (namelen)\n> + printf(\"%.*s <%.*s>%c\",\n> + (int)namelen, name, (int)maillen, mail, term);\n> + else\n> + printf(\"<%.*s>%c\", (int)maillen, mail, term);\n> +}\n"},{"id":"223043","messageId":"CACsJy8ADu+RNCgqHQG20nC+0S7sZKwMCg=yX+i906dwhvFrEtQ@mail.gmail.com","threadId":"34398","inReplyTo":"CAPig+cRxuqOvRZQ5WZmUWruvEQytYM+RduYdkkN4_f8UdcuQnQ@mail.gmail.com","subject":"Re: [RFC/PATCH 1/4] builtin: add git-check-mailmap command","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-07-11T06:47:38Z","receivedAt":"2013-07-11T06:47:38Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jul 11, 2013 at 12:50 PM, Eric Sunshine wrote:\n>>> + maybe_flush_or_die(stdout, \"contact to stdout\");\n>>\n>> On error this function will print\n>>\n>> write failure on 'contact to stdout'\n>>\n>> maybe maybe_flush_or_die(stdout, \"write contact to stdout\") or\n>> something? From i18n point of view, maybe_flush_or_die should not\n>> compose a sentence this way. Let the second argument be a complete\n>> sentence so that translators have more freedom. But that's a different\n>> issue.\n>\n> Indeed, it's not ideal. I chose \"contact to stdout\" for consistency\n> with other callers, not because of a fondness for it.\n>\n> % git grep maybe_flush_or_die\n> builtin/blame.c: maybe_flush_or_die(stdout, \"stdout\");\n> builtin/check-attr.c: maybe_flush_or_die(stdout, \"attribute to stdout\");\n> builtin/check-attr.c: maybe_flush_or_die(stdout, \"attribute to stdout\");\n> builtin/check-ignore.c: maybe_flush_or_die(stdout, \"check-ignore to stdout\");\n> builtin/check-ignore.c: maybe_flush_or_die(stdout, \"ignore to stdout\");\n> builtin/hash-object.c: maybe_flush_or_die(stdout, \"hash to stdout\");\n> builtin/rev-list.c: maybe_flush_or_die(stdout, \"stdout\");\n> log-tree.c: maybe_flush_or_die(stdout, \"stdout\");\n>\n> They seem pretty evenly split between just \"stdout\" and \"foo to\n> stdout\". (I actually prefer plain \"stdout\" and will happily change it\n> to that.)\n\nThen probably stick to the convention. If this i18n thing is fixed, it\nhas to be fixed everywhere anyway.\n-- \nDuy\n"},{"id":"223046","messageId":"CAPig+cSNvXQTvhdU-9YTRa3M_BDPBB0jH87y-C16E+_Qqo_Lxg@mail.gmail.com","threadId":"34398","inReplyTo":"CALWbr2zJTBSptGsOr6tqrr4KcVd2GOWCkgy4GgdZ2+0Vz7DU4w@mail.gmail.com","subject":"Re: [RFC/PATCH 1/4] builtin: add git-check-mailmap command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-11T10:28:18Z","receivedAt":"2013-07-11T10:28:18Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jul 11, 2013 at 2:45 AM, Antoine Pelisse wrote:\n> On Wed, Jul 10, 2013 at 9:03 PM, Eric Sunshine wrote:\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>> + 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> Would it be useful to check the return value of this function, to\n> display a message when the name can't mapped ?\n\nI thought about it but decided against the added complexity (at least\ninitially) for a few reasons.\n\nThere are only two callers in the existing code which even look at the\nreturn value:\n\n % git grep 'map_user('\n builtin/blame.c: map_user(&mailmap, &mailbuf, &maillen,\n builtin/shortlog.c: map_user(&log->mailmap, &mailbuf, &maillen,\n&namebuf, &namelen);\n pretty.c: map_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);\n pretty.c: return mail_map->nr && map_user(mail_map, email,\nemail_len, name, name_len);\n revision.c: if (map_user(mailmap, &mail, &maillen, &name, &namelen)) {\n\nOf those, mailmap_name() in pretty.c merely passes the return value\nalong to its callers, but its callers don't care about it:\n\n % git grep 'mailmap_name('\n pretty.c: mailmap_name(&mail, &maillen, &name, &namelen);\n\ncommit_rewrite_person() in revision.c uses the return value apparently\nonly as an optimization to decide if it should go through the effort\nof replacing the \"old\" person with the re-mapped person, and then\npasses the return value along to its callers, none of which care:\n\n % git grep 'commit_rewrite_person('\n revision.c: commit_rewrite_person(&buf, \"\\nauthor \", opt->mailmap);\n revision.c: commit_rewrite_person(&buf, \"\\ncommitter \", opt->mailmap);\n\nThe sort of optimization taken by commit_rewrite_person() is\neffectively lost to script and porcelain clients of check-mailmap\nsince the cost of executing the command probably swamps any savings\nthe client might otherwise achieve by knowing whether the person was\nremapped. Also, modulo possible whitespace changes, a client can\ncompare what it passed in to what is received back to determine if\nremapping occurred.\n\nAs plumbing, the expectation is that most clients will pass a value in\nand use the output without further interpretation. If check-mailmap\nunconditionally adds some \"mapped\" or \"not mapped\" indicator to each\nreturned value, then that places extra burden on all clients by\nforcing them to parse the result. Alternately, if the indicator is\ncontrolled by a command-line option, then (at the least) that\nincreases documentation costs.\n\nFor people using check-mailmap as a .mailmap debugging aid, such an\nindicator might indeed be useful, however, it seems prudent to keep\nthings simple initially by omitting the bells and whistles until\npractical experience shows that more features (and complexity) are\nwarranted.\n\n>> + if (namelen)\n>> + printf(\"%.*s <%.*s>%c\",\n>> + (int)namelen, name, (int)maillen, mail, term);\n>> + else\n>> + printf(\"<%.*s>%c\", (int)maillen, mail, term);\n>> +}\n"}]}