* [PATCH 0/2] Use %as and %cs as pretty format flags @ 2008-08-28 11:09 Nathan Panike 2008-08-28 23:15 ` Jeff King 0 siblings, 1 reply; 9+ messages in thread From: Nathan Panike @ 2008-08-28 11:09 UTC (permalink / raw) To: git The patch series below allows git to use %as and %cs as flags for pretty-printing the format of a date. Nathan W. Panike (2): Document %as and %cs as a pretty format flag Give %as and %cs meaning for pretty format Documentation/pretty-formats.txt | 2 ++ pretty.c | 3 +++ 2 files changed, 5 insertions(+), 0 deletions(-) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use %as and %cs as pretty format flags 2008-08-28 11:09 [PATCH 0/2] Use %as and %cs as pretty format flags Nathan Panike @ 2008-08-28 23:15 ` Jeff King 2008-08-28 23:36 ` Junio C Hamano 0 siblings, 1 reply; 9+ messages in thread From: Jeff King @ 2008-08-28 23:15 UTC (permalink / raw) To: Nathan Panike; +Cc: Junio C Hamano, git On Thu, Aug 28, 2008 at 06:09:37AM -0500, Nathan Panike wrote: > The patch series below allows git to use %as and %cs as flags for > pretty-printing the format of a date. Your description leaves a little to be desired (here and in the patches themselves). I had to read the patch to figure out that these are formatting specifiers for the date format "short". That being said, I think this is probably reasonable just for the sake of completeness (and I doubt we are wasting a useful formatting combo, since %a* is likely to remain dedicated to author information). I wonder if there should be "%al" for "local". However, it makes me wonder even more if '%ad' should simply respect the --date= parameter (this wouldn't allow you to mix and match dates in a single format, but I don't think that is what is desired). Or whether we should have some syntax for "%ad(short)" or something, where the argument would be handed off to the date format parser. But that is probably overengineering. -Peff PS Junio, I think this will make your "git one" alias much shorter. :) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use %as and %cs as pretty format flags 2008-08-28 23:15 ` Jeff King @ 2008-08-28 23:36 ` Junio C Hamano 2008-08-28 23:54 ` Jeff King 2008-08-29 3:43 ` Avery Pennarun 0 siblings, 2 replies; 9+ messages in thread From: Junio C Hamano @ 2008-08-28 23:36 UTC (permalink / raw) To: Jeff King; +Cc: Nathan Panike, git Jeff King <peff@peff.net> writes: > On Thu, Aug 28, 2008 at 06:09:37AM -0500, Nathan Panike wrote: > >> The patch series below allows git to use %as and %cs as flags for >> pretty-printing the format of a date. > > Your description leaves a little to be desired (here and in the patches > themselves). I had to read the patch to figure out that these are > formatting specifiers for the date format "short". > > That being said, I think this is probably reasonable just for the sake > of completeness (and I doubt we are wasting a useful formatting combo, > since %a* is likely to remain dedicated to author information). I wonder > if there should be "%al" for "local". > > However, it makes me wonder even more if '%ad' should simply respect the > --date= parameter (this wouldn't allow you to mix and match dates in a > single format, but I don't think that is what is desired). Or whether we > should have some syntax for "%ad(short)" or something, where the > argument would be handed off to the date format parser. But that is > probably overengineering. I was actually thinking about rejecting this, asking for something that allows to express all the other %[ai][dDri] format can express, and perhaps more. So I think "%ad(short)" is a good direction to go, except that 'd' is already taken. Perhaps %a(date), %a(shortdate,local),...? Oh, and before anybody asks, even if we do %a(specifier), you can keep writing "%ad" if you are used to it. I am not talking about deprecating the existing ones, but making future extensions easier without forcing people to remember cryptic one-letter format specifiers. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use %as and %cs as pretty format flags 2008-08-28 23:36 ` Junio C Hamano @ 2008-08-28 23:54 ` Jeff King 2008-08-29 0:10 ` Nathan W. Panike 2008-08-29 8:12 ` Jakub Narebski 2008-08-29 3:43 ` Avery Pennarun 1 sibling, 2 replies; 9+ messages in thread From: Jeff King @ 2008-08-28 23:54 UTC (permalink / raw) To: Junio C Hamano; +Cc: Nathan Panike, git On Thu, Aug 28, 2008 at 04:36:51PM -0700, Junio C Hamano wrote: > I was actually thinking about rejecting this, asking for something that > allows to express all the other %[ai][dDri] format can express, and > perhaps more. So I think "%ad(short)" is a good direction to go, except > that 'd' is already taken. Perhaps %a(date), %a(shortdate,local),...? I was thinking we could accept %ad _or_ %ad(short), but of course introducing the latter can break existing "%ad(my other random text)" which is a bad idea. I really think some consideration should be given to introducing arbitrary "arguments" to formatting specifiers, of which this is one example. Another that has been mentioned is pulling an arbitrary element from a list. How do you feel about a brand new syntax (and supporting the old, of course) that is syntactically a little easier to extend. Like: %(macro, key=val, key=val) e.g. %(authordate, format=short, tz=local) where the syntax can be easily parsed without understanding what "authordate" means. Jakub already suggested something akin to RPM's macro expansion, though I haven't looked too closely at it. > Oh, and before anybody asks, even if we do %a(specifier), you can keep > writing "%ad" if you are used to it. I am not talking about deprecating > the existing ones, but making future extensions easier without forcing > people to remember cryptic one-letter format specifiers. Yes, I don't see a need to get rid of the existing ones. Introducing a new syntax does have the possibility of breaking existing scripts, since we just leave unrecognized expansions in place (in fact, just adding %as has the potential to break scripts!). Worst case, we can introduce a --pretty=otherformat:, but I don't know if people are really expecting "%a(blah)" to be left untouched currently. I don't think we ever claimed that % was magical. -Peff ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use %as and %cs as pretty format flags 2008-08-28 23:54 ` Jeff King @ 2008-08-29 0:10 ` Nathan W. Panike 2008-08-29 0:54 ` Jeff King 2008-08-29 8:12 ` Jakub Narebski 1 sibling, 1 reply; 9+ messages in thread From: Nathan W. Panike @ 2008-08-29 0:10 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, git On Thu, Aug 28, 2008 at 6:54 PM, Jeff King <peff@peff.net> wrote: > On Thu, Aug 28, 2008 at 04:36:51PM -0700, Junio C Hamano wrote: > >> I was actually thinking about rejecting this, asking for something that >> allows to express all the other %[ai][dDri] format can express, and >> perhaps more. So I think "%ad(short)" is a good direction to go, except >> that 'd' is already taken. Perhaps %a(date), %a(shortdate,local),...? > > I was thinking we could accept %ad _or_ %ad(short), but of course > introducing the latter can break existing "%ad(my other random text)" > which is a bad idea. > > I really think some consideration should be given to introducing > arbitrary "arguments" to formatting specifiers, of which this is one > example. Another that has been mentioned is pulling an arbitrary element > from a list. > > How do you feel about a brand new syntax (and supporting the old, of > course) that is syntactically a little easier to extend. Like: > > %(macro, key=val, key=val) > > e.g. > > %(authordate, format=short, tz=local) The genesis of this patch was the documentation says that %ad represents 'author date' without any more specific information. I thought that I could do git show --date=short --pretty=format:"%ad %h" and get the date in short form---but alas, this did not work. So I wrote up the patch to get a short date format. Your ideas are much more powerful. I like them. Thanks for git. Nathan Panike ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use %as and %cs as pretty format flags 2008-08-29 0:10 ` Nathan W. Panike @ 2008-08-29 0:54 ` Jeff King 2008-08-29 1:08 ` Junio C Hamano 0 siblings, 1 reply; 9+ messages in thread From: Jeff King @ 2008-08-29 0:54 UTC (permalink / raw) To: Nathan W. Panike; +Cc: Junio C Hamano, git On Thu, Aug 28, 2008 at 07:10:45PM -0500, Nathan W. Panike wrote: > The genesis of this patch was the documentation says that %ad > represents 'author date' without any more specific information. I > thought that I could do > > git show --date=short --pretty=format:"%ad %h" > > and get the date in short form---but alas, this did not work. So I Regardless of expansions to --pretty=format:, that is something we should probably be doing _anyway_. The current behavior is just confusing, and %ad is simply documented as "date", not in any particular format (and I think anyone relying on it _ignoring_ --date= is insane, since that --date is otherwise not doing anything). So how about this? -- >8 -- pretty=format: respect date format options When running a command like: git log --pretty=format:%ad --date=short the date option was ignored. This patch causes it to use whatever format was specified by --date (or by --relative-date, etc), just as the non-user formats would do. Signed-off-by: Jeff King <peff@peff.net> --- Documentation/pretty-formats.txt | 2 +- archive.c | 2 +- builtin-commit.c | 2 +- commit.h | 3 ++- pretty.c | 17 +++++++++++------ t/t6006-rev-list-format.sh | 6 ++++++ 6 files changed, 22 insertions(+), 10 deletions(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index c11d495..388d492 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -103,7 +103,7 @@ The placeholders are: - '%an': author name - '%aN': author name (respecting .mailmap) - '%ae': author email -- '%ad': author date +- '%ad': author date (format respects --date= option) - '%aD': author date, RFC2822 style - '%ar': author date, relative - '%at': author date, UNIX timestamp diff --git a/archive.c b/archive.c index 5b40e26..e2280df 100644 --- a/archive.c +++ b/archive.c @@ -48,7 +48,7 @@ static void format_subst(const struct commit *commit, strbuf_add(&fmt, b + 8, c - b - 8); strbuf_add(buf, src, b - src); - format_commit_message(commit, fmt.buf, buf); + format_commit_message(commit, fmt.buf, buf, DATE_NORMAL); len -= c + 1 - src; src = c + 1; } diff --git a/builtin-commit.c b/builtin-commit.c index 649c8be..c870037 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -882,7 +882,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1) if (!log_tree_commit(&rev, commit)) { struct strbuf buf = STRBUF_INIT; - format_commit_message(commit, "%h: %s", &buf); + format_commit_message(commit, "%h: %s", &buf, DATE_NORMAL); printf("%s\n", buf.buf); strbuf_release(&buf); } diff --git a/commit.h b/commit.h index d163c74..de15f4d 100644 --- a/commit.h +++ b/commit.h @@ -67,7 +67,8 @@ extern int non_ascii(int); struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */ extern void get_commit_format(const char *arg, struct rev_info *); extern void format_commit_message(const struct commit *commit, - const void *format, struct strbuf *sb); + const void *format, struct strbuf *sb, + enum date_mode dmode); extern void pretty_print_commit(enum cmit_fmt fmt, const struct commit*, struct strbuf *, int abbrev, const char *subject, diff --git a/pretty.c b/pretty.c index 33ef34a..a29c290 100644 --- a/pretty.c +++ b/pretty.c @@ -310,7 +310,7 @@ static int mailmap_name(struct strbuf *sb, const char *email) } static size_t format_person_part(struct strbuf *sb, char part, - const char *msg, int len) + const char *msg, int len, enum date_mode dmode) { /* currently all placeholders have same length */ const int placeholder_len = 2; @@ -377,7 +377,7 @@ static size_t format_person_part(struct strbuf *sb, char part, switch (part) { case 'd': /* date */ - strbuf_addstr(sb, show_date(date, tz, DATE_NORMAL)); + strbuf_addstr(sb, show_date(date, tz, dmode)); return placeholder_len; case 'D': /* date, RFC2822 style */ strbuf_addstr(sb, show_date(date, tz, DATE_RFC2822)); @@ -409,6 +409,7 @@ struct chunk { struct format_commit_context { const struct commit *commit; + enum date_mode dmode; /* These offsets are relative to the start of the commit message. */ int commit_header_parsed; @@ -584,10 +585,12 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder, return 1; case 'a': /* author ... */ return format_person_part(sb, placeholder[1], - msg + c->author.off, c->author.len); + msg + c->author.off, c->author.len, + c->dmode); case 'c': /* committer ... */ return format_person_part(sb, placeholder[1], - msg + c->committer.off, c->committer.len); + msg + c->committer.off, c->committer.len, + c->dmode); case 'e': /* encoding */ strbuf_add(sb, msg + c->encoding.off, c->encoding.len); return 1; @@ -599,12 +602,14 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder, } void format_commit_message(const struct commit *commit, - const void *format, struct strbuf *sb) + const void *format, struct strbuf *sb, + enum date_mode dmode) { struct format_commit_context context; memset(&context, 0, sizeof(context)); context.commit = commit; + context.dmode = dmode; strbuf_expand(sb, format, format_commit_item, &context); } @@ -770,7 +775,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit, const char *encoding; if (fmt == CMIT_FMT_USERFORMAT) { - format_commit_message(commit, user_format, sb); + format_commit_message(commit, user_format, sb, dmode); return; } diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh index 9176484..485ad4d 100755 --- a/t/t6006-rev-list-format.sh +++ b/t/t6006-rev-list-format.sh @@ -139,6 +139,12 @@ commit 131a310eb913d107dd3c09a65d1651175898735d commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873 EOF +test_expect_success '%ad respects --date=' ' + echo 2005-04-07 >expect.ad-short && + git log -1 --date=short --pretty=tformat:%ad >output.ad-short master && + test_cmp expect.ad-short output.ad-short +' + test_expect_success 'empty email' ' test_tick && C=$(GIT_AUTHOR_EMAIL= git commit-tree HEAD^{tree} </dev/null) && -- 1.6.0.1.173.g2bf39.dirty ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use %as and %cs as pretty format flags 2008-08-29 0:54 ` Jeff King @ 2008-08-29 1:08 ` Junio C Hamano 0 siblings, 0 replies; 9+ messages in thread From: Junio C Hamano @ 2008-08-29 1:08 UTC (permalink / raw) To: Jeff King; +Cc: Nathan W. Panike, git Jeff King <peff@peff.net> writes: > On Thu, Aug 28, 2008 at 07:10:45PM -0500, Nathan W. Panike wrote: > >> The genesis of this patch was the documentation says that %ad >> represents 'author date' without any more specific information. I >> thought that I could do >> >> git show --date=short --pretty=format:"%ad %h" >> >> and get the date in short form---but alas, this did not work. So I > > Regardless of expansions to --pretty=format:, that is something we > should probably be doing _anyway_. The current behavior is just > confusing, and %ad is simply documented as "date", not in any particular > format (and I think anyone relying on it _ignoring_ --date= is insane, > since that --date is otherwise not doing anything). > > So how about this? > > -- >8 -- > pretty=format: respect date format options > > When running a command like: > > git log --pretty=format:%ad --date=short > > the date option was ignored. This patch causes it to use > whatever format was specified by --date (or by > --relative-date, etc), just as the non-user formats would > do. > > Signed-off-by: Jeff King <peff@peff.net> Like the idea; haven't looked at the patch yet, though. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use %as and %cs as pretty format flags 2008-08-28 23:54 ` Jeff King 2008-08-29 0:10 ` Nathan W. Panike @ 2008-08-29 8:12 ` Jakub Narebski 1 sibling, 0 replies; 9+ messages in thread From: Jakub Narebski @ 2008-08-29 8:12 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Nathan Panike, git Jeff King <peff@peff.net> writes: > On Thu, Aug 28, 2008 at 04:36:51PM -0700, Junio C Hamano wrote: > > > I was actually thinking about rejecting this, asking for something that > > allows to express all the other %[ai][dDri] format can express, and > > perhaps more. So I think "%ad(short)" is a good direction to go, except > > that 'd' is already taken. Perhaps %a(date), %a(shortdate,local),...? > > I was thinking we could accept %ad _or_ %ad(short), but of course > introducing the latter can break existing "%ad(my other random text)" > which is a bad idea. > > I really think some consideration should be given to introducing > arbitrary "arguments" to formatting specifiers, of which this is one > example. Another that has been mentioned is pulling an arbitrary element > from a list. > > How do you feel about a brand new syntax (and supporting the old, of > course) that is syntactically a little easier to extend. Like: > > %(macro, key=val, key=val) > > e.g. > > %(authordate, format=short, tz=local) > > where the syntax can be easily parsed without understanding what > "authordate" means. Jakub already suggested something akin to RPM's > macro expansion, though I haven't looked too closely at it. I'd rather we do not introduce yet another pretty-printing / formatting syntax, and use the same syntax as git-for-each-ref uses for extra formatting, As a special case for the date-type fields, you may specify a format for the date by adding one of `:default`, `:relative`, `:short`, `:local`, `:iso8601` or `:rfc2822` to the end of the fieldname; e.g. `%(taggerdate:relative)`. which incindentally (or not) is the same as rpm uses Alternate output formats may be requested by following the tag with :type- tag. Currently, the following types are supported: :armor Wrap a public key in ASCII armor. :base64 Encode binary data using base64. :date Use strftime(3) "%c" format. :day Use strftime(3) "%a %b %d %Y" format. :depflags Format dependency flags. :fflags Format file flags. :hex Format in hexadecimal. :octal Format in octal. :perms Format file permissions. :shescape Escape single quotes for use in a script. :triggertype Display trigger suffix. P.S. I Agree with %ad and the like respecting --date=<type> -- Jakub Narebski Poland ShadeHawk on #git ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use %as and %cs as pretty format flags 2008-08-28 23:36 ` Junio C Hamano 2008-08-28 23:54 ` Jeff King @ 2008-08-29 3:43 ` Avery Pennarun 1 sibling, 0 replies; 9+ messages in thread From: Avery Pennarun @ 2008-08-29 3:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, Nathan Panike, git On Thu, Aug 28, 2008 at 7:36 PM, Junio C Hamano <gitster@pobox.com> wrote: > I was actually thinking about rejecting this, asking for something that > allows to express all the other %[ai][dDri] format can express, and > perhaps more. So I think "%ad(short)" is a good direction to go, except > that 'd' is already taken. Perhaps %a(date), %a(shortdate,local),...? These format strings are getting increasingly complicated without really gaining much in generality. Why not "%ad(strftime-compatible-format-string)" ? You could differentiate localtime vs. gmtime with %ad vs. %aD, or something like that. Have fun, Avery ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2008-08-29 8:13 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-08-28 11:09 [PATCH 0/2] Use %as and %cs as pretty format flags Nathan Panike 2008-08-28 23:15 ` Jeff King 2008-08-28 23:36 ` Junio C Hamano 2008-08-28 23:54 ` Jeff King 2008-08-29 0:10 ` Nathan W. Panike 2008-08-29 0:54 ` Jeff King 2008-08-29 1:08 ` Junio C Hamano 2008-08-29 8:12 ` Jakub Narebski 2008-08-29 3:43 ` Avery Pennarun
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).