Re: [PATCH 4/4] reflog-walk: always make HEAD@{0} show indexed selectors

On Mon, May 07, 2012 at 05:37:52PM -0400, Jeff King wrote:

quoted

 	strbuf_addf(sb, "%s@{", printed_ref);
 	if (commit_reflog->selector == SELECTOR_DATE ||
-	    (commit_reflog->selector == SELECTOR_NONE && dmode)) {
+	    (commit_reflog->selector == SELECTOR_NONE && (dmode != DATE_DEFAULT))) {
 		info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
 		strbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode));
 	} else {

I think some of the callers set dmode to DATE_NORMAL explicitly. So this
code would be confused into thinking that the user had asked for it
explicitly. Or maybe it happens before the date_mode_explicit check, and
it would be OK. I'd have to do audit the code.

I just took a look at what you built on top of this topic (55ccf85)
instead of the bit quoted above. I also found it ugly not to pass the
explicit flag all the way down to the point-of-use. I had a nagging
feeling that the original did not do it that way for some good reason,
but looking at your patch, I cannot fathom what that reason could
possibly be. So it looks good to me.

-Peff

PS It would have been nice to see the patch on the list for review. I
   only noticed it because it hit 'next', and had a minor conflict with
   my patches in the area.