{"thread":{"id":"18102","subject":"orthogonal cases of log --date option","startedAt":"2009-03-03T08:18:56Z","lastAt":"2009-03-06T12:12:58Z","messageCount":17,"participants":["Miles Bader","Jeff King","Junio C Hamano","Jay Soffian"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"106789","messageId":"buo8wnnrpcf.fsf@dhlpc061.dev.necel.com","threadId":"18102","inReplyTo":null,"subject":"orthogonal cases of log --date option","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2009-03-03T08:18:56Z","receivedAt":"2009-03-03T08:18:56Z","isPatch":false,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"I can use \"git log --date=iso\" to get YYYY-MM-DD format for dates, or\n\"git log --date=local\" to force the dates to use my local time zone, but\nif I use _both_ of these options together, it uses only the last one,\nand ignores any preceding --date (even those in this case, the two\n--date options affect orthogonal properties of dates). Is there a way\nto get YYYY-MM-DD format dates, but in my local time-zone?\n\nThanks,\n\n-Miles\n\n-- \nI'd rather be consing.\n"},{"id":"106794","messageId":"20090303083436.GD3158@coredump.intra.peff.net","threadId":"18102","inReplyTo":"buo8wnnrpcf.fsf@dhlpc061.dev.necel.com","subject":"Re: orthogonal cases of log --date option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-03T08:34:36Z","receivedAt":"2009-03-03T08:34:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 03, 2009 at 05:18:56PM +0900, Miles Bader wrote:\n\n> I can use \"git log --date=iso\" to get YYYY-MM-DD format for dates, or\n> \"git log --date=local\" to force the dates to use my local time zone, but\n> if I use _both_ of these options together, it uses only the last one,\n> and ignores any preceding --date (even those in this case, the two\n> --date options affect orthogonal properties of dates).\n\nYuck. It sounds like --date=local is really the wrong way to have\nimplemented it. It really should be:\n\n git log --date=iso --local-dates\n\nI don't think there is currently a way to do what you want, but it is\nnot too late to add an option (and keep --date=local as a synonym for\n--date=default --local-dates for compatibility).\n\n> Is there a way to get YYYY-MM-DD format dates, but in my local\n> time-zone?\n\nShort of using --date=raw and munging the output with perl, I don't\nthink so.\n\n-Peff\n"},{"id":"106796","messageId":"7vtz6bdmfi.fsf@gitster.siamese.dyndns.org","threadId":"18102","inReplyTo":"buo8wnnrpcf.fsf@dhlpc061.dev.necel.com","subject":"Re: orthogonal cases of log --date option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-03T08:45:37Z","receivedAt":"2009-03-03T08:45:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miles Bader writes:\n\n> I can use \"git log --date=iso\" to get YYYY-MM-DD format for dates, or\n> \"git log --date=local\" to force the dates to use my local time zone, but\n> if I use _both_ of these options together, it uses only the last one,\n> and ignores any preceding --date (even those in this case, the two\n> --date options affect orthogonal properties of dates). Is there a way\n> to get YYYY-MM-DD format dates, but in my local time-zone?\n\nNo, there isn't.\n\nBut this patch may help you get started.\n\nJust like any other patches I send out to only show a way to competent\npeople I can trust to carry it forward, it is not even compile tested,\nthough.\n\n---\n builtin-for-each-ref.c | 2 +-\n builtin-log.c | 2 +-\n cache.h | 15 ++++++++-------\n date.c | 27 +++++++++++++++------------\n revision.c | 2 +-\n 5 files changed, 26 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c\nindex e46b7ad..3a9f64b 100644\n--- a/builtin-for-each-ref.c\n+++ b/builtin-for-each-ref.c\n@@ -361,7 +361,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam\n \tformatp = strchr(atomname, ':');\n \tif (formatp != NULL) {\n \t\tformatp++;\n-\t\tdate_mode = parse_date_format(formatp);\n+\t\tdate_mode = parse_date_format(formatp, 0);\n \t}\n \n \tif (!eoemail)\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 2ae39af..618922a 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -41,7 +41,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \tDIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);\n \n \tif (default_date_mode)\n-\t\trev->date_mode = parse_date_format(default_date_mode);\n+\t\trev->date_mode = parse_date_format(default_date_mode, 0);\n \n \targc = setup_revisions(argc, argv, rev, \"HEAD\");\n \ndiff --git a/cache.h b/cache.h\nindex 189151d..a3ecf63 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -692,19 +692,20 @@ extern struct object *peel_to_type(const char *name, int namelen,\n \n enum date_mode {\n \tDATE_NORMAL = 0,\n-\tDATE_RELATIVE,\n-\tDATE_SHORT,\n-\tDATE_LOCAL,\n-\tDATE_ISO8601,\n-\tDATE_RFC2822,\n-\tDATE_RAW\n+\tDATE_RELATIVE = 1,\n+\tDATE_SHORT = 2,\n+\tDATE_ISO8601 = 3,\n+\tDATE_RFC2822 = 4,\n+\tDATE_RAW = 5,\n+\n+\tDATE_LOCAL = 16, /* OR'ed in to others */\n };\n \n const char *show_date(unsigned long time, int timezone, enum date_mode mode);\n int parse_date(const char *date, char *buf, int bufsize);\n void datestamp(char *buf, int bufsize);\n unsigned long approxidate(const char *);\n-enum date_mode parse_date_format(const char *format);\n+enum date_mode parse_date_format(const char *format, enum date_mode so_far);\n \n #define IDENT_WARN_ON_NO_NAME 1\n #define IDENT_ERROR_ON_NO_NAME 2\ndiff --git a/date.c b/date.c\nindex d75dff4..8d04418 100644\n--- a/date.c\n+++ b/date.c\n@@ -89,6 +89,10 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)\n \tstruct tm *tm;\n \tstatic char timebuf[200];\n \n+\tif (mode & DATE_LOCAL)\n+\t\ttz = local_tzoffset(time);\n+\tmode &= ~DATE_LOCAL;\n+\n \tif (mode == DATE_RAW) {\n \t\tsnprintf(timebuf, sizeof(timebuf), \"%lu %+05d\", time, tz);\n \t\treturn timebuf;\n@@ -136,9 +140,6 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)\n \t\t/* Else fall back on absolute format.. */\n \t}\n \n-\tif (mode == DATE_LOCAL)\n-\t\ttz = local_tzoffset(time);\n-\n \ttm = time_to_tm(time, tz);\n \tif (!tm)\n \t\treturn NULL;\n@@ -604,24 +605,26 @@ int parse_date(const char *date, char *result, int maxlen)\n \treturn date_string(then, offset, result, maxlen);\n }\n \n-enum date_mode parse_date_format(const char *format)\n+enum date_mode parse_date_format(const char *format, enum date_mode so_far)\n {\n+\tint or_in_local = so_far & DATE_LOCAL;\n+\n \tif (!strcmp(format, \"relative\"))\n-\t\treturn DATE_RELATIVE;\n+\t\treturn DATE_RELATIVE | or_in_local;\n \telse if (!strcmp(format, \"iso8601\") ||\n \t\t !strcmp(format, \"iso\"))\n-\t\treturn DATE_ISO8601;\n+\t\treturn DATE_ISO8601 | or_in_local;\n \telse if (!strcmp(format, \"rfc2822\") ||\n \t\t !strcmp(format, \"rfc\"))\n-\t\treturn DATE_RFC2822;\n+\t\treturn DATE_RFC2822 | or_in_local;\n \telse if (!strcmp(format, \"short\"))\n-\t\treturn DATE_SHORT;\n-\telse if (!strcmp(format, \"local\"))\n-\t\treturn DATE_LOCAL;\n+\t\treturn DATE_SHORT | or_in_local;\n \telse if (!strcmp(format, \"default\"))\n-\t\treturn DATE_NORMAL;\n+\t\treturn DATE_NORMAL | or_in_local;\n \telse if (!strcmp(format, \"raw\"))\n-\t\treturn DATE_RAW;\n+\t\treturn DATE_RAW | or_in_local;\n+\telse if (!strcmp(format, \"local\"))\n+\t\treturn DATE_LOCAL | so_far;\n \telse\n \t\tdie(\"unknown date format %s\", format);\n }\ndiff --git a/revision.c b/revision.c\nindex 286e416..be9bbc4 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1177,7 +1177,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--relative-date\")) {\n \t\trevs->date_mode = DATE_RELATIVE;\n \t} else if (!strncmp(arg, \"--date=\", 7)) {\n-\t\trevs->date_mode = parse_date_format(arg + 7);\n+\t\trevs->date_mode = parse_date_format(arg + 7, revs->date_mode);\n \t} else if (!strcmp(arg, \"--log-size\")) {\n \t\trevs->show_log_size = 1;\n \t}\n"},{"id":"107053","messageId":"20090305104304.GA17760@coredump.intra.peff.net","threadId":"18102","inReplyTo":"7vtz6bdmfi.fsf@gitster.siamese.dyndns.org","subject":"Re: orthogonal cases of log --date option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-05T10:43:05Z","receivedAt":"2009-03-05T10:43:05Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 03, 2009 at 12:45:37AM -0800, Junio C Hamano wrote:\n\n> Miles Bader writes:\n> \n> > I can use \"git log --date=iso\" to get YYYY-MM-DD format for dates, or\n> > \"git log --date=local\" to force the dates to use my local time zone, but\n> > if I use _both_ of these options together, it uses only the last one,\n> > and ignores any preceding --date (even those in this case, the two\n> > --date options affect orthogonal properties of dates). Is there a way\n> > to get YYYY-MM-DD format dates, but in my local time-zone?\n> \n> No, there isn't.\n> \n> But this patch may help you get started.\n\nFWIW, I think this is the wrong direction. You are working around the\nlack of orthogonality in the interface by tweaking things in the\nimplementation. I think you are better to fix the interface, but support\n--date=local for historical reasons. IOW,\n\n git log --local-dates --date=short\n\nwith\n\n git log --date=local\n\nas a historical synonym for\n\n git log --local-dates --date=default\n\nThis makes the interface simpler to understand: --date remains a\nselector, and --date=local is a special case that new people don't need\nto think about or understand.\n\n-Peff\n"},{"id":"107105","messageId":"76718490903051304j6d8138f7qa5492ac15edd6460@mail.gmail.com","threadId":"18102","inReplyTo":"20090305104304.GA17760@coredump.intra.peff.net","subject":"Re: orthogonal cases of log --date option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-05T21:04:44Z","receivedAt":"2009-03-05T21:04:44Z","isPatch":false,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Thu, Mar 5, 2009 at 5:43 AM, Jeff King wrote:\n>> But this patch may help you get started.\n>\n> FWIW, I think this is the wrong direction. You are working around the\n> lack of orthogonality in the interface by tweaking things in the\n> implementation. I think you are better to fix the interface, but support\n> --date=local for historical reasons. IOW,\n>\n>  git log --local-dates --date=short\n>\n> with\n>\n>  git log --date=local\n>\n> as a historical synonym for\n>\n>  git log --local-dates --date=default\n>\n> This makes the interface simpler to understand: --date remains a\n> selector, and --date=local is a special case that new people don't need\n> to think about or understand.\n\nI started to pick this up and I want to clarify what you meant by\ninterface. Was it the CLI you had an issue with? Because that I\nunderstand and it's easy to support the CLI changes you outline above.\n\nOr did you have a problem with how Junio was going about passing along\nboth bits (i.e. 1. date format; 2. local or not) in an enum? Because I\nhave to tell you, I started looking at what it would take to switch\nthe enum to something like:\n\nstruct date_mode {\n\tenum {\n\t\tDATE_NORMAL = 0,\n\t\tDATE_RELATIVE,\n\t\tDATE_SHORT,\n\t\tDATE_ISO8601,\n\t\tDATE_RFC2822,\n\t\tDATE_RAW\n\t} format;\n\tunsigned int local;\n};\n\nIt's a significantly more invasive change.\n\nj.\n"},{"id":"107107","messageId":"20090305211120.GB20157@coredump.intra.peff.net","threadId":"18102","inReplyTo":"76718490903051304j6d8138f7qa5492ac15edd6460@mail.gmail.com","subject":"Re: orthogonal cases of log --date option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-05T21:11:20Z","receivedAt":"2009-03-05T21:11:20Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 05, 2009 at 04:04:44PM -0500, Jay Soffian wrote:\n\n> > This makes the interface simpler to understand: --date remains a\n> > selector, and --date=local is a special case that new people don't need\n> > to think about or understand.\n> \n> I started to pick this up and I want to clarify what you meant by\n> interface. Was it the CLI you had an issue with? Because that I\n> understand and it's easy to support the CLI changes you outline above.\n\nI meant the CLI.\n\n> Or did you have a problem with how Junio was going about passing along\n> both bits (i.e. 1. date format; 2. local or not) in an enum? Because I\n> have to tell you, I started looking at what it would take to switch\n> the enum to something like:\n\nI find that a bit confusing, too, but at least it is not something users\nsee. So I don't feel as strongly about it.\n\n> struct date_mode {\n> \tenum {\n> \t\tDATE_NORMAL = 0,\n> \t\tDATE_RELATIVE,\n> \t\tDATE_SHORT,\n> \t\tDATE_ISO8601,\n> \t\tDATE_RFC2822,\n> \t\tDATE_RAW\n> \t} format;\n> \tunsigned int local;\n> };\n> \n> It's a significantly more invasive change.\n\nYep, it is more invasive. But I consider it more maintainable in the\nlong run.\n\n-Peff\n"},{"id":"107113","messageId":"7vy6vjy5js.fsf@gitster.siamese.dyndns.org","threadId":"18102","inReplyTo":"20090305211120.GB20157@coredump.intra.peff.net","subject":"Re: orthogonal cases of log --date option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-05T22:21:27Z","receivedAt":"2009-03-05T22:21:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Yep, it is more invasive. But I consider it more maintainable in the\n> long run.\n\nWhy do you think it is more confusing to ask \"--date=local --date=iso\"\nthan asking \"--local-time --date=iso\"? If the patch under discussion were\nnot mine, I would have said that --date=local that flips the \"lie about\ntimezone\" bit and tells us to use the \"default\" format is a brilliant and\nelegant solution.\n\nI honestly do not see the point of what you are proposing to make\n\"selector\" and \"format\" independent; unless you are shooting for\n\"--use-tz=Indian/Christmas --date=iso\", that is.\n\nThe \"--use-tz=zonename\" might make some sense. The required change would\nlook more like:\n\n (1) Introduce:\n\n const char *force_output_tz;\n\n that defaults to NULL;\n\n (2) Option parser for --use-tz=Indian/Christmas would store\n arg+9 to force_output_tz;\n\n (3) Option parser for --date=local would set the date format to\n \"default\", and store \"localtime\" to force_output_tz. We discard\n DATE_LOCAL enum.\n\n (4) In show_date(), instead of \n\n\tif (mode == DATE_LOCAL)\n\t\ttz = local_tzoffset(time);\n\n you'd do:\n\n\tif (forced_output_tz)\n\t\ttz = zonename_to_tzoffset(time, force_output_tz);\n\n zonename_to_tzoffset() would look up the zoneinfo database and\n compute the offset in our internal (hour*100+min) format.\n\nBut I do not think that is what you were proposing.\n"},{"id":"107128","messageId":"87bpsffmn5.fsf@catnip.gol.com","threadId":"18102","inReplyTo":"20090305104304.GA17760@coredump.intra.peff.net","subject":"Re: orthogonal cases of log --date option","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2009-03-06T01:47:10Z","receivedAt":"2009-03-06T01:47:10Z","isPatch":false,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Jeff King writes:\n> git log --local-dates --date=default\n>\n> This makes the interface simpler to understand: --date remains a\n> selector, and --date=local is a special case that new people don't need\n> to think about or understand.\n\nI agree, a separate option is more clear.\n\nI suppose (as Junio later commented) that an explicit \"--date-tz\" option\nmight be cool too, though I suppose 99% of the time, people would only\never specify \"--date-tz=local\" or \"--date-gz=gmt\"...\n\n[one use-case I can think of for a more general option would be\nif you want to see a log from somebody else's point of view ... \"when\nwas john in san francisco doing all that hacking?\"]\n\n-Miles\n\n-- \nThe trouble with most people is that they think with their hopes or\nfears or wishes rather than with their minds. -- Will Durant\n"},{"id":"107147","messageId":"20090306052318.GB3426@sigill.intra.peff.net","threadId":"18102","inReplyTo":"7vy6vjy5js.fsf@gitster.siamese.dyndns.org","subject":"Re: orthogonal cases of log --date option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-06T05:23:18Z","receivedAt":"2009-03-06T05:23:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 05, 2009 at 02:21:27PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Yep, it is more invasive. But I consider it more maintainable in the\n> > long run.\n> \n> Why do you think it is more confusing to ask \"--date=local --date=iso\"\n> than asking \"--local-time --date=iso\"? If the patch under discussion were\n> not mine, I would have said that --date=local that flips the \"lie about\n> timezone\" bit and tells us to use the \"default\" format is a brilliant and\n> elegant solution.\n\nBecause from the user's perspective --foo={bar,baz,bleep} is about\nselecting exactly one of {bar,baz,bleep}. Having --date=local is like\nhaving --pretty=abbrev-commit. Yes, it is vaguely related to date\ndisplay, but it is orthogonal to the selection of one of the mutually\nexclusive options.\n\nMaybe I am alone in viewing the options this way. But at the very least,\nthe documentation that reads:\n\n --date={relative,local,default,iso,rfc,short}\n\nshould be updated to show that _some_ options are mutually exclusive but\nothers are not.\n\n> I honestly do not see the point of what you are proposing to make\n> \"selector\" and \"format\" independent; unless you are shooting for\n> \"--use-tz=Indian/Christmas --date=iso\", that is.\n\nNo, I am not really proposing --use-tz as I have no use for it. But it\nis an example of why syntactically keeping the orthogonal concept of\n\"which timezone to use\" away from \"which date style to use\" is useful.\nThe only reason --date=local _doesn't_ look terrible is that it is a\nsimple boolean. Any date-related flags that took an argument would look\nstupid as --date=option=foo.\n\n-Peff\n"},{"id":"107153","messageId":"7vmybzw3el.fsf@gitster.siamese.dyndns.org","threadId":"18102","inReplyTo":"20090306052318.GB3426@sigill.intra.peff.net","subject":"Re: orthogonal cases of log --date option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-06T06:50:42Z","receivedAt":"2009-03-06T06:50:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Because from the user's perspective --foo={bar,baz,bleep} is about\n> selecting exactly one of {bar,baz,bleep}.\n\nI do not feel very strongly about this either way, and without any prior\nend user \"Huh?\" input, I would probably have argued like you myself, but\nI saw the original message from Miles about giving more than one --date\nand getting perplexed to see that it did not work, so...\n\nI am not likely to use --tz=Indian/Christmas myself; GMT and local might\nhowever be useful in some situations, though.\n"},{"id":"107155","messageId":"76718490903052258j277fa8e9g963deae1c3264a22@mail.gmail.com","threadId":"18102","inReplyTo":"7vmybzw3el.fsf@gitster.siamese.dyndns.org","subject":"Re: orthogonal cases of log --date option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-06T06:58:36Z","receivedAt":"2009-03-06T06:58:36Z","isPatch":false,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Mar 6, 2009 at 1:50 AM, Junio C Hamano wrote:\n> Jeff King writes:\n>\n>> Because from the user's perspective --foo={bar,baz,bleep} is about\n>> selecting exactly one of {bar,baz,bleep}.\n>\n> I do not feel very strongly about this either way, and without any prior\n> end user \"Huh?\" input, I would probably have argued like you myself, but\n> I saw the original message from Miles about giving more than one --date\n> and getting perplexed to see that it did not work, so...\n>\n> I am not likely to use --tz=Indian/Christmas myself; GMT and local might\n> however be useful in some situations, though.\n\nSo I don't mind picking this up, but I'd like some guidance. There are\ntwo issues:\n\n1) The CLI. You and Jeff don't seem to have an agreement here, but\nfrankly, this is the easy part.\n\n2) The internal implementation. Your implementation (enum -> bitfield)\nis clever, but Jeff seems to prefer what I suggested (going to a\nstruct). The latter is quite a bit more work.\n\nIf we only care about fixing the original issue, I'll just pickup your\npatch, make sure it compiles, and add some tests. I certainly don't\nwant to do more work than is needed, unless there's a good reason to\ndo so.\n\nj.\n"},{"id":"107161","messageId":"7v1vtbw03h.fsf@gitster.siamese.dyndns.org","threadId":"18102","inReplyTo":"76718490903052258j277fa8e9g963deae1c3264a22@mail.gmail.com","subject":"Re: orthogonal cases of log --date option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-06T08:02:10Z","receivedAt":"2009-03-06T08:02:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> On Fri, Mar 6, 2009 at 1:50 AM, Junio C Hamano wrote:\n>> Jeff King writes:\n>>\n>>> Because from the user's perspective --foo={bar,baz,bleep} is about\n>>> selecting exactly one of {bar,baz,bleep}.\n>>\n>> I do not feel very strongly about this either way, and without any prior\n>> end user \"Huh?\" input, I would probably have argued like you myself, but\n>> I saw the original message from Miles about giving more than one --date\n>> and getting perplexed to see that it did not work, so...\n>>\n>> I am not likely to use --tz=Indian/Christmas myself; GMT and local might\n>> however be useful in some situations, though.\n>\n> So I don't mind picking this up, but I'd like some guidance. There are\n> two issues:\n>\n> 1) The CLI. You and Jeff don't seem to have an agreement here, but\n> frankly, this is the easy part.\n>\n> 2) The internal implementation. Your implementation (enum -> bitfield)\n> is clever, but Jeff seems to prefer what I suggested (going to a\n> struct). The latter is quite a bit more work.\n\nIs it? Isn't it just the matter of doing something like this?\n\n\tstruct date_mode {\n \tenum {\n DATE_NORMAL = 0,\n DATE_RELATIVE,\n ...\n DATE_RAW\n } format;\n enum {\n DATE_ORIGINAL = 0,\n DATE_LOCAL\n /* perhaps \",DATE_GMT\" later... */\n } tz_offset;\n\t};\n\n\t/* In revision.c::handle_revision_opt() */\n ...\n\t} else if (!strcmp(arg, \"--date=local\")) {\n\t\trevs->date_mode.format = DATE_NORMAL;\n \trevs->date_mode.tz_offset = DATE_LOCAL;\n\t} else if (!prefixcmp(arg, \"--date=\")) {\n \trevs->date_mode.format = parse_date_format(arg + 7);\n\t} else if (!strcmp(arg, \"--tz=local\")) {\n \trevs->date_mode.tz_offset = DATE_LOCAL;\n\t}\n\t...\n\n /* In date.c::show_date() */\n\t...\n const char *show_date(unsigned long time, int tz, struct date_mode *mode_)\n\t{\n \tint mode = mode_->format;\n\n\t\tif (mode_->tz_offset == DATE_LOCAL)\n\t\t\ttz = local_tzoffset(time);\n\n\t\t...\n\t\t/* and remove the existing\n if (mode == DATE_LOCAL)\n \ttz = local_tzoffset(time);\n\t\t that appears later in the code\n\t\t*/\n\t...\n"},{"id":"107167","messageId":"76718490903060031u2bbc5c08k6a635dcdb1c8bc32@mail.gmail.com","threadId":"18102","inReplyTo":"7v1vtbw03h.fsf@gitster.siamese.dyndns.org","subject":"Re: orthogonal cases of log --date option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-06T08:31:55Z","receivedAt":"2009-03-06T08:31:55Z","isPatch":false,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Mar 6, 2009 at 3:02 AM, Junio C Hamano wrote:\n>\n> Is it?  Isn't it just the matter of doing something like this?\n>\n>        struct date_mode {\n>                enum {\n>                DATE_NORMAL = 0,\n>                DATE_RELATIVE,\n>                ...\n>                DATE_RAW\n>                } format;\n>                enum {\n>                DATE_ORIGINAL = 0,\n>                DATE_LOCAL\n>                /* perhaps \",DATE_GMT\" later... */\n>                } tz_offset;\n>        };\n>\n>        /* In revision.c::handle_revision_opt() */\n>        ...\n>        } else if (!strcmp(arg, \"--date=local\")) {\n>                revs->date_mode.format = DATE_NORMAL;\n>                revs->date_mode.tz_offset = DATE_LOCAL;\n>        } else if (!prefixcmp(arg, \"--date=\")) {\n>                revs->date_mode.format = parse_date_format(arg + 7);\n>        } else if (!strcmp(arg, \"--tz=local\")) {\n>                revs->date_mode.tz_offset = DATE_LOCAL;\n>        }\n>        ...\n>\n>        /* In date.c::show_date() */\n>        ...\n>        const char *show_date(unsigned long time, int tz, struct date_mode *mode_)\n>        {\n>                int mode = mode_->format;\n>\n>                if (mode_->tz_offset == DATE_LOCAL)\n>                        tz = local_tzoffset(time);\n>\n>                ...\n>                /* and remove the existing\n>                if (mode == DATE_LOCAL)\n>                        tz = local_tzoffset(time);\n>                   that appears later in the code\n>                */\n>        ...\n\n\nYeah, that part is easy. I wasn't sure the best way to handle places\nwhere a constant date_mode is used e.g.:\n\npp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,\n\t\t encoding);\n\nI had started on:\n\nstatic const struct date_mode date_rfc2822 = {DATE_RFC2822, DATE_ORIGINAL};\n...\npp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, &date_rfc2822,\n\t\t encoding);\n\nBut I imagine there's maybe a better way to do that.\n\nj.\n"},{"id":"107169","messageId":"7vk573t4cm.fsf@gitster.siamese.dyndns.org","threadId":"18102","inReplyTo":"76718490903060031u2bbc5c08k6a635dcdb1c8bc32@mail.gmail.com","subject":"Re: orthogonal cases of log --date option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-06T08:58:33Z","receivedAt":"2009-03-06T08:58:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> Yeah, that part is easy. I wasn't sure the best way to handle places\n> where a constant date_mode is used e.g.:\n>\n> pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,\n> \t\t encoding);\n\nOne approach that will be hated by libgit2 effort would be to keep the\ndate_mode an enum as before (sans DATE_LOCAL) and make the \"tz_offset\"\nthing as a setting global to the process, defined in environ.c\n"},{"id":"107191","messageId":"20090306120916.GA3437@sigill.intra.peff.net","threadId":"18102","inReplyTo":"7vmybzw3el.fsf@gitster.siamese.dyndns.org","subject":"Re: orthogonal cases of log --date option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-06T12:09:16Z","receivedAt":"2009-03-06T12:09:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 05, 2009 at 10:50:42PM -0800, Junio C Hamano wrote:\n\n> > Because from the user's perspective --foo={bar,baz,bleep} is about\n> > selecting exactly one of {bar,baz,bleep}.\n> \n> I do not feel very strongly about this either way, and without any prior\n> end user \"Huh?\" input, I would probably have argued like you myself, but\n> I saw the original message from Miles about giving more than one --date\n> and getting perplexed to see that it did not work, so...\n\nInteresting. I saw the original message from Miles as \"I tried to use\nmore than one --date, but that seems wrong to me because these concepts\nare orthogonal\". But I do recognize that it is somewhat a matter of\nperception and style.\n\nAnyway, as our one user in this thread, he has said the separate option\nis more clear. So I think we should go with that.\n\n-Peff\n"},{"id":"107192","messageId":"20090306121058.GB3437@sigill.intra.peff.net","threadId":"18102","inReplyTo":"76718490903052258j277fa8e9g963deae1c3264a22@mail.gmail.com","subject":"Re: orthogonal cases of log --date option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-06T12:10:58Z","receivedAt":"2009-03-06T12:10:58Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 06, 2009 at 01:58:36AM -0500, Jay Soffian wrote:\n\n> 1) The CLI. You and Jeff don't seem to have an agreement here, but\n> frankly, this is the easy part.\n> \n> 2) The internal implementation. Your implementation (enum -> bitfield)\n> is clever, but Jeff seems to prefer what I suggested (going to a\n> struct). The latter is quite a bit more work.\n\nMy argument for (2), btw, is that the code is easier to read and more\nmaintainable if it follows the structure of the CLI. But I don't think\nit's as important as getting the CLI right.\n\n-Peff\n"},{"id":"107193","messageId":"20090306121258.GC3437@sigill.intra.peff.net","threadId":"18102","inReplyTo":"7vk573t4cm.fsf@gitster.siamese.dyndns.org","subject":"Re: orthogonal cases of log --date option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-06T12:12:58Z","receivedAt":"2009-03-06T12:12:58Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 06, 2009 at 12:58:33AM -0800, Junio C Hamano wrote:\n\n> Jay Soffian writes:\n> \n> > Yeah, that part is easy. I wasn't sure the best way to handle places\n> > where a constant date_mode is used e.g.:\n> >\n> > pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,\n> > \t\t encoding);\n> \n> One approach that will be hated by libgit2 effort would be to keep the\n> date_mode an enum as before (sans DATE_LOCAL) and make the \"tz_offset\"\n> thing as a setting global to the process, defined in environ.c\n\nI haven't looked, but might it not be possible to put it in the rev_info\nstruct next to the date format, but not as part of a struct? You will\nthen sometimes have to pass two options around (the format and the tz\noffset) instead of one (the struct with both), but it makes split usage\nmuch easier:\n\n pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,\n revs->tz_offset, encoding);\n\n-Peff\n"}]}