{"thread":{"id":"26644","subject":"[Bug] %[a|c]d placeholder does not respect --date= option in combination with git archive","startedAt":"2011-03-03T09:30:16Z","lastAt":"2011-03-11T08:33:30Z","messageCount":15,"participants":["Dietmar Winkler","Jeff King","Will Palmer","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"162712","messageId":"4D6F5FA8.5030105@gmx.de","threadId":"26644","inReplyTo":null,"subject":"[Bug] %[a|c]d placeholder does not respect --date= option in combination with git archive","fromName":"Dietmar Winkler","fromEmail":"dietmarw@gmx.de","sentAt":"2011-03-03T09:30:16Z","receivedAt":"2011-03-03T09:30:16Z","isPatch":false,"sender":{"key":"dietmarw@gmx.de","avatar":"https://gravatar.com/avatar/1ea50950a453bd8d50d8170dcebb52912082c8b2ba9c90a3e85438a14272e270?d=mp&s=160"},"body":"-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA1\n\nIt seems like that the place holders %ad and %cd do not respect the\n- --date= option when used as part of the export substitution.\n\nIn my file I have the place holder $Format:%ad$ and in .git/config the\nsetting log.date = short is present.\n\nI can very this the date setting by running\n\tgit log --pretty=format:%ad\nand I get:\n2011-03-03\n2011-02-28\n2011-02-28\n2011-02-28\n2011-02-28\n\n\nNow if I run on the same repo\n\tgit archive --format=zip HEAD -o out.zip\nand check the place holder in the exported zip file it is actually\nreplaced with:\n\nThu, 3 Mar 2011 10:06:43 +0100\n\nand not\n\n2011-03-03\n\nThe same happens with the place holder %cd\n\n/Dietmar/\n\n\n-----BEGIN PGP SIGNATURE-----\nVersion: GnuPG v1.4.10 (GNU/Linux)\n\niJwEAQECAAYFAk1vX6gACgkQCXG8gXafJGFfHwQApYJ/y0bS0D97fMKGjjBjjZQv\nsOrPbwxIaPII2RGeRqxlQLjDL2kYnXHObTor+3rLWbNHLXPjjPw/2r4YIeCxz/f+\nvA8ro9o4dTAyvGiUc/xUhu/U4XaVuV4Rl3QX83oaGmuefHRGc5/esex4R4mnzVdW\nQBPVvqqs25gyiu7zV6s=\n=lVtM\n-----END PGP SIGNATURE-----\n"},{"id":"162723","messageId":"20110303151019.GC1074@sigill.intra.peff.net","threadId":"26644","inReplyTo":"4D6F5FA8.5030105@gmx.de","subject":"Re: [Bug] %[a|c]d placeholder does not respect --date= option in combination with git archive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-03T15:10:19Z","receivedAt":"2011-03-03T15:10:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 03, 2011 at 10:30:16AM +0100, Dietmar Winkler wrote:\n\n> In my file I have the place holder $Format:%ad$ and in .git/config the\n> setting log.date = short is present.\n> [...]\n> Now if I run on the same repo\n> \tgit archive --format=zip HEAD -o out.zip\n> and check the place holder in the exported zip file it is actually\n> replaced with:\n> \n> Thu, 3 Mar 2011 10:06:43 +0100\n> \n> and not\n> \n> 2011-03-03\n\nI am not sure that this is a bug. The log.date parameter is about the\nlog command, not necessarily other format substitutions. If it were any\nother date format, I would say the right answer is that you should be\nusing one of the format-specific date specifiers. But annoyingly, there\nis no such specifier for \"short\". Which means there is no way to\nactually get the output that you want.\n\nI remember at some point discussing extending the specifier syntax to\nallow things like \"%(ad,date=short)\", but it was never implemented. I\nthink that would be the cleanest way to do what you want.\n\nThe second cleanest would be adding an archive.date variable. Which is\nmuch simpler, obviously. But I think making \"log.date\" start applying to\narchive substitutions is going to surprise some people and possibly\nbreak their setups.\n\n-Peff\n"},{"id":"162761","messageId":"4D70BA9C.1080902@gmx.de","threadId":"26644","inReplyTo":"20110303151019.GC1074@sigill.intra.peff.net","subject":"Re: [Bug] %[a|c]d placeholder does not respect --date= option in combination with git archive","fromName":"Dietmar Winkler","fromEmail":"dietmarw@gmx.de","sentAt":"2011-03-04T10:10:36Z","receivedAt":"2011-03-04T10:10:36Z","isPatch":false,"sender":{"key":"dietmarw@gmx.de","avatar":"https://gravatar.com/avatar/1ea50950a453bd8d50d8170dcebb52912082c8b2ba9c90a3e85438a14272e270?d=mp&s=160"},"body":"Jeff and list,\n\nDen 03. mars 2011 16:10, skrev Jeff King:\n> I am not sure that this is a bug. The log.date parameter is about the\n> log command, not necessarily other format substitutions. \n\nWell in\nhttp://www.kernel.org/pub/software/scm/git/docs/gitattributes.html it says:\n\n \"The placeholders are the same as those for the option\n--pretty=format: of git-log(1), except that they need to be wrapped like\nthis: $Format:PLACEHOLDERS$ in the file.\"\n\nAnd in git log the list includes (besides the various date formats) also\n\n %ad: author date (format respects --date= option)\n ...\n %cd: committer date *\n\n*) actually here the string \"(format respects --date= option)\" is\nmissing. Otherwise what committer date format are we speaking about ;)\n\nSo either the documentation should make clear that the substitution will\n*not* work or (and this would be preferable) fix the substitution so\nthat it works as documented.\n\n> I remember at some point discussing extending the specifier syntax to\n> allow things like \"%(ad,date=short)\", but it was never implemented. I\n> think that would be the cleanest way to do what you want.\n\nYes that would be even better since it would give one the freedom of\ndefining different format for the subsitutions in different places in a\nproject. Shame it was not accepted.\n\n> The second cleanest would be adding an archive.date variable. Which is\n> much simpler, obviously. But I think making \"log.date\" start applying to\n> archive substitutions is going to surprise some people and possibly\n> break their setups.\n\nHow should this surprise people? If the used %ad they would have\nexpected a configuration depended substitution to start with. If they\nwanted a log.date *independent* substitution they should have (according\nto the documentation) some of the other formats (e.g., %ar, %ai, ...).\nSo I don't really see this as a reason for not fixing this bug.\n\n\n/Dietmar/\n"},{"id":"162834","messageId":"20110305195020.GA3089@sigill.intra.peff.net","threadId":"26644","inReplyTo":"4D70BA9C.1080902@gmx.de","subject":"Re: [Bug] %[a|c]d placeholder does not respect --date= option in combination with git archive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-05T19:50:20Z","receivedAt":"2011-03-05T19:50:20Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 04, 2011 at 11:10:36AM +0100, Dietmar Winkler wrote:\n\n> Well in\n> http://www.kernel.org/pub/software/scm/git/docs/gitattributes.html it says:\n> \n> \"The placeholders are the same as those for the option\n> --pretty=format: of git-log(1), except that they need to be wrapped like\n> this: $Format:PLACEHOLDERS$ in the file.\"\n> \n> And in git log the list includes (besides the various date formats) also\n> \n> %ad: author date (format respects --date= option)\n> ...\n> %cd: committer date *\n> \n> *) actually here the string \"(format respects --date= option)\" is\n> missing. Otherwise what committer date format are we speaking about ;)\n> \n> So either the documentation should make clear that the substitution will\n> *not* work or (and this would be preferable) fix the substitution so\n> that it works as documented.\n\nYeah, the documentation is misleadingly vague there. I've improved it in\nthe patch series below.\n\n> > I remember at some point discussing extending the specifier syntax to\n> > allow things like \"%(ad,date=short)\", but it was never implemented. I\n> > think that would be the cleanest way to do what you want.\n> \n> Yes that would be even better since it would give one the freedom of\n> defining different format for the subsitutions in different places in a\n> project. Shame it was not accepted.\n\nI think we got bogged down in what exactly the extended format should\nlook like and then nothing got done. I spent a few hours yesterday\nlooking again at how bad it would be to extend the syntax to handle both\nthe traditional format and '%(foo,arg=value)' but there are lot of\ncorner cases.\n\nSo this morning I scrapped that and just added \"%ad(mode)\" which was\nmuch simpler, and matches syntactically with some of our other commands.\nIt's in the series below.\n\n> > The second cleanest would be adding an archive.date variable. Which is\n> > much simpler, obviously. But I think making \"log.date\" start applying to\n> > archive substitutions is going to surprise some people and possibly\n> > break their setups.\n> \n> How should this surprise people? If the used %ad they would have\n> expected a configuration depended substitution to start with. If they\n> wanted a log.date *independent* substitution they should have (according\n> to the documentation) some of the other formats (e.g., %ar, %ai, ...).\n> So I don't really see this as a reason for not fixing this bug.\n\nImagine a project which uses \"git archive\" as part of its scripts for\nbuilding a distribution tarball. I.e., you run \"make dist\" or similar,\nand it produces the tarball. The gitattributes and $Format:%ad$\nplaceholders are contained in the upstream repository. So anybody who\nclones it can run \"make dist\" and get the identical tarball.\n\nNow imagine as a developer on the project, you prefer to see your logs\nwith a different date format. So you set log.date to \"short\". But if\ngit-archive behaves as you want it to, then your \"make dist\" is now\nbroken. It generates different results to everyone else's.\n\nAnyway, hopefully the point becomes moot with this patch series, which\nlets you do %ad(short) in your format strings:\n\n [1/2]: pretty.c: give format_person_part the whole placeholder\n [2/2]: pretty.c: allow date formats in user format strings\n\n-Peff\n"},{"id":"162835","messageId":"20110305195156.GA32095@sigill.intra.peff.net","threadId":"26644","inReplyTo":"20110305195020.GA3089@sigill.intra.peff.net","subject":"[PATCH 1/2] pretty.c: give format_person_part the whole placeholder","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-05T19:51:56Z","receivedAt":"2011-03-05T19:51:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Until now it only got to see the next character. Giving it\nthe whole string will make it possible to add longer\nplaceholders in a future patch.\n\nSigned-off-by: Jeff King \n---\nI split this out because the patch ends up so noisy.\n\n pretty.c | 20 ++++++++++----------\n 1 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 8549934..00bcf83 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -440,7 +440,7 @@ static int mailmap_name(char *email, int email_len, char *name, int name_len)\n \treturn mail_map->nr && map_user(mail_map, email, email_len, name, name_len);\n }\n \n-static size_t format_person_part(struct strbuf *sb, char part,\n+static size_t format_person_part(struct strbuf *sb, const char *part,\n \t\t\t\t const char *msg, int len, enum date_mode dmode)\n {\n \t/* currently all placeholders have same length */\n@@ -477,7 +477,7 @@ static size_t format_person_part(struct strbuf *sb, char part,\n \t\tgoto skip;\n \tend = mail_end-msg;\n \n-\tif (part == 'N' || part == 'E') { /* mailmap lookup */\n+\tif (*part == 'N' || *part == 'E') { /* mailmap lookup */\n \t\tstrlcpy(person_name, name_start, name_end-name_start+1);\n \t\tstrlcpy(person_mail, mail_start, mail_end-mail_start+1);\n \t\tmailmap_name(person_mail, sizeof(person_mail), person_name, sizeof(person_name));\n@@ -486,11 +486,11 @@ static size_t format_person_part(struct strbuf *sb, char part,\n \t\tmail_start = person_mail;\n \t\tmail_end = mail_start + strlen(person_mail);\n \t}\n-\tif (part == 'n' || part == 'N') {\t/* name */\n+\tif (*part == 'n' || *part == 'N') {\t/* name */\n \t\tstrbuf_add(sb, name_start, name_end-name_start);\n \t\treturn placeholder_len;\n \t}\n-\tif (part == 'e' || part == 'E') {\t/* email */\n+\tif (*part == 'e' || *part == 'E') {\t/* email */\n \t\tstrbuf_add(sb, mail_start, mail_end-mail_start);\n \t\treturn placeholder_len;\n \t}\n@@ -504,7 +504,7 @@ static size_t format_person_part(struct strbuf *sb, char part,\n \tif (msg + start == ep)\n \t\tgoto skip;\n \n-\tif (part == 't') {\t/* date, UNIX timestamp */\n+\tif (*part == 't') {\t/* date, UNIX timestamp */\n \t\tstrbuf_add(sb, msg + start, ep - (msg + start));\n \t\treturn placeholder_len;\n \t}\n@@ -518,7 +518,7 @@ static size_t format_person_part(struct strbuf *sb, char part,\n \t\t\ttz = -tz;\n \t}\n \n-\tswitch (part) {\n+\tswitch (*part) {\n \tcase 'd':\t/* date */\n \t\tstrbuf_addstr(sb, show_date(date, tz, dmode));\n \t\treturn placeholder_len;\n@@ -538,8 +538,8 @@ skip:\n \t * bogus commit, 'sb' cannot be updated, but we still need to\n \t * compute a valid return value.\n \t */\n-\tif (part == 'n' || part == 'e' || part == 't' || part == 'd'\n-\t || part == 'D' || part == 'r' || part == 'i')\n+\tif (*part == 'n' || *part == 'e' || *part == 't' || *part == 'd'\n+\t || *part == 'D' || *part == 'r' || *part == 'i')\n \t\treturn placeholder_len;\n \n \treturn 0; /* unknown placeholder */\n@@ -899,11 +899,11 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \n \tswitch (placeholder[0]) {\n \tcase 'a':\t/* author ... */\n-\t\treturn format_person_part(sb, placeholder[1],\n+\t\treturn format_person_part(sb, placeholder + 1,\n \t\t\t\t msg + c->author.off, c->author.len,\n \t\t\t\t c->pretty_ctx->date_mode);\n \tcase 'c':\t/* committer ... */\n-\t\treturn format_person_part(sb, placeholder[1],\n+\t\treturn format_person_part(sb, placeholder + 1,\n \t\t\t\t msg + c->committer.off, c->committer.len,\n \t\t\t\t c->pretty_ctx->date_mode);\n \tcase 'e':\t/* encoding */\n-- \n1.7.4.rc1.24.g38985d\n"},{"id":"162839","messageId":"20110305200010.GB32095@sigill.intra.peff.net","threadId":"26644","inReplyTo":"20110305195020.GA3089@sigill.intra.peff.net","subject":"[PATCH 2/2] pretty.c: allow date formats in user format strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-05T20:00:10Z","receivedAt":"2011-03-05T20:00:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"You can now do \"%ad(short)\" or similar (using any format\nthat works for --date). This makes some formats like %aD\nredundant (since you can do \"%ad(rfc)\"), but of course we\nkeep them for compatibility.\n\nWhile we're updating the docs, let's explain in more detail\nhow the placeholder mode, the --date= option, and the\nlog.date config all interact.\n\nSigned-off-by: Jeff King \n---\nMy only reservation here is the strdup() we need to call\nparse_date_format(). We usually try to keep the formatting parsing\nlightweight since it gets re-parsed for each commit.\n\nMy timings for logging all of git.git showed that the slowdown is lost\nin the noise, so it's probably not worth caring about.\n\n Documentation/pretty-formats.txt | 21 +++++++++++++++++++--\n pretty.c | 33 +++++++++++++++++++++++++++++----\n t/t6006-rev-list-format.sh | 12 ++++++++++++\n 3 files changed, 60 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 561cc9f..a73a9ac 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -109,7 +109,7 @@ The placeholders are:\n - '%aN': author name (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1])\n - '%ae': author email\n - '%aE': author email (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1])\n-- '%ad': author date (format respects --date= option)\n+- '%ad': author date (see below for format information)\n - '%aD': author date, RFC2822 style\n - '%ar': author date, relative\n - '%at': author date, UNIX timestamp\n@@ -118,7 +118,7 @@ The placeholders are:\n - '%cN': committer name (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1])\n - '%ce': committer email\n - '%cE': committer email (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1])\n-- '%cd': committer date\n+- '%cd': committer date (see below for format information)\n - '%cD': committer date, RFC2822 style\n - '%cr': committer date, relative\n - '%ct': committer date, UNIX timestamp\n@@ -151,6 +151,23 @@ insert an empty string unless we are traversing reflog entries (e.g., by\n `git log -g`). The `%d` placeholder will use the \"short\" decoration\n format if `--decorate` was not already provided on the command line.\n \n+Dates given by `%ad` and `%cd` are formatted according to the following\n+rules:\n+\n+ 1. A date mode in parentheses may follow the placeholder. For example,\n+ `%ad(iso8601)` will format the author date in the ISO8601 format.\n+ You may specify any mode valid for the `--date=` option of\n+ linkgit:git-log[1].\n+\n+ 2. If no date mode is specified, and the command respects the\n+ `--date=` option, the mode specified by that option is used.\n+\n+ 3. Otherwise, if the format is used by the log family of commands and\n+ the `log.date` config option is set, the mode specified by that\n+ option is used.\n+\n+ 4. Otherwise, the format is equivalent to that of --date=default.\n+\n If you add a `{plus}` (plus sign) after '%' of a placeholder, a line-feed\n is inserted immediately before the expansion if and only if the\n placeholder expands to a non-empty string.\ndiff --git a/pretty.c b/pretty.c\nindex 00bcf83..d0bf2a0 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -440,6 +440,27 @@ static int mailmap_name(char *email, int email_len, char *name, int name_len)\n \treturn mail_map->nr && map_user(mail_map, email, email_len, name, name_len);\n }\n \n+static size_t format_date(struct strbuf *sb, const char *part,\n+\t\t\t unsigned long date, int tz, enum date_mode dmode)\n+{\n+\tint consumed = 0;\n+\tif (*part == '(') {\n+\t\tchar *v;\n+\t\tconsumed++;\n+\t\twhile (part[consumed] && part[consumed] != ')')\n+\t\t\tconsumed++;\n+\t\t/* yuck, we do this malloc for every commit */\n+\t\tv = xstrndup(part + 1, consumed - 1);\n+\t\tdmode = parse_date_format(v);\n+\t\tfree(v);\n+\t\tif (part[consumed] == ')')\n+\t\t\tconsumed++;\n+\t}\n+\tif (sb)\n+\t\tstrbuf_addstr(sb, show_date(date, tz, dmode));\n+\treturn consumed;\n+}\n+\n static size_t format_person_part(struct strbuf *sb, const char *part,\n \t\t\t\t const char *msg, int len, enum date_mode dmode)\n {\n@@ -519,9 +540,9 @@ static size_t format_person_part(struct strbuf *sb, const char *part,\n \t}\n \n \tswitch (*part) {\n-\tcase 'd':\t/* date */\n-\t\tstrbuf_addstr(sb, show_date(date, tz, dmode));\n-\t\treturn placeholder_len;\n+\tcase 'd':\t/* date, possibly with format */\n+\t\treturn placeholder_len +\n+\t\t\tformat_date(sb, part + 1, date, tz, dmode);\n \tcase 'D':\t/* date, RFC2822 style */\n \t\tstrbuf_addstr(sb, show_date(date, tz, DATE_RFC2822));\n \t\treturn placeholder_len;\n@@ -538,9 +559,13 @@ skip:\n \t * bogus commit, 'sb' cannot be updated, but we still need to\n \t * compute a valid return value.\n \t */\n-\tif (*part == 'n' || *part == 'e' || *part == 't' || *part == 'd'\n+\tif (*part == 'n' || *part == 'e' || *part == 't'\n \t || *part == 'D' || *part == 'r' || *part == 'i')\n \t\treturn placeholder_len;\n+\t/* handle 'd' separately, as it is variable length */\n+\tif (*part == 'd')\n+\t\treturn placeholder_len +\n+\t\t\tformat_date(NULL, part + 1, 0, 0, 0);\n \n \treturn 0; /* unknown placeholder */\n }\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex d918cc0..b9cef1f 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -176,6 +176,18 @@ test_expect_success '%ad respects --date=' '\n \ttest_cmp expect.ad-short output.ad-short\n '\n \n+test_format 'date-with-mode' '%ad(short)%n%ad(iso)' <<'EOF'\n+commit f58db70b055c5718631e5c61528b28b12090cdea\n+2005-04-07\n+2005-04-07 15:13:13 -0700\n+commit 131a310eb913d107dd3c09a65d1651175898735d\n+2005-04-07\n+2005-04-07 15:13:13 -0700\n+commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873\n+2005-04-07\n+2005-04-07 15:13:13 -0700\n+EOF\n+\n test_expect_success 'empty email' '\n \ttest_tick &&\n \tC=$(GIT_AUTHOR_EMAIL= git commit-tree HEAD^{tree} wrote:\n> You can now do \"%ad(short)\" or similar (using any format\n> that works for --date). This makes some formats like %aD\n> redundant (since you can do \"%ad(rfc)\"), but of course we\n> keep them for compatibility.\n>\n\nThe more I see long formats like this, the more I think it would make\nsense to make formats %(likeThis), the way for-each-ref does.\nIdeally, these formats could even be unified, at some point.\n\nI tried this a long while ago, as part of my attempt to make all\npre-defined formats work in terms of format strings, but that turned\ninto too much of a bloated mess to bother submitting. I don't know\nif there's enough interest in such a thing to justify trying again (or to\njustify rebasing the bloated version, cleaning it up and submitting it\nas-is, for that matter)\n\nPoint is: we're going to keep having more and more format options,\nI think that's a given. At some point, these short mnemonics will just\nstop making sense, and it makes sense to have an escape plan when\nthat happens.\n\n> While we're updating the docs, let's explain in more detail\n> how the placeholder mode, the --date= option, and the\n> log.date config all interact.\n>\n> Signed-off-by: Jeff King \n> ---\n> My only reservation here is the strdup() we need to call\n> parse_date_format(). We usually try to keep the formatting parsing\n> lightweight since it gets re-parsed for each commit.\n>\n> My timings for logging all of git.git showed that the slowdown is lost\n> in the noise, so it's probably not worth caring about.\n>\n>  Documentation/pretty-formats.txt |   21 +++++++++++++++++++--\n>  pretty.c                         |   33 +++++++++++++++++++++++++++++----\n>  t/t6006-rev-list-format.sh       |   12 ++++++++++++\n>  3 files changed, 60 insertions(+), 6 deletions(-)\n>\n> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\n> index 561cc9f..a73a9ac 100644\n> --- a/Documentation/pretty-formats.txt\n> +++ b/Documentation/pretty-formats.txt\n> @@ -109,7 +109,7 @@ The placeholders are:\n>  - '%aN': author name (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1])\n>  - '%ae': author email\n>  - '%aE': author email (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1])\n> -- '%ad': author date (format respects --date= option)\n> +- '%ad': author date (see below for format information)\n>  - '%aD': author date, RFC2822 style\n>  - '%ar': author date, relative\n>  - '%at': author date, UNIX timestamp\n> @@ -118,7 +118,7 @@ The placeholders are:\n>  - '%cN': committer name (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1])\n>  - '%ce': committer email\n>  - '%cE': committer email (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1])\n> -- '%cd': committer date\n> +- '%cd': committer date (see below for format information)\n>  - '%cD': committer date, RFC2822 style\n>  - '%cr': committer date, relative\n>  - '%ct': committer date, UNIX timestamp\n> @@ -151,6 +151,23 @@ insert an empty string unless we are traversing reflog entries (e.g., by\n>  `git log -g`). The `%d` placeholder will use the \"short\" decoration\n>  format if `--decorate` was not already provided on the command line.\n>\n> +Dates given by `%ad` and `%cd` are formatted according to the following\n> +rules:\n> +\n> +  1. A date mode in parentheses may follow the placeholder. For example,\n> +     `%ad(iso8601)` will format the author date in the ISO8601 format.\n> +     You may specify any mode valid for the `--date=` option of\n> +     linkgit:git-log[1].\n> +\n> +  2. If no date mode is specified, and the command respects the\n> +     `--date=` option, the mode specified by that option is used.\n> +\n> +  3. Otherwise, if the format is used by the log family of commands and\n> +     the `log.date` config option is set, the mode specified by that\n> +     option is used.\n> +\n> +  4. Otherwise, the format is equivalent to that of --date=default.\n> +\n>  If you add a `{plus}` (plus sign) after '%' of a placeholder, a line-feed\n>  is inserted immediately before the expansion if and only if the\n>  placeholder expands to a non-empty string.\n> diff --git a/pretty.c b/pretty.c\n> index 00bcf83..d0bf2a0 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -440,6 +440,27 @@ static int mailmap_name(char *email, int email_len, char *name, int name_len)\n>        return mail_map->nr && map_user(mail_map, email, email_len, name, name_len);\n>  }\n>\n> +static size_t format_date(struct strbuf *sb, const char *part,\n> +                         unsigned long date, int tz, enum date_mode dmode)\n> +{\n> +       int consumed = 0;\n> +       if (*part == '(') {\n> +               char *v;\n> +               consumed++;\n> +               while (part[consumed] && part[consumed] != ')')\n> +                       consumed++;\n> +               /* yuck, we do this malloc for every commit */\n> +               v = xstrndup(part + 1, consumed - 1);\n> +               dmode = parse_date_format(v);\n> +               free(v);\n> +               if (part[consumed] == ')')\n> +                       consumed++;\n> +       }\n> +       if (sb)\n> +               strbuf_addstr(sb, show_date(date, tz, dmode));\n> +       return consumed;\n> +}\n> +\n>  static size_t format_person_part(struct strbuf *sb, const char *part,\n>                                 const char *msg, int len, enum date_mode dmode)\n>  {\n> @@ -519,9 +540,9 @@ static size_t format_person_part(struct strbuf *sb, const char *part,\n>        }\n>\n>        switch (*part) {\n> -       case 'd':       /* date */\n> -               strbuf_addstr(sb, show_date(date, tz, dmode));\n> -               return placeholder_len;\n> +       case 'd':       /* date, possibly with format */\n> +               return placeholder_len +\n> +                       format_date(sb, part + 1, date, tz, dmode);\n>        case 'D':       /* date, RFC2822 style */\n>                strbuf_addstr(sb, show_date(date, tz, DATE_RFC2822));\n>                return placeholder_len;\n> @@ -538,9 +559,13 @@ skip:\n>         * bogus commit, 'sb' cannot be updated, but we still need to\n>         * compute a valid return value.\n>         */\n> -       if (*part == 'n' || *part == 'e' || *part == 't' || *part == 'd'\n> +       if (*part == 'n' || *part == 'e' || *part == 't'\n>            || *part == 'D' || *part == 'r' || *part == 'i')\n>                return placeholder_len;\n> +       /* handle 'd' separately, as it is variable length */\n> +       if (*part == 'd')\n> +               return placeholder_len +\n> +                       format_date(NULL, part + 1, 0, 0, 0);\n>\n>        return 0; /* unknown placeholder */\n>  }\n> diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\n> index d918cc0..b9cef1f 100755\n> --- a/t/t6006-rev-list-format.sh\n> +++ b/t/t6006-rev-list-format.sh\n> @@ -176,6 +176,18 @@ test_expect_success '%ad respects --date=' '\n>        test_cmp expect.ad-short output.ad-short\n>  '\n>\n> +test_format 'date-with-mode' '%ad(short)%n%ad(iso)' <<'EOF'\n> +commit f58db70b055c5718631e5c61528b28b12090cdea\n> +2005-04-07\n> +2005-04-07 15:13:13 -0700\n> +commit 131a310eb913d107dd3c09a65d1651175898735d\n> +2005-04-07\n> +2005-04-07 15:13:13 -0700\n> +commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873\n> +2005-04-07\n> +2005-04-07 15:13:13 -0700\n> +EOF\n> +\n>  test_expect_success 'empty email' '\n>        test_tick &&\n>        C=$(GIT_AUTHOR_EMAIL= git commit-tree HEAD^{tree} --\n> 1.7.4.rc1.24.g38985d\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"162921","messageId":"20110307161758.GB11934@sigill.intra.peff.net","threadId":"26644","inReplyTo":"AANLkTikaN=wsg6RLFaFxh=L3RCYjKkVGFR4VTrQ=KRZk@mail.gmail.com","subject":"Re: Fwd: [PATCH 2/2] pretty.c: allow date formats in user format strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-07T16:17:58Z","receivedAt":"2011-03-07T16:17:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 06, 2011 at 09:54:01PM +0000, Will Palmer wrote:\n\n> On Sat, Mar 5, 2011 at 8:00 PM, Jeff King wrote:\n> > You can now do \"%ad(short)\" or similar (using any format\n> > that works for --date). This makes some formats like %aD\n> > redundant (since you can do \"%ad(rfc)\"), but of course we\n> > keep them for compatibility.\n> >\n> \n> The more I see long formats like this, the more I think it would make\n> sense to make formats %(likeThis), the way for-each-ref does.\n> Ideally, these formats could even be unified, at some point.\n\nYeah, I totally agree. One problem is that everytime an extended format\ncomes up it gets bikeshedded to death as everybody mentions their\nfavorite format and/or feature, and then nobody codes it.\n\n> I tried this a long while ago, as part of my attempt to make all\n> pre-defined formats work in terms of format strings, but that turned\n> into too much of a bloated mess to bother submitting. I don't know\n> if there's enough interest in such a thing to justify trying again (or to\n> justify rebasing the bloated version, cleaning it up and submitting it\n> as-is, for that matter)\n\nI think there is interest. I'd be curious to see what you have. A few\ndays ago, when working on this series, I tried to make a\nminimally-invasive change to allow \"%(ad)\" to work alongside \"%ad\", with\na generic arguments format like %(ad:flag:key=value). Which would allow\nexisting shorthand, for-each-ref-style %(refname:short), and leave room\nfor arbitrary extension of each placeholder (alongside more\nhuman-readable placeholder names).\n\nThe problem I ran into was the internal code interface. We parse the\nformat string each time we expand it. This works OK for simple\nprintf-like stuff. But ideally we can handle something like:\n\n %(ad:key=embedded\\:colon:key2=embedded\\)paren)\n\nIt's hard to make a nice interface to that which doesn't involve copying\nthe quoted string out into a non-quoted version. But we don't want to be\ndoing a bunch of parsing and allocation per-expansion. It's slow, and\nthis expansion happens inside a fairly tight loop in many cases (e.g.,\nduring rev-list).\n\nSo I think the whole thing needs to be factored into two phases: a\nparsing phase where we build some internal parse tree, and then an\nexpansion phase where we walk the parse tree for each commit (or ref, or\nwhatever is being expanded).\n\n> Point is: we're going to keep having more and more format options,\n> I think that's a given. At some point, these short mnemonics will just\n> stop making sense, and it makes sense to have an escape plan when\n> that happens.\n\nAgreed. And I think it is possible to do it in a backwards-compatible\nway; support %(longname:options) for everything, and keep short-hands\nlike %h and %ad for existing elements without options.\n\n-Peff\n"},{"id":"162925","messageId":"1299518898.3024.10.camel@wpalmer.simply-domain","threadId":"26644","inReplyTo":"20110307161758.GB11934@sigill.intra.peff.net","subject":"Re: Fwd: [PATCH 2/2] pretty.c: allow date formats in user format strings","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2011-03-07T17:28:18Z","receivedAt":"2011-03-07T17:28:18Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"On Mon, 2011-03-07 at 11:17 -0500, Jeff King wrote:\n> On Sun, Mar 06, 2011 at 09:54:01PM +0000, Will Palmer wrote:\n> \n> > On Sat, Mar 5, 2011 at 8:00 PM, Jeff King wrote:\n> > > You can now do \"%ad(short)\" or similar (using any format\n> > > that works for --date). This makes some formats like %aD\n> > > redundant (since you can do \"%ad(rfc)\"), but of course we\n> > > keep them for compatibility.\n> > >\n> > \n> > The more I see long formats like this, the more I think it would make\n> > sense to make formats %(likeThis), the way for-each-ref does.\n> > Ideally, these formats could even be unified, at some point.\n> \n> Yeah, I totally agree. One problem is that everytime an extended format\n> comes up it gets bikeshedded to death as everybody mentions their\n> favorite format and/or feature, and then nobody codes it.\n> \n> > I tried this a long while ago, as part of my attempt to make all\n> > pre-defined formats work in terms of format strings, but that turned\n> > into too much of a bloated mess to bother submitting. I don't know\n> > if there's enough interest in such a thing to justify trying again (or to\n> > justify rebasing the bloated version, cleaning it up and submitting it\n> > as-is, for that matter)\n> \n> I think there is interest. I'd be curious to see what you have. A few\n> days ago, when working on this series, I tried to make a\n> minimally-invasive change to allow \"%(ad)\" to work alongside \"%ad\", with\n> a generic arguments format like %(ad:flag:key=value). Which would allow\n> existing shorthand, for-each-ref-style %(refname:short), and leave room\n> for arbitrary extension of each placeholder (alongside more\n> human-readable placeholder names).\n> \n> The problem I ran into was the internal code interface. We parse the\n> format string each time we expand it. This works OK for simple\n> printf-like stuff. But ideally we can handle something like:\n> %(ad:key=embedded\\:colon:key2=embedded\\)paren)\n> \n> It's hard to make a nice interface to that which doesn't involve copying\n> the quoted string out into a non-quoted version. But we don't want to be\n> doing a bunch of parsing and allocation per-expansion. It's slow, and\n> this expansion happens inside a fairly tight loop in many cases (e.g.,\n> during rev-list).\n\nExactly the problem I ran into.\n\n> \n> So I think the whole thing needs to be factored into two phases: a\n> parsing phase where we build some internal parse tree, and then an\n> expansion phase where we walk the parse tree for each commit (or ref, or\n> whatever is being expanded).\n\nAnd exactly the solution I implemented.\nAt the time, it felt like needless bloat, but perhaps the problem has\ngotten to the point where it's worth it.\n\nI assume rebasing what I have right now would be problematic, but it\nsounds like it's about time to give it another go.\n\nThe code was ever only in a \"proof of concept\" stage- I had it working\nfor single revisions, but in a way which wasn't yet compatible with any\nof the other parts of log, iirc.\n\nI'll try getting a rebase started tonight, but in the mean time\nI /think/ the latest code is at \nhttps://github.com/wpalmer/git/tree/pretty/parse-format-poc\n\nWarning: quite ugly.\n\nIf you have comments, I would not mind hearing them (though off-list\nmight be better)\n\n> \n> > Point is: we're going to keep having more and more format options,\n> > I think that's a given. At some point, these short mnemonics will just\n> > stop making sense, and it makes sense to have an escape plan when\n> > that happens.\n> \n> Agreed. And I think it is possible to do it in a backwards-compatible\n> way; support %(longname:options) for everything, and keep short-hands\n> like %h and %ad for existing elements without options.\n> \n> -Peff\n"},{"id":"162929","messageId":"1299523834.1835.17.camel@walleee","threadId":"26644","inReplyTo":"1299518898.3024.10.camel@wpalmer.simply-domain","subject":"Re: Fwd: [PATCH 2/2] pretty.c: allow date formats in user format strings","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2011-03-07T18:50:34Z","receivedAt":"2011-03-07T18:50:34Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"On Mon, 2011-03-07 at 17:28 +0000, Will Palmer wrote:\n> On Mon, 2011-03-07 at 11:17 -0500, Jeff King wrote:\n> > On Sun, Mar 06, 2011 at 09:54:01PM +0000, Will Palmer wrote:\n> > \n> > > On Sat, Mar 5, 2011 at 8:00 PM, Jeff King wrote:\n> > > > You can now do \"%ad(short)\" or similar (using any format\n> > > > that works for --date). This makes some formats like %aD\n> > > > redundant (since you can do \"%ad(rfc)\"), but of course we\n> > > > keep them for compatibility.\n> > > >\n> > > \n> > > The more I see long formats like this, the more I think it would make\n> > > sense to make formats %(likeThis), the way for-each-ref does.\n> > > Ideally, these formats could even be unified, at some point.\n> > \n> > Yeah, I totally agree. One problem is that everytime an extended format\n> > comes up it gets bikeshedded to death as everybody mentions their\n> > favorite format and/or feature, and then nobody codes it.\n> > \n> > > I tried this a long while ago, as part of my attempt to make all\n> > > pre-defined formats work in terms of format strings, but that turned\n> > > into too much of a bloated mess to bother submitting. I don't know\n> > > if there's enough interest in such a thing to justify trying again (or to\n> > > justify rebasing the bloated version, cleaning it up and submitting it\n> > > as-is, for that matter)\n> > \n> > I think there is interest. I'd be curious to see what you have. A few\n> > days ago, when working on this series, I tried to make a\n> > minimally-invasive change to allow \"%(ad)\" to work alongside \"%ad\", with\n> > a generic arguments format like %(ad:flag:key=value). Which would allow\n> > existing shorthand, for-each-ref-style %(refname:short), and leave room\n> > for arbitrary extension of each placeholder (alongside more\n> > human-readable placeholder names).\n> > \n> > The problem I ran into was the internal code interface. We parse the\n> > format string each time we expand it. This works OK for simple\n> > printf-like stuff. But ideally we can handle something like:\n> > %(ad:key=embedded\\:colon:key2=embedded\\)paren)\n> > \n> > It's hard to make a nice interface to that which doesn't involve copying\n> > the quoted string out into a non-quoted version. But we don't want to be\n> > doing a bunch of parsing and allocation per-expansion. It's slow, and\n> > this expansion happens inside a fairly tight loop in many cases (e.g.,\n> > during rev-list).\n> \n> Exactly the problem I ran into.\n> \n> > \n> > So I think the whole thing needs to be factored into two phases: a\n> > parsing phase where we build some internal parse tree, and then an\n> > expansion phase where we walk the parse tree for each commit (or ref, or\n> > whatever is being expanded).\n> \n> And exactly the solution I implemented.\n> At the time, it felt like needless bloat, but perhaps the problem has\n> gotten to the point where it's worth it.\n> \n> I assume rebasing what I have right now would be problematic, but it\n> sounds like it's about time to give it another go.\n> \n> The code was ever only in a \"proof of concept\" stage- I had it working\n> for single revisions, but in a way which wasn't yet compatible with any\n> of the other parts of log, iirc.\n> \n> I'll try getting a rebase started tonight, but in the mean time\n> I /think/ the latest code is at \n> https://github.com/wpalmer/git/tree/pretty/parse-format-poc\n> \n\nI'm home now, and apparently that should have been:\nhttps://github.com/wpalmer/git/tree/pretty/parse-format\n\nI assume the code is very hard to follow, as it was pretty much written\nwith the mindset of \"get it done now, fix it later\". Looking into it\nagain, I see that part of the reason I abandoned it was not being able\nto determine a good way to split things into logical commits. It's\nalmost entirely an \"everything works or nothing works\" change.\n\nThere was of course one section of it that I managed to split out, which\nis the \"format aliases\" code, already merged. I assume that this code\nhas absolutely never been used since inclusion, as what it was actually\nintended to support was never finished.\n\nTo see it in action, try:\n ./git log --pretty='%h%(opt-color ? %Cred) foo'\n\nuncommenting the //parts_debug(parsed, 0); line in pretty.c will show\noff the built format tree.\n\n> Warning: quite ugly.\n> \n> If you have comments, I would not mind hearing them (though off-list\n> might be better)\n> \n> > \n> > > Point is: we're going to keep having more and more format options,\n> > > I think that's a given. At some point, these short mnemonics will just\n> > > stop making sense, and it makes sense to have an escape plan when\n> > > that happens.\n> > \n> > Agreed. And I think it is possible to do it in a backwards-compatible\n> > way; support %(longname:options) for everything, and keep short-hands\n> > like %h and %ad for existing elements without options.\n> > \n> > -Peff\n> \n> \n"},{"id":"162932","messageId":"20110307192640.GB20930@sigill.intra.peff.net","threadId":"26644","inReplyTo":"1299523834.1835.17.camel@walleee","subject":"Re: Fwd: [PATCH 2/2] pretty.c: allow date formats in user format strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-07T19:26:41Z","receivedAt":"2011-03-07T19:26:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 07, 2011 at 06:50:34PM +0000, Will Palmer wrote:\n\n> I'm home now, and apparently that should have been:\n> https://github.com/wpalmer/git/tree/pretty/parse-format\n> \n> I assume the code is very hard to follow, as it was pretty much written\n> with the mindset of \"get it done now, fix it later\". Looking into it\n> again, I see that part of the reason I abandoned it was not being able\n> to determine a good way to split things into logical commits. It's\n> almost entirely an \"everything works or nothing works\" change.\n\nI haven't looked at your code yet, but the breakdown of patches I would\nexpect / hope for is something like:\n\n 1. introduce infrastructure for creating parse-tree from strbuf_expand\n format, with some tests\n\n 2. port format_commit_* over to new system; I would expect that the\n caller code will have to be part of both the parsing and the\n expansion, since the generic code can't know that \"%ad\" is\n meaningful (and we want to keep it for backwards compatibility).\n Leave format_commit_message as a parse + expand wrapper for simple\n callers who don't care about speed.\n\n 3. Add generic \"%(key:option)\" support to the new infrastructure,\n forward-porting format_commit_* as necessary (and hopefully the\n change are minimal...).\n\nSo those are all big commits, obviously, but hopefully it lets us review\nin three stages: does the new infrastructure look good, does porting an\nexisting caller (and probably the most complex caller) clean up the\ncaller code, and then finally, does the new syntax look good?\n\nBut of course the devil is in the details, so probably that breakdown\nhas some flaw in it. :) I'll see when I look at your code how close to\nreality I came.\n\n-Peff\n"},{"id":"162967","messageId":"1299572985.4071.30.camel@walleee","threadId":"26644","inReplyTo":"20110307192640.GB20930@sigill.intra.peff.net","subject":"Re: Fwd: [PATCH 2/2] pretty.c: allow date formats in user format strings","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2011-03-08T08:29:45Z","receivedAt":"2011-03-08T08:29:45Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"On Mon, 2011-03-07 at 14:26 -0500, Jeff King wrote:\n> On Mon, Mar 07, 2011 at 06:50:34PM +0000, Will Palmer wrote:\n> \n> > I'm home now, and apparently that should have been:\n> > https://github.com/wpalmer/git/tree/pretty/parse-format\n> > \n> > I assume the code is very hard to follow, as it was pretty much written\n> > with the mindset of \"get it done now, fix it later\". Looking into it\n> > again, I see that part of the reason I abandoned it was not being able\n> > to determine a good way to split things into logical commits. It's\n> > almost entirely an \"everything works or nothing works\" change.\n> \n> I haven't looked at your code yet, but the breakdown of patches I would\n> expect / hope for is something like:\n> \n> 1. introduce infrastructure for creating parse-tree from strbuf_expand\n> format, with some tests\n> \n> 2. port format_commit_* over to new system; I would expect that the\n> caller code will have to be part of both the parsing and the\n> expansion, since the generic code can't know that \"%ad\" is\n> meaningful (and we want to keep it for backwards compatibility).\n> Leave format_commit_message as a parse + expand wrapper for simple\n> callers who don't care about speed.\n> \n> 3. Add generic \"%(key:option)\" support to the new infrastructure,\n> forward-porting format_commit_* as necessary (and hopefully the\n> change are minimal...).\n> \n> So those are all big commits, obviously,\n\nYeah, looks about right. It's mostly the \"those commits will still be\npretty big\" that I was concerned with. There's also the question of:\nas my end-goal is conditional formatting, should these \"smaller, but\nstill big\" commits try to make sense independently, or, for example,\nshould I lay out a basic structure in the earlier commits, filled-out\nwith a relatively simple loop, and only later expand that into the\nrecursive function / parse-tree structure; or, should I start with the\n\"fancy\" structures, even before they have a justification?\n\n - the \"simple first\" way sounds tempting, but it has the result of\n pretty much \"inventing\" in-between code which is never intended to\n actually be used. (even if it is intended to compile and work just \n fine)\n - the \"write it as it will be\", however, is going to result in commits\n which may not make any sense one after another, and really only make\n sense in the end. I don't know if that's okay.\n\nNeither of these options sound fun for bisecting, and yet it's such a\nbig change (in terms of \"everyone uses log, so every user is effected\")\nthat ease of bisectability seems like a very important consideration.\n\nWhat I don't want to do is start the patch over from scratch, with only\nthe \"long formats\"/\"unification with for-each-ref\" in mind, only to\nsubmit another patch following up later on that needs to completely\nchange the structures again to fit with the \"parse tree\" idea. Given\nthat the basic %(opt-color?...) test works, I expect that the current\nstate of the tree-structure is at least fairly close to what it should\nbe, though I also expect that someone with more experience writing\nparsers may want to slap me for the way that structure is built.\nCriticism is anticipated and appreciated.\n\n>\n> ...................................... but hopefully it lets us review\n> in three stages: does the new infrastructure look good, does porting an\n> existing caller (and probably the most complex caller) clean up the\n> caller code, and then finally, does the new syntax look good?\n> \n> But of course the devil is in the details, so probably that breakdown\n> has some flaw in it. :) I'll see when I look at your code how close to\n> reality I came.\n> \n> -Peff\n\nEr, good luck :)\nAs a side-note: It turns out that rebasing to the current \"next\" was not\ntoo difficult. The result hasn't been pushed yet (I need to do a little\nbe of forensic work to make sure a behaviour I'm seeing isn't a\nrebase-induced regression), but it does imply that at least pretty.c\nshould still make a fair amount of sense, even though it's about a year\nold. Most of the problems I expected of the rebase, it turns out would\nhave been in sections which I hadn't actually done yet.\n"},{"id":"163084","messageId":"7v39mw9f7a.fsf@alter.siamese.dyndns.org","threadId":"26644","inReplyTo":"20110307161758.GB11934@sigill.intra.peff.net","subject":"Re: Fwd: [PATCH 2/2] pretty.c: allow date formats in user format strings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-09T21:06:17Z","receivedAt":"2011-03-09T21:06:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So I think the whole thing needs to be factored into two phases: a\n> parsing phase where we build some internal parse tree, and then an\n> expansion phase where we walk the parse tree for each commit (or ref, or\n> whatever is being expanded).\n\nYou are right. I think for-each-ref expander has an attempt for\noptimization of this exact kind.\n\n>> Point is: we're going to keep having more and more format options,\n>> I think that's a given. At some point, these short mnemonics will just\n>> stop making sense, and it makes sense to have an escape plan when\n>> that happens.\n>\n> Agreed. And I think it is possible to do it in a backwards-compatible\n> way; support %(longname:options) for everything, and keep short-hands\n> like %h and %ad for existing elements without options.\n\nYes, I think %( is not taken in the pretty-format language, so we should\nbe able to do this.\n\nI wanted to take your earlier \"'%ad' or '%ad(format)'\" patch but refrained\nfrom doing so. The above line of reasoning is much better for the long\nterm health of the project.\n"},{"id":"163179","messageId":"20110310223148.GD15828@sigill.intra.peff.net","threadId":"26644","inReplyTo":"7v39mw9f7a.fsf@alter.siamese.dyndns.org","subject":"Re: Fwd: [PATCH 2/2] pretty.c: allow date formats in user format strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-10T22:31:48Z","receivedAt":"2011-03-10T22:31:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 09, 2011 at 01:06:17PM -0800, Junio C Hamano wrote:\n\n> > Agreed. And I think it is possible to do it in a backwards-compatible\n> > way; support %(longname:options) for everything, and keep short-hands\n> > like %h and %ad for existing elements without options.\n> \n> Yes, I think %( is not taken in the pretty-format language, so we should\n> be able to do this.\n> \n> I wanted to take your earlier \"'%ad' or '%ad(format)'\" patch but refrained\n> from doing so. The above line of reasoning is much better for the long\n> term health of the project.\n\nOK. Do you want me to throw away the %ad(format) patch for now, then, in\nfavor of building it on top of a more sane syntax?\n\nI had originally planned to do %ad(format) for now, and then worry about\nsyntax later. Since we already have a variety of of other placeholders\nwith similar syntax (e.g., %w(), %C()). But I don't care too much either\nway; it is not a feature I personally wanted, so delay doesn't bother\nme. Dietmar (the original requestor) may feel differently, of course. :)\n\n-Peff\n"},{"id":"163211","messageId":"4D79DE5A.9060700@gmx.de","threadId":"26644","inReplyTo":"20110310223148.GD15828@sigill.intra.peff.net","subject":"Re: Fwd: [PATCH 2/2] pretty.c: allow date formats in user format strings","fromName":"Dietmar Winkler","fromEmail":"dietmarw@gmx.de","sentAt":"2011-03-11T08:33:30Z","receivedAt":"2011-03-11T08:33:30Z","isPatch":true,"sender":{"key":"dietmarw@gmx.de","avatar":"https://gravatar.com/avatar/1ea50950a453bd8d50d8170dcebb52912082c8b2ba9c90a3e85438a14272e270?d=mp&s=160"},"body":"Den 10. mars 2011 23:31, skrev Jeff King:\n> OK. Do you want me to throw away the %ad(format) patch for now, then, in\n> favor of building it on top of a more sane syntax?\n> \n> I had originally planned to do %ad(format) for now, and then worry about\n> syntax later. Since we already have a variety of of other placeholders\n> with similar syntax (e.g., %w(), %C()). But I don't care too much either\n> way; it is not a feature I personally wanted, so delay doesn't bother\n> me. Dietmar (the original requestor) may feel differently, of course. :)\n\nAs much as I would like to have such a feature (and a documentation that\nis in synch with the implementation ;) I don't heavily rely on it. I'm\nhappy to wait a bit longer in favour of a more complete and clean\nimplementation which also opens up for other %(longname:options) support.\n-- \nDietmar.\n"}]}