{"thread":{"id":"30352","subject":"Bug in git-stash(.sh) ?","startedAt":"2012-04-27T22:57:36Z","lastAt":"2012-05-10T17:35:52Z","messageCount":26,"participants":["Eli Barzilay","Junio C Hamano","Andreas Schwab","Yann Hodique","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"190227","messageId":"20379.9312.943088.350379@winooski.ccs.neu.edu","threadId":"30352","inReplyTo":null,"subject":"Bug in git-stash(.sh) ?","fromName":"Eli Barzilay","fromEmail":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","sentAt":"2012-04-27T22:57:36Z","receivedAt":"2012-04-27T22:57:36Z","isPatch":false,"sender":{"key":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","avatar":null},"body":"[Note: cross-posted to the magit list to see if anyone else has this\nproblem.]\n\nFor a while now I had a problem when I try to do stash operations via\nmagit -- for example, it shows this in the process buffer:\n\n $ git --no-pager stash apply stash@{2012-04-27 08:53:30 -0400}\n Too many revisions specified: stash@{2012-04-27 08:53:30 -0400}\n\nI tracked this down to this part of the script:\n\n\tREV=$(git rev-parse --no-flags --symbolic \"$@\") || exit 1\n\t...\n\tset -- $REV\n\nwhere $REV has one symbolic name but the name has spaces in it. (This\nwas introduced two years ago, in ef76312.)\n\nRemoving the --symbolic flag could solve this but it looks like it's\nneeded for error reporting. Instead, I tweaked IFS so it's split\ncorrectly and added some quotations later in the script where $1 and\n$REV are used without quotes. (I also moved the \"REV=...\" line next\nto the \"set -- $REV\", since the chunk of code between them isn't using\n$REV.)\n\nThe following is the diff -- if it looks right I can send a properly\nformatted patch.\n\n\n-------------------------------------------------------------------------------\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 4e2c7f8..10a264b 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -33,6 +33,8 @@ else\n reset_color=\n fi\n \n+NEWLINE=\"\n+\"\n no_changes () {\n \tgit diff-index --quiet --cached HEAD --ignore-submodules -- &&\n \tgit diff-files --quiet --ignore-submodules &&\n@@ -327,8 +329,6 @@ parse_flags_and_rev()\n \ti_tree=\n \tu_tree=\n \n-\tREV=$(git rev-parse --no-flags --symbolic \"$@\") || exit 1\n-\n \tFLAGS=\n \tfor opt\n \tdo\n@@ -345,7 +345,9 @@ parse_flags_and_rev()\n \t\tesac\n \tdone\n \n-\tset -- $REV\n+\tREV=$(git rev-parse --no-flags --symbolic \"$@\") || exit 1\n+\n+\tOIFS=\"$IFS\"; IFS=\"$NEWLINE\"; set -- $REV; IFS=\"$OIFS\"\n \n \tcase $# in\n \t\t0)\n@@ -360,13 +362,13 @@ parse_flags_and_rev()\n \t\t;;\n \tesac\n \n-\tREV=$(git rev-parse --quiet --symbolic --verify $1 2>/dev/null) || {\n+\tREV=$(git rev-parse --quiet --symbolic --verify \"$1\" 2>/dev/null) || {\n \t\treference=\"$1\"\n \t\tdie \"$(eval_gettext \"\\$reference is not valid reference\")\"\n \t}\n \n-\ti_commit=$(git rev-parse --quiet --verify $REV^2 2>/dev/null) &&\n-\tset -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2: 2>/dev/null) &&\n+\ti_commit=$(git rev-parse --quiet --verify \"$REV^2\" 2>/dev/null) &&\n+\tset -- $(git rev-parse \"$REV\" \"$REV^1\" \"$REV:\" \"$REV^1:\" \"$REV^2:\" 2>/dev/null) &&\n \ts=$1 &&\n \tw_commit=$1 &&\n \tb_commit=$2 &&\n@@ -377,8 +379,8 @@ parse_flags_and_rev()\n \ttest \"$ref_stash\" = \"$(git rev-parse --symbolic-full-name \"${REV%@*}\")\" &&\n \tIS_STASH_REF=t\n \n-\tu_commit=$(git rev-parse --quiet --verify $REV^3 2>/dev/null) &&\n-\tu_tree=$(git rev-parse $REV^3: 2>/dev/null)\n+\tu_commit=$(git rev-parse --quiet --verify \"$REV^3\" 2>/dev/null) &&\n+\tu_tree=$(git rev-parse \"$REV^3:\" 2>/dev/null)\n }\n \n is_stash_like()\n-------------------------------------------------------------------------------\n\n-- \n ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"},{"id":"190229","messageId":"xmqqvckk93ta.fsf@junio.mtv.corp.google.com","threadId":"30352","inReplyTo":"20379.9312.943088.350379@winooski.ccs.neu.edu","subject":"Re: Bug in git-stash(.sh) ?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-27T23:02:09Z","receivedAt":"2012-04-27T23:02:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eli Barzilay writes:\n\n> For a while now I had a problem when I try to do stash operations via\n> magit -- for example, it shows this in the process buffer:\n>\n> $ git --no-pager stash apply stash@{2012-04-27 08:53:30 -0400}\n> Too many revisions specified: stash@{2012-04-27 08:53:30 -0400}\n\nNot surprised; as far as I understand, ever since the original design,\nthe stash entries are meant to be _counted_, i.e. stash@{0}, stash@{1},\nstash@{2}, ... and never timed.\n\nI do not mind a fix, but I would prefer a solution that does *not*\ninvolve $IFS hack that would not work with a string with LF in it.\n"},{"id":"190231","messageId":"CALO-gut4csy5wef4iGPGD5jVPc1f0iFBfS3MUWrOwc2yczdviw@mail.gmail.com","threadId":"30352","inReplyTo":"xmqqvckk93ta.fsf@junio.mtv.corp.google.com","subject":"Re: Bug in git-stash(.sh) ?","fromName":"Eli Barzilay","fromEmail":"eli@barzilay.org","sentAt":"2012-04-28T00:16:11Z","receivedAt":"2012-04-28T00:16:11Z","isPatch":false,"sender":{"key":"eli@barzilay.org","avatar":"https://avatars.githubusercontent.com/u/185905?v=4"},"body":"On Fri, Apr 27, 2012 at 19:02, Junio C Hamano wrote:\n>\n> Not surprised; as far as I understand, ever since the original design,\n> the stash entries are meant to be _counted_, i.e. stash@{0},\n> stash@{1}, stash@{2}, ... and never timed.\n\nAh, that's an issue for magit, but I'd rather have it working anyway.\n\n\n> I do not mind a fix, but I would prefer a solution that does *not*\n> involve $IFS hack that would not work with a string with LF in it.\n\nI didn't like that either, and I think that it's possible to avoid it by\ndropping the --symbolic for that test, and re-parse if needed for an\nerror messag. I'll try that and send a patch.\n\n-- \n ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:\n http://www.barzilay.org/ Maze is Life!\n"},{"id":"190232","messageId":"m2pqasb8mr.fsf@linux-m68k.org","threadId":"30352","inReplyTo":"20379.9312.943088.350379@winooski.ccs.neu.edu","subject":"Re: Bug in git-stash(.sh) ?","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-04-28T07:47:24Z","receivedAt":"2012-04-28T07:47:24Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Eli Barzilay writes:\n\n> For a while now I had a problem when I try to do stash operations via\n> magit -- for example, it shows this in the process buffer:\n>\n> $ git --no-pager stash apply stash@{2012-04-27 08:53:30 -0400}\n> Too many revisions specified: stash@{2012-04-27 08:53:30 -0400}\n\nFWIW, replacing the spaces by dots will avoid the bug.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"190239","messageId":"87wr4za9mr.fsf@gmail.com","threadId":"30352","inReplyTo":"20379.9312.943088.350379@winooski.ccs.neu.edu","subject":"Re: Bug in git-stash(.sh) ?","fromName":"Yann Hodique","fromEmail":"yann.hodique@gmail.com","sentAt":"2012-04-28T20:23:24Z","receivedAt":"2012-04-28T20:23:24Z","isPatch":false,"sender":{"key":"yann.hodique@gmail.com","avatar":"https://gravatar.com/avatar/47ab8ea6d0d9ecdbe95e507dd34a1de11b75c52d9f9b095320874f24d4ac306a?d=mp&s=160"},"body":">>>>> \"Eli\" == Eli Barzilay writes:\n\n> [Note: cross-posted to the magit list to see if anyone else has\n> this problem.]\n\n> For a while now I had a problem when I try to do stash operations via\n> magit -- for example, it shows this in the process buffer:\n\n> $ git --no-pager stash apply stash@{2012-04-27 08:53:30 -0400}\n> Too many revisions specified: stash@{2012-04-27 08:53:30 -0400}\n\nHow exactly do you make magit generate these calls?\nAFAICT, Magit should operate on whatever \"git stash list\" outputs,\nmeaning stash@{N}. So I guess I'm missing something.\n\nYann.\n\n-- \nThe strictest limits are self-imposed.\n\n -- FRIEDRE GINAZ, Philosophy of the Swordmaster\n"},{"id":"190240","messageId":"20380.33897.666338.766096@winooski.ccs.neu.edu","threadId":"30352","inReplyTo":"CALO-gut4csy5wef4iGPGD5jVPc1f0iFBfS3MUWrOwc2yczdviw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org","subject":"Re: Bug in git-stash(.sh) ?","fromName":"Eli Barzilay","fromEmail":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","sentAt":"2012-04-28T23:59:37Z","receivedAt":"2012-04-28T23:59:37Z","isPatch":false,"sender":{"key":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","avatar":null},"body":"Earlier today, Andreas Schwab wrote:\n> \n> FWIW, replacing the spaces by dots will avoid the bug.\n\n(Yeah, but I don't see a quick way to do that replacement without\nresorting to bashisms.)\n\n\nYesterday, Eli Barzilay wrote:\n> I didn't like that either, and I think that it's possible to avoid\n> it by dropping the --symbolic for that test, and re-parse if needed\n> for an error messag. I'll try that and send a patch.\n\nI'm attaching a patch rather than including it inline since it's\nbigger than I thought, and since I'm not sure that it should be used.\nIt makes the script use $REV for sha1 revisions, and $SREV for the\n--symbolic versions that were used previously. With this, date refs\nwork for \"stash apply\" but they *don't* work for \"stash drop\" -- looks\nlike a number is required for that.\n\nHowever, I thought that a much better solution is to not show the\ndates to begin with, since that would make things work as expected...\nThe fact that things seem to be working fine for the whole world\nexcept for me made me look into my config file, and ...\n\nThree hours ago, Yann Hodique wrote:\n> \n> How exactly do you make magit generate these calls? AFAICT, Magit\n> should operate on whatever \"git stash list\" outputs, meaning\n> stash@{N}. So I guess I'm missing something.\n\n... right: the offending configuration I had was log.date = iso. This\ncalls for a simple chane for git-stash.sh to use `--date default':\n\n\tgit log --date default --format=\"%gd: %gs\" -g \"$@\" $ref_stash --\n\nwhich follows. This is independent of the other patch. In any case,\nit is also questionable -- reading the documentation for %gd:\n\n · %gD: reflog selector, e.g., refs/stash@{1}\n · %gd: shortened reflog selector, e.g., stash@{1}\n\nmakes it look like the problem is there -- in get_reflog_selector() --\nwhich has explicit code for showing the dates. (This was done in\n8f8f5476.)\n\nAnother point is being able to see these dates, eg, make \"stash list\"\nshow the stash{N} and also show the dates. It looks to me like the\ndate code in get_reflog_selector() should be *removed* since it can be\nprinted with \"%cd\" or \"%ad\" in the log line. And it might be nicer to\nadd the date to the \"stash list\" output, something like:\n\n\tgit log --date default --format=\"%gd: %gs (%cd)\" -g \"$@\" $ref_stash --\n\n\nThis is the patch mentioned in the beginning:\n\n\n\n\n-- \n ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"},{"id":"190254","messageId":"20120429220132.GB4491@sigill.intra.peff.net","threadId":"30352","inReplyTo":"20380.33897.666338.766096@winooski.ccs.neu.edu","subject":"Re: Bug in git-stash(.sh) ?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-29T22:01:32Z","receivedAt":"2012-04-29T22:01:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 28, 2012 at 07:59:37PM -0400, Eli Barzilay wrote:\n\n> > How exactly do you make magit generate these calls? AFAICT, Magit\n> > should operate on whatever \"git stash list\" outputs, meaning\n> > stash@{N}. So I guess I'm missing something.\n> \n> ... right: the offending configuration I had was log.date = iso. This\n> calls for a simple chane for git-stash.sh to use `--date default':\n> \n> \tgit log --date default --format=\"%gd: %gs\" -g \"$@\" $ref_stash --\n\nI seem to remember dealing with this once a long time ago. And while\n\"--date=default\" works, it is papering over the symptom of a larger\nproblem, which is that \"log\" should not use a non-commandline date to\nmake the stash selector decision. Searching turned up this discussion:\n\n http://thread.gmane.org/gmane.comp.version-control.git/128569\n\nwhich led to f4ea32f (improve reflog date/number heuristic, 2009-09-24).\nThat fixed the case of:\n\n git config log.date iso\n git log -g --oneline\n\nBut later, 8f8f547 (Introduce new pretty formats %g[sdD] for reflog\ninformation, 2009-10-19) added another way to show selectors, and it did\nnot respect the date_mode_explicit flag from f4ea32f. Which I think is a\nbug.\n\nSo the right solution is to pass the date_mode_explicit flag through to\nthe pretty-print --format code, and then pass it along to the reflog\ncode.\n\n> Another point is being able to see these dates, eg, make \"stash list\"\n> show the stash{N} and also show the dates.\n\nYou can do so with:\n\n git stash list --date=iso\n\nbut there is no way to do it automatically via config (and indeed, you\ncan see that it creates problems for scripts when you do so. :) ).\n\n> It looks to me like the date code in get_reflog_selector() should be\n> *removed* since it can be printed with \"%cd\" or \"%ad\" in the log line.\n\nNo, all three are distinct dates. For example, from my git.git reflog:\n\n $ git log -g --format='%gd / %cd / %ad' --date=short\n HEAD@{2012-04-29} / 2009-09-29 / 2009-09-24\n\nThat's a commit (which happens to be f4ea32f) that was written on\n2009-09-24 (author date), sent as a patch to the list and applied\nupstream on 2009-09-29 (committer date), and reached my HEAD reflog via\n\"git checkout f4ea32f\" three years later.\n\n-Peff\n"},{"id":"190255","messageId":"7vlilexkcq.fsf@alter.siamese.dyndns.org","threadId":"30352","inReplyTo":"20380.33897.666338.766096@winooski.ccs.neu.edu","subject":"Re: Bug in git-stash(.sh) ?","fromName":"Junio C Hamano","fromEmail":"gitster-e+axbwqsrlaavxtiumwx3w@public.gmane.org","sentAt":"2012-04-29T22:07:49Z","receivedAt":"2012-04-29T22:07:49Z","isPatch":false,"sender":{"key":"gitster-e+axbwqsrlaavxtiumwx3w@public.gmane.org","avatar":null},"body":"Eli Barzilay writes:\n\n> ... In any case,\n> it is also questionable -- reading the documentation for %gd:\n>\n> · %gD: reflog selector, e.g., refs/stash@{1}\n> · %gd: shortened reflog selector, e.g., stash@{1}\n>\n> makes it look like the problem is there -- in get_reflog_selector() --\n> which has explicit code for showing the dates. (This was done in\n> 8f8f5476.)\n\nI think the root cause of the bug is that there are three cases:\n\n - If we ask for \"log -g ref@{0}\", we should show them counted no matter what.\n\n - If we ask for \"log -g ref@{now}\", we should show them timed no matter what.\n\n - If we ask for \"log -g ref\" without specifier, we show them counted by\n default, but we try to be nice and show them timed when we can infer\n from other context that the user wanted to see them timed.\n\nAn ancient 4e244cb (log --reflog: honour --relative-date, 2007-02-08) was\nwhat introduced the \"explicit code for showing the dates\", but it was done\nsomewhat poorly---it does not differentiate the first and third case.\n\nOnce we fix *that* bug, to disable the \"timed\" codepath altogether when\nthe caller gives \"ref@{0}\" to explicitly ask for counted output, we can\nfix it a lot easily.\n\nAnd the patch to do so should look like this; I'll leave it to the readers\nto add whatever tests that are appropriate.\n\n git-stash.sh | 2 +-\n reflog-walk.c | 16 ++++++++++++----\n 2 files changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex fe4ab28..590c1f3 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -265,7 +265,7 @@ have_stash () {\n \n list_stash () {\n \thave_stash || return 0\n-\tgit log --format=\"%gd: %gs\" -g \"$@\" $ref_stash --\n+\tgit log --format=\"%gd: %gs\" -g \"$@\" \"$ref_stash@{0}\" --\n }\n \n show_stash () {\ndiff --git a/reflog-walk.c b/reflog-walk.c\nindex 86d1884..6fe60a8 100644\n--- a/reflog-walk.c\n+++ b/reflog-walk.c\n@@ -126,7 +126,10 @@ static void add_commit_info(struct commit *commit, void *util,\n }\n \n struct commit_reflog {\n-\tint flag, recno;\n+\tint recno;\n+#define REFLOG_COUNTED 01\n+#define REFLOG_TIMED 02\n+\tunsigned flags;\n \tstruct complete_reflogs *reflogs;\n };\n \n@@ -150,6 +153,7 @@ int add_reflog_for_walk(struct reflog_walk_info *info,\n \tstruct complete_reflogs *reflogs;\n \tchar *branch, *at = strchr(name, '@');\n \tstruct commit_reflog *commit_reflog;\n+\tunsigned flags = 0;\n \n \tif (commit->object.flags & UNINTERESTING)\n \t\tdie (\"Cannot walk reflogs for %s\", name);\n@@ -162,6 +166,9 @@ int add_reflog_for_walk(struct reflog_walk_info *info,\n \t\tif (*ep != '}') {\n \t\t\trecno = -1;\n \t\t\ttimestamp = approxidate(at + 2);\n+\t\t\tflags = REFLOG_TIMED;\n+\t\t} else {\n+\t\t\tflags = REFLOG_COUNTED;\n \t\t}\n \t} else\n \t\trecno = 0;\n@@ -199,8 +206,8 @@ int add_reflog_for_walk(struct reflog_walk_info *info,\n \t}\n \n \tcommit_reflog = xcalloc(sizeof(struct commit_reflog), 1);\n-\tif (recno < 0) {\n-\t\tcommit_reflog->flag = 1;\n+\tcommit_reflog->flags = flags;\n+\tif (flags & REFLOG_TIMED) {\n \t\tcommit_reflog->recno = get_reflog_recno_by_time(reflogs, timestamp);\n \t\tif (commit_reflog->recno < 0) {\n \t\t\tfree(branch);\n@@ -267,7 +274,8 @@ void get_reflog_selector(struct strbuf *sb,\n \t}\n \n \tstrbuf_addf(sb, \"%s@{\", printed_ref);\n-\tif (commit_reflog->flag || dmode) {\n+\tif ((! (commit_reflog->flags && (REFLOG_COUNTED | REFLOG_TIMED)) && dmode) ||\n+\t (commit_reflog->flags & REFLOG_TIMED)) {\n \t\tinfo = &commit_reflog->reflogs->items[commit_reflog->recno+1];\n \t\tstrbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode));\n \t} else {\n"},{"id":"190256","messageId":"20381.49180.329586.983166@winooski.ccs.neu.edu","threadId":"30352","inReplyTo":"20120429220132.GB4491-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org","subject":"Re: Bug in git-stash(.sh) ?","fromName":"Eli Barzilay","fromEmail":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","sentAt":"2012-04-29T22:26:36Z","receivedAt":"2012-04-29T22:26:36Z","isPatch":false,"sender":{"key":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","avatar":null},"body":"A few minutes ago, Jeff King wrote:\n> On Sat, Apr 28, 2012 at 07:59:37PM -0400, Eli Barzilay wrote:\n> \n> > > How exactly do you make magit generate these calls? AFAICT, Magit\n> > > should operate on whatever \"git stash list\" outputs, meaning\n> > > stash@{N}. So I guess I'm missing something.\n> > \n> > ... right: the offending configuration I had was log.date = iso. This\n> > calls for a simple chane for git-stash.sh to use `--date default':\n> > \n> > \tgit log --date default --format=\"%gd: %gs\" -g \"$@\" $ref_stash --\n> \n> I seem to remember dealing with this once a long time ago. And while\n> \"--date=default\" works, it is papering over the symptom of a larger\n> problem, which is that \"log\" should not use a non-commandline date\n> to make the stash selector decision. Searching turned up this\n> discussion:\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/128569\n\nAh, that looks like almost exactly the problem I started with...\n\n\n> which led to f4ea32f (improve reflog date/number heuristic,\n> 2009-09-24). That fixed the case of:\n> \n> git config log.date iso\n> git log -g --oneline\n> \n> But later, 8f8f547 (Introduce new pretty formats %g[sdD] for reflog\n> information, 2009-10-19) added another way to show selectors, and it\n> did not respect the date_mode_explicit flag from f4ea32f. Which I\n> think is a bug.\n> \n> So the right solution is to pass the date_mode_explicit flag through\n> to the pretty-print --format code, and then pass it along to the\n> reflog code.\n\nAssuming that I followed all of that correctly, it still seems bogus\nto do that, given that %gd and %gD are described as producing reflog\nselector, and given that Junio's note that stash operations are really\nintended to be used only with these selectos. What looks more\nsensible to me given the necessity of %gd (and the fact that it's\ndifferent from %cd/%ad) is to change things as follows:\n\n * %gd produces only the date, with the \"default\" having the same\n meaning as elsewhere (so it doesn't show the index numbers)\n * %gD is useless\n * Some new %gi uses the index number: stash@{1}, and %gI produces\n refs/stash@{1}, unrelated to any date setting\n * git-stash.sh uses %gi so the output has the numbers\n * Some new option for \"stash list\" for the format string, so it's\n possible to show the dates if you want to with something like\n git stash list --format:\"%gi: %gs (%gd)\"\n\nWith this the output has the number independent of log.date setting,\nand I get a --format if I want to see something else, which makes more\nsense than --date being explicit or not. IOW, I'd expect this:\n\n> git stash list --date=iso\n\nto not have any effect.\n\nThis is not a backwards compatible change, but my guess is that\nexisting uses of %g[dD] are suffering from a similar problem anyway.\n(So another option maybe making %gd use the number and something else\nfor the date version.)\n\n(But my opinion is of course limited to my short encounter with all of\nthis...)\n\n-- \n ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"},{"id":"190257","messageId":"20381.49810.943013.33117@winooski.ccs.neu.edu","threadId":"30352","inReplyTo":"7vlilexkcq.fsf-s2KvWo2KEQL18tm6hw+yZpy9Z0UEorGK@public.gmane.org","subject":"Re: Bug in git-stash(.sh) ?","fromName":"Eli Barzilay","fromEmail":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","sentAt":"2012-04-29T22:37:06Z","receivedAt":"2012-04-29T22:37:06Z","isPatch":false,"sender":{"key":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","avatar":null},"body":"30 minutes ago, Junio C Hamano wrote:\n> \n> I think the root cause of the bug is that there are three cases:\n> \n> - If we ask for \"log -g ref@{0}\", we should show them counted no\n> matter what.\n> \n> - If we ask for \"log -g ref@{now}\", we should show them timed no\n> matter what.\n> \n> - If we ask for \"log -g ref\" without specifier, we show them\n> counted by default, but we try to be nice and show them timed\n> when we can infer from other context that the user wanted to see\n> them timed.\n\nAh, I was unaware (unsurprisingly) that *that's* how an explict date\nformat is (supposed to?) requested -- but then what happens with\n\n git log --date iso -g \"ref@{0}\"\n\n?\n\n-- \n ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"},{"id":"190418","messageId":"20120501134254.GA11900@sigill.intra.peff.net","threadId":"30352","inReplyTo":"20381.49180.329586.983166@winooski.ccs.neu.edu","subject":"Re: Bug in git-stash(.sh) ?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-01T13:42:55Z","receivedAt":"2012-05-01T13:42:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 29, 2012 at 06:26:36PM -0400, Eli Barzilay wrote:\n\n> > which led to f4ea32f (improve reflog date/number heuristic,\n> > 2009-09-24). That fixed the case of:\n> > \n> > git config log.date iso\n> > git log -g --oneline\n> > \n> > But later, 8f8f547 (Introduce new pretty formats %g[sdD] for reflog\n> > information, 2009-10-19) added another way to show selectors, and it\n> > did not respect the date_mode_explicit flag from f4ea32f. Which I\n> > think is a bug.\n> > \n> > So the right solution is to pass the date_mode_explicit flag through\n> > to the pretty-print --format code, and then pass it along to the\n> > reflog code.\n> \n> Assuming that I followed all of that correctly, it still seems bogus\n> to do that, given that %gd and %gD are described as producing reflog\n> selector, and given that Junio's note that stash operations are really\n> intended to be used only with these selectos.\n\nKeep in mind this bug is not about stash at all; it is about showing\nreflog selectors. Those are a more general mechanism, and are used for\nmore than just stash. The fact that user config affects the format of\n\"%gd\" is a bug; it should follow the same rules as the regular reflog\npretty-printing (and the behavior of neither should be affected by user\nconfig, as scripts rely on the output being consistent).\n\nOnce that is fixed, then we can consider whether something more should\nhappen for stash (though I am inclined to say that is enough; it is a\nfeature that you can do \"git stash list --date=relative\" to see the\nstash timestamps).\n\n> What looks more sensible to me given the necessity of %gd (and the\n> fact that it's different from %cd/%ad) is to change things as follows:\n> \n> * %gd produces only the date, with the \"default\" having the same\n> meaning as elsewhere (so it doesn't show the index numbers)\n\n%gd is part of the public interface and will not change its semantics\n(or at least not without a long deprecation period). It's a shame that\n\"d\" is taken for the selector, when it would be better to mean \"date\" as\nit does for author and committer. But I don't know if it's worth\nchanging at this point.\n\nWe could add new placeholders with different semantics, though. When I\nadded reflog identity placeholders a few months ago, there was a brief\ndiscussion on adding a date placeholder:\n\n http://article.gmane.org/gmane.comp.version-control.git/185043\n\nbut the related work hasn't progressed.\n\n> * Some new %gi uses the index number: stash@{1}, and %gI produces\n> refs/stash@{1}, unrelated to any date setting\n> * git-stash.sh uses %gi so the output has the numbers\n> * Some new option for \"stash list\" for the format string, so it's\n> possible to show the dates if you want to with something like\n> git stash list --format:\"%gi: %gs (%gd)\"\n\nI don't have a huge problem with that. But what issue is it really\nsolving? Are people using \"git stash list --date=iso\" and then getting\nconfused by the output? Or is it simply a matter of mistakenly applying\nthe config when it should not be? The latter needs fixed in either case.\n\n-Peff\n"},{"id":"190421","messageId":"20120501150211.GA14185@sigill.intra.peff.net","threadId":"30352","inReplyTo":"7vlilexkcq.fsf-s2KvWo2KEQL18tm6hw+yZpy9Z0UEorGK@public.gmane.org","subject":"Re: Bug in git-stash(.sh) ?","fromName":"Jeff King","fromEmail":"peff-adepduraxsq@public.gmane.org","sentAt":"2012-05-01T15:02:11Z","receivedAt":"2012-05-01T15:02:11Z","isPatch":false,"sender":{"key":"peff-adepduraxsq@public.gmane.org","avatar":null},"body":"On Sun, Apr 29, 2012 at 03:07:49PM -0700, Junio C Hamano wrote:\n\n> Eli Barzilay writes:\n> \n> > ... In any case,\n> > it is also questionable -- reading the documentation for %gd:\n> >\n> > · %gD: reflog selector, e.g., refs/stash@{1}\n> > · %gd: shortened reflog selector, e.g., stash@{1}\n> >\n> > makes it look like the problem is there -- in get_reflog_selector() --\n> > which has explicit code for showing the dates. (This was done in\n> > 8f8f5476.)\n> \n> I think the root cause of the bug is that there are three cases:\n> \n> - If we ask for \"log -g ref@{0}\", we should show them counted no matter what.\n> \n> - If we ask for \"log -g ref@{now}\", we should show them timed no matter what.\n> \n> - If we ask for \"log -g ref\" without specifier, we show them counted by\n> default, but we try to be nice and show them timed when we can infer\n> from other context that the user wanted to see them timed.\n\nRight. My argument is that the context in your third point was always\nintended to be about command-line options. Respecting the log.date\nconfig there is a bug (and not just in breaking intent; it also breaks\nscriptability). It was fixed for the regular pretty-print code path, but\nwas broken again when the \"%gd\" code path was added.\n\n> An ancient 4e244cb (log --reflog: honour --relative-date, 2007-02-08) was\n> what introduced the \"explicit code for showing the dates\", but it was done\n> somewhat poorly---it does not differentiate the first and third case.\n\nIf that is the case (and I haven't checked either way, but it does not\nsurprise me at all), then I believe that is a separate bug. And we\nshould fix that, too.\n\n> Once we fix *that* bug, to disable the \"timed\" codepath altogether when\n> the caller gives \"ref@{0}\" to explicitly ask for counted output, we can\n> fix it a lot easily.\n> [...]\n> -\tgit log --format=\"%gd: %gs\" -g \"$@\" $ref_stash --\n> +\tgit log --format=\"%gd: %gs\" -g \"$@\" \"$ref_stash@{0}\" --\n\nThat will solve the problem for stash, but the config bug would remain\nfor every _other_ user of \"git log -g --format=%gd\". So that needs fixed\neither way.\n\nHowever, I really wonder if this is the right thing. If I do:\n\n git stash list --date=relative\n\nisn't it a feature that I get to see the date at which each stash was\nmade? Why are we taking it away? I can see if it were the only way to\nfix the problem with log.date, but that has another solution. Are people\nreally calling \"stash list\" with a date on the command line and getting\nconfused by the output? My understanding was that the observed problem\nwas purely a bad interaction with log.date, which should not be\nrespected at all.\n\n-Peff\n"},{"id":"190667","messageId":"20386.53745.200846.115335@winooski.ccs.neu.edu","threadId":"30352","inReplyTo":"20120501134254.GA11900-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org","subject":"[git] Re: Bug in git-stash(.sh) ?","fromName":"Eli Barzilay","fromEmail":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","sentAt":"2012-05-03T18:44:01Z","receivedAt":"2012-05-03T18:44:01Z","isPatch":false,"sender":{"key":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","avatar":null},"body":"Two days ago, Jeff King wrote:\n> On Sun, Apr 29, 2012 at 06:26:36PM -0400, Eli Barzilay wrote:\n> \n> > > which led to f4ea32f (improve reflog date/number heuristic,\n> > > 2009-09-24). That fixed the case of:\n> > > \n> > > git config log.date iso\n> > > git log -g --oneline\n> > > \n> > > But later, 8f8f547 (Introduce new pretty formats %g[sdD] for reflog\n> > > information, 2009-10-19) added another way to show selectors, and it\n> > > did not respect the date_mode_explicit flag from f4ea32f. Which I\n> > > think is a bug.\n> > > \n> > > So the right solution is to pass the date_mode_explicit flag through\n> > > to the pretty-print --format code, and then pass it along to the\n> > > reflog code.\n> > \n> > Assuming that I followed all of that correctly, it still seems bogus\n> > to do that, given that %gd and %gD are described as producing reflog\n> > selector, and given that Junio's note that stash operations are really\n> > intended to be used only with these selectos.\n> \n> Keep in mind this bug is not about stash at all; it is about showing\n> reflog selectors. Those are a more general mechanism, and are used for\n> more than just stash. The fact that user config affects the format of\n> \"%gd\" is a bug; it should follow the same rules as the regular reflog\n> pretty-printing (and the behavior of neither should be affected by user\n> config, as scripts rely on the output being consistent).\n> \n> Once that is fixed, then we can consider whether something more should\n> happen for stash (though I am inclined to say that is enough; it is a\n> feature that you can do \"git stash list --date=relative\" to see the\n> stash timestamps).\n\nSince the general problem is bigger, how about just the quick patch of\nadding --date=default in the list_stash function as a stopgap? That\nseems to be close enough to how it should work anyway.\n\n\n> > What looks more sensible to me given the necessity of %gd (and the\n> > fact that it's different from %cd/%ad) is to change things as\n> > follows:\n> > \n> > * %gd produces only the date, with the \"default\" having the same\n> > meaning as elsewhere (so it doesn't show the index numbers)\n> \n> %gd is part of the public interface and will not change its semantics\n> (or at least not without a long deprecation period). It's a shame\n> that \"d\" is taken for the selector, when it would be better to mean\n> \"date\" as it does for author and committer. But I don't know if it's\n> worth changing at this point.\n\n(Yeah, I can see that.)\n\n\n> > * Some new %gi uses the index number: stash@{1}, and %gI produces\n> > refs/stash@{1}, unrelated to any date setting\n> > * git-stash.sh uses %gi so the output has the numbers\n> > * Some new option for \"stash list\" for the format string, so it's\n> > possible to show the dates if you want to with something like\n> > git stash list --format:\"%gi: %gs (%gd)\"\n> \n> I don't have a huge problem with that. But what issue is it really\n> solving? Are people using \"git stash list --date=iso\" and then\n> getting confused by the output? Or is it simply a matter of\n> mistakenly applying the config when it should not be? The latter\n> needs fixed in either case.\n\nIt's basically an attempt to have a %gi that is disconnected from date\noptions (config or flags), which solves the config problem in a\ntrivial way (no date options are used)...\n\n-- \n ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"},{"id":"190717","messageId":"20120504052106.GA15970@sigill.intra.peff.net","threadId":"30352","inReplyTo":"20386.53745.200846.115335-a5nvgYPMCZcx/1z6v04GWfZ8FUJU4vz8@public.gmane.org","subject":"Re: [git] Re: Bug in git-stash(.sh) ?","fromName":"Jeff King","fromEmail":"peff-adepduraxsq@public.gmane.org","sentAt":"2012-05-04T05:21:07Z","receivedAt":"2012-05-04T05:21:07Z","isPatch":false,"sender":{"key":"peff-adepduraxsq@public.gmane.org","avatar":null},"body":"On Thu, May 03, 2012 at 02:44:01PM -0400, Eli Barzilay wrote:\n\n> > Once that is fixed, then we can consider whether something more should\n> > happen for stash (though I am inclined to say that is enough; it is a\n> > feature that you can do \"git stash list --date=relative\" to see the\n> > stash timestamps).\n> \n> Since the general problem is bigger, how about just the quick patch of\n> adding --date=default in the list_stash function as a stopgap? That\n> seems to be close enough to how it should work anyway.\n\nIt is bigger in scope, but the fix is still pretty small. I was trying\nto trick^W gently prod you into making a patch, but that does not seem\nto have worked. :) So here is a series that fixes it, and we don't have\nto worry about a stopgap.\n\n [1/4]: t1411: add more selector index/date tests\n [2/4]: log: respect date_mode_explicit --format:%gd\n [3/4]: reflog-walk: clean up \"flag\" field of commit_reflog struct\n [4/4]: reflog-walk: always make HEAD@{0} show indexed selectors\n\nThe first two fix and test the bug I mentioned, and as a result solve\nthe stash problem. The second two fix and test the bug that Junio\nmentioned. This doesn't affect stash, but it's the right thing for \"git\nlog\" to do.\n\n> > > * Some new %gi uses the index number: stash@{1}, and %gI produces\n> > > refs/stash@{1}, unrelated to any date setting\n> > > * git-stash.sh uses %gi so the output has the numbers\n> > > * Some new option for \"stash list\" for the format string, so it's\n> > > possible to show the dates if you want to with something like\n> > > git stash list --format:\"%gi: %gs (%gd)\"\n> > \n> > I don't have a huge problem with that. But what issue is it really\n> > solving? Are people using \"git stash list --date=iso\" and then\n> > getting confused by the output? Or is it simply a matter of\n> > mistakenly applying the config when it should not be? The latter\n> > needs fixed in either case.\n> \n> It's basically an attempt to have a %gi that is disconnected from date\n> options (config or flags), which solves the config problem in a\n> trivial way (no date options are used)...\n\nI don't have a problem at all with %gi; I think it would be a good\naddition. I just think that stash shouldn't use, as the \"--date\" thing\nis a feature that there is no reason to deny to stash users (it just\nneeds to be less buggy :) ).\n\n-Peff\n"},{"id":"190718","messageId":"20120504052314.GA16107@sigill.intra.peff.net","threadId":"30352","inReplyTo":"20120504052106.GA15970-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org","subject":"[PATCH 1/4] t1411: add more selector index/date tests","fromName":"Jeff King","fromEmail":"peff-adepduraxsq@public.gmane.org","sentAt":"2012-05-04T05:23:14Z","receivedAt":"2012-05-04T05:23:14Z","isPatch":true,"sender":{"key":"peff-adepduraxsq@public.gmane.org","avatar":null},"body":"We already check that @{now} and \"--date\" cause the\ndisplayed selector to use the date for both the multiline\nand oneline formats. However, we miss several cases:\n\n 1. The --format=%gd selector is not tested at all.\n\n 2. We do not check how the log.date config interacts with the\n \"--date\" magic (according to f4ea32f, it should not\n impact the output).\n\nDoing so reveals that the combination of both (log.date\ncombined with the %gd format) does not behave as expected.\n\nSigned-off-by: Jeff King \n---\nThis takes us up to 9 tests (3 cases by 3 formats). It's almost enough\nto make me want to write loops, but I think the boilerplate would end up\njust making it more confusing to read.\n\n t/t1411-reflog-show.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 45 insertions(+)\n\ndiff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh\nindex caa687b..4706f4c 100755\n--- a/t/t1411-reflog-show.sh\n+++ b/t/t1411-reflog-show.sh\n@@ -65,6 +65,14 @@ test_expect_success 'using @{now} syntax shows reflog date (oneline)' '\n '\n \n cat >expect <<'EOF'\n+HEAD@{Thu Apr 7 15:13:13 2005 -0700}\n+EOF\n+test_expect_success 'using @{now} syntax shows reflog date (format=%gd)' '\n+\tgit log -g -1 --format=%gd HEAD@{now} >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<'EOF'\n Reflog: HEAD@{1112911993 -0700} (C O Mitter )\n Reflog message: commit (initial): one\n EOF\n@@ -82,6 +90,43 @@ test_expect_success 'using --date= shows reflog date (oneline)' '\n \ttest_cmp expect actual\n '\n \n+cat >expect <<'EOF'\n+HEAD@{1112911993 -0700}\n+EOF\n+test_expect_success 'using --date= shows reflog date (format=%gd)' '\n+\tgit log -g -1 --format=%gd --date=raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<'EOF'\n+Reflog: HEAD@{0} (C O Mitter )\n+Reflog message: commit (initial): one\n+EOF\n+test_expect_success 'log.date does not invoke \"--date\" magic (multiline)' '\n+\ttest_config log.date raw &&\n+\tgit log -g -1 >tmp &&\n+\tgrep ^Reflog actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<'EOF'\n+e46513e HEAD@{0}: commit (initial): one\n+EOF\n+test_expect_success 'log.date does not invoke \"--date\" magic (oneline)' '\n+\ttest_config log.date raw &&\n+\tgit log -g -1 --oneline >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<'EOF'\n+HEAD@{0}\n+EOF\n+test_expect_failure 'log.date does not invoke \"--date\" magic (format=%gd)' '\n+\ttest_config log.date raw &&\n+\tgit log -g -1 --format=%gd >actual &&\n+\ttest_cmp expect actual\n+'\n+\n : >expect\n test_expect_success 'empty reflog file' '\n \tgit branch empty &&\n-- \n1.7.10.1.10.ge534bc3\n"},{"id":"190720","messageId":"20120504052518.GB16107@sigill.intra.peff.net","threadId":"30352","inReplyTo":"20120504052106.GA15970-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org","subject":"[PATCH 2/4] log: respect date_mode_explicit with --format:%gd","fromName":"Jeff King","fromEmail":"peff-adepduraxsq@public.gmane.org","sentAt":"2012-05-04T05:25:18Z","receivedAt":"2012-05-04T05:25:18Z","isPatch":true,"sender":{"key":"peff-adepduraxsq@public.gmane.org","avatar":null},"body":"When we show a reflog selector (e.g., via \"git log -g\"), we\nperform some DWIM magic: while we normally show the entry's\nindex (e.g., HEAD@{1}), if the user has given us a date\nwith \"--date\", then we show a date-based select (e.g.,\nHEAD@{yesterday}).\n\nHowever, we don't want to trigger this magic if the\nalternate date format we got was from the \"log.date\"\nconfiguration; that is not sufficiently strong context for\nus to invoke this particular magic. To fix this, commit\nf4ea32f (improve reflog date/number heuristic, 2009-09-24)\nintroduced a \"date_mode_explicit\" flag in rev_info. This\nflag is set only when we see a \"--date\" option on the\ncommand line, and we a vanilla date to the reflog code if\nthe date was not explicit.\n\nLater, commit 8f8f547 (Introduce new pretty formats %g[sdD]\nfor reflog information, 2009-10-19) added another way to\nshow selectors, and it did not respect the date_mode_explicit\nflag from f4ea32f.\n\nThis patch propagates the date_mode_explicit flag to the\npretty-print code, which can then use it to pass the\nappropriate date field to the reflog code. This brings the\nbehavior of \"%gd\" in line with the other formats, and means\nthat its output is independent of any user configuration.\n\nSigned-off-by: Jeff King \n---\nI'm not happy that users of pretty_print_context have to manually\nremember to copy in the date_mode_explicit flag; it would be nice if it\njust came with the date_mode field for free. But that would mean\nchanging the type of date_mode to hold the extra bit, and would disrupt\ncallers all over the code base.\n\n builtin/rev-list.c | 1 +\n commit.h | 1 +\n log-tree.c | 1 +\n pretty.c | 4 +++-\n t/t1411-reflog-show.sh | 2 +-\n 5 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 4c4d404..ff5a383 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -109,6 +109,7 @@ static void show_commit(struct commit *commit, void *data)\n \t\tstruct pretty_print_context ctx = {0};\n \t\tctx.abbrev = revs->abbrev;\n \t\tctx.date_mode = revs->date_mode;\n+\t\tctx.date_mode_explicit = revs->date_mode_explicit;\n \t\tctx.fmt = revs->commit_format;\n \t\tpretty_print_commit(&ctx, commit, &buf);\n \t\tif (revs->graph) {\ndiff --git a/commit.h b/commit.h\nindex ccaa20b..d617fa3 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -84,6 +84,7 @@ struct pretty_print_context {\n \tconst char *after_subject;\n \tint preserve_subject;\n \tenum date_mode date_mode;\n+\tunsigned date_mode_explicit:1;\n \tint need_8bit_cte;\n \tint show_notes;\n \tstruct reflog_walk_info *reflog_info;\ndiff --git a/log-tree.c b/log-tree.c\nindex 34c49e7..634f142 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -652,6 +652,7 @@ void show_log(struct rev_info *opt)\n \tif (ctx.need_8bit_cte >= 0)\n \t\tctx.need_8bit_cte = has_non_ascii(opt->add_signoff);\n \tctx.date_mode = opt->date_mode;\n+\tctx.date_mode_explicit = opt->date_mode_explicit;\n \tctx.abbrev = opt->diffopt.abbrev;\n \tctx.after_subject = extra_headers;\n \tctx.preserve_subject = opt->preserve_subject;\ndiff --git a/pretty.c b/pretty.c\nindex f2dee30..2bc64b3 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1009,7 +1009,9 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \t\t\tif (c->pretty_ctx->reflog_info)\n \t\t\t\tget_reflog_selector(sb,\n \t\t\t\t\t\t c->pretty_ctx->reflog_info,\n-\t\t\t\t\t\t c->pretty_ctx->date_mode,\n+\t\t\t\t\t\t c->pretty_ctx->date_mode_explicit ?\n+\t\t\t\t\t\t c->pretty_ctx->date_mode :\n+\t\t\t\t\t\t DATE_NORMAL,\n \t\t\t\t\t\t (placeholder[1] == 'd'));\n \t\t\treturn 2;\n \t\tcase 's':\t/* reflog message */\ndiff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh\nindex 4706f4c..88247f8 100755\n--- a/t/t1411-reflog-show.sh\n+++ b/t/t1411-reflog-show.sh\n@@ -121,7 +121,7 @@ test_expect_success 'log.date does not invoke \"--date\" magic (oneline)' '\n cat >expect <<'EOF'\n HEAD@{0}\n EOF\n-test_expect_failure 'log.date does not invoke \"--date\" magic (format=%gd)' '\n+test_expect_success 'log.date does not invoke \"--date\" magic (format=%gd)' '\n \ttest_config log.date raw &&\n \tgit log -g -1 --format=%gd >actual &&\n \ttest_cmp expect actual\n-- \n1.7.10.1.10.ge534bc3\n"},{"id":"190721","messageId":"20120504052626.GC16107@sigill.intra.peff.net","threadId":"30352","inReplyTo":"20120504052106.GA15970@sigill.intra.peff.net","subject":"[PATCH 3/4] reflog-walk: clean up \"flag\" field of commit_reflog struct","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-04T05:26:26Z","receivedAt":"2012-05-04T05:26:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we prepare to walk a reflog, we parse the specification\nand pull some information from it, such as which reflog to\nlook in (e.g., HEAD), and where to start (e.g., HEAD@{10} or\nHEAD@{yesterday}). The resulting struct has a \"recno\" field\nto show where in the reflog we are starting. It also has a\n\"flag\" field; if true, it means the recno field came from\nparsing a date like HEAD@{yesterday}.\n\nThere are two problems with this:\n\n 1. \"flag\" is an absolutely terrible name, as it conveys\n nothing about the meaning\n\n 2. you can tell \"HEAD\" from \"HEAD@{yesterday}\", but you\n can't differentiate \"HEAD\" from \"HEAD{0}\"\n\nThis patch converts the flag into a tri-state (and gives it\na better name!).\n\nSigned-off-by: Jeff King \n---\n reflog-walk.c | 15 ++++++++++++---\n 1 file changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/reflog-walk.c b/reflog-walk.c\nindex 86d1884..3549318 100644\n--- a/reflog-walk.c\n+++ b/reflog-walk.c\n@@ -126,7 +126,12 @@ static void add_commit_info(struct commit *commit, void *util,\n }\n \n struct commit_reflog {\n-\tint flag, recno;\n+\tint recno;\n+\tenum selector_type {\n+\t\tSELECTOR_NONE,\n+\t\tSELECTOR_INDEX,\n+\t\tSELECTOR_DATE\n+\t} selector;\n \tstruct complete_reflogs *reflogs;\n };\n \n@@ -150,6 +155,7 @@ int add_reflog_for_walk(struct reflog_walk_info *info,\n \tstruct complete_reflogs *reflogs;\n \tchar *branch, *at = strchr(name, '@');\n \tstruct commit_reflog *commit_reflog;\n+\tenum selector_type selector = SELECTOR_NONE;\n \n \tif (commit->object.flags & UNINTERESTING)\n \t\tdie (\"Cannot walk reflogs for %s\", name);\n@@ -162,7 +168,10 @@ int add_reflog_for_walk(struct reflog_walk_info *info,\n \t\tif (*ep != '}') {\n \t\t\trecno = -1;\n \t\t\ttimestamp = approxidate(at + 2);\n+\t\t\tselector = SELECTOR_DATE;\n \t\t}\n+\t\telse\n+\t\t\tselector = SELECTOR_INDEX;\n \t} else\n \t\trecno = 0;\n \n@@ -200,7 +209,6 @@ int add_reflog_for_walk(struct reflog_walk_info *info,\n \n \tcommit_reflog = xcalloc(sizeof(struct commit_reflog), 1);\n \tif (recno < 0) {\n-\t\tcommit_reflog->flag = 1;\n \t\tcommit_reflog->recno = get_reflog_recno_by_time(reflogs, timestamp);\n \t\tif (commit_reflog->recno < 0) {\n \t\t\tfree(branch);\n@@ -209,6 +217,7 @@ int add_reflog_for_walk(struct reflog_walk_info *info,\n \t\t}\n \t} else\n \t\tcommit_reflog->recno = reflogs->nr - recno - 1;\n+\tcommit_reflog->selector = selector;\n \tcommit_reflog->reflogs = reflogs;\n \n \tadd_commit_info(commit, commit_reflog, &info->reflogs);\n@@ -267,7 +276,7 @@ void get_reflog_selector(struct strbuf *sb,\n \t}\n \n \tstrbuf_addf(sb, \"%s@{\", printed_ref);\n-\tif (commit_reflog->flag || dmode) {\n+\tif (commit_reflog->selector == SELECTOR_DATE || dmode) {\n \t\tinfo = &commit_reflog->reflogs->items[commit_reflog->recno+1];\n \t\tstrbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode));\n \t} else {\n-- \n1.7.10.1.10.ge534bc3\n"},{"id":"190722","messageId":"20120504052725.GD16107@sigill.intra.peff.net","threadId":"30352","inReplyTo":"20120504052106.GA15970-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org","subject":"[PATCH 4/4] reflog-walk: always make HEAD@{0} show indexed selectors","fromName":"Jeff King","fromEmail":"peff-adepduraxsq@public.gmane.org","sentAt":"2012-05-04T05:27:25Z","receivedAt":"2012-05-04T05:27:25Z","isPatch":true,"sender":{"key":"peff-adepduraxsq@public.gmane.org","avatar":null},"body":"When we are showing reflog selectors during a walk, we infer\nfrom context whether the user wanted to see the index in\neach selector, or the reflog date. The current rules are:\n\n 1. if the user asked for an explicit date format in the\n output, show the date\n\n 2. if the user asked for ref@{now}, show the date\n\n 3. if neither is true, show the index\n\nHowever, if we see \"ref@{0}\", that should be a strong clue\nthat the user wants to see the counted version. In fact, it\nshould be much stronger than the date format in (1). The\nuser may have been setting the date format to use in another\npart of the output (e.g., in --format=\"%gd (%ad)\", they may\nhave wanted to influence the author date).\n\nThis patch flips the rules to:\n\n 1. if the user asked for ref@{0}, always show the index\n\n 2. if the user asked for ref@{now}, always show the date\n\n 3. otherwise, we have just \"ref\"; show them counted by\n default, but respect the presence of \"--date\" as a clue\n that the user wanted them date-based\n\nSigned-off-by: Jeff King \n---\n reflog-walk.c | 3 ++-\n t/t1411-reflog-show.sh | 8 ++++++++\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/reflog-walk.c b/reflog-walk.c\nindex 3549318..b974258 100644\n--- a/reflog-walk.c\n+++ b/reflog-walk.c\n@@ -276,7 +276,8 @@ void get_reflog_selector(struct strbuf *sb,\n \t}\n \n \tstrbuf_addf(sb, \"%s@{\", printed_ref);\n-\tif (commit_reflog->selector == SELECTOR_DATE || dmode) {\n+\tif (commit_reflog->selector == SELECTOR_DATE ||\n+\t (commit_reflog->selector == SELECTOR_NONE && dmode)) {\n \t\tinfo = &commit_reflog->reflogs->items[commit_reflog->recno+1];\n \t\tstrbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode));\n \t} else {\ndiff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh\nindex 88247f8..7d9b5e3 100755\n--- a/t/t1411-reflog-show.sh\n+++ b/t/t1411-reflog-show.sh\n@@ -127,6 +127,14 @@ test_expect_success 'log.date does not invoke \"--date\" magic (format=%gd)' '\n \ttest_cmp expect actual\n '\n \n+cat >expect <<'EOF'\n+HEAD@{0}\n+EOF\n+test_expect_success '--date magic does not override explicit @{0} syntax' '\n+\tgit log -g -1 --format=%gd --date=raw HEAD@{0} >actual &&\n+\ttest_cmp expect actual\n+'\n+\n : >expect\n test_expect_success 'empty reflog file' '\n \tgit branch empty &&\n-- \n1.7.10.1.10.ge534bc3\n"},{"id":"190768","messageId":"7v7gwrc212.fsf@alter.siamese.dyndns.org","threadId":"30352","inReplyTo":"20120504052725.GD16107-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org","subject":"Re: [PATCH 4/4] reflog-walk: always make HEAD@{0} show indexed selectors","fromName":"Junio C Hamano","fromEmail":"gitster-e+axbwqsrlaavxtiumwx3w@public.gmane.org","sentAt":"2012-05-04T17:02:49Z","receivedAt":"2012-05-04T17:02:49Z","isPatch":true,"sender":{"key":"gitster-e+axbwqsrlaavxtiumwx3w@public.gmane.org","avatar":null},"body":"Jeff King writes:\n\n> This patch flips the rules to:\n>\n> 1. if the user asked for ref@{0}, always show the index\n>\n> 2. if the user asked for ref@{now}, always show the date\n>\n> 3. otherwise, we have just \"ref\"; show them counted by\n> default, but respect the presence of \"--date\" as a clue\n> that the user wanted them date-based\n\nThe revision.c parser for \"git log --date=default -g master\" would flip\nthe \"explicit\" bit, revs->date_mode is set to DATE_NORMAL, and that value\nwill eventually come as dmode here.\n\n> diff --git a/reflog-walk.c b/reflog-walk.c\n> index 3549318..b974258 100644\n> --- a/reflog-walk.c\n> +++ b/reflog-walk.c\n> @@ -276,7 +276,8 @@ void get_reflog_selector(struct strbuf *sb,\n> \t}\n> \n> \tstrbuf_addf(sb, \"%s@{\", printed_ref);\n> -\tif (commit_reflog->selector == SELECTOR_DATE || dmode) {\n> +\tif (commit_reflog->selector == SELECTOR_DATE ||\n> +\t (commit_reflog->selector == SELECTOR_NONE && dmode)) {\n> \t\tinfo = &commit_reflog->reflogs->items[commit_reflog->recno+1];\n> \t\tstrbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode));\n\nBut DATE_NORMAL happens to be zero ;-) \"git log --date=default -g master\"\nwould still show the counted version.\n\nI personally do not care about that behaviour, but I know that I will\nlater later have to deal with people who do care, which is annoying.\n\nProbably we would internally need to define two values to ask for the\nDATE_NORMAL output. Move DATE_NORMAL to non-zero value, introduce a new\nDATE_DEFAULT that is zero, and make their output identical, perhaps\nsomething like the attached (not even compile tested).\n\nThe implicit comparison to zero in the above is a bad code (but that is\na problem from the very old days).\n\ndiff --git a/cache.h b/cache.h\nindex 58ff054..fe42e80 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -876,7 +876,8 @@ extern struct object *peel_to_type(const char *name, int namelen,\n \t\t\t\t struct object *o, enum object_type);\n \n enum date_mode {\n-\tDATE_NORMAL = 0,\n+\tDATE_DEFAULT = 0,\n+\tDATE_NORMAL,\n \tDATE_RELATIVE,\n \tDATE_SHORT,\n \tDATE_LOCAL,\ndiff --git a/reflog-walk.c b/reflog-walk.c\nindex b974258..d002516 100644\n--- a/reflog-walk.c\n+++ b/reflog-walk.c\n@@ -277,7 +277,7 @@ void get_reflog_selector(struct strbuf *sb,\n \n \tstrbuf_addf(sb, \"%s@{\", printed_ref);\n \tif (commit_reflog->selector == SELECTOR_DATE ||\n-\t (commit_reflog->selector == SELECTOR_NONE && dmode)) {\n+\t (commit_reflog->selector == SELECTOR_NONE && (dmode != DATE_DEFAULT))) {\n \t\tinfo = &commit_reflog->reflogs->items[commit_reflog->recno+1];\n \t\tstrbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode));\n \t} else {\n"},{"id":"190778","messageId":"20388.9885.608325.489624@winooski.ccs.neu.edu","threadId":"30352","inReplyTo":"20120504052106.GA15970-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org","subject":"Re: [git] Re: Bug in git-stash(.sh) ?","fromName":"Eli Barzilay","fromEmail":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","sentAt":"2012-05-04T18:57:33Z","receivedAt":"2012-05-04T18:57:33Z","isPatch":false,"sender":{"key":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","avatar":null},"body":"Earlier today, Jeff King wrote:\n> On Thu, May 03, 2012 at 02:44:01PM -0400, Eli Barzilay wrote:\n> \n> > > Once that is fixed, then we can consider whether something more\n> > > should happen for stash (though I am inclined to say that is\n> > > enough; it is a feature that you can do \"git stash list\n> > > --date=relative\" to see the stash timestamps).\n> > \n> > Since the general problem is bigger, how about just the quick\n> > patch of adding --date=default in the list_stash function as a\n> > stopgap? That seems to be close enough to how it should work\n> > anyway.\n> \n> It is bigger in scope, but the fix is still pretty small. I was\n> trying to trick^W gently prod you into making a patch, but that does\n> not seem to have worked. :)\n\nApologies -- it wasn't clear to me what should be done to address the\ngeneral problem, and I don't think that I'd do a good job with that\nanyway.\n\nI can still make a proper patch with the fix for git-stash.sh that\navoids the problem with the spaces.\n\n-- \n ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"},{"id":"190808","messageId":"20388.23034.73009.601819@winooski.ccs.neu.edu","threadId":"30352","inReplyTo":"20388.9885.608325.489624-a5nvgYPMCZcx/1z6v04GWfZ8FUJU4vz8@public.gmane.org","subject":"Re: [git] Re: Bug in git-stash(.sh) ?","fromName":"Eli Barzilay","fromEmail":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","sentAt":"2012-05-04T22:36:42Z","receivedAt":"2012-05-04T22:36:42Z","isPatch":false,"sender":{"key":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","avatar":null},"body":"Four hours ago, Eli Barzilay wrote:\n> \n> I can still make a proper patch with the fix for git-stash.sh that\n> avoids the problem with the spaces.\n\nHere's another idea for an easier patch that instead of fixing the\nissue with space just spits out a better error: before throwing the\n\"Too many revisions specified\" error, check if \"$*\" matches \"{.*}\"\nand if \"$1\" matches \"{[^}]*\" and in that case make the error say that\nspaces are not supported. The change is therefore only in the error,\nand with a fixed output, that should be enough.\n\n(Please tell me if the above or this is desirable...)\n\n-- \n ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"},{"id":"191042","messageId":"20120507213752.GA19911@sigill.intra.peff.net","threadId":"30352","inReplyTo":"7v7gwrc212.fsf-s2KvWo2KEQL18tm6hw+yZpy9Z0UEorGK@public.gmane.org","subject":"Re: [PATCH 4/4] reflog-walk: always make HEAD@{0} show indexed selectors","fromName":"Jeff King","fromEmail":"peff-adepduraxsq@public.gmane.org","sentAt":"2012-05-07T21:37:52Z","receivedAt":"2012-05-07T21:37:52Z","isPatch":true,"sender":{"key":"peff-adepduraxsq@public.gmane.org","avatar":null},"body":"On Fri, May 04, 2012 at 10:02:49AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > This patch flips the rules to:\n> >\n> > 1. if the user asked for ref@{0}, always show the index\n> >\n> > 2. if the user asked for ref@{now}, always show the date\n> >\n> > 3. otherwise, we have just \"ref\"; show them counted by\n> > default, but respect the presence of \"--date\" as a clue\n> > that the user wanted them date-based\n> \n> The revision.c parser for \"git log --date=default -g master\" would flip\n> the \"explicit\" bit, revs->date_mode is set to DATE_NORMAL, and that value\n> will eventually come as dmode here.\n> [...]\n> But DATE_NORMAL happens to be zero ;-) \"git log --date=default -g master\"\n> would still show the counted version.\n\nYeah, I noticed that, but decided not to tackle it, as nobody had really\ncomplained (and you can get the behavior you want with master@{now}).\nHowever, I agree it would be better for \"--date=default\" to trigger the\ndate-based selector.\n\n> I personally do not care about that behaviour, but I know that I will\n> later later have to deal with people who do care, which is annoying.\n\nMaybe. It has been that way for years and nobody has yet complained. :)\n\n> Probably we would internally need to define two values to ask for the\n> DATE_NORMAL output. Move DATE_NORMAL to non-zero value, introduce a new\n> DATE_DEFAULT that is zero, and make their output identical, perhaps\n> something like the attached (not even compile tested).\n\nI think that is the right way forward. I am worried that we will end up\nwith parts of the code that do not handle the distinction properly (see\nbelow). But maybe it is best to try it and shake the bugs out.\n\n> The implicit comparison to zero in the above is a bad code (but that is\n> a problem from the very old days).\n\nIt is. The enum at least explicitly starts at 0 for this reason, but I\ndon't mind at all if it is updated to an explicit '!= DATE_NORMAL'.\n\n> diff --git a/cache.h b/cache.h\n> index 58ff054..fe42e80 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -876,7 +876,8 @@ extern struct object *peel_to_type(const char *name, int namelen,\n> \t\t\t\t struct object *o, enum object_type);\n> \n> enum date_mode {\n> -\tDATE_NORMAL = 0,\n> +\tDATE_DEFAULT = 0,\n> +\tDATE_NORMAL,\n> \tDATE_RELATIVE,\n> \tDATE_SHORT,\n> \tDATE_LOCAL,\n> diff --git a/reflog-walk.c b/reflog-walk.c\n> index b974258..d002516 100644\n> --- a/reflog-walk.c\n> +++ b/reflog-walk.c\n> @@ -277,7 +277,7 @@ void get_reflog_selector(struct strbuf *sb,\n> \n> \tstrbuf_addf(sb, \"%s@{\", printed_ref);\n> \tif (commit_reflog->selector == SELECTOR_DATE ||\n> -\t (commit_reflog->selector == SELECTOR_NONE && dmode)) {\n> +\t (commit_reflog->selector == SELECTOR_NONE && (dmode != DATE_DEFAULT))) {\n> \t\tinfo = &commit_reflog->reflogs->items[commit_reflog->recno+1];\n> \t\tstrbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode));\n> \t} else {\n\nI think some of the callers set dmode to DATE_NORMAL explicitly. So this\ncode would be confused into thinking that the user had asked for it\nexplicitly. Or maybe it happens before the date_mode_explicit check, and\nit would be OK. I'd have to do audit the code.\n\n-Peff\n"},{"id":"191309","messageId":"20120510153754.GA23941@sigill.intra.peff.net","threadId":"30352","inReplyTo":"20120507213752.GA19911-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org","subject":"Re: [PATCH 4/4] reflog-walk: always make HEAD@{0} show indexed selectors","fromName":"Jeff King","fromEmail":"peff-adepduraxsq@public.gmane.org","sentAt":"2012-05-10T15:37:54Z","receivedAt":"2012-05-10T15:37:54Z","isPatch":true,"sender":{"key":"peff-adepduraxsq@public.gmane.org","avatar":null},"body":"On Mon, May 07, 2012 at 05:37:52PM -0400, Jeff King wrote:\n\n> > \tstrbuf_addf(sb, \"%s@{\", printed_ref);\n> > \tif (commit_reflog->selector == SELECTOR_DATE ||\n> > -\t (commit_reflog->selector == SELECTOR_NONE && dmode)) {\n> > +\t (commit_reflog->selector == SELECTOR_NONE && (dmode != DATE_DEFAULT))) {\n> > \t\tinfo = &commit_reflog->reflogs->items[commit_reflog->recno+1];\n> > \t\tstrbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode));\n> > \t} else {\n> \n> I think some of the callers set dmode to DATE_NORMAL explicitly. So this\n> code would be confused into thinking that the user had asked for it\n> explicitly. Or maybe it happens before the date_mode_explicit check, and\n> it would be OK. I'd have to do audit the code.\n\nI just took a look at what you built on top of this topic (55ccf85)\ninstead of the bit quoted above. I also found it ugly not to pass the\nexplicit flag all the way down to the point-of-use. I had a nagging\nfeeling that the original did not do it that way for some good reason,\nbut looking at your patch, I cannot fathom what that reason could\npossibly be. So it looks good to me.\n\n-Peff\n\nPS It would have been nice to see the patch on the list for review. I\n only noticed it because it hit 'next', and had a minor conflict with\n my patches in the area.\n"},{"id":"191318","messageId":"7vd36cng6n.fsf@alter.siamese.dyndns.org","threadId":"30352","inReplyTo":"20120510153754.GA23941-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org","subject":"Re: [PATCH 4/4] reflog-walk: always make HEAD@{0} show indexed selectors","fromName":"Junio C Hamano","fromEmail":"gitster-e+axbwqsrlaavxtiumwx3w@public.gmane.org","sentAt":"2012-05-10T16:39:44Z","receivedAt":"2012-05-10T16:39:44Z","isPatch":true,"sender":{"key":"gitster-e+axbwqsrlaavxtiumwx3w@public.gmane.org","avatar":null},"body":"Jeff King writes:\n\n> PS It would have been nice to see the patch on the list for review. I\n> only noticed it because it hit 'next', and had a minor conflict with\n> my patches in the area.\n\nHeh, it was sent before I gave you this message:\n\n From: Junio C Hamano \n Subject: Re: [PATCH 4/4] reflog-walk: always make HEAD@{0} show indexed selectors\n To: Jeff King \n Date: Mon, 07 May 2012 14:54:24 -0700\n\n Jeff King writes:\n\n > I think some of the callers set dmode to DATE_NORMAL explicitly. So this\n > code would be confused into thinking that the user had asked for it\n > explicitly. Or maybe it happens before the date_mode_explicit check, and\n > it would be OK. I'd have to do audit the code.\n\n Yeah, that is why today's update I sent does not use DATE_DEFAULT, which\n is after all a hack to piggy-back logically a separate bit in the same\n variable. What we are trying to tell these two functions are (1) does the\n caller prefer to use counted notation or dated notation? and (2) if the\n output shows the timestamp, what format should be used.\n\nThe message \"today's update I sent\" refers to is this.\n\nI suspect that the original thread came from a message cross-posted to\nmagit list whose participants somehow seem to mangle e-mail addresses when\ninteracting with gmane, and it ended up mangling our e-mail addresses but\nthe e-mail relay at gmane probably dropped the messages.\n\n-- >8 --\n\nJunio C Hamano writes:\n\n Administrivia: these gmane-mangled e-mail addresses are extremely\n annoying. Please do not cross post with any insane list that choose\n to turn that feature on when sending message to the git list; thanks.\n\n> Jeff King writes:\n>\n>> This patch flips the rules to:\n>>\n>> 1. if the user asked for ref@{0}, always show the index\n>>\n>> 2. if the user asked for ref@{now}, always show the date\n>>\n>> 3. otherwise, we have just \"ref\"; show them counted by\n>> default, but respect the presence of \"--date\" as a clue\n>> that the user wanted them date-based\n>\n> The revision.c parser for \"git log --date=default -g master\" would flip\n> the \"explicit\" bit, revs->date_mode is set to DATE_NORMAL, and that value\n> will eventually come as dmode here.\n> ...\n> But DATE_NORMAL happens to be zero ;-) \"git log --date=default -g master\"\n> would still show the counted version.\n>\n> I personally do not care about that behaviour, but I know that I will\n> later later have to deal with people who do care, which is annoying.\n\nHow about doing it this way? After all, we are internally holding a bit\nbut the problem is that bit is lost near the tip of the callchain.\n\nThe two integer arguments to get_reflog_selector() and the two integer\narguments to show_reflog_message() might want to become two bits in one\n\"unsigned flags\", but the former wants \"do we want date? do we want a\nshort output?\" two bits, while the latter wants \"do we want date? do we\nwant a oneline output?\", so I didn't bother squashing them into one flag\nword with three-bit assigned. Perhaps we should, but I dunno.\n\n-- >8 --\nSubject: [PATCH] reflog-walk: tell --date=default from not having --date at all\n\nIntroduction of opt->date_mode_explicit was a step in the right direction,\nbut lost that crucial bit at the very end of the callchain, and the callee\ncould not tell an explicitly specified \"I want *date* but in default format\"\nfrom the built-in default value passed when there was no --date specified.\n\nSigned-off-by: Junio C Hamano \n---\n log-tree.c | 7 +++----\n pretty.c | 5 ++---\n reflog-walk.c | 8 ++++----\n reflog-walk.h | 4 ++--\n t/t1411-reflog-show.sh | 8 ++++----\n 5 files changed, 15 insertions(+), 17 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 5f9e59a..588117e 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -493,10 +493,9 @@ void show_log(struct rev_info *opt)\n \t\t\t * graph info here.\n \t\t\t */\n \t\t\tshow_reflog_message(opt->reflog_info,\n-\t\t\t\t opt->commit_format == CMIT_FMT_ONELINE,\n-\t\t\t\t opt->date_mode_explicit ?\n-\t\t\t\t\topt->date_mode :\n-\t\t\t\t\tDATE_NORMAL);\n+\t\t\t\t\t opt->commit_format == CMIT_FMT_ONELINE,\n+\t\t\t\t\t opt->date_mode,\n+\t\t\t\t\t opt->date_mode_explicit);\n \t\t\tif (opt->commit_format == CMIT_FMT_ONELINE)\n \t\t\t\treturn;\n \t\t}\ndiff --git a/pretty.c b/pretty.c\nindex efd62e8..25944de 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -956,9 +956,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \t\t\tif (c->pretty_ctx->reflog_info)\n \t\t\t\tget_reflog_selector(sb,\n \t\t\t\t\t\t c->pretty_ctx->reflog_info,\n-\t\t\t\t\t\t c->pretty_ctx->date_mode_explicit ?\n-\t\t\t\t\t\t c->pretty_ctx->date_mode :\n-\t\t\t\t\t\t DATE_NORMAL,\n+\t\t\t\t\t\t c->pretty_ctx->date_mode,\n+\t\t\t\t\t\t c->pretty_ctx->date_mode_explicit,\n \t\t\t\t\t\t (placeholder[1] == 'd'));\n \t\t\treturn 2;\n \t\tcase 's':\t/* reflog message */\ndiff --git a/reflog-walk.c b/reflog-walk.c\nindex b84e80f..0c904fb 100644\n--- a/reflog-walk.c\n+++ b/reflog-walk.c\n@@ -252,7 +252,7 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)\n \n void get_reflog_selector(struct strbuf *sb,\n \t\t\t struct reflog_walk_info *reflog_info,\n-\t\t\t enum date_mode dmode,\n+\t\t\t enum date_mode dmode, int force_date,\n \t\t\t int shorten)\n {\n \tstruct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;\n@@ -273,7 +273,7 @@ void get_reflog_selector(struct strbuf *sb,\n \n \tstrbuf_addf(sb, \"%s@{\", printed_ref);\n \tif (commit_reflog->selector == SELECTOR_DATE ||\n-\t (commit_reflog->selector == SELECTOR_NONE && dmode)) {\n+\t (commit_reflog->selector == SELECTOR_NONE && force_date)) {\n \t\tinfo = &commit_reflog->reflogs->items[commit_reflog->recno+1];\n \t\tstrbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode));\n \t} else {\n@@ -302,7 +302,7 @@ void get_reflog_message(struct strbuf *sb,\n }\n \n void show_reflog_message(struct reflog_walk_info *reflog_info, int oneline,\n-\tenum date_mode dmode)\n+\t\t\t enum date_mode dmode, int force_date)\n {\n \tif (reflog_info && reflog_info->last_commit_reflog) {\n \t\tstruct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;\n@@ -310,7 +310,7 @@ void show_reflog_message(struct reflog_walk_info *reflog_info, int oneline,\n \t\tstruct strbuf selector = STRBUF_INIT;\n \n \t\tinfo = &commit_reflog->reflogs->items[commit_reflog->recno+1];\n-\t\tget_reflog_selector(&selector, reflog_info, dmode, 0);\n+\t\tget_reflog_selector(&selector, reflog_info, dmode, force_date, 0);\n \t\tif (oneline) {\n \t\t\tprintf(\"%s: %s\", selector.buf, info->message);\n \t\t}\ndiff --git a/reflog-walk.h b/reflog-walk.h\nindex 7bd2cd4..3adccb0 100644\n--- a/reflog-walk.h\n+++ b/reflog-walk.h\n@@ -11,12 +11,12 @@ extern int add_reflog_for_walk(struct reflog_walk_info *info,\n extern void fake_reflog_parent(struct reflog_walk_info *info,\n \t\tstruct commit *commit);\n extern void show_reflog_message(struct reflog_walk_info *info, int,\n-\t\tenum date_mode);\n+\t\t\t\tenum date_mode, int force_date);\n extern void get_reflog_message(struct strbuf *sb,\n \t\tstruct reflog_walk_info *reflog_info);\n extern void get_reflog_selector(struct strbuf *sb,\n \t\tstruct reflog_walk_info *reflog_info,\n-\t\tenum date_mode dmode,\n+\t\tenum date_mode dmode, int force_date,\n \t\tint shorten);\n \n #endif\ndiff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh\nindex 7d9b5e3..9a105fe 100755\n--- a/t/t1411-reflog-show.sh\n+++ b/t/t1411-reflog-show.sh\n@@ -73,20 +73,20 @@ test_expect_success 'using @{now} syntax shows reflog date (format=%gd)' '\n '\n \n cat >expect <<'EOF'\n-Reflog: HEAD@{1112911993 -0700} (C O Mitter )\n+Reflog: HEAD@{Thu Apr 7 15:13:13 2005 -0700} (C O Mitter )\n Reflog message: commit (initial): one\n EOF\n test_expect_success 'using --date= shows reflog date (multiline)' '\n-\tgit log -g -1 --date=raw >tmp &&\n+\tgit log -g -1 --date=default >tmp &&\n \tgrep ^Reflog actual &&\n \ttest_cmp expect actual\n '\n \n cat >expect <<'EOF'\n-e46513e HEAD@{1112911993 -0700}: commit (initial): one\n+e46513e HEAD@{Thu Apr 7 15:13:13 2005 -0700}: commit (initial): one\n EOF\n test_expect_success 'using --date= shows reflog date (oneline)' '\n-\tgit log -g -1 --oneline --date=raw >actual &&\n+\tgit log -g -1 --oneline --date=default >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n1.7.10.1.500.g37b1e9a\n"},{"id":"191327","messageId":"20120510171912.GA29972@sigill.intra.peff.net","threadId":"30352","inReplyTo":"7vd36cng6n.fsf-s2KvWo2KEQL18tm6hw+yZpy9Z0UEorGK@public.gmane.org","subject":"OT: gmane address mangling selectors","fromName":"Jeff King","fromEmail":"peff-adepduraxsq@public.gmane.org","sentAt":"2012-05-10T17:19:12Z","receivedAt":"2012-05-10T17:19:12Z","isPatch":false,"sender":{"key":"peff-adepduraxsq@public.gmane.org","avatar":null},"body":"On Thu, May 10, 2012 at 09:39:44AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > PS It would have been nice to see the patch on the list for review. I\n> > only noticed it because it hit 'next', and had a minor conflict with\n> > my patches in the area.\n> \n> Heh, it was sent before I gave you this message:\n\nAh, I never got that message. Probably because...\n\n> To: Jeff King \n\n...I have no clue where that address would end up.\n\n> Junio C Hamano writes:\n> \n> Administrivia: these gmane-mangled e-mail addresses are extremely\n> annoying. Please do not cross post with any insane list that choose\n> to turn that feature on when sending message to the git list; thanks.\n\nWhere do they come from? I notice that my address has been mangled\nabove, but I do not use gmane to either post or read the list. In fact,\nlooking at the start of the thread in my mailbox, I see:\n\n 1. Eli posts[1], to: git@vger and magit@googlegroups; his from address\n looks normal, and there is no reply-to.\n\n 2. You reply, to: a gmane-mangled address, cc: git@vger.\n\nThe presence of the magit list is obviously the unusual thing here, but\nhe did not involve gmane at all. If I recall correctly, you read the\nlist via gmane. So I believe it is your workflow that introduces the\nmangled addresses on the reading end, not the sender.\n\nIt seems that this gmane \"feature\" is turned on by the presence of the\nmagit list, which I guess is configured at gmane to obfuscate emails.\nBut I don't see how the sender should be expected to know or care about\nthis gmane nonsense. It is your fault that the gateway through which you\nread the messages is doing the mangling. So the right fix is not to\narbitrarily restrict cc-ing of other lists, but to fix the gmane bug[2].\n\n-Peff\n\n[1] mid:<20379.9312.943088.350379-a5nvgYPMCZcx/1z6v04GWfZ8FUJU4vz8@public.gmane.org>; the gmane link\n at http://thread.gmane.org/gmane.comp.version-control.git.magit/1308\n is mangled, but the original message as sent by vger looks fine (a\n copy of the headers that I received is included below).\n\n[2] Without looking at the gmane code at all, I suspect the faulty logic\n is \"mangle addresses for privacy if the message went to any list\n which requests this feature\". But that is not right. The privacy was\n lost as soon as it went to any list that does _not_ mangle. And the\n unmangled version should be given to people accessing the sane list\n (it is slightly pointless to still mangle for the other list, since\n the privacy has been lost, but at least it is not actively bad). But\n given that my search for the mid at gmane turns up the magit\n version, I am guessing that they are storing only a single version\n of the message (mangled).\n\n-- >8 --\nReturn-Path: \nDelivered-To: peff-AdEPDUrAXsQ@public.gmane.org\nReceived: (qmail 2012 invoked by uid 107); 28 Apr 2012 00:03:12 -0000\nX-Spam-Level: *\nX-Spam-Status: No, hits=-3.4 required=4.9\n\ttests=BAYES_00,KB_DATE_CONTAINS_TAB,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,TAB_IN_FROM\nX-Spam-Check-By: peff.net\nReceived: from vger.kernel.org (HELO vger.kernel.org) (209.132.180.67)\n by peff.net (qpsmtpd/0.84) with ESMTP; Fri, 27 Apr 2012 20:03:07 -0400\nReceived: (majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org) by vger.kernel.org via listexpand\n\tid S1758065Ab2D1ACs (ORCPT );\n\tFri, 27 Apr 2012 20:02:48 -0400\nReceived: from winooski.ccs.neu.edu ([129.10.115.117]:52945 \"EHLO\n\twinooski.ccs.neu.edu\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1757659Ab2D1ACg (ORCPT );\n\tFri, 27 Apr 2012 20:02:36 -0400\nX-Greylist: delayed 3897 seconds by postgrey-1.27 at vger.kernel.org; Fri, 27 Apr 2012 20:02:36 EDT\nReceived: from winooski.ccs.neu.edu (localhost.localdomain [127.0.0.1])\n\tby winooski.ccs.neu.edu (8.14.4/8.14.4) with ESMTP id q3RMvbEt019273;\n\tFri, 27 Apr 2012 18:57:37 -0400\nReceived: (from eli@localhost)\n\tby winooski.ccs.neu.edu (8.14.4/8.14.4/Submit) id q3RMvbHJ019269;\n\tFri, 27 Apr 2012 18:57:37 -0400\nFrom:\tEli Barzilay \nMIME-Version: 1.0\nContent-Type: text/plain; charset=us-ascii\nContent-Transfer-Encoding: 7bit\nMessage-ID: <20379.9312.943088.350379-a5nvgYPMCZcx/1z6v04GWfZ8FUJU4vz8@public.gmane.org>\nDate:\tFri, 27 Apr 2012 18:57:36 -0400\nTo:\tgit-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, magit-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org\nSubject: Bug in git-stash(.sh) ?\nX-Mailer: VM 8.2.0a under 23.2.1 (x86_64-redhat-linux-gnu)\nSender:\tgit-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org\nPrecedence: bulk\nList-ID: \nX-Mailing-List:\tgit-u79uwXL29TY76Z2rM5mHXA@public.gmane.org\nStatus: RO\nContent-Length: 3206\nLines: 100\n"},{"id":"191329","messageId":"20395.64632.583415.101007@winooski.ccs.neu.edu","threadId":"30352","inReplyTo":"20120510171912.GA29972-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org","subject":"Re: OT: gmane address mangling selectors","fromName":"Eli Barzilay","fromEmail":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","sentAt":"2012-05-10T17:35:52Z","receivedAt":"2012-05-10T17:35:52Z","isPatch":false,"sender":{"key":"eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org","avatar":null},"body":"A few minutes ago, Jeff King wrote:\n> \n> The presence of the magit list is obviously the unusual thing here,\n> but he did not involve gmane at all. If I recall correctly, you read\n> the list via gmane. So I believe it is your workflow that introduces\n> the mangled addresses on the reading end, not the sender.\n\nIIRC, gmane intercepts a first-time post to a newsgroup->mailing list\nusing a mangled email that it gets and later forwards on your behalf\nwhen you prove that you're human. Or something like that. The weird\nthing is that it did that for a personal email -- maybe someone posted\na reply with CCs, where the usual thing is to have only newsgroups?\n\n-- \n ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"}]}