{"thread":{"id":"23598","subject":"[PATCH v2 0/3] pretty: format aliases","startedAt":"2010-04-25T21:56:16Z","lastAt":"2010-04-26T22:11:30Z","messageCount":27,"participants":["Will Palmer","Jonathan Nieder","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"140367","messageId":"1272232579-18895-1-git-send-email-wmpalmer@gmail.com","threadId":"23598","inReplyTo":null,"subject":"[PATCH v2 0/3] pretty: format aliases","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2010-04-25T21:56:16Z","receivedAt":"2010-04-25T21:56:16Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"The following patch series adds the ability to configure aliases for\nuser-defined formats. The first two patches define new placeholders and\nmodify the output of existing placeholders to allow aliases to be more\nconsistent with the way builtin formats are handled. The final patch\nadds support for the aliases themselves.\n\nThere were a couple of places where I wasn't entirely sure about which\ncolor setting I should be following, but I've tried to be consistent\nthroughout. It may be that I could have simply followed diffopt's color\noption in all cases, in which case various modifications to show_log()\nwere entirely unnecessary. I'll await judgement at the hands of one who\ngroks those sections more than I do, but I think what I've done feels\ncorrect.\n\nMy original goal was to make it possible to define all of the builtin\nformats as builtin aliases to format strings, but complications\nregarding how --parents and --decorate would be handled require further\nthought and discussion. For example, we could simply make\n\"--format=%H --decorate\" synonymous with \"--format=%H%d\", but I'm not\nsure if that feels clean enough.\n\nFor now, I think this is at a point where its good-enough to submit, if\nonly as a starting point for some discussion as to where to head next.\n\nThis is the second version of the patch. Following feedback from Jeff\nKing , I realized that the modification to the arguments\nof show_log() were unnecessary, as they only made a difference within\nshow-branch.c, which does not accept a --format option in any case.\n\nWill Palmer (3):\n pretty: add conditional %C?colorname placeholders\n pretty: make %H/%h dependent on --abbrev[-commit]\n pretty: add aliases for pretty formats\n\n Documentation/config.txt | 8 ++\n Documentation/pretty-formats.txt | 1 +\n builtin/log.c | 2 +-\n builtin/rev-list.c | 2 +\n builtin/shortlog.c | 7 +-\n commit.h | 2 +\n log-tree.c | 3 +\n pretty.c | 248 ++++++++++++++++++++++++++++++--------\n shortlog.h | 2 +-\n t/t4205-log-pretty-formats.sh | 87 +++++++++++++\n 10 files changed, 307 insertions(+), 55 deletions(-)\n create mode 100755 t/t4205-log-pretty-formats.sh\n"},{"id":"140368","messageId":"1272232579-18895-2-git-send-email-wmpalmer@gmail.com","threadId":"23598","inReplyTo":"1272232579-18895-1-git-send-email-wmpalmer@gmail.com","subject":"[PATCH v2 1/3] pretty: add conditional %C?colorname placeholders","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2010-04-25T21:56:17Z","receivedAt":"2010-04-25T21:56:17Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"Many commands are able to colorize, or not, depending on a user's\nconfiguration and whether output is being sent to a terminal. However,\nif a user explicitly specifies a --pretty format, color will always be\noutput, regardless of the destination. This would generally be okay, in\na \"do what I tell you, whether or not I should tell you to\" sense, but\nthe assumption fell apart when an alias was defined which may be run in\nvarious contexts: there was no way to specify \"use this color, but only\nif you normally would display color at all\"\n\nHere we add the %C?colorname placeholders which act just as the\n%Ccolorname placeholders, with the exception that the pretty_context is\nchecked to see if color should be used according to configuration\n\nSigned-off-by: Will Palmer \n---\n Documentation/pretty-formats.txt | 1 +\n builtin/log.c | 2 +-\n builtin/rev-list.c | 1 +\n builtin/shortlog.c | 5 ++-\n commit.h | 1 +\n log-tree.c | 1 +\n pretty.c | 49 ++++++++++++++++++++++++-------------\n shortlog.h | 2 +-\n t/t4205-log-pretty-formats.sh | 44 ++++++++++++++++++++++++++++++++++\n 9 files changed, 85 insertions(+), 21 deletions(-)\n create mode 100755 t/t4205-log-pretty-formats.sh\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 1686a54..53eb903 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -132,6 +132,7 @@ The placeholders are:\n - '%Cblue': switch color to blue\n - '%Creset': reset color\n - '%C(...)': color specification, as described in color.branch.* config option\n+- '%C?...: switch to specified color, if relevant color.* config option specifies that color is ok\n - '%m': left, right or boundary mark\n - '%n': newline\n - '%%': a raw '%'\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 6208703..a9fc6ea 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -740,7 +740,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tlog.in1 = 2;\n \tlog.in2 = 4;\n \tfor (i = 0; i < nr; i++)\n-\t\tshortlog_add_commit(&log, list[i]);\n+\t\tshortlog_add_commit(&log, list[i], rev);\n \n \tshortlog_output(&log);\n \ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 51ceb19..5a53862 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -99,6 +99,7 @@ static void show_commit(struct commit *commit, void *data)\n \t\tstruct pretty_print_context ctx = {0};\n \t\tctx.abbrev = revs->abbrev;\n \t\tctx.date_mode = revs->date_mode;\n+\t\tctx.use_color = DIFF_OPT_TST(&revs->diffopt, COLOR_DIFF);\n \t\tpretty_print_commit(revs->commit_format, commit, &buf, &ctx);\n \t\tif (revs->graph) {\n \t\t\tif (buf.len) {\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 06320f5..7aee491 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -136,13 +136,14 @@ static void read_from_stdin(struct shortlog *log)\n \t}\n }\n \n-void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n+void shortlog_add_commit(struct shortlog *log, struct commit *commit, struct rev_info *rev)\n {\n \tconst char *author = NULL, *buffer;\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct strbuf ufbuf = STRBUF_INIT;\n \tstruct pretty_print_context ctx = {0};\n \n+\tctx.use_color = DIFF_OPT_TST(&rev->diffopt, COLOR_DIFF);\n \tpretty_print_commit(CMIT_FMT_RAW, commit, &buf, &ctx);\n \tbuffer = buf.buf;\n \twhile (*buffer && *buffer != '\\n') {\n@@ -183,7 +184,7 @@ static void get_from_rev(struct rev_info *rev, struct shortlog *log)\n \tif (prepare_revision_walk(rev))\n \t\tdie(\"revision walk setup failed\");\n \twhile ((commit = get_revision(rev)) != NULL)\n-\t\tshortlog_add_commit(log, commit);\n+\t\tshortlog_add_commit(log, commit, rev);\n }\n \n static int parse_uint(char const **arg, int comma, int defval)\ndiff --git a/commit.h b/commit.h\nindex 26ec8c0..b6caf91 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -71,6 +71,7 @@ struct pretty_print_context\n \tenum date_mode date_mode;\n \tint need_8bit_cte;\n \tint show_notes;\n+\tint use_color;\n \tstruct reflog_walk_info *reflog_info;\n };\n \ndiff --git a/log-tree.c b/log-tree.c\nindex d3ae969..6bb4748 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -282,6 +282,7 @@ void show_log(struct rev_info *opt)\n \tint abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40;\n \tconst char *extra_headers = opt->extra_headers;\n \tstruct pretty_print_context ctx = {0};\n+\tctx.use_color = DIFF_OPT_TST(&opt->diffopt, COLOR_DIFF);\n \n \topt->loginfo = NULL;\n \tctx.show_notes = opt->show_notes;\ndiff --git a/pretty.c b/pretty.c\nindex 7cb3a2a..fdb5e16 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -634,35 +634,50 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \tstruct format_commit_context *c = context;\n \tconst struct commit *commit = c->commit;\n \tconst char *msg = commit->buffer;\n+\tconst char *color_start;\n \tstruct commit_list *p;\n+\tint use_color = c->pretty_ctx->use_color;\n+\tint conditional_color = 0;\n \tint h1, h2;\n \n \t/* these are independent of the commit */\n \tswitch (placeholder[0]) {\n \tcase 'C':\n-\t\tif (placeholder[1] == '(') {\n+\t\tcolor_start = placeholder;\n+\t\tif (placeholder[1] == '?') {\n+\t\t\tcolor_start++;\n+\t\t\tconditional_color = 1;\n+\t\t}\n+\t\tif (color_start[1] == '(') {\n \t\t\tconst char *end = strchr(placeholder + 2, ')');\n \t\t\tchar color[COLOR_MAXLEN];\n \t\t\tif (!end)\n \t\t\t\treturn 0;\n-\t\t\tcolor_parse_mem(placeholder + 2,\n-\t\t\t\t\tend - (placeholder + 2),\n-\t\t\t\t\t\"--pretty format\", color);\n-\t\t\tstrbuf_addstr(sb, color);\n+\t\t\tif ( !conditional_color || use_color ) {\n+\t\t\t\tcolor_parse_mem(color_start + 2,\n+\t\t\t\t\t\tend - (color_start + 2),\n+\t\t\t\t\t\t\"--pretty format\", color);\n+\t\t\t\tstrbuf_addstr(sb, color);\n+\t\t\t}\n \t\t\treturn end - placeholder + 1;\n \t\t}\n-\t\tif (!prefixcmp(placeholder + 1, \"red\")) {\n-\t\t\tstrbuf_addstr(sb, GIT_COLOR_RED);\n-\t\t\treturn 4;\n-\t\t} else if (!prefixcmp(placeholder + 1, \"green\")) {\n-\t\t\tstrbuf_addstr(sb, GIT_COLOR_GREEN);\n-\t\t\treturn 6;\n-\t\t} else if (!prefixcmp(placeholder + 1, \"blue\")) {\n-\t\t\tstrbuf_addstr(sb, GIT_COLOR_BLUE);\n-\t\t\treturn 5;\n-\t\t} else if (!prefixcmp(placeholder + 1, \"reset\")) {\n-\t\t\tstrbuf_addstr(sb, GIT_COLOR_RESET);\n-\t\t\treturn 6;\n+\n+\t\tif (!prefixcmp(color_start + 1, \"red\")) {\n+\t\t\tif ( !conditional_color || use_color )\n+\t\t\t\tstrbuf_addstr(sb, GIT_COLOR_RED);\n+\t\t\treturn 4 + (conditional_color ? 1 : 0);\n+\t\t} else if (!prefixcmp(color_start + 1, \"green\")) {\n+\t\t\tif ( !conditional_color || use_color )\n+\t\t\t\tstrbuf_addstr(sb, GIT_COLOR_GREEN);\n+\t\t\treturn 6 + (conditional_color ? 1 : 0);\n+\t\t} else if (!prefixcmp(color_start + 1, \"blue\")) {\n+\t\t\tif ( !conditional_color || use_color )\n+\t\t\t\tstrbuf_addstr(sb, GIT_COLOR_BLUE);\n+\t\t\treturn 5 + (conditional_color ? 1 : 0);\n+\t\t} else if (!prefixcmp(color_start + 1, \"reset\")) {\n+\t\t\tif ( !conditional_color || use_color )\n+\t\t\t\tstrbuf_addstr(sb, GIT_COLOR_RESET);\n+\t\t\treturn 6 + (conditional_color ? 1 : 0);\n \t\t} else\n \t\t\treturn 0;\n \tcase 'n':\t\t/* newline */\ndiff --git a/shortlog.h b/shortlog.h\nindex bc02cc2..43498a0 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -20,7 +20,7 @@ struct shortlog {\n \n void shortlog_init(struct shortlog *log);\n \n-void shortlog_add_commit(struct shortlog *log, struct commit *commit);\n+void shortlog_add_commit(struct shortlog *log, struct commit *commit, struct rev_info *rev);\n \n void shortlog_output(struct shortlog *log);\n \ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nnew file mode 100755\nindex 0000000..28d2948\n--- /dev/null\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -0,0 +1,44 @@\n+#!/bin/sh\n+#\n+# Released into Public Domain by Will Palmer 2010\n+#\n+\n+test_description='Test pretty formats'\n+. ./test-lib.sh\n+\n+test_expect_success \"set up basic repos\" \">foo && git add foo && git commit -m initial\"\n+\n+for flag in false true always; do\n+for color in red green blue reset; do\n+\n+\tmake_expected=\"git config --get-color no.such.slot $color >expected\"\n+\ttest_expect_success \"%C$color with color.ui $flag\" \\\n+\t\t\"$make_expected &&\n+\t\tgit config color.ui $flag &&\n+\t\tgit log -1 --pretty=format:'%C$color' > actual &&\n+\t\tcmp expected actual\"\n+\n+\n+\ttest_expect_success \"%C($color) with color.ui $flag\" \\\n+\t\t\"$make_expected &&\n+\t\tgit config color.ui $flag &&\n+\t\tgit log -1 --pretty=format:'%C($color)' > actual &&\n+\t\tcmp expected actual\"\n+\n+\t[ ! \"$flag\" = \"always\" ] && make_expected=\">expected\"\n+\ttest_expect_success \"%C?$color with color.ui $flag\" \\\n+\t\t\"$make_expected &&\n+\t\tgit config color.ui $flag &&\n+\t\tgit log -1 --pretty=format:'%C?$color' > actual &&\n+\t\tcmp expected actual\"\n+\n+\ttest_expect_success \"%C?($color) with color.ui $flag\" \\\n+\t\t\"$make_expected &&\n+\t\tgit config color.ui $flag &&\n+\t\tgit log -1 --pretty=format:'%C?($color)' > actual &&\n+\t\tcmp expected actual\"\n+\n+done\n+done\n+\n+test_done\n-- \n1.7.1.rc1.13.gbb0a0a.dirty\n"},{"id":"140369","messageId":"1272232579-18895-3-git-send-email-wmpalmer@gmail.com","threadId":"23598","inReplyTo":"1272232579-18895-2-git-send-email-wmpalmer@gmail.com","subject":"[PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2010-04-25T21:56:18Z","receivedAt":"2010-04-25T21:56:18Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"Prior to this, the output of %H was always 40 characters long, and the\noutput of %h always DEFAULT_ABBREV characters long, without regard to\nwhether --abbrev-commit or --abbrev had been passed.\n\nHere we make \"git log --pretty=%H --abbrev-commit\" synonymous with\n\"git log --pretty=%h\", and make %h/abbreviated-%H respect the length\nspecified for --abbrev.\n\nThe same is applied to other commit-placeholders %P and %p, and\n--abbrev is respected for %t, though %T is not changed.\n\nSigned-off-by: Will Palmer \n---\n builtin/rev-list.c | 1 +\n builtin/shortlog.c | 2 ++\n commit.h | 1 +\n log-tree.c | 2 ++\n pretty.c | 30 +++++++++++++++++++-----------\n 5 files changed, 25 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 5a53862..1d1e59c 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -98,6 +98,7 @@ static void show_commit(struct commit *commit, void *data)\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\tstruct pretty_print_context ctx = {0};\n \t\tctx.abbrev = revs->abbrev;\n+\t\tctx.abbrev_commit = revs->abbrev_commit;\n \t\tctx.date_mode = revs->date_mode;\n \t\tctx.use_color = DIFF_OPT_TST(&revs->diffopt, COLOR_DIFF);\n \t\tpretty_print_commit(revs->commit_format, commit, &buf, &ctx);\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 7aee491..5c0721c 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -143,6 +143,8 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit, struct rev\n \tstruct strbuf ufbuf = STRBUF_INIT;\n \tstruct pretty_print_context ctx = {0};\n \n+\tctx.abbrev = rev->abbrev;\n+\tctx.abbrev_commit = rev->abbrev_commit;\n \tctx.use_color = DIFF_OPT_TST(&rev->diffopt, COLOR_DIFF);\n \tpretty_print_commit(CMIT_FMT_RAW, commit, &buf, &ctx);\n \tbuffer = buf.buf;\ndiff --git a/commit.h b/commit.h\nindex b6caf91..7a476a0 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -72,6 +72,7 @@ struct pretty_print_context\n \tint need_8bit_cte;\n \tint show_notes;\n \tint use_color;\n+\tint abbrev_commit;\n \tstruct reflog_walk_info *reflog_info;\n };\n \ndiff --git a/log-tree.c b/log-tree.c\nindex 6bb4748..0a2309c 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -282,6 +282,8 @@ void show_log(struct rev_info *opt)\n \tint abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40;\n \tconst char *extra_headers = opt->extra_headers;\n \tstruct pretty_print_context ctx = {0};\n+\tctx.abbrev = opt->abbrev;\n+\tctx.abbrev_commit = opt->abbrev_commit;\n \tctx.use_color = DIFF_OPT_TST(&opt->diffopt, COLOR_DIFF);\n \n \topt->loginfo = NULL;\ndiff --git a/pretty.c b/pretty.c\nindex fdb5e16..f884f48 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -725,13 +725,16 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \n \tswitch (placeholder[0]) {\n \tcase 'H':\t\t/* commit hash */\n-\t\tstrbuf_addstr(sb, sha1_to_hex(commit->object.sha1));\n-\t\treturn 1;\n \tcase 'h':\t\t/* abbreviated commit hash */\n+\t\tif (placeholder[0] != 'h' && !c->pretty_ctx->abbrev_commit) {\n+\t\t\tstrbuf_addstr(sb, sha1_to_hex(commit->object.sha1));\n+\t\t\treturn 1;\n+\t\t}\n+\n \t\tif (add_again(sb, &c->abbrev_commit_hash))\n \t\t\treturn 1;\n \t\tstrbuf_addstr(sb, find_unique_abbrev(commit->object.sha1,\n-\t\t DEFAULT_ABBREV));\n+\t\t\t\t\t\t c->pretty_ctx->abbrev));\n \t\tc->abbrev_commit_hash.len = sb->len - c->abbrev_commit_hash.off;\n \t\treturn 1;\n \tcase 'T':\t\t/* tree hash */\n@@ -741,24 +744,29 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \t\tif (add_again(sb, &c->abbrev_tree_hash))\n \t\t\treturn 1;\n \t\tstrbuf_addstr(sb, find_unique_abbrev(commit->tree->object.sha1,\n-\t\t DEFAULT_ABBREV));\n+\t\t\t\t\t\t c->pretty_ctx->abbrev));\n \t\tc->abbrev_tree_hash.len = sb->len - c->abbrev_tree_hash.off;\n \t\treturn 1;\n \tcase 'P':\t\t/* parent hashes */\n-\t\tfor (p = commit->parents; p; p = p->next) {\n-\t\t\tif (p != commit->parents)\n-\t\t\t\tstrbuf_addch(sb, ' ');\n-\t\t\tstrbuf_addstr(sb, sha1_to_hex(p->item->object.sha1));\n-\t\t}\n-\t\treturn 1;\n \tcase 'p':\t\t/* abbreviated parent hashes */\n+\t\tif (placeholder[0] != 'p' && !c->pretty_ctx->abbrev_commit) {\n+\t\t\tfor (p = commit->parents; p; p = p->next) {\n+\t\t\t\tif (p != commit->parents)\n+\t\t\t\t\tstrbuf_addch(sb, ' ');\n+\t\t\t\tstrbuf_addstr(sb,\n+\t\t\t\t\t sha1_to_hex(p->item->object.sha1));\n+\t\t\t}\n+\t\t\treturn 1;\n+\t\t}\n+\n \t\tif (add_again(sb, &c->abbrev_parent_hashes))\n \t\t\treturn 1;\n \t\tfor (p = commit->parents; p; p = p->next) {\n \t\t\tif (p != commit->parents)\n \t\t\t\tstrbuf_addch(sb, ' ');\n \t\t\tstrbuf_addstr(sb, find_unique_abbrev(\n-\t\t\t\t\tp->item->object.sha1, DEFAULT_ABBREV));\n+\t\t\t\t p->item->object.sha1,\n+\t\t\t\t c->pretty_ctx->abbrev));\n \t\t}\n \t\tc->abbrev_parent_hashes.len = sb->len -\n \t\t c->abbrev_parent_hashes.off;\n-- \n1.7.1.rc1.13.gbb0a0a.dirty\n"},{"id":"140370","messageId":"1272232579-18895-4-git-send-email-wmpalmer@gmail.com","threadId":"23598","inReplyTo":"1272232579-18895-3-git-send-email-wmpalmer@gmail.com","subject":"[PATCH v2 3/3] pretty: add aliases for pretty formats","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2010-04-25T21:56:19Z","receivedAt":"2010-04-25T21:56:19Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"previously the only ways to alias a --pretty format within git were\neither to set the format as your default format (via the format.pretty\nconfiguration variable), or by using a regular git alias. This left the\ndefinition of more complicated formats to the realm of \"builtin or\nnothing\", with user-defined formats usually being reserved for quick\none-offs.\n\nHere we allow user-defined formats to enjoy more or less the same\nbenefits of builtins. By defining format.pretty.myalias, \"myalias\" can\nbe used in place of whatever would normally come after --pretty=. This\ncan be a format:, tformat:, raw (ie, defaulting to tformat), or the name\nof another builtin or user-defined pretty format.\n\nSigned-off-by: Will Palmer \n---\n Documentation/config.txt | 8 ++\n pretty.c | 169 +++++++++++++++++++++++++++++++++++------\n t/t4205-log-pretty-formats.sh | 53 ++++++++++++-\n 3 files changed, 202 insertions(+), 28 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 92f851e..6573d18 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -894,6 +894,14 @@ format.pretty::\n \tSee linkgit:git-log[1], linkgit:git-show[1],\n \tlinkgit:git-whatchanged[1].\n \n+format.pretty.::\n+\tAlias for a --pretty= format string, as specified in\n+\tlinkgit:git-log[1]. Any aliases defined here can be used just\n+\tas the builtin pretty formats could. For example, defining\n+\t\"format.pretty.hash = format:%H\" would cause the invocation\n+\t\"git log --pretty=hash\" to be equivalent to running\n+\t\"git log --pretty=format:%H\".\n+\n format.thread::\n \tThe default threading style for 'git format-patch'. Can be\n \ta boolean value, or `shallow` or `deep`. `shallow` threading\ndiff --git a/pretty.c b/pretty.c\nindex f884f48..d49fec7 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -11,6 +11,16 @@\n #include \"reflog-walk.h\"\n \n static char *user_format;\n+static struct cmt_fmt_map {\n+\tconst char *name;\n+\tenum cmit_fmt format;\n+\tconst char *user_format;\n+\tint is_tformat;\n+\tint is_alias;\n+} *commit_formats = NULL;\n+static size_t commit_formats_len = 0;\n+static size_t commit_formats_alloc = 0;\n+static struct cmt_fmt_map *find_commit_format(const char *sought);\n \n static void save_user_format(struct rev_info *rev, const char *cp, int is_tformat)\n {\n@@ -21,22 +31,134 @@ static void save_user_format(struct rev_info *rev, const char *cp, int is_tforma\n \trev->commit_format = CMIT_FMT_USERFORMAT;\n }\n \n-void get_commit_format(const char *arg, struct rev_info *rev)\n+static int git_pretty_formats_config(const char *var, const char *value, void *cb)\n+{\n+\tif (!prefixcmp(var, \"format.pretty.\")) {\n+\t\tstruct cmt_fmt_map user_format = {0};\n+\t\tconst char *fmt;\n+\n+\t\tuser_format.name = xstrdup(&var[14]);\n+\t\tuser_format.format = CMIT_FMT_USERFORMAT;\n+\t\tgit_config_string(&fmt, var, value);\n+\t\tif (!prefixcmp(fmt, \"format:\") || !prefixcmp(fmt, \"tformat:\")) {\n+\t\t\tuser_format.is_tformat = fmt[0] == 't';\n+\t\t\tfmt = strchr(fmt, ':') + 1;\n+\t\t} else if (strchr(fmt, '%'))\n+\t\t\tuser_format.is_tformat = 1;\n+\t\telse\n+\t\t\tuser_format.is_alias = 1;\n+\t\tuser_format.user_format = fmt;\n+\n+\t\tALLOC_GROW(commit_formats, commit_formats_len+1,\n+\t\t\t commit_formats_alloc);\n+\t\tmemcpy(&commit_formats[ commit_formats_len ], &user_format,\n+\t\t sizeof(user_format));\n+\t\tcommit_formats_len++;\n+\t}\n+\treturn 0;\n+}\n+\n+static void setup_commit_formats(void)\n {\n \tint i;\n-\tstatic struct cmt_fmt_map {\n-\t\tconst char *n;\n-\t\tsize_t cmp_len;\n-\t\tenum cmit_fmt v;\n-\t} cmt_fmts[] = {\n-\t\t{ \"raw\",\t1,\tCMIT_FMT_RAW },\n-\t\t{ \"medium\",\t1,\tCMIT_FMT_MEDIUM },\n-\t\t{ \"short\",\t1,\tCMIT_FMT_SHORT },\n-\t\t{ \"email\",\t1,\tCMIT_FMT_EMAIL },\n-\t\t{ \"full\",\t5,\tCMIT_FMT_FULL },\n-\t\t{ \"fuller\",\t5,\tCMIT_FMT_FULLER },\n-\t\t{ \"oneline\",\t1,\tCMIT_FMT_ONELINE },\n+\tconst char **attempted_aliases = NULL;\n+\tsize_t attempted_aliases_alloc = 0;\n+\tsize_t attempted_aliases_len;\n+\tstruct cmt_fmt_map builtin_formats[] = {\n+\t\t{ \"raw\",\tCMIT_FMT_RAW,\t\tNULL,\t0 },\n+\t\t{ \"medium\",\tCMIT_FMT_MEDIUM,\tNULL,\t0 },\n+\t\t{ \"short\",\tCMIT_FMT_SHORT,\t\tNULL,\t0 },\n+\t\t{ \"email\",\tCMIT_FMT_EMAIL,\t\tNULL,\t0 },\n+\t\t{ \"full\",\tCMIT_FMT_FULL,\t\tNULL,\t0 },\n+\t\t{ \"fuller\",\tCMIT_FMT_FULLER,\tNULL,\t0 },\n+\t\t{ \"oneline\",\tCMIT_FMT_ONELINE,\tNULL,\t1 }\n \t};\n+\tcommit_formats_len = ARRAY_SIZE(builtin_formats);\n+\tALLOC_GROW(commit_formats, commit_formats_len, commit_formats_alloc);\n+\tmemcpy(commit_formats, builtin_formats,\n+\t sizeof(*builtin_formats)*ARRAY_SIZE(builtin_formats));\n+\n+\tgit_config(git_pretty_formats_config, NULL);\n+\n+\tfor (i = ARRAY_SIZE(builtin_formats); i < commit_formats_len; i++) {\n+\t\tattempted_aliases_len = 0;\n+\t\tstruct cmt_fmt_map *aliased_format = &commit_formats[i];\n+\t\tconst char *fmt = commit_formats[i].user_format;\n+\t\tint j;\n+\n+\t\tif (!commit_formats[i].is_alias)\n+\t\t\tcontinue;\n+\n+\t\twhile ((aliased_format = find_commit_format(fmt))) {\n+\t\t\tif (!aliased_format->is_alias)\n+\t\t\t\tbreak;\n+\n+\t\t\tfmt = aliased_format->user_format;\n+\t\t\tfor (j=0; jformat;\n+\t\t\tcommit_formats[i].user_format = aliased_format->user_format;\n+\t\t\tcommit_formats[i].is_tformat = aliased_format->is_tformat;\n+\t\t\tcommit_formats[i].is_alias = 0;\n+\t\t} else\n+\t\t\tcommit_formats[i].format = CMIT_FMT_UNSPECIFIED;\n+\t}\n+}\n+\n+static struct cmt_fmt_map *find_commit_format(const char *sought)\n+{\n+\tstruct cmt_fmt_map *found = NULL;\n+\tsize_t found_match_len = 0;\n+\n+\tif (!commit_formats)\n+\t\tsetup_commit_formats();\n+\n+\tint i;\n+\tfor (i = 0; i < commit_formats_len; i++) {\n+\t\tconst char *candidate = commit_formats[i].name;\n+\t\tconst char *s = sought;\n+\t\tsize_t match_len = 0;\n+\n+\t\tif (commit_formats[i].format == CMIT_FMT_UNSPECIFIED)\n+\t\t\tcontinue;\n+\n+\t\tfor (; s++, candidate++;) {\n+\t\t\tif (!*candidate && *s) {\n+\t\t\t\tmatch_len = 0;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tif (!*candidate || !*s) {\n+\t\t\t\tmatch_len = s - sought;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tif (*s != *candidate) {\n+\t\t\t\tmatch_len = 0;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t\tif (match_len > found_match_len) {\n+\t\t\tfound = &commit_formats[i];\n+\t\t}\n+\t}\n+\treturn found;\n+}\n+\n+void get_commit_format(const char *arg, struct rev_info *rev)\n+{\n+\tstruct cmt_fmt_map *commit_format;\n \n \trev->use_terminator = 0;\n \tif (!arg || !*arg) {\n@@ -47,21 +169,22 @@ void get_commit_format(const char *arg, struct rev_info *rev)\n \t\tsave_user_format(rev, strchr(arg, ':') + 1, arg[0] == 't');\n \t\treturn;\n \t}\n-\tfor (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {\n-\t\tif (!strncmp(arg, cmt_fmts[i].n, cmt_fmts[i].cmp_len) &&\n-\t\t !strncmp(arg, cmt_fmts[i].n, strlen(arg))) {\n-\t\t\tif (cmt_fmts[i].v == CMIT_FMT_ONELINE)\n-\t\t\t\trev->use_terminator = 1;\n-\t\t\trev->commit_format = cmt_fmts[i].v;\n-\t\t\treturn;\n-\t\t}\n-\t}\n+\n \tif (strchr(arg, '%')) {\n \t\tsave_user_format(rev, arg, 1);\n \t\treturn;\n \t}\n \n-\tdie(\"invalid --pretty format: %s\", arg);\n+\tcommit_format = find_commit_format(arg);\n+\tif( !commit_format )\n+\t\tdie(\"invalid --pretty format: %s\", arg);\n+\n+\trev->commit_format = commit_format->format;\n+\trev->use_terminator = commit_format->is_tformat;\n+\tif( commit_format->format == CMIT_FMT_USERFORMAT ){\n+\t\tsave_user_format(rev, commit_format->user_format,\n+\t\t\t\t commit_format->is_tformat);\n+\t}\n }\n \n /*\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 28d2948..ee3e934 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -6,7 +6,15 @@\n test_description='Test pretty formats'\n . ./test-lib.sh\n \n-test_expect_success \"set up basic repos\" \">foo && git add foo && git commit -m initial\"\n+test_expect_success \"set up basic repos\" \\\n+\t\">foo &&\n+\t>bar &&\n+\tgit add foo &&\n+\ttest_tick &&\n+\tgit commit -m initial &&\n+\tgit add bar &&\n+\ttest_tick &&\n+\tgit commit -m 'add bar'\"\n \n for flag in false true always; do\n for color in red green blue reset; do\n@@ -16,29 +24,64 @@ for color in red green blue reset; do\n \t\t\"$make_expected &&\n \t\tgit config color.ui $flag &&\n \t\tgit log -1 --pretty=format:'%C$color' > actual &&\n-\t\tcmp expected actual\"\n+\t\ttest_cmp expected actual\"\n \n \n \ttest_expect_success \"%C($color) with color.ui $flag\" \\\n \t\t\"$make_expected &&\n \t\tgit config color.ui $flag &&\n \t\tgit log -1 --pretty=format:'%C($color)' > actual &&\n-\t\tcmp expected actual\"\n+\t\ttest_cmp expected actual\"\n \n \t[ ! \"$flag\" = \"always\" ] && make_expected=\">expected\"\n \ttest_expect_success \"%C?$color with color.ui $flag\" \\\n \t\t\"$make_expected &&\n \t\tgit config color.ui $flag &&\n \t\tgit log -1 --pretty=format:'%C?$color' > actual &&\n-\t\tcmp expected actual\"\n+\t\ttest_cmp expected actual\"\n \n \ttest_expect_success \"%C?($color) with color.ui $flag\" \\\n \t\t\"$make_expected &&\n \t\tgit config color.ui $flag &&\n \t\tgit log -1 --pretty=format:'%C?($color)' > actual &&\n-\t\tcmp expected actual\"\n+\t\ttest_cmp expected actual\"\n \n done\n done\n+test_expect_success \"reset color flags\" \"git config --unset color.ui\"\n+\n+test_expect_success \"alias builtin format\" \\\n+\t\"git log --pretty=oneline >expected &&\n+\tgit config format.pretty.test-alias oneline &&\n+\tgit log --pretty=test-alias >actual &&\n+\ttest_cmp expected actual\"\n+\n+test_expect_success \"alias user-defined format\" \\\n+\t\"git log --pretty='format:%h' >expected &&\n+\tgit config format.pretty.test-alias 'format:%h' &&\n+\tgit log --pretty=test-alias >actual &&\n+\ttest_cmp expected actual\"\n+\n+test_expect_success \"alias user-defined tformat\" \\\n+\t\"git log --pretty='tformat:%h' >expected &&\n+\tgit config format.pretty.test-alias 'tformat:%h' &&\n+\tgit log --pretty=test-alias >actual &&\n+\ttest_cmp expected actual\"\n+\n+test_expect_code 128 \"alias non-existant format\" \\\n+\t\"git config format.pretty.test-alias format-that-will-never-exist &&\n+\tgit log --pretty=test-alias\"\n+\n+test_expect_success \"alias of an alias\" \\\n+\t\"git log --pretty='tformat:%h' >expected &&\n+\tgit config format.pretty.test-foo 'tformat:%h' &&\n+\tgit config format.pretty.test-bar test-foo &&\n+\tgit log --pretty=test-bar >actual &&\n+\ttest_cmp expected actual\"\n+\n+test_expect_code 128 \"alias loop\" \\\n+\t\"git config format.pretty.test-foo test-bar &&\n+\tgit config format.pretty.test-bar test-foo &&\n+\tgit log --pretty=test-foo\"\n \n test_done\n-- \n1.7.1.rc1.13.gbb0a0a.dirty\n"},{"id":"140377","messageId":"20100426021347.GA29669@progeny.tock","threadId":"23598","inReplyTo":"1272232579-18895-2-git-send-email-wmpalmer@gmail.com","subject":"Re: [PATCH v2 1/3] pretty: add conditional %C?colorname placeholders","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T02:13:47Z","receivedAt":"2010-04-26T02:13:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Will,\n\nWill Palmer wrote:\n\n> Here we add the %C?colorname placeholders which act just as the\n> %Ccolorname placeholders, with the exception that the pretty_context is\n> checked to see if color should be used according to configuration\n\nThanks for tackling this.\n\nI have thought a little about a related problem: some commands have\nconfiguration for the colors they use, like:\n\n color.grep.\n Use customized color for grep colorization. specifies which\n part of the line to use the specified color, and is one of\n\n context\n non-matching text in context lines (when using -A, -B, or -C)\n\n filename\n filename prefix (when not using -h)\n\nThis is nice because in certain situations (e.g. different background\ncolors), the default colors might not be suitable. As an example of\nthis, the ‘commit ’ line of ‘git log’ output uses color.diff.commit.\n\nSo it would be nice to be able to use %C(diff.commit) and\nautomatically use the right color, if color is enabled.\n\nWhy not make %C always check? I can understand that it would be\nannoying when first trying to use %C. On the other hand, it would be\nmore convenient for writing format.pretty configuration that should be\nshared with old git, and I assume anyone using %C for the first time\nwould be looking at the manual, which could warn her.\n\nCheers,\nJonathan\n"},{"id":"140378","messageId":"20100426031012.GA29953@progeny.tock","threadId":"23598","inReplyTo":"1272232579-18895-3-git-send-email-wmpalmer@gmail.com","subject":"Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T03:11:37Z","receivedAt":"2010-04-26T03:11:37Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Will Palmer wrote:\n\n> Here we make \"git log --pretty=%H --abbrev-commit\" synonymous with\n> \"git log --pretty=%h\", and make %h/abbreviated-%H respect the length\n> specified for --abbrev.\n> \n> The same is applied to other commit-placeholders %P and %p, and\n> --abbrev is respected for %t, though %T is not changed.\n> \n> Signed-off-by: Will Palmer \n> ---\n> builtin/rev-list.c | 1 +\n> builtin/shortlog.c | 2 ++\n> commit.h | 1 +\n> log-tree.c | 2 ++\n> pretty.c | 30 +++++++++++++++++++-----------\n> 5 files changed, 25 insertions(+), 11 deletions(-)\n\nI agree that this is the right to do, since this is how the built-in\nformats work (the ‘commit ...’ line follows the semantics of your %H,\nand ‘Merge: ...’ line your %p, for example).\n\nDocumentation and tests?\n\n> diff --git a/builtin/rev-list.c b/builtin/rev-list.c\n> index 5a53862..1d1e59c 100644\n> --- a/builtin/rev-list.c\n> +++ b/builtin/rev-list.c\n> @@ -98,6 +98,7 @@ static void show_commit(struct commit *commit, void *data)\n> \t\tstruct strbuf buf = STRBUF_INIT;\n> \t\tstruct pretty_print_context ctx = {0};\n> \t\tctx.abbrev = revs->abbrev;\n> +\t\tctx.abbrev_commit = revs->abbrev_commit;\n> \t\tctx.date_mode = revs->date_mode;\n> \t\tctx.use_color = DIFF_OPT_TST(&revs->diffopt, COLOR_DIFF);\n> \t\tpretty_print_commit(revs->commit_format, commit, &buf, &ctx);\n\nMakes sense.\n\n> diff --git a/builtin/shortlog.c b/builtin/shortlog.c\n> index 7aee491..5c0721c 100644\n> --- a/builtin/shortlog.c\n> +++ b/builtin/shortlog.c\n> @@ -143,6 +143,8 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit, struct rev\n> \tstruct strbuf ufbuf = STRBUF_INIT;\n> \tstruct pretty_print_context ctx = {0};\n> \n> +\tctx.abbrev = rev->abbrev;\n> +\tctx.abbrev_commit = rev->abbrev_commit;\n> \tctx.use_color = DIFF_OPT_TST(&rev->diffopt, COLOR_DIFF);\n> \tpretty_print_commit(CMIT_FMT_RAW, commit, &buf, &ctx);\n> \tbuffer = buf.buf;\n\nShortlog doesn’t print commit hashes, does it?\n\n> diff --git a/commit.h b/commit.h\n> index b6caf91..7a476a0 100644\n> --- a/commit.h\n> +++ b/commit.h\n> @@ -72,6 +72,7 @@ struct pretty_print_context\n> \tint need_8bit_cte;\n> \tint show_notes;\n> \tint use_color;\n> +\tint abbrev_commit;\n> \tstruct reflog_walk_info *reflog_info;\n> };\n> \n\nnitpick: I’d stick this up with abbrev and maybe add a comment to\nexplain their distinct uses.\n\n> diff --git a/log-tree.c b/log-tree.c\n> index 6bb4748..0a2309c 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -282,6 +282,8 @@ void show_log(struct rev_info *opt)\n> \tint abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40;\n> \tconst char *extra_headers = opt->extra_headers;\n> \tstruct pretty_print_context ctx = {0};\n> +\tctx.abbrev = opt->abbrev;\n> +\tctx.abbrev_commit = opt->abbrev_commit;\n> \tctx.use_color = DIFF_OPT_TST(&opt->diffopt, COLOR_DIFF);\n> \n> \topt->loginfo = NULL;\n\nThere is a\n\nctx.abbrev = opt->diffopt.abbrev;\n\nlater in the same function; how do these interact?\n\n> @@ -741,24 +744,29 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n[...]\n> \t\t\tstrbuf_addstr(sb, find_unique_abbrev(\n> -\t\t\t\t\tp->item->object.sha1, DEFAULT_ABBREV));\n> +\t\t\t\t p->item->object.sha1,\n> +\t\t\t\t c->pretty_ctx->abbrev));\n\nnitpick: the new indentation makes these look like parameters to\nstrbuf_addstr.\n\nHere’s an alternative implementation of the more controversial half of\nyour patch, for your amusement. The big downside is that it requires\none to specify --abbrev-commit before the --format option.\n\nThanks for the pleasant read.\n\nJonathan\n\ndiff --git a/pretty.c b/pretty.c\nindex 7cb3a2a..1008a41 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -12,10 +12,31 @@\n \n static char *user_format;\n \n+static void abbreviate_commit_hashes(char *fmt)\n+{\n+\tchar *p;\n+\tfor (p = fmt; p != NULL; p = strchr(p + 1, '%')) {\n+\t\tp++;\n+\t\tswitch (*p) {\n+\t\tcase 'H':\n+\t\t\t*p = 'h';\n+\t\t\tbreak;\n+\t\tcase 'P':\n+\t\t\t*p = 'p';\n+\t\t\tbreak;\n+\t\tcase 'T':\n+\t\tdefault:\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+}\n+\n static void save_user_format(struct rev_info *rev, const char *cp, int is_tformat)\n {\n \tfree(user_format);\n \tuser_format = xstrdup(cp);\n+\tif (rev->abbrev_commit)\n+\t\tabbreviate_commit_hashes(user_format);\n \tif (is_tformat)\n \t\trev->use_terminator = 1;\n \trev->commit_format = CMIT_FMT_USERFORMAT;\n"},{"id":"140379","messageId":"20100426032642.GA14421@coredump.intra.peff.net","threadId":"23598","inReplyTo":"20100426021347.GA29669@progeny.tock","subject":"Re: [PATCH v2 1/3] pretty: add conditional %C?colorname placeholders","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-26T03:26:42Z","receivedAt":"2010-04-26T03:26:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 25, 2010 at 09:13:47PM -0500, Jonathan Nieder wrote:\n\n> This is nice because in certain situations (e.g. different background\n> colors), the default colors might not be suitable. As an example of\n> this, the ‘commit ’ line of ‘git log’ output uses color.diff.commit.\n> \n> So it would be nice to be able to use %C(diff.commit) and\n> automatically use the right color, if color is enabled.\n\nFWIW, I like this idea very much. And it would be a bit more natural for\n%C(diff.commit) to respect diff.color, whereas we could perhaps keep\n%Cred as \"always on\" for backwards compatibility.\n\nHowever:\n\n> Why not make %C always check? I can understand that it would be\n> annoying when first trying to use %C. On the other hand, it would be\n\nI am a little nervous that we would be breaking scripts that pipe the\ncolorized output for later display to the user. Right now doing\nsomething like[1]:\n\n log=`git log --format=%Cred%h`\n test \"x$log\" != x && echo \"foo: $log\"\n\ncolorizes, but we would be breaking it. And for new values like\n%C(diff.commit), we are not breaking anything (because it is a new\nsyntax), but a script like the one above may want to convert, and there\nis no way to say \"respect the color _config_, but don't respect\nisatty(1)\". Saying \"--color\" doesn't work, because it overrides the\ncolor config. We can cheat a little with GIT_PAGER_IN_USE, but that will\nhave funny interactions with pager.color.\n\n[1] Yes, I know this snippet is contrived, but it seems within the realm\nof possibility. git-add--interactive reads the diff output in both\nregular and color forms, though it takes some care to look at the user's\nconfig and decide whether to show the color.\n\n-Peff\n"},{"id":"140380","messageId":"20100426033147.GB14421@coredump.intra.peff.net","threadId":"23598","inReplyTo":"20100426031012.GA29953@progeny.tock","subject":"Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-26T03:31:47Z","receivedAt":"2010-04-26T03:31:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 25, 2010 at 10:11:37PM -0500, Jonathan Nieder wrote:\n\n> Here’s an alternative implementation of the more controversial half of\n> your patch, for your amusement. The big downside is that it requires\n> one to specify --abbrev-commit before the --format option.\n\nThat is not insurmountable, as we could just check after the parsing\nstage. But there is a worse problem:\n\n> +static void abbreviate_commit_hashes(char *fmt)\n> +{\n> +\tchar *p;\n> +\tfor (p = fmt; p != NULL; p = strchr(p + 1, '%')) {\n> +\t\tp++;\n> +\t\tswitch (*p) {\n> +\t\tcase 'H':\n> +\t\t\t*p = 'h';\n> +\t\t\tbreak;\n> +\t\tcase 'P':\n> +\t\t\t*p = 'p';\n> +\t\t\tbreak;\n> +\t\tcase 'T':\n> +\t\tdefault:\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +}\n\nYou parse '%%H' incorrectly. I would really rather not see ad-hoc\nparsers for the format like this, but rather use or extend strbuf_expand\nas appropriate. That would make things less painful if and when we\ndecide to tweak the syntax.\n\nI think a lot of this might be more pleasant if we had some extensible\nand backwards compatible syntax like %(placeholder, arg, ...) and could\ndo \"%(H, abbrev=always)\" or \"%(H, abbrev=config)\". Yeah, it's long, but\nit is readable and explicit.\n\n-Peff\n"},{"id":"140381","messageId":"20100426033813.GA30544@progeny.tock","threadId":"23598","inReplyTo":"20100426033147.GB14421@coredump.intra.peff.net","subject":"Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T03:38:13Z","receivedAt":"2010-04-26T03:38:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n> On Sun, Apr 25, 2010 at 10:11:37PM -0500, Jonathan Nieder wrote:\n>\n>> Here’s an alternative implementation of the more controversial half of\n>> your patch, for your amusement. The big downside is that it requires\n>> one to specify --abbrev-commit before the --format option.\n>\n> That is not insurmountable, as we could just check after the parsing\n> stage. But there is a worse problem:\n>\n>> +static void abbreviate_commit_hashes(char *fmt)\n>> +{\n>> +\tchar *p;\n>> +\tfor (p = fmt; p != NULL; p = strchr(p + 1, '%')) {\n>> +\t\tp++;\n>> +\t\tswitch (*p) {\n>> +\t\tcase 'H':\n>> +\t\t\t*p = 'h';\n>> +\t\t\tbreak;\n>> +\t\tcase 'P':\n>> +\t\t\t*p = 'p';\n>> +\t\t\tbreak;\n>> +\t\tcase 'T':\n>> +\t\tdefault:\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t}\n>> +}\n>\n> You parse '%%H' incorrectly.\n\nI’m pretty sure I don’t.\n\n> I would really rather not see ad-hoc\n> parsers for the format like this, but rather use or extend strbuf_expand\n> as appropriate. That would make things less painful if and when we\n> decide to tweak the syntax.\n\nHowever, this point still applies.\n\nJonathan\n"},{"id":"140382","messageId":"20100426034135.GB30544@progeny.tock","threadId":"23598","inReplyTo":"20100426033813.GA30544@progeny.tock","subject":"Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T03:41:35Z","receivedAt":"2010-04-26T03:41:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n> Jeff King wrote:\n>> On Sun, Apr 25, 2010 at 10:11:37PM -0500, Jonathan Nieder wrote:\n\n>>> +static void abbreviate_commit_hashes(char *fmt)\n>>> +{\n>>> +\tchar *p;\n>>> +\tfor (p = fmt; p != NULL; p = strchr(p + 1, '%')) {\n>>> +\t\tp++;\n>>> +\t\tswitch (*p) {\n>>> +\t\tcase 'H':\n>>> +\t\t\t*p = 'h';\n>>> +\t\t\tbreak;\n>>> +\t\tcase 'P':\n>>> +\t\t\t*p = 'p';\n>>> +\t\t\tbreak;\n>>> +\t\tcase 'T':\n>>> +\t\tdefault:\n>>> +\t\t\tbreak;\n>>> +\t\t}\n>>> +\t}\n>>> +}\n>>\n>> You parse '%%H' incorrectly.\n>\n> I’m pretty sure I don’t.\n\nAggh, I see it now. The first line should be\n\n for (p = strchr(fmt, '%'); ...\n\nas I would have noticed with even a little testing.\n\nSorry for the nonsense.\nJonathan\n"},{"id":"140383","messageId":"20100426034254.GA16500@coredump.intra.peff.net","threadId":"23598","inReplyTo":"20100426033813.GA30544@progeny.tock","subject":"Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-26T03:42:54Z","receivedAt":"2010-04-26T03:42:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 25, 2010 at 10:38:13PM -0500, Jonathan Nieder wrote:\n\n> > You parse '%%H' incorrectly.\n> \n> I’m pretty sure I don’t.\n\nSorry, you're right. I should read more carefully.\n\n-Peff\n"},{"id":"140384","messageId":"20100426034520.GB16500@coredump.intra.peff.net","threadId":"23598","inReplyTo":"20100426034135.GB30544@progeny.tock","subject":"Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-26T03:45:21Z","receivedAt":"2010-04-26T03:45:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 25, 2010 at 10:41:35PM -0500, Jonathan Nieder wrote:\n\n> Jonathan Nieder wrote:\n> > Jeff King wrote:\n> >> On Sun, Apr 25, 2010 at 10:11:37PM -0500, Jonathan Nieder wrote:\n> \n> >>> +static void abbreviate_commit_hashes(char *fmt)\n> >>> +{\n> >>> +\tchar *p;\n> >>> +\tfor (p = fmt; p != NULL; p = strchr(p + 1, '%')) {\n> >>> +\t\tp++;\n> >>> +\t\tswitch (*p) {\n> >>> +\t\tcase 'H':\n> >>> +\t\t\t*p = 'h';\n> >>> +\t\t\tbreak;\n> >>> +\t\tcase 'P':\n> >>> +\t\t\t*p = 'p';\n> >>> +\t\t\tbreak;\n> >>> +\t\tcase 'T':\n> >>> +\t\tdefault:\n> >>> +\t\t\tbreak;\n> >>> +\t\t}\n> >>> +\t}\n> >>> +}\n> >>\n> >> You parse '%%H' incorrectly.\n> >\n> > I’m pretty sure I don’t.\n> \n> Aggh, I see it now. The first line should be\n> \n> for (p = strchr(fmt, '%'); ...\n> \n> as I would have noticed with even a little testing.\n\nActually, we are both failing. You _do_ parse %%H right, but you don't\nparse \"WHO\" correctly. So yours is a different bug than what I thought.\n:)\n\n-Peff\n"},{"id":"140386","messageId":"20100426041411.GB30566@progeny.tock","threadId":"23598","inReplyTo":"20100426032642.GA14421@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/3] pretty: add conditional %C?colorname placeholders","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T04:14:12Z","receivedAt":"2010-04-26T04:14:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> I am a little nervous that we would be breaking scripts that pipe the\n> colorized output for later display to the user. Right now doing\n> something like[1]:\n> \n> log=`git log --format=%Cred%h`\n> test \"x$log\" != x && echo \"foo: $log\"\n> \n> colorizes, but we would be breaking it. And for new values like\n> %C(diff.commit), we are not breaking anything (because it is a new\n> syntax), but a script like the one above may want to convert, and there\n> is no way to say \"respect the color _config_, but don't respect\n> isatty(1)\". Saying \"--color\" doesn't work, because it overrides the\n> color config. We can cheat a little with GIT_PAGER_IN_USE, but that will\n> have funny interactions with pager.color.\n\nVery interesting. I think the natural solution (for new colors) would\nbe a --color=config option, which would require parsing options before\nchecking configuration.\n\nDoes this sound sane?\n\nJonathan\n"},{"id":"140392","messageId":"20100426072516.GA30416@progeny.tock","threadId":"23598","inReplyTo":"1272232579-18895-4-git-send-email-wmpalmer@gmail.com","subject":"Re: [PATCH v2 3/3] pretty: add aliases for pretty formats","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T07:25:16Z","receivedAt":"2010-04-26T07:25:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Will Palmer wrote:\n\n> format.pretty.::\n> Alias for a --pretty= format string, as specified in\n> linkgit:git-log[1]. Any aliases defined here can be used just\n> as the builtin pretty formats could. For example, defining\n> \"format.pretty.hash = format:%H\" would cause the invocation\n> \"git log --pretty=hash\" to be equivalent to running\n> \"git log --pretty=format:%H\".\n\nAh, so I could use \n\n\t[format \"pretty\"]\n\t\twrapped = \"format:\\\n\t%C(yellow)commit %H%n\\\n\tMerge: %p%n\\\n\tAuthor:\t%aN <%aE>%n\\\n\tDate:\t%ad%n%n%w(80,4,4)%s%n\\\n\t%+b\"\n\nand then by default I get the standard medium, but with --format=wrapped,\nI get my imitation of it. Sounds very useful, thanks.\n\n> diff --git a/pretty.c b/pretty.c\n> index f884f48..d49fec7 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -11,6 +11,16 @@\n> #include \"reflog-walk.h\"\n> \n> static char *user_format;\n> +static struct cmt_fmt_map {\n> +\tconst char *name;\n> +\tenum cmit_fmt format;\n> +\tconst char *user_format;\n> +\tint is_tformat;\n> +\tint is_alias;\n> +} *commit_formats = NULL;\n> +static size_t commit_formats_len = 0;\n> +static size_t commit_formats_alloc = 0;\n> +static struct cmt_fmt_map *find_commit_format(const char *sought);\n> \n> static void save_user_format(struct rev_info *rev, const char *cp, int is_tformat)\n> {\n> @@ -21,22 +31,134 @@ static void save_user_format(struct rev_info *rev, const char *cp, int is_tforma\n> \trev->commit_format = CMIT_FMT_USERFORMAT;\n> }\n> \n> -void get_commit_format(const char *arg, struct rev_info *rev)\n> +static int git_pretty_formats_config(const char *var, const char *value, void *cb)\n> +{\n> +\tif (!prefixcmp(var, \"format.pretty.\")) {\n\nSimpler to use\n\n\tif (prefixcmp(var, \"format.pretty.\"))\n\t\treturn 0;\n\nto avoid keeping the reader in suspense.\n\n> +\t\tstruct cmt_fmt_map user_format = {0};\n> +\t\tconst char *fmt;\n> +\n> +\t\tuser_format.name = xstrdup(&var[14]);\n> +\t\tuser_format.format = CMIT_FMT_USERFORMAT;\n> +\t\tgit_config_string(&fmt, var, value);\n\ngit_config_string() does a strdup(), but we were about to either\ndiscard the value or do that ourselves anyway...\n\n> +\t\tif (!prefixcmp(fmt, \"format:\") || !prefixcmp(fmt, \"tformat:\")) {\n> +\t\t\tuser_format.is_tformat = fmt[0] == 't';\n> +\t\t\tfmt = strchr(fmt, ':') + 1;\n> +\t\t} else if (strchr(fmt, '%'))\n> +\t\t\tuser_format.is_tformat = 1;\n> +\t\telse\n> +\t\t\tuser_format.is_alias = 1;\n> +\t\tuser_format.user_format = fmt;\n\n... or rather we would be, if this reused code from get_commit_format/\nsave_user_format.\n\n> +\n> +\t\tALLOC_GROW(commit_formats, commit_formats_len+1,\n> +\t\t\t commit_formats_alloc);\n> +\t\tmemcpy(&commit_formats[ commit_formats_len ], &user_format,\n> +\t\t sizeof(user_format));\n> +\t\tcommit_formats_len++;\n\nWhy not build it in place? Not for performance reasons (that could go\neither way); it is just that that would seem simpler to me.\n\n> +\t}\n> +\treturn 0;\n> +}\n\nRegarding the next piece: I suspect the review would be easier if\nit had been more than one patch. Maybe three patches:\n\n 1 restructure get_commit_format to read from a (dynamic) list of\n supported formats that is not its responsibility\n\n 2 infrastructure for format aliases (this is not needed for\n --format=datelist where datelist = \"tformat:%h %cd\")\n\n 3 new configuration for user-defined formats and format aliases\n\nMaybe 3 could come before 2, since it seems like complicated.\n\nThe new call graph looks like this:\n\n setup_revisions() ->\n handle_revision_opt() ->\n get_commit_format() ->\n find_commit_format() ->\n setup_commit_formats() ->\n git_config() ->\n git_pretty_formats_config()\n\nThis means we have to have searched for a repository before parsing\nthese arguments; this constraint already exists for parsing the actual\nrevision arguments (maybe some day we will defer handling those\narguments for some reason).\n\nI would have put the setup_commit_formats() call in setup_revisions()\nto make this more obvious, but I suppose this way you save time if no\n--format option is used.\n\n> +\n> +static void setup_commit_formats(void)\n> {\n> \tint i;\n> -\tstatic struct cmt_fmt_map {\n> -\t\tconst char *n;\n> -\t\tsize_t cmp_len;\n> -\t\tenum cmit_fmt v;\n> -\t} cmt_fmts[] = {\n> -\t\t{ \"raw\",\t1,\tCMIT_FMT_RAW },\n> -\t\t{ \"medium\",\t1,\tCMIT_FMT_MEDIUM },\n> -\t\t{ \"short\",\t1,\tCMIT_FMT_SHORT },\n> -\t\t{ \"email\",\t1,\tCMIT_FMT_EMAIL },\n> -\t\t{ \"full\",\t5,\tCMIT_FMT_FULL },\n> -\t\t{ \"fuller\",\t5,\tCMIT_FMT_FULLER },\n> -\t\t{ \"oneline\",\t1,\tCMIT_FMT_ONELINE },\n> +\tconst char **attempted_aliases = NULL;\n> +\tsize_t attempted_aliases_alloc = 0;\n> +\tsize_t attempted_aliases_len;\n> +\tstruct cmt_fmt_map builtin_formats[] = {\n> +\t\t{ \"raw\",\tCMIT_FMT_RAW,\t\tNULL,\t0 },\n> +\t\t{ \"medium\",\tCMIT_FMT_MEDIUM,\tNULL,\t0 },\n> +\t\t{ \"short\",\tCMIT_FMT_SHORT,\t\tNULL,\t0 },\n> +\t\t{ \"email\",\tCMIT_FMT_EMAIL,\t\tNULL,\t0 },\n> +\t\t{ \"full\",\tCMIT_FMT_FULL,\t\tNULL,\t0 },\n> +\t\t{ \"fuller\",\tCMIT_FMT_FULLER,\tNULL,\t0 },\n> +\t\t{ \"oneline\",\tCMIT_FMT_ONELINE,\tNULL,\t1 }\n\nnitpick: Might be less noisy if the null format string field were the\nlast field.\n\n> \t};\n> +\tcommit_formats_len = ARRAY_SIZE(builtin_formats);\n> +\tALLOC_GROW(commit_formats, commit_formats_len, commit_formats_alloc);\n> +\tmemcpy(commit_formats, builtin_formats,\n> +\t sizeof(*builtin_formats)*ARRAY_SIZE(builtin_formats));\n> +\n> +\tgit_config(git_pretty_formats_config, NULL);\n\nI suspect the body of the next loop should be its own function to keep\nthe reader’s attention.\n\n> +\n> +\tfor (i = ARRAY_SIZE(builtin_formats); i < commit_formats_len; i++) {\n> +\t\tattempted_aliases_len = 0;\n> +\t\tstruct cmt_fmt_map *aliased_format = &commit_formats[i];\n> +\t\tconst char *fmt = commit_formats[i].user_format;\n> +\t\tint j;\n\ndeclaration after statement\n\n> +\n> +\t\tif (!commit_formats[i].is_alias)\n> +\t\t\tcontinue;\n> +\n> +\t\twhile ((aliased_format = find_commit_format(fmt))) {\n[...]\n\nIs this the right time to do this check? Maybe we could check lazily\nwhen the format is used.\n\n> +\t\t\tif (!aliased_format->is_alias)\n> +\t\t\t\tbreak;\n> +\n> +\t\t\tfmt = aliased_format->user_format;\n> +\t\t\tfor (j=0; j +\t\t\t\tif (!strcmp(fmt, attempted_aliases[j])) {\n> +\t\t\t\t\taliased_format = NULL;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n> +\t\t\t}\n\nExample:\n\n\t[format \"pretty\"]\n\t\tfoo = nonsense\n\t\tone = foo\n\t\ttwo = foo\n\none is treated as an alias, but two is not. Why?\n\n> +\t\t\tif (!aliased_format)\n> +\t\t\t\tbreak;\n\nIs to escape from the wider loop? I think this is a valid use for goto.\n\n> +\n> +\t\t\tALLOC_GROW(attempted_aliases, attempted_aliases_len+1,\n> +\t\t\t\t attempted_aliases_alloc);\n> +\t\t\tattempted_aliases[attempted_aliases_len] = fmt;\n> +\t\t\tattempted_aliases_len++;\n> +\t\t}\n\nExample:\n\n\t[format \"pretty\"]\n\t\tfoo = medium\n\t\txyzzy = one\n\t\tone = foo\n\t\ttwo = foo\n\t\tfrotz = two\n\nAt the end of this loop, attempted_aliases contains:\n\n\tone\n\tfoo\n\ttwo\n\nEvery alias which is itself referred to by an alias is listed.\n\n> +\t\tif (aliased_format) {\n> +\t\t\tcommit_formats[i].format = aliased_format->format;\n> +\t\t\tcommit_formats[i].user_format = aliased_format->user_format;\n> +\t\t\tcommit_formats[i].is_tformat = aliased_format->is_tformat;\n> +\t\t\tcommit_formats[i].is_alias = 0;\n> +\t\t} else\n> +\t\t\tcommit_formats[i].format = CMIT_FMT_UNSPECIFIED;\n> +\t}\n> +}\n\nWhy go to the trouble to build attempted_aliases when it is never used?\n\nI suspect I’ve completely misunderstood, so I’m stopping here. Maybe\nsomeone else can clear it up or take over.\n\nJonathan\n"},{"id":"140394","messageId":"u2i5b9751661004260047n168ad87bx6083fc201bfb21d9@mail.gmail.com","threadId":"23598","inReplyTo":"20100426031012.GA29953@progeny.tock","subject":"Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2010-04-26T07:47:54Z","receivedAt":"2010-04-26T07:47:54Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"> I agree that this is the right to do, since this is how the built-in\n> formats work (the ‘commit ...’ line follows the semantics of your %H,\n> and ‘Merge: ...’ line your %p, for example).\n>\n> Documentation and tests?\nTests, noted. I'll include some in the next version of the patch.\nAs for documentation, I'm not entirely sure what to add, as it seemed\nlike the change\nmerely implements what I would expect when reading the docs for\ngit-log. Still, noted.\nI'll stick a short note in each of %H, %h, %P, %p, and %t\n\n\n> Shortlog doesn’t print commit hashes, does it?\n\nShortlog accepts --format, though this doesn't seem to be documented\n(if I type \"man\" and search\nfor \"format\"), so perhaps it should be.\n\n>\n>> diff --git a/commit.h b/commit.h\n>> index b6caf91..7a476a0 100644\n>> --- a/commit.h\n>> +++ b/commit.h\n>> @@ -72,6 +72,7 @@ struct pretty_print_context\n>>       int need_8bit_cte;\n>>       int show_notes;\n>>       int use_color;\n>> +     int abbrev_commit;\n>>       struct reflog_walk_info *reflog_info;\n>>  };\n>>\n>\n> nitpick: I’d stick this up with abbrev and maybe add a comment to\n> explain their distinct uses.\nNoted. Thanks\n\n>\n>> diff --git a/log-tree.c b/log-tree.c\n>> index 6bb4748..0a2309c 100644\n>> --- a/log-tree.c\n>> +++ b/log-tree.c\n>> @@ -282,6 +282,8 @@ void show_log(struct rev_info *opt)\n>>       int abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40;\n>>       const char *extra_headers = opt->extra_headers;\n>>       struct pretty_print_context ctx = {0};\n>> +     ctx.abbrev = opt->abbrev;\n>> +     ctx.abbrev_commit = opt->abbrev_commit;\n>>       ctx.use_color = DIFF_OPT_TST(&opt->diffopt, COLOR_DIFF);\n>>\n>>       opt->loginfo = NULL;\n>\n> There is a\n>\n> ctx.abbrev = opt->diffopt.abbrev;\n>\n> later in the same function; how do these interact?\n\nI hadn't caught that. My guess: stupidly doing the same thing twice.\nI'll double-check,\nand take out one of them if that's the case.\n\nWhat all this\nhas shown me is that there are really too many ways to specify \"context when\nprinting information about a commit\". I don't like it, and the lot of\nthem can probably\nbe re-factored, perhaps by getting rid of pretty_context and passing\nrev_info around\neverywhere. I don't know the full implications of that and it seems\noutside the scope\nof this change.\n\n\n>\n>> @@ -741,24 +744,29 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n> [...]\n>>                       strbuf_addstr(sb, find_unique_abbrev(\n>> -                                     p->item->object.sha1, DEFAULT_ABBREV));\n>> +                                   p->item->object.sha1,\n>> +                                   c->pretty_ctx->abbrev));\n>\n> nitpick: the new indentation makes these look like parameters to\n> strbuf_addstr.\n\nNoted. I'll restore the extra indent, which I had assumed was a typo.\n\n>\n> Here’s an alternative implementation of the more controversial half of\n> your patch, for your amusement.  The big downside is that it requires\n> one to specify --abbrev-commit before the --format option.\n>\n> Thanks for the pleasant read.\n>\n> Jonathan\n>\n> diff --git a/pretty.c b/pretty.c\n> index 7cb3a2a..1008a41 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -12,10 +12,31 @@\n>\n>  static char *user_format;\n>\n> +static void abbreviate_commit_hashes(char *fmt)\n> +{\n> +       char *p;\n> +       for (p = fmt; p != NULL; p = strchr(p + 1, '%')) {\n> +               p++;\n> +               switch (*p) {\n> +               case 'H':\n> +                       *p = 'h';\n> +                       break;\n> +               case 'P':\n> +                       *p = 'p';\n> +                       break;\n> +               case 'T':\n> +               default:\n> +                       break;\n> +               }\n> +       }\n> +}\n> +\n>  static void save_user_format(struct rev_info *rev, const char *cp, int is_tformat)\n>  {\n>        free(user_format);\n>        user_format = xstrdup(cp);\n> +       if (rev->abbrev_commit)\n> +               abbreviate_commit_hashes(user_format);\n>        if (is_tformat)\n>                rev->use_terminator = 1;\n>        rev->commit_format = CMIT_FMT_USERFORMAT;\n>\n\nI had been thinking that this wouldn't be safe, but then that was my\nbeing overly-cautious:\nit's just been xstrdup()ed, so what we're parsing is ours, no real\nreason not to do it.\nI think the \"must be specified after --abbrev-commit\" is a rather\nlarge nail, though. If you work\nout the bugs mentioned by Jeff King, and it works, I'll stick it in\nthere, as I don't like\nfalling through case statements any more than the next guy. (well,\nmaybe a little more than\nthe next guy).\n\nThanks for the review!\n-- Will Palmer\n"},{"id":"140395","messageId":"i2x5b9751661004260115o6a195e21q52e8f65e12dc5339@mail.gmail.com","threadId":"23598","inReplyTo":"20100426072516.GA30416@progeny.tock","subject":"Re: [PATCH v2 3/3] pretty: add aliases for pretty formats","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2010-04-26T08:15:04Z","receivedAt":"2010-04-26T08:15:04Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"I don't have time right now to reply line-by-line to this, so this is\njust an ACK that it was\nreceived, and I'll reply later today.\n\n\nOn Mon, Apr 26, 2010 at 8:25 AM, Jonathan Nieder wrote:\n> Will Palmer wrote:\n>\n>> format.pretty.::\n>>        Alias for a --pretty= format string, as specified in\n>>        linkgit:git-log[1]. Any aliases defined here can be used just\n>>        as the builtin pretty formats could. For example, defining\n>>        \"format.pretty.hash = format:%H\" would cause the invocation\n>>        \"git log --pretty=hash\" to be equivalent to running\n>>        \"git log --pretty=format:%H\".\n>\n> Ah, so I could use\n>\n>        [format \"pretty\"]\n>                wrapped = \"format:\\\n>        %C(yellow)commit %H%n\\\n>        Merge: %p%n\\\n>        Author: %aN <%aE>%n\\\n>        Date:   %ad%n%n%w(80,4,4)%s%n\\\n>        %+b\"\n>\n> and then by default I get the standard medium, but with --format=wrapped,\n> I get my imitation of it.  Sounds very useful, thanks.\n>\n>> diff --git a/pretty.c b/pretty.c\n>> index f884f48..d49fec7 100644\n>> --- a/pretty.c\n>> +++ b/pretty.c\n>> @@ -11,6 +11,16 @@\n>>  #include \"reflog-walk.h\"\n>>\n>>  static char *user_format;\n>> +static struct cmt_fmt_map {\n>> +     const char *name;\n>> +     enum cmit_fmt format;\n>> +     const char *user_format;\n>> +     int is_tformat;\n>> +     int is_alias;\n>> +} *commit_formats = NULL;\n>> +static size_t commit_formats_len = 0;\n>> +static size_t commit_formats_alloc = 0;\n>> +static struct cmt_fmt_map *find_commit_format(const char *sought);\n>>\n>>  static void save_user_format(struct rev_info *rev, const char *cp, int is_tformat)\n>>  {\n>> @@ -21,22 +31,134 @@ static void save_user_format(struct rev_info *rev, const char *cp, int is_tforma\n>>       rev->commit_format = CMIT_FMT_USERFORMAT;\n>>  }\n>>\n>> -void get_commit_format(const char *arg, struct rev_info *rev)\n>> +static int git_pretty_formats_config(const char *var, const char *value, void *cb)\n>> +{\n>> +     if (!prefixcmp(var, \"format.pretty.\")) {\n>\n> Simpler to use\n>\n>        if (prefixcmp(var, \"format.pretty.\"))\n>                return 0;\n>\n> to avoid keeping the reader in suspense.\n>\n>> +             struct cmt_fmt_map user_format = {0};\n>> +             const char *fmt;\n>> +\n>> +             user_format.name = xstrdup(&var[14]);\n>> +             user_format.format = CMIT_FMT_USERFORMAT;\n>> +             git_config_string(&fmt, var, value);\n>\n> git_config_string() does a strdup(), but we were about to either\n> discard the value or do that ourselves anyway...\n>\n>> +             if (!prefixcmp(fmt, \"format:\") || !prefixcmp(fmt, \"tformat:\")) {\n>> +                     user_format.is_tformat = fmt[0] == 't';\n>> +                     fmt = strchr(fmt, ':') + 1;\n>> +             } else if (strchr(fmt, '%'))\n>> +                     user_format.is_tformat = 1;\n>> +             else\n>> +                     user_format.is_alias = 1;\n>> +             user_format.user_format = fmt;\n>\n> ... or rather we would be, if this reused code from get_commit_format/\n> save_user_format.\n>\n>> +\n>> +             ALLOC_GROW(commit_formats, commit_formats_len+1,\n>> +                        commit_formats_alloc);\n>> +             memcpy(&commit_formats[ commit_formats_len ], &user_format,\n>> +                    sizeof(user_format));\n>> +             commit_formats_len++;\n>\n> Why not build it in place?  Not for performance reasons (that could go\n> either way); it is just that that would seem simpler to me.\n>\n>> +     }\n>> +     return 0;\n>> +}\n>\n> Regarding the next piece: I suspect the review would be easier if\n> it had been more than one patch.  Maybe three patches:\n>\n>  1 restructure get_commit_format to read from a (dynamic) list of\n>   supported formats that is not its responsibility\n>\n>  2 infrastructure for format aliases (this is not needed for\n>   --format=datelist where datelist = \"tformat:%h %cd\")\n>\n>  3 new configuration for user-defined formats and format aliases\n>\n> Maybe 3 could come before 2, since it seems like complicated.\n>\n> The new call graph looks like this:\n>\n>  setup_revisions() ->\n>   handle_revision_opt() ->\n>   get_commit_format() ->\n>   find_commit_format() ->\n>   setup_commit_formats() ->\n>   git_config() ->\n>   git_pretty_formats_config()\n>\n> This means we have to have searched for a repository before parsing\n> these arguments; this constraint already exists for parsing the actual\n> revision arguments (maybe some day we will defer handling those\n> arguments for some reason).\n>\n> I would have put the setup_commit_formats() call in setup_revisions()\n> to make this more obvious, but I suppose this way you save time if no\n> --format option is used.\n>\n>> +\n>> +static void setup_commit_formats(void)\n>>  {\n>>       int i;\n>> -     static struct cmt_fmt_map {\n>> -             const char *n;\n>> -             size_t cmp_len;\n>> -             enum cmit_fmt v;\n>> -     } cmt_fmts[] = {\n>> -             { \"raw\",        1,      CMIT_FMT_RAW },\n>> -             { \"medium\",     1,      CMIT_FMT_MEDIUM },\n>> -             { \"short\",      1,      CMIT_FMT_SHORT },\n>> -             { \"email\",      1,      CMIT_FMT_EMAIL },\n>> -             { \"full\",       5,      CMIT_FMT_FULL },\n>> -             { \"fuller\",     5,      CMIT_FMT_FULLER },\n>> -             { \"oneline\",    1,      CMIT_FMT_ONELINE },\n>> +     const char **attempted_aliases = NULL;\n>> +     size_t attempted_aliases_alloc = 0;\n>> +     size_t attempted_aliases_len;\n>> +     struct cmt_fmt_map builtin_formats[] = {\n>> +             { \"raw\",        CMIT_FMT_RAW,           NULL,   0 },\n>> +             { \"medium\",     CMIT_FMT_MEDIUM,        NULL,   0 },\n>> +             { \"short\",      CMIT_FMT_SHORT,         NULL,   0 },\n>> +             { \"email\",      CMIT_FMT_EMAIL,         NULL,   0 },\n>> +             { \"full\",       CMIT_FMT_FULL,          NULL,   0 },\n>> +             { \"fuller\",     CMIT_FMT_FULLER,        NULL,   0 },\n>> +             { \"oneline\",    CMIT_FMT_ONELINE,       NULL,   1 }\n>\n> nitpick: Might be less noisy if the null format string field were the\n> last field.\n>\n>>       };\n>> +     commit_formats_len = ARRAY_SIZE(builtin_formats);\n>> +     ALLOC_GROW(commit_formats, commit_formats_len, commit_formats_alloc);\n>> +     memcpy(commit_formats, builtin_formats,\n>> +            sizeof(*builtin_formats)*ARRAY_SIZE(builtin_formats));\n>> +\n>> +     git_config(git_pretty_formats_config, NULL);\n>\n> I suspect the body of the next loop should be its own function to keep\n> the reader’s attention.\n>\n>> +\n>> +     for (i = ARRAY_SIZE(builtin_formats); i < commit_formats_len; i++) {\n>> +             attempted_aliases_len = 0;\n>> +             struct cmt_fmt_map *aliased_format = &commit_formats[i];\n>> +             const char *fmt = commit_formats[i].user_format;\n>> +             int j;\n>\n> declaration after statement\n>\n>> +\n>> +             if (!commit_formats[i].is_alias)\n>> +                     continue;\n>> +\n>> +             while ((aliased_format = find_commit_format(fmt))) {\n> [...]\n>\n> Is this the right time to do this check?  Maybe we could check lazily\n> when the format is used.\n>\n>> +                     if (!aliased_format->is_alias)\n>> +                             break;\n>> +\n>> +                     fmt = aliased_format->user_format;\n>> +                     for (j=0; j> +                             if (!strcmp(fmt, attempted_aliases[j])) {\n>> +                                     aliased_format = NULL;\n>> +                                     break;\n>> +                             }\n>> +                     }\n>\n> Example:\n>\n>        [format \"pretty\"]\n>                foo = nonsense\n>                one = foo\n>                two = foo\n>\n> one is treated as an alias, but two is not.  Why?\n>\n>> +                     if (!aliased_format)\n>> +                             break;\n>\n> Is to escape from the wider loop?  I think this is a valid use for goto.\n>\n>> +\n>> +                     ALLOC_GROW(attempted_aliases, attempted_aliases_len+1,\n>> +                                attempted_aliases_alloc);\n>> +                     attempted_aliases[attempted_aliases_len] = fmt;\n>> +                     attempted_aliases_len++;\n>> +             }\n>\n> Example:\n>\n>        [format \"pretty\"]\n>                foo = medium\n>                xyzzy = one\n>                one = foo\n>                two = foo\n>                frotz = two\n>\n> At the end of this loop, attempted_aliases contains:\n>\n>        one\n>        foo\n>        two\n>\n> Every alias which is itself referred to by an alias is listed.\n>\n>> +             if (aliased_format) {\n>> +                     commit_formats[i].format = aliased_format->format;\n>> +                     commit_formats[i].user_format = aliased_format->user_format;\n>> +                     commit_formats[i].is_tformat = aliased_format->is_tformat;\n>> +                     commit_formats[i].is_alias = 0;\n>> +             } else\n>> +                     commit_formats[i].format = CMIT_FMT_UNSPECIFIED;\n>> +     }\n>> +}\n>\n> Why go to the trouble to build attempted_aliases when it is never used?\n>\n> I suspect I’ve completely misunderstood, so I’m stopping here.  Maybe\n> someone else can clear it up or take over.\n>\n> Jonathan\n>\n"},{"id":"140399","messageId":"20100426095206.GA31483@progeny.tock","threadId":"23598","inReplyTo":"u2i5b9751661004260047n168ad87bx6083fc201bfb21d9@mail.gmail.com","subject":"Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T09:53:29Z","receivedAt":"2010-04-26T09:53:29Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Will Palmer wrote:\n> Jonathan Nieder wrote:\n\n>> Shortlog doesn’t print commit hashes, does it?\n>\n> Shortlog accepts --format, though this doesn't seem to be documented\n> (if I type \"man\" and search\n> for \"format\"), so perhaps it should be.\n\nOh, neat! Maybe this would save you the trouble.\n\nJonathan Nieder (3):\n t4201 (shortlog): guard setup with test_expect_success\n t4201 (shortlog): Test output format with multiple authors\n shortlog: Document and test --format option\n\nWill Palmer (1):\n pretty: Respect --abbrev option\n\n Documentation/git-shortlog.txt | 8 +++\n builtin/shortlog.c | 3 +-\n pretty.c | 7 ++-\n shortlog.h | 1 +\n t/t4201-shortlog.sh | 116 +++++++++++++++++++++++++++++++--------\n t/t6006-rev-list-format.sh | 19 +++++++\n 6 files changed, 126 insertions(+), 28 deletions(-)\n"},{"id":"140400","messageId":"20100426095805.GB31483@progeny.tock","threadId":"23598","inReplyTo":"20100426095206.GA31483@progeny.tock","subject":"[PATCH 1/4] t4201 (shortlog): guard setup with test_expect_success","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T09:58:05Z","receivedAt":"2010-04-26T09:58:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Follow the current prevailing style. This also has the benefit of\ncapturing any stray output and noticing if any of the setup commands\nstart failing.\n\nSigned-off-by: Jonathan Nieder \n---\n t/t4201-shortlog.sh | 68 +++++++++++++++++++++++++++++---------------------\n 1 files changed, 39 insertions(+), 29 deletions(-)\n\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex a01e55b..438a826 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -8,30 +8,38 @@ test_description='git shortlog\n \n . ./test-lib.sh\n \n-echo 1 > a1\n-git add a1\n-tree=$(git write-tree)\n-commit=$( (echo \"Test\"; echo) | git commit-tree $tree )\n-git update-ref HEAD $commit\n-\n-echo 2 > a1\n-git commit --quiet -m \"This is a very, very long first line for the commit message to see if it is wrapped correctly\" a1\n-\n-# test if the wrapping is still valid when replacing all i's by treble clefs.\n-echo 3 > a1\n-git commit --quiet -m \"$(echo \"This is a very, very long first line for the commit message to see if it is wrapped correctly\" | sed \"s/i/1234/g\" | tr 1234 '\\360\\235\\204\\236')\" a1\n-\n-# now fsck up the utf8\n-git config i18n.commitencoding non-utf-8\n-echo 4 > a1\n-git commit --quiet -m \"$(echo \"This is a very, very long first line for the commit message to see if it is wrapped correctly\" | sed \"s/i/1234/g\" | tr 1234 '\\370\\235\\204\\236')\" a1\n-\n-echo 5 > a1\n-git commit --quiet -m \"a\t\t\t\t\t\t\t\t12\t34\t56\t78\" a1\n-\n-git shortlog -w HEAD > out\n+test_expect_success 'setup' '\n+\techo 1 >a1 &&\n+\tgit add a1 &&\n+\ttree=$(git write-tree) &&\n+\tcommit=$(printf \"%s\\n\" \"Test\" \"\" | git commit-tree \"$tree\") &&\n+\tgit update-ref HEAD \"$commit\" &&\n+\n+\techo 2 >a1 &&\n+\tgit commit --quiet -m \"This is a very, very long first line for the commit message to see if it is wrapped correctly\" a1 &&\n+\n+\t# test if the wrapping is still valid\n+\t# when replacing all is by treble clefs.\n+\techo 3 >a1 &&\n+\tgit commit --quiet -m \"$(\n+\t\techo \"This is a very, very long first line for the commit message to see if it is wrapped correctly\" |\n+\t\tsed \"s/i/1234/g\" |\n+\t\ttr 1234 \"\\360\\235\\204\\236\")\" a1 &&\n+\n+\t# now fsck up the utf8\n+\tgit config i18n.commitencoding non-utf-8 &&\n+\techo 4 >a1 &&\n+\tgit commit --quiet -m \"$(\n+\t\techo \"This is a very, very long first line for the commit message to see if it is wrapped correctly\" |\n+\t\tsed \"s/i/1234/g\" |\n+\t\ttr 1234 \"\\370\\235\\204\\236\")\" a1 &&\n+\n+\techo 5 >a1 &&\n+\tgit commit --quiet -m \"a\t\t\t\t\t\t\t\t12\t34\t56\t78\" a1\n+'\n \n-cat > expect << EOF\n+test_expect_success 'shortlog wrapping' '\n+\tcat >expect <<\\EOF &&\n A U Thor (5):\n Test\n This is a very, very long first line for the commit message to see if\n@@ -44,13 +52,15 @@ A U Thor (5):\n 56\t78\n \n EOF\n+\tgit shortlog -w HEAD >out &&\n+\ttest_cmp expect out\n+'\n \n-test_expect_success 'shortlog wrapping' 'test_cmp expect out'\n-\n-git log HEAD > log\n-GIT_DIR=non-existing git shortlog -w < log > out\n-\n-test_expect_success 'shortlog from non-git directory' 'test_cmp expect out'\n+test_expect_success 'shortlog from non-git directory' '\n+\tgit log HEAD >log &&\n+\tGIT_DIR=non-existing git shortlog -w out &&\n+\ttest_cmp expect out\n+'\n \n iconvfromutf8toiso88591() {\n \tprintf \"%s\" \"$*\" | iconv -f UTF-8 -t ISO8859-1\n-- \n1.7.1.3.g5f1e.dirty\n"},{"id":"140401","messageId":"20100426095900.GC31483@progeny.tock","threadId":"23598","inReplyTo":"20100426095206.GA31483@progeny.tock","subject":"[PATCH 2/4] t4201 (shortlog): Test output format with multiple authors","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T09:59:00Z","receivedAt":"2010-04-26T09:59:00Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Signed-off-by: Jonathan Nieder \n---\nThis could be squashed with patch 1, if you like.\n\n t/t4201-shortlog.sh | 7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 438a826..6bfd0c0 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -36,6 +36,10 @@ test_expect_success 'setup' '\n \n \techo 5 >a1 &&\n \tgit commit --quiet -m \"a\t\t\t\t\t\t\t\t12\t34\t56\t78\" a1\n+\n+\techo 6 >a1 &&\n+\tgit commit --quiet -m \"Commit by someone else\" \\\n+\t\t--author=\"Someone else \" a1\n '\n \n test_expect_success 'shortlog wrapping' '\n@@ -51,6 +55,9 @@ A U Thor (5):\n a\t\t\t\t\t\t\t\t12\t34\n 56\t78\n \n+Someone else (1):\n+ Commit by someone else\n+\n EOF\n \tgit shortlog -w HEAD >out &&\n \ttest_cmp expect out\n-- \n1.7.1.3.g5f1e.dirty\n"},{"id":"140402","messageId":"20100426095937.GD31483@progeny.tock","threadId":"23598","inReplyTo":"20100426095206.GA31483@progeny.tock","subject":"[PATCH 3/4] shortlog: Document and test --format option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T09:59:37Z","receivedAt":"2010-04-26T09:59:37Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Do not document the --pretty synonym, since it takes too long to\nexplain the name to people.\n\nSigned-off-by: Jonathan Nieder \n---\n Documentation/git-shortlog.txt | 8 ++++++\n t/t4201-shortlog.sh | 53 +++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 60 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex dfd4d0c..15e92ed 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -39,6 +39,14 @@ OPTIONS\n --email::\n \tShow the email address of each author.\n \n+--format[='']::\n+\tInstead of the commit subject, use some other information to\n+\tdescribe each commit. '' can be any string accepted\n+\tby the `--format` option of 'git log', such as '* [%h] %s'.\n+\t(See the \"PRETTY FORMATS\" section of linkgit:git-log[1].)\n+\n+\tEach pretty-printed commit will be rewrapped before it is shown.\n+\n -w[[,[,]]]::\n \tLinewrap the output by wrapping each line at `width`. The first\n \tline of each entry is indented by `indent1` spaces, and the second\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 6bfd0c0..899ddbe 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -39,7 +39,58 @@ test_expect_success 'setup' '\n \n \techo 6 >a1 &&\n \tgit commit --quiet -m \"Commit by someone else\" \\\n-\t\t--author=\"Someone else \" a1\n+\t\t--author=\"Someone else \" a1 &&\n+\n+\tcat >expect.template <<-\\EOF\n+\tA U Thor (5):\n+\t SUBJECT\n+\t SUBJECT\n+\t SUBJECT\n+\t SUBJECT\n+\t SUBJECT\n+\n+\tSomeone else (1):\n+\t SUBJECT\n+\n+\tEOF\n+'\n+\n+fuzz() {\n+\tfile=$1 &&\n+\tsed \"\n+\t\t\ts/$_x40/OBJECT_NAME/g\n+\t\t\ts/$_x05/OBJID/g\n+\t\t\ts/^ \\{6\\}[CTa].*/ SUBJECT/g\n+\t\t\ts/^ \\{8\\}[^ ].*/ CONTINUATION/g\n+\t\t\" <\"$file\" >\"$file.fuzzy\" &&\n+\tsed \"/CONTINUATION/ d\" <\"$file.fuzzy\"\n+}\n+\n+test_expect_success 'default output format' '\n+\tgit shortlog >log &&\n+\tfuzz log >log.predictable &&\n+\ttest_cmp expect.template log.predictable\n+'\n+\n+test_expect_success 'pretty format' '\n+\tsed s/SUBJECT/OBJECT_NAME/ expect.template >expect &&\n+\tgit shortlog --format=\"%H\" >log &&\n+\tfuzz log >log.predictable &&\n+\ttest_cmp expect log.predictable\n+'\n+\n+test_expect_failure '--abbrev' '\n+\tsed s/SUBJECT/OBJID/ expect.template >expect &&\n+\tgit shortlog --format=\"%h\" --abbrev=5 >log &&\n+\tfuzz log >log.predictable &&\n+\ttest_cmp expect log.predictable\n+'\n+\n+test_expect_success 'output from user-defined format is re-wrapped' '\n+\tsed \"s/SUBJECT/two lines/\" expect.template >expect &&\n+\tgit shortlog --format=\"two%nlines\" >log &&\n+\tfuzz log >log.predictable &&\n+\ttest_cmp expect log.predictable\n '\n \n test_expect_success 'shortlog wrapping' '\n-- \n1.7.1.3.g5f1e.dirty\n"},{"id":"140403","messageId":"20100426100032.GE31483@progeny.tock","threadId":"23598","inReplyTo":"20100426095206.GA31483@progeny.tock","subject":"[PATCH 4/4] pretty: Respect --abbrev option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T10:00:32Z","receivedAt":"2010-04-26T10:00:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Will Palmer \n\nPrior to this, the output of git log -1 --format=%h was always 7\ncharacters long, without regard to whether --abbrev had been passed.\n\nSigned-off-by: Will Palmer \nSigned-off-by: Jonathan Nieder \n---\nThat’s the end of the series. Thanks for reading.\n\n builtin/shortlog.c | 3 ++-\n pretty.c | 7 ++++---\n shortlog.h | 1 +\n t/t4201-shortlog.sh | 2 +-\n t/t6006-rev-list-format.sh | 19 +++++++++++++++++++\n 5 files changed, 27 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 06320f5..5089502 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -162,7 +162,7 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \t\t sha1_to_hex(commit->object.sha1));\n \tif (log->user_format) {\n \t\tstruct pretty_print_context ctx = {0};\n-\t\tctx.abbrev = DEFAULT_ABBREV;\n+\t\tctx.abbrev = log->abbrev;\n \t\tctx.subject = \"\";\n \t\tctx.after_subject = \"\";\n \t\tctx.date_mode = DATE_NORMAL;\n@@ -290,6 +290,7 @@ parse_done:\n \t}\n \n \tlog.user_format = rev.commit_format == CMIT_FMT_USERFORMAT;\n+\tlog.abbrev = rev.abbrev;\n \n \t/* assume HEAD if from a tty */\n \tif (!nongit && !rev.pending.nr && isatty(0))\ndiff --git a/pretty.c b/pretty.c\nindex 7cb3a2a..1430616 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -716,7 +716,7 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \t\tif (add_again(sb, &c->abbrev_commit_hash))\n \t\t\treturn 1;\n \t\tstrbuf_addstr(sb, find_unique_abbrev(commit->object.sha1,\n-\t\t DEFAULT_ABBREV));\n+\t\t c->pretty_ctx->abbrev));\n \t\tc->abbrev_commit_hash.len = sb->len - c->abbrev_commit_hash.off;\n \t\treturn 1;\n \tcase 'T':\t\t/* tree hash */\n@@ -726,7 +726,7 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \t\tif (add_again(sb, &c->abbrev_tree_hash))\n \t\t\treturn 1;\n \t\tstrbuf_addstr(sb, find_unique_abbrev(commit->tree->object.sha1,\n-\t\t DEFAULT_ABBREV));\n+\t\t c->pretty_ctx->abbrev));\n \t\tc->abbrev_tree_hash.len = sb->len - c->abbrev_tree_hash.off;\n \t\treturn 1;\n \tcase 'P':\t\t/* parent hashes */\n@@ -743,7 +743,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \t\t\tif (p != commit->parents)\n \t\t\t\tstrbuf_addch(sb, ' ');\n \t\t\tstrbuf_addstr(sb, find_unique_abbrev(\n-\t\t\t\t\tp->item->object.sha1, DEFAULT_ABBREV));\n+\t\t\t\t\tp->item->object.sha1,\n+\t\t\t\t\tc->pretty_ctx->abbrev));\n \t\t}\n \t\tc->abbrev_parent_hashes.len = sb->len -\n \t\t c->abbrev_parent_hashes.off;\ndiff --git a/shortlog.h b/shortlog.h\nindex bc02cc2..de4f86f 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -12,6 +12,7 @@ struct shortlog {\n \tint in1;\n \tint in2;\n \tint user_format;\n+\tint abbrev;\n \n \tchar *common_repo_prefix;\n \tint email;\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 899ddbe..c49ca98 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -79,7 +79,7 @@ test_expect_success 'pretty format' '\n \ttest_cmp expect log.predictable\n '\n \n-test_expect_failure '--abbrev' '\n+test_expect_success '--abbrev' '\n \tsed s/SUBJECT/OBJID/ expect.template >expect &&\n \tgit shortlog --format=\"%h\" --abbrev=5 >log &&\n \tfuzz log >log.predictable &&\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex a49b7c5..dd9b3b9 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -191,6 +191,19 @@ test_expect_success 'add LF before non-empty (2)' '\n \tgrep \"^$\" actual\n '\n \n+test_expect_success '--abbrev' '\n+\techo SHORT SHORT SHORT >expect2 &&\n+\techo LONG LONG LONG >expect3 &&\n+\tgit log -1 --format=\"%h %h %h\" HEAD >actual1 &&\n+\tgit log -1 --abbrev=5 --format=\"%h %h %h\" HEAD >actual2 &&\n+\tgit log -1 --abbrev=5 --format=\"%H %H %H\" HEAD >actual3 &&\n+\tsed -e \"s/$_x40/LONG/g\" -e \"s/$_x05/SHORT/g\" fuzzy2 &&\n+\tsed -e \"s/$_x40/LONG/g\" -e \"s/$_x05/SHORT/g\" fuzzy3 &&\n+\ttest_cmp expect2 fuzzy2 &&\n+\ttest_cmp expect3 fuzzy3 &&\n+\t! test_cmp actual1 actual2\n+'\n+\n test_expect_success '\"%h %gD: %gs\" is same as git-reflog' '\n \tgit reflog >expect &&\n \tgit log -g --format=\"%h %gD: %gs\" >actual &&\n@@ -203,6 +216,12 @@ test_expect_success '\"%h %gD: %gs\" is same as git-reflog (with date)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '\"%h %gD: %gs\" is same as git-reflog (with --abbrev)' '\n+\tgit reflog --abbrev=13 --date=raw >expect &&\n+\tgit log -g --abbrev=13 --format=\"%h %gD: %gs\" --date=raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '%gd shortens ref name' '\n \techo \"master@{0}\" >expect.gd-short &&\n \tgit log -g -1 --format=%gd refs/heads/master >actual.gd-short &&\n-- \n1.7.1.3.g5f1e.dirty\n"},{"id":"140405","messageId":"p2w5b9751661004260313kd1b809earb8d9fd8c4d09adef@mail.gmail.com","threadId":"23598","inReplyTo":"20100426095206.GA31483@progeny.tock","subject":"Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2010-04-26T10:13:42Z","receivedAt":"2010-04-26T10:13:42Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"On Mon, Apr 26, 2010 at 10:53 AM, Jonathan Nieder wrote:\n> Will Palmer wrote:\n>> Jonathan Nieder wrote:\n>\n>>> Shortlog doesn’t print commit hashes, does it?\n>>\n>> Shortlog accepts --format, though this doesn't seem to be documented\n>> (if I type \"man\" and search\n>> for \"format\"), so perhaps it should be.\n>\n> Oh, neat!  Maybe this would save you the trouble.\n>\n> Jonathan Nieder (3):\n>  t4201 (shortlog): guard setup with test_expect_success\n>  t4201 (shortlog): Test output format with multiple authors\n>  shortlog: Document and test --format option\n\nThanks!\n\nI wasn't sure if you intended this to be submitted as a separate\nseries or added on top of my series,\nas my mail client grouped them together (I really need to stop using\ngmail for the mailing list...). If the latter,\nI'd appreciate it being sent in as a separate series, as I'd consider\nthe shortlog documentation/testing to be a\nseparate topic. (Though I suppose I should add tests for shortlog to\nthe tests I wrote)\n"},{"id":"140406","messageId":"20100426101911.GA8841@progeny.tock","threadId":"23598","inReplyTo":"p2w5b9751661004260313kd1b809earb8d9fd8c4d09adef@mail.gmail.com","subject":"Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T10:19:11Z","receivedAt":"2010-04-26T10:19:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Will Palmer wrote:\n\n> I wasn't sure if you intended this to be submitted as a separate\n> series or added on top of my series,\n> as my mail client grouped them together (I really need to stop using\n> gmail for the mailing list...).\n\nI sent it as a reply, but the patches are against master. The last of\nthe four patches is from your series with a few small changes and its\nin-message From: line is set accordingly. Sorry for the confusion.\n\nJonathan\n"},{"id":"140407","messageId":"4BD56985.2030506@gmail.com","threadId":"23598","inReplyTo":"20100426101911.GA8841@progeny.tock","subject":"Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2010-04-26T10:23:01Z","receivedAt":"2010-04-26T10:23:01Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"Jonathan Nieder wrote:\n> I sent it as a reply, but the patches are against master. The last of\n> the four patches is from your series with a few small changes and its\n> in-message From: line is set accordingly. Sorry for the confusion.\n>\n> Jonathan\n> \n\nI think the documentation / test changes are needed, but my\nown patch will probably go through several more revisions\nbefore it is ready to be included, which is why I suggested\nsubmitting your series separately.\n"},{"id":"140408","messageId":"20100426102830.GB8841@progeny.tock","threadId":"23598","inReplyTo":"4BD56985.2030506@gmail.com","subject":"Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T10:28:30Z","receivedAt":"2010-04-26T10:28:30Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Will Palmer wrote:\n\n> I think the documentation / test changes are needed, but my\n> own patch will probably go through several more revisions\n> before it is ready to be included, which is why I suggested\n> submitting your series separately.\n\nOh, I only took the uncontroversial part. ;-)\n\nJonathan\n"},{"id":"140418","messageId":"20100426142831.GA25209@coredump.intra.peff.net","threadId":"23598","inReplyTo":"20100426041411.GB30566@progeny.tock","subject":"Re: [PATCH v2 1/3] pretty: add conditional %C?colorname placeholders","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-26T14:28:32Z","receivedAt":"2010-04-26T14:28:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 25, 2010 at 11:14:12PM -0500, Jonathan Nieder wrote:\n\n> > %C(diff.commit), we are not breaking anything (because it is a new\n> > syntax), but a script like the one above may want to convert, and there\n> > is no way to say \"respect the color _config_, but don't respect\n> > isatty(1)\". Saying \"--color\" doesn't work, because it overrides the\n> > color config. We can cheat a little with GIT_PAGER_IN_USE, but that will\n> > have funny interactions with pager.color.\n> \n> Very interesting. I think the natural solution (for new colors) would\n> be a --color=config option, which would require parsing options before\n> checking configuration.\n> \n> Does this sound sane?\n\nYeah, that makes sense to me.\n\n-Peff\n"},{"id":"140455","messageId":"1272319890.5368.15.camel@walleee","threadId":"23598","inReplyTo":"20100426072516.GA30416@progeny.tock","subject":"Re: [PATCH v2 3/3] pretty: add aliases for pretty formats","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2010-04-26T22:11:30Z","receivedAt":"2010-04-26T22:11:30Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"On Mon, 2010-04-26 at 02:25 -0500, Jonathan Nieder wrote:\n... a lot ...\n\nNow that I've had a chance to fully read through this, I see that I\ncan't actually just go through line-by-line and respond to it off the\ntop of my head. I've made a note of everything, and anything which is\nnot directly addressed in the next version, should at least be addressed\nin the cover-letter.\n\nThank you very much for taking time to review this\n\n-- \n-- Will\n"}]}