On Mon, May 07, 2012 at 05:37:52PM -0400, Jeff King wrote:
> > 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html