{"thread":{"id":"11489","subject":"[PATCH] Avoid a useless prefix lookup in strbuf_expand()","startedAt":"2008-01-06T00:10:01Z","lastAt":"2008-01-06T00:10:01Z","messageCount":1,"participants":["Marco Costalba"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"64555","messageId":"e5bfff550801051610l3086ee7yccd8c23b8249a817@mail.gmail.com","threadId":"11489","inReplyTo":null,"subject":"[PATCH] Avoid a useless prefix lookup in strbuf_expand()","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2008-01-06T00:10:01Z","receivedAt":"2008-01-06T00:10:01Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"Currently the --prett=format prefix is looked up in a\ntight loop in strbuf_expand(), if found is passed as parameter\nto format_commit_item() that does another search using a\nswitch statement to select the proper operation according to\nthe kind of prefix.\n\nBecause the switch statement is already able to discard unknown\nmatches we don't need the prefix lookup before to call format_commit_item()\n\nThis patch removes an useless loop in a very fasth path,\nused by, as example, by 'git log' with --pretty=format option\n\nSigned-off-by: Marco Costalba <mcostalba@gmail.com>\n---\n\n\nTo apply on top of \"[PATCH] Fix an off by one bug in pretty.c\"\n\n\nI send also as attached file because I thing my mailer will\nword wrap this one.\n\n\npretty.c |  228 +++++++++++++++++++++++++++++--------------------------------\n strbuf.c |   19 ++---\n strbuf.h |    4 +-\n 3 files changed, 118 insertions(+), 133 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 3ce5e6f..5132b1f 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -282,76 +282,93 @@ static char *logmsg_reencode(const struct\n \treturn out;\n }\n\n-static void format_person_part(struct strbuf *sb, char part,\n-                               const char *msg, int len)\n+static int parse_tz(char *ep, const char *msg, int len) {\n+\n+\tint tz = 0, start = ep - msg + 1;\n+\n+\tfor ( ;start < len && isspace(msg[start]); start++)\n+\t\t; /* do nothing */\n+\n+\tif (start + 1 < len) {\n+\t\ttz = strtoul(msg + start + 1, NULL, 10);\n+\t\tif (msg[start] == '-')\n+\t\t\ttz = -tz;\n+\t}\n+\treturn tz;\n+}\n+\n+static size_t format_person_part(struct strbuf *sb, char part,\n+                                 const char *msg, int len)\n {\n-\tint start, end, tz = 0;\n-\tunsigned long date;\n+\tint start, end, tz = 0, date_valid;\n+\tunsigned long date = 0;\n \tchar *ep;\n\n-\t/* parse name */\n+\t/* advance 'end' to point to name end delimiter */\n \tfor (end = 0; end < len && msg[end] != '<'; end++)\n \t\t; /* do nothing */\n-\tstart = end + 1;\n-\twhile (end > 0 && isspace(msg[end - 1]))\n-\t\tend--;\n-\tif (part == 'n') {\t/* name */\n+\n+\tif (part == 'n') { /* name */\n+\t\twhile (end > 0 && isspace(msg[end - 1]))\n+\t\t\tend--;\n+\n \t\tstrbuf_add(sb, msg, end);\n-\t\treturn;\n+\t\treturn 2;\n \t}\n+\tstart = ++end; /* save email start delimiter */\n\n-\tif (start >= len)\n-\t\treturn;\n-\n-\t/* parse email */\n-\tfor (end = start; end < len && msg[end] != '>'; end++)\n+\t/* advance 'end' to point to email end delimiter */\n+\tfor ( ; end < len && msg[end] != '>'; end++)\n \t\t; /* do nothing */\n\n-\tif (end >= len)\n-\t\treturn;\n-\n-\tif (part == 'e') {\t/* email */\n-\t\tstrbuf_add(sb, msg + start, end - start);\n-\t\treturn;\n+\tif (part == 'e') { /* email */\n+\t\tif (end - start > 0)\n+\t\t\tstrbuf_add(sb, msg + start, end - start);\n+\t\treturn 2;\n \t}\n\n-\t/* parse date */\n+\t/* advance 'start' to point to date start delimiter */\n \tfor (start = end + 1; start < len && isspace(msg[start]); start++)\n \t\t; /* do nothing */\n-\tif (start >= len)\n-\t\treturn;\n-\tdate = strtoul(msg + start, &ep, 10);\n-\tif (msg + start == ep)\n-\t\treturn;\n\n-\tif (part == 't') {\t/* date, UNIX timestamp */\n-\t\tstrbuf_add(sb, msg + start, ep - (msg + start));\n-\t\treturn;\n-\t}\n+\tdate_valid = start < len;\n\n-\t/* parse tz */\n-\tfor (start = ep - msg + 1; start < len && isspace(msg[start]); start++)\n-\t\t; /* do nothing */\n-\tif (start + 1 < len) {\n-\t\ttz = strtoul(msg + start + 1, NULL, 10);\n-\t\tif (msg[start] == '-')\n-\t\t\ttz = -tz;\n+\tif (date_valid)\n+\t\tdate = strtoul(msg + start, &ep, 10);\n+\n+\tif (part == 't') { /* date, UNIX timestamp */\n+\t\tif (date_valid && msg + start != ep)\n+\t\t\tstrbuf_add(sb, msg + start, ep - (msg + start));\n+\t\treturn 2;\n \t}\n\n \tswitch (part) {\n-\tcase 'd':\t/* date */\n-\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_NORMAL));\n-\t\treturn;\n-\tcase 'D':\t/* date, RFC2822 style */\n-\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_RFC2822));\n-\t\treturn;\n-\tcase 'r':\t/* date, relative */\n-\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_RELATIVE));\n-\t\treturn;\n-\tcase 'i':\t/* date, ISO 8601 */\n-\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_ISO8601));\n-\t\treturn;\n+\tcase 'd': /* date */\n+\t\tif (date_valid) {\n+\t\t\ttz = parse_tz(ep, msg, len);\n+\t\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_NORMAL));\n+\t\t}\n+\t\treturn 2;\n+\tcase 'D': /* date, RFC2822 style */\n+\t\tif (date_valid) {\n+\t\t\ttz = parse_tz(ep, msg, len);\n+\t\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_RFC2822));\n+\t\t}\n+\t\treturn 2;\n+\tcase 'r': /* date, relative */\n+\t\tif (date_valid) {\n+\t\t\ttz = parse_tz(ep, msg, len);\n+\t\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_RELATIVE));\n+\t\t}\n+\t\treturn 2;\n+\tcase 'i': /* date, ISO 8601 */\n+\t\tif (date_valid) {\n+\t\t\ttz = parse_tz(ep, msg, len);\n+\t\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_ISO8601));\n+\t\t}\n+\t\treturn 2;\n \t}\n+\treturn 0; /* unknown person part */\n }\n\n struct chunk {\n@@ -432,8 +449,8 @@ static void parse_commit_header(struct format_\n \tcontext->commit_header_parsed = 1;\n }\n\n-static void format_commit_item(struct strbuf *sb, const char *placeholder,\n-                               void *context)\n+static size_t format_commit_item(struct strbuf *sb, const char *placeholder,\n+                                 void *context)\n {\n \tstruct format_commit_context *c = context;\n \tconst struct commit *commit = c->commit;\n@@ -443,23 +460,23 @@ static void format_commit_item(struct strbuf *sb,\n \t/* these are independent of the commit */\n \tswitch (placeholder[0]) {\n \tcase 'C':\n-\t\tswitch (placeholder[3]) {\n-\t\tcase 'd':\t/* red */\n+\t\tif (!prefixcmp(placeholder + 1, \"red\")) {\n \t\t\tstrbuf_addstr(sb, \"\\033[31m\");\n-\t\t\treturn;\n-\t\tcase 'e':\t/* green */\n+\t\t\treturn 4;\n+\t\t} else if (!prefixcmp(placeholder + 1, \"green\")) {\n \t\t\tstrbuf_addstr(sb, \"\\033[32m\");\n-\t\t\treturn;\n-\t\tcase 'u':\t/* blue */\n+\t\t\treturn 6;\n+\t\t} else if (!prefixcmp(placeholder + 1, \"blue\")) {\n \t\t\tstrbuf_addstr(sb, \"\\033[34m\");\n-\t\t\treturn;\n-\t\tcase 's':\t/* reset color */\n+\t\t\treturn 5;\n+\t\t} else if (!prefixcmp(placeholder + 1, \"reset\")) {\n \t\t\tstrbuf_addstr(sb, \"\\033[m\");\n-\t\t\treturn;\n-\t\t}\n+\t\t\treturn 6;\n+\t\t} else\n+\t\t\treturn 0;\n \tcase 'n':\t\t/* newline */\n \t\tstrbuf_addch(sb, '\\n');\n-\t\treturn;\n+\t\treturn 1;\n \t}\n\n \t/* these depend on the commit */\n@@ -469,34 +486,34 @@ static void format_commit_item(struct strbuf *sb,\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;\n+\t\treturn 1;\n \tcase 'h':\t\t/* abbreviated commit hash */\n \t\tif (add_again(sb, &c->abbrev_commit_hash))\n-\t\t\treturn;\n+\t\t\treturn 1;\n \t\tstrbuf_addstr(sb, find_unique_abbrev(commit->object.sha1,\n \t\t                                     DEFAULT_ABBREV));\n \t\tc->abbrev_commit_hash.len = sb->len - c->abbrev_commit_hash.off;\n-\t\treturn;\n+\t\treturn 1;\n \tcase 'T':\t\t/* tree hash */\n \t\tstrbuf_addstr(sb, sha1_to_hex(commit->tree->object.sha1));\n-\t\treturn;\n+\t\treturn 1;\n \tcase 't':\t\t/* abbreviated tree hash */\n \t\tif (add_again(sb, &c->abbrev_tree_hash))\n-\t\t\treturn;\n+\t\t\treturn 1;\n \t\tstrbuf_addstr(sb, find_unique_abbrev(commit->tree->object.sha1,\n \t\t                                     DEFAULT_ABBREV));\n \t\tc->abbrev_tree_hash.len = sb->len - c->abbrev_tree_hash.off;\n-\t\treturn;\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;\n+\t\treturn 1;\n \tcase 'p':\t\t/* abbreviated parent hashes */\n \t\tif (add_again(sb, &c->abbrev_parent_hashes))\n-\t\t\treturn;\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@@ -505,14 +522,14 @@ static void format_commit_item(struct strbuf\n*sb, const char *placeholder,\n \t\t}\n \t\tc->abbrev_parent_hashes.len = sb->len -\n \t\t                              c->abbrev_parent_hashes.off;\n-\t\treturn;\n+\t\treturn 1;\n \tcase 'm':\t\t/* left/right/bottom */\n \t\tstrbuf_addch(sb, (commit->object.flags & BOUNDARY)\n \t\t                 ? '-'\n \t\t                 : (commit->object.flags & SYMMETRIC_LEFT)\n \t\t                 ? '<'\n \t\t                 : '>');\n-\t\treturn;\n+\t\treturn 1;\n \t}\n\n \t/* For the rest we have to parse the commit header. */\n@@ -520,66 +537,37 @@ static void format_commit_item(struct strbuf *sb,\n \t\tparse_commit_header(c);\n\n \tswitch (placeholder[0]) {\n-\tcase 's':\n+\tcase 's':\t\t/* subject */\n \t\tstrbuf_add(sb, msg + c->subject.off, c->subject.len);\n-\t\treturn;\n-\tcase 'a':\n-\t\tformat_person_part(sb, placeholder[1],\n-\t\t                   msg + c->author.off, c->author.len);\n-\t\treturn;\n-\tcase 'c':\n-\t\tformat_person_part(sb, placeholder[1],\n-\t\t                   msg + c->committer.off, c->committer.len);\n-\t\treturn;\n-\tcase 'e':\n+\t\treturn 1;\n+\tcase 'a':\t\t/* author ... */\n+\t\treturn format_person_part(sb, placeholder[1],\n+\t\t                       msg + c->author.off,\n+\t\t                       c->author.len);\n+\n+\tcase 'c':\t\t/* committer ... */\n+\t\treturn format_person_part(sb, placeholder[1],\n+\t\t                       msg + c->committer.off,\n+\t\t                       c->committer.len);\n+\n+\tcase 'e':\t\t/* encoding */\n \t\tstrbuf_add(sb, msg + c->encoding.off, c->encoding.len);\n-\t\treturn;\n-\tcase 'b':\n+\t\treturn 1;\n+\tcase 'b':\t\t/* body */\n \t\tstrbuf_addstr(sb, msg + c->body_off);\n-\t\treturn;\n+\t\treturn 1;\n \t}\n+\treturn 0;\t\t/* unknown placeholder */\n }\n\n void format_commit_message(const struct commit *commit,\n                            const void *format, struct strbuf *sb)\n {\n-\tconst char *placeholders[] = {\n-\t\t\"H\",\t\t/* commit hash */\n-\t\t\"h\",\t\t/* abbreviated commit hash */\n-\t\t\"T\",\t\t/* tree hash */\n-\t\t\"t\",\t\t/* abbreviated tree hash */\n-\t\t\"P\",\t\t/* parent hashes */\n-\t\t\"p\",\t\t/* abbreviated parent hashes */\n-\t\t\"an\",\t\t/* author name */\n-\t\t\"ae\",\t\t/* author email */\n-\t\t\"ad\",\t\t/* author date */\n-\t\t\"aD\",\t\t/* author date, RFC2822 style */\n-\t\t\"ar\",\t\t/* author date, relative */\n-\t\t\"at\",\t\t/* author date, UNIX timestamp */\n-\t\t\"ai\",\t\t/* author date, ISO 8601 */\n-\t\t\"cn\",\t\t/* committer name */\n-\t\t\"ce\",\t\t/* committer email */\n-\t\t\"cd\",\t\t/* committer date */\n-\t\t\"cD\",\t\t/* committer date, RFC2822 style */\n-\t\t\"cr\",\t\t/* committer date, relative */\n-\t\t\"ct\",\t\t/* committer date, UNIX timestamp */\n-\t\t\"ci\",\t\t/* committer date, ISO 8601 */\n-\t\t\"e\",\t\t/* encoding */\n-\t\t\"s\",\t\t/* subject */\n-\t\t\"b\",\t\t/* body */\n-\t\t\"Cred\",\t\t/* red */\n-\t\t\"Cgreen\",\t/* green */\n-\t\t\"Cblue\",\t/* blue */\n-\t\t\"Creset\",\t/* reset color */\n-\t\t\"n\",\t\t/* newline */\n-\t\t\"m\",\t\t/* left/right/bottom */\n-\t\tNULL\n-\t};\n \tstruct format_commit_context context;\n\n \tmemset(&context, 0, sizeof(context));\n \tcontext.commit = commit;\n-\tstrbuf_expand(sb, format, placeholders, format_commit_item, &context);\n+\tstrbuf_expand(sb, format, format_commit_item, &context);\n }\n\n static void pp_header(enum cmit_fmt fmt,\ndiff --git a/strbuf.c b/strbuf.c\nindex 5efcfc8..32ab8e5 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -146,11 +146,12 @@ void strbuf_addf(struct strbuf *sb,\n \tstrbuf_setlen(sb, sb->len + len);\n }\n\n-void strbuf_expand(struct strbuf *sb, const char *format,\n-                   const char **placeholders, expand_fn_t fn, void *context)\n+void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn,\n+                   void *context)\n {\n \tfor (;;) {\n-\t\tconst char *percent, **p;\n+\t\tconst char *percent;\n+\t\tsize_t consumed;\n\n \t\tpercent = strchrnul(format, '%');\n \t\tstrbuf_add(sb, format, percent - format);\n@@ -158,14 +159,10 @@ void strbuf_expand(struct strbuf *sb,\n \t\t\tbreak;\n \t\tformat = percent + 1;\n\n-\t\tfor (p = placeholders; *p; p++) {\n-\t\t\tif (!prefixcmp(format, *p))\n-\t\t\t\tbreak;\n-\t\t}\n-\t\tif (*p) {\n-\t\t\tfn(sb, *p, context);\n-\t\t\tformat += strlen(*p);\n-\t\t} else\n+\t\tconsumed = fn(sb, format, context);\n+\t\tif (consumed)\n+\t\t\tformat += consumed;\n+\t\telse\n \t\t\tstrbuf_addch(sb, '%');\n \t}\n }\ndiff --git a/strbuf.h b/strbuf.h\nindex 36d61db..faec229 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -103,8 +103,8 @@ static inline void strbuf_addbuf(struct strbuf *sb,\n }\n extern void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len);\n\n-typedef void (*expand_fn_t) (struct strbuf *sb, const char\n*placeholder, void *context);\n-extern void strbuf_expand(struct strbuf *sb, const char *format,\nconst char **placeholders, expand_fn_t fn, void *context);\n+typedef size_t (*expand_fn_t) (struct strbuf *sb, const char\n*placeholder, void *context);\n+extern void strbuf_expand(struct strbuf *sb, const char *format,\nexpand_fn_t fn, void *context);\n\n __attribute__((format(printf,2,3)))\n extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);\n\n\nSubject: [PATCH] Avoid a useless prefix lookup in strbuf_expand()\n\nCurrently the --prett=format prefix is looked up in a\ntight loop in strbuf_expand(), if found is passed as parameter\nto format_commit_item() that does another search using a\nswitch statement to select the proper operation according to\nthe kind of prefix.\n\nBecause the switch statement is already able to discard unknown\nmatches we don't need the prefix lookup before to call format_commit_item()\n\nThis patch removes an useless loop in a very fasth path,\nused by, as example, by 'git log' with --pretty=format option\n\nSigned-off-by: Marco Costalba <mcostalba@gmail.com>\n---\n\n\nTo apply on top of \"[PATCH] Fix an off by one bug in pretty.c\"\n\n\npretty.c |  228 +++++++++++++++++++++++++++++--------------------------------\n strbuf.c |   19 ++---\n strbuf.h |    4 +-\n 3 files changed, 118 insertions(+), 133 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 3ce5e6f..5132b1f 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -282,76 +282,93 @@ static char *logmsg_reencode(const struct \n \treturn out;\n }\n \n-static void format_person_part(struct strbuf *sb, char part,\n-                               const char *msg, int len)\n+static int parse_tz(char *ep, const char *msg, int len) {\n+\n+\tint tz = 0, start = ep - msg + 1;\n+\n+\tfor ( ;start < len && isspace(msg[start]); start++)\n+\t\t; /* do nothing */\n+\n+\tif (start + 1 < len) {\n+\t\ttz = strtoul(msg + start + 1, NULL, 10);\n+\t\tif (msg[start] == '-')\n+\t\t\ttz = -tz;\n+\t}\n+\treturn tz;\n+}\n+\n+static size_t format_person_part(struct strbuf *sb, char part,\n+                                 const char *msg, int len)\n {\n-\tint start, end, tz = 0;\n-\tunsigned long date;\n+\tint start, end, tz = 0, date_valid;\n+\tunsigned long date = 0;\n \tchar *ep;\n \n-\t/* parse name */\n+\t/* advance 'end' to point to name end delimiter */\n \tfor (end = 0; end < len && msg[end] != '<'; end++)\n \t\t; /* do nothing */\n-\tstart = end + 1;\n-\twhile (end > 0 && isspace(msg[end - 1]))\n-\t\tend--;\n-\tif (part == 'n') {\t/* name */\n+\n+\tif (part == 'n') { /* name */\n+\t\twhile (end > 0 && isspace(msg[end - 1]))\n+\t\t\tend--;\n+\n \t\tstrbuf_add(sb, msg, end);\n-\t\treturn;\n+\t\treturn 2;\n \t}\n+\tstart = ++end; /* save email start delimiter */\n \n-\tif (start >= len)\n-\t\treturn;\n-\n-\t/* parse email */\n-\tfor (end = start; end < len && msg[end] != '>'; end++)\n+\t/* advance 'end' to point to email end delimiter */\n+\tfor ( ; end < len && msg[end] != '>'; end++)\n \t\t; /* do nothing */\n \n-\tif (end >= len)\n-\t\treturn;\n-\n-\tif (part == 'e') {\t/* email */\n-\t\tstrbuf_add(sb, msg + start, end - start);\n-\t\treturn;\n+\tif (part == 'e') { /* email */\n+\t\tif (end - start > 0)\n+\t\t\tstrbuf_add(sb, msg + start, end - start);\n+\t\treturn 2;\n \t}\n \n-\t/* parse date */\n+\t/* advance 'start' to point to date start delimiter */\n \tfor (start = end + 1; start < len && isspace(msg[start]); start++)\n \t\t; /* do nothing */\n-\tif (start >= len)\n-\t\treturn;\n-\tdate = strtoul(msg + start, &ep, 10);\n-\tif (msg + start == ep)\n-\t\treturn;\n \n-\tif (part == 't') {\t/* date, UNIX timestamp */\n-\t\tstrbuf_add(sb, msg + start, ep - (msg + start));\n-\t\treturn;\n-\t}\n+\tdate_valid = start < len;\n \n-\t/* parse tz */\n-\tfor (start = ep - msg + 1; start < len && isspace(msg[start]); start++)\n-\t\t; /* do nothing */\n-\tif (start + 1 < len) {\n-\t\ttz = strtoul(msg + start + 1, NULL, 10);\n-\t\tif (msg[start] == '-')\n-\t\t\ttz = -tz;\n+\tif (date_valid)\n+\t\tdate = strtoul(msg + start, &ep, 10);\n+\n+\tif (part == 't') { /* date, UNIX timestamp */\n+\t\tif (date_valid && msg + start != ep)\n+\t\t\tstrbuf_add(sb, msg + start, ep - (msg + start));\n+\t\treturn 2;\n \t}\n \n \tswitch (part) {\n-\tcase 'd':\t/* date */\n-\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_NORMAL));\n-\t\treturn;\n-\tcase 'D':\t/* date, RFC2822 style */\n-\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_RFC2822));\n-\t\treturn;\n-\tcase 'r':\t/* date, relative */\n-\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_RELATIVE));\n-\t\treturn;\n-\tcase 'i':\t/* date, ISO 8601 */\n-\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_ISO8601));\n-\t\treturn;\n+\tcase 'd': /* date */\n+\t\tif (date_valid) {\n+\t\t\ttz = parse_tz(ep, msg, len);\n+\t\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_NORMAL));\n+\t\t}\n+\t\treturn 2;\n+\tcase 'D': /* date, RFC2822 style */\n+\t\tif (date_valid) {\n+\t\t\ttz = parse_tz(ep, msg, len);\n+\t\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_RFC2822));\n+\t\t}\n+\t\treturn 2;\n+\tcase 'r': /* date, relative */\n+\t\tif (date_valid) {\n+\t\t\ttz = parse_tz(ep, msg, len);\n+\t\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_RELATIVE));\n+\t\t}\n+\t\treturn 2;\n+\tcase 'i': /* date, ISO 8601 */\n+\t\tif (date_valid) {\n+\t\t\ttz = parse_tz(ep, msg, len);\n+\t\t\tstrbuf_addstr(sb, show_date(date, tz, DATE_ISO8601));\n+\t\t}\n+\t\treturn 2;\n \t}\n+\treturn 0; /* unknown person part */\n }\n \n struct chunk {\n@@ -432,8 +449,8 @@ static void parse_commit_header(struct format_\n \tcontext->commit_header_parsed = 1;\n }\n \n-static void format_commit_item(struct strbuf *sb, const char *placeholder,\n-                               void *context)\n+static size_t format_commit_item(struct strbuf *sb, const char *placeholder,\n+                                 void *context)\n {\n \tstruct format_commit_context *c = context;\n \tconst struct commit *commit = c->commit;\n@@ -443,23 +460,23 @@ static void format_commit_item(struct strbuf *sb, \n \t/* these are independent of the commit */\n \tswitch (placeholder[0]) {\n \tcase 'C':\n-\t\tswitch (placeholder[3]) {\n-\t\tcase 'd':\t/* red */\n+\t\tif (!prefixcmp(placeholder + 1, \"red\")) {\n \t\t\tstrbuf_addstr(sb, \"\\033[31m\");\n-\t\t\treturn;\n-\t\tcase 'e':\t/* green */\n+\t\t\treturn 4;\n+\t\t} else if (!prefixcmp(placeholder + 1, \"green\")) {\n \t\t\tstrbuf_addstr(sb, \"\\033[32m\");\n-\t\t\treturn;\n-\t\tcase 'u':\t/* blue */\n+\t\t\treturn 6;\n+\t\t} else if (!prefixcmp(placeholder + 1, \"blue\")) {\n \t\t\tstrbuf_addstr(sb, \"\\033[34m\");\n-\t\t\treturn;\n-\t\tcase 's':\t/* reset color */\n+\t\t\treturn 5;\n+\t\t} else if (!prefixcmp(placeholder + 1, \"reset\")) {\n \t\t\tstrbuf_addstr(sb, \"\\033[m\");\n-\t\t\treturn;\n-\t\t}\n+\t\t\treturn 6;\n+\t\t} else\n+\t\t\treturn 0;\n \tcase 'n':\t\t/* newline */\n \t\tstrbuf_addch(sb, '\\n');\n-\t\treturn;\n+\t\treturn 1;\n \t}\n \n \t/* these depend on the commit */\n@@ -469,34 +486,34 @@ static void format_commit_item(struct strbuf *sb, \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;\n+\t\treturn 1;\n \tcase 'h':\t\t/* abbreviated commit hash */\n \t\tif (add_again(sb, &c->abbrev_commit_hash))\n-\t\t\treturn;\n+\t\t\treturn 1;\n \t\tstrbuf_addstr(sb, find_unique_abbrev(commit->object.sha1,\n \t\t                                     DEFAULT_ABBREV));\n \t\tc->abbrev_commit_hash.len = sb->len - c->abbrev_commit_hash.off;\n-\t\treturn;\n+\t\treturn 1;\n \tcase 'T':\t\t/* tree hash */\n \t\tstrbuf_addstr(sb, sha1_to_hex(commit->tree->object.sha1));\n-\t\treturn;\n+\t\treturn 1;\n \tcase 't':\t\t/* abbreviated tree hash */\n \t\tif (add_again(sb, &c->abbrev_tree_hash))\n-\t\t\treturn;\n+\t\t\treturn 1;\n \t\tstrbuf_addstr(sb, find_unique_abbrev(commit->tree->object.sha1,\n \t\t                                     DEFAULT_ABBREV));\n \t\tc->abbrev_tree_hash.len = sb->len - c->abbrev_tree_hash.off;\n-\t\treturn;\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;\n+\t\treturn 1;\n \tcase 'p':\t\t/* abbreviated parent hashes */\n \t\tif (add_again(sb, &c->abbrev_parent_hashes))\n-\t\t\treturn;\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@@ -505,14 +522,14 @@ static void format_commit_item(struct strbuf *sb, const char *placeholder,\n \t\t}\n \t\tc->abbrev_parent_hashes.len = sb->len -\n \t\t                              c->abbrev_parent_hashes.off;\n-\t\treturn;\n+\t\treturn 1;\n \tcase 'm':\t\t/* left/right/bottom */\n \t\tstrbuf_addch(sb, (commit->object.flags & BOUNDARY)\n \t\t                 ? '-'\n \t\t                 : (commit->object.flags & SYMMETRIC_LEFT)\n \t\t                 ? '<'\n \t\t                 : '>');\n-\t\treturn;\n+\t\treturn 1;\n \t}\n \n \t/* For the rest we have to parse the commit header. */\n@@ -520,66 +537,37 @@ static void format_commit_item(struct strbuf *sb,\n \t\tparse_commit_header(c);\n \n \tswitch (placeholder[0]) {\n-\tcase 's':\n+\tcase 's':\t\t/* subject */\n \t\tstrbuf_add(sb, msg + c->subject.off, c->subject.len);\n-\t\treturn;\n-\tcase 'a':\n-\t\tformat_person_part(sb, placeholder[1],\n-\t\t                   msg + c->author.off, c->author.len);\n-\t\treturn;\n-\tcase 'c':\n-\t\tformat_person_part(sb, placeholder[1],\n-\t\t                   msg + c->committer.off, c->committer.len);\n-\t\treturn;\n-\tcase 'e':\n+\t\treturn 1;\n+\tcase 'a':\t\t/* author ... */\n+\t\treturn format_person_part(sb, placeholder[1],\n+\t\t                       msg + c->author.off,\n+\t\t                       c->author.len);\n+\n+\tcase 'c':\t\t/* committer ... */\n+\t\treturn format_person_part(sb, placeholder[1],\n+\t\t                       msg + c->committer.off,\n+\t\t                       c->committer.len);\n+\n+\tcase 'e':\t\t/* encoding */\n \t\tstrbuf_add(sb, msg + c->encoding.off, c->encoding.len);\n-\t\treturn;\n-\tcase 'b':\n+\t\treturn 1;\n+\tcase 'b':\t\t/* body */\n \t\tstrbuf_addstr(sb, msg + c->body_off);\n-\t\treturn;\n+\t\treturn 1;\n \t}\n+\treturn 0;\t\t/* unknown placeholder */\n }\n \n void format_commit_message(const struct commit *commit,\n                            const void *format, struct strbuf *sb)\n {\n-\tconst char *placeholders[] = {\n-\t\t\"H\",\t\t/* commit hash */\n-\t\t\"h\",\t\t/* abbreviated commit hash */\n-\t\t\"T\",\t\t/* tree hash */\n-\t\t\"t\",\t\t/* abbreviated tree hash */\n-\t\t\"P\",\t\t/* parent hashes */\n-\t\t\"p\",\t\t/* abbreviated parent hashes */\n-\t\t\"an\",\t\t/* author name */\n-\t\t\"ae\",\t\t/* author email */\n-\t\t\"ad\",\t\t/* author date */\n-\t\t\"aD\",\t\t/* author date, RFC2822 style */\n-\t\t\"ar\",\t\t/* author date, relative */\n-\t\t\"at\",\t\t/* author date, UNIX timestamp */\n-\t\t\"ai\",\t\t/* author date, ISO 8601 */\n-\t\t\"cn\",\t\t/* committer name */\n-\t\t\"ce\",\t\t/* committer email */\n-\t\t\"cd\",\t\t/* committer date */\n-\t\t\"cD\",\t\t/* committer date, RFC2822 style */\n-\t\t\"cr\",\t\t/* committer date, relative */\n-\t\t\"ct\",\t\t/* committer date, UNIX timestamp */\n-\t\t\"ci\",\t\t/* committer date, ISO 8601 */\n-\t\t\"e\",\t\t/* encoding */\n-\t\t\"s\",\t\t/* subject */\n-\t\t\"b\",\t\t/* body */\n-\t\t\"Cred\",\t\t/* red */\n-\t\t\"Cgreen\",\t/* green */\n-\t\t\"Cblue\",\t/* blue */\n-\t\t\"Creset\",\t/* reset color */\n-\t\t\"n\",\t\t/* newline */\n-\t\t\"m\",\t\t/* left/right/bottom */\n-\t\tNULL\n-\t};\n \tstruct format_commit_context context;\n \n \tmemset(&context, 0, sizeof(context));\n \tcontext.commit = commit;\n-\tstrbuf_expand(sb, format, placeholders, format_commit_item, &context);\n+\tstrbuf_expand(sb, format, format_commit_item, &context);\n }\n \n static void pp_header(enum cmit_fmt fmt,\ndiff --git a/strbuf.c b/strbuf.c\nindex 5efcfc8..32ab8e5 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -146,11 +146,12 @@ void strbuf_addf(struct strbuf *sb, \n \tstrbuf_setlen(sb, sb->len + len);\n }\n \n-void strbuf_expand(struct strbuf *sb, const char *format,\n-                   const char **placeholders, expand_fn_t fn, void *context)\n+void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn,\n+                   void *context)\n {\n \tfor (;;) {\n-\t\tconst char *percent, **p;\n+\t\tconst char *percent;\n+\t\tsize_t consumed;\n \n \t\tpercent = strchrnul(format, '%');\n \t\tstrbuf_add(sb, format, percent - format);\n@@ -158,14 +159,10 @@ void strbuf_expand(struct strbuf *sb, \n \t\t\tbreak;\n \t\tformat = percent + 1;\n \n-\t\tfor (p = placeholders; *p; p++) {\n-\t\t\tif (!prefixcmp(format, *p))\n-\t\t\t\tbreak;\n-\t\t}\n-\t\tif (*p) {\n-\t\t\tfn(sb, *p, context);\n-\t\t\tformat += strlen(*p);\n-\t\t} else\n+\t\tconsumed = fn(sb, format, context);\n+\t\tif (consumed)\n+\t\t\tformat += consumed;\n+\t\telse\n \t\t\tstrbuf_addch(sb, '%');\n \t}\n }\ndiff --git a/strbuf.h b/strbuf.h\nindex 36d61db..faec229 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -103,8 +103,8 @@ static inline void strbuf_addbuf(struct strbuf *sb, \n }\n extern void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len);\n \n-typedef void (*expand_fn_t) (struct strbuf *sb, const char *placeholder, void *context);\n-extern void strbuf_expand(struct strbuf *sb, const char *format, const char **placeholders, expand_fn_t fn, void *context);\n+typedef size_t (*expand_fn_t) (struct strbuf *sb, const char *placeholder, void *context);\n+extern void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn, void *context);\n \n __attribute__((format(printf,2,3)))\n extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);\n"}]}