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

[Junio C Hamano <gitster-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>]

Jeff King <peff-AdEPDUrAXsQ@public.gmane.org> writes:

> 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.

Heh, it was sent before I gave you this message:

        From: Junio C Hamano <gitster-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
        Subject: Re: [PATCH 4/4] reflog-walk: always make HEAD@{0} show indexed selectors
        To: Jeff King <peff-AdEPDUrAXsQ-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org>
        Date: Mon, 07 May 2012 14:54:24 -0700

        Jeff King <peff-AdEPDUrAXsQ-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org> writes:

        > 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.

        Yeah, that is why today's update I sent does not use DATE_DEFAULT, which
        is after all a hack to piggy-back logically a separate bit in the same
        variable.  What we are trying to tell these two functions are (1) does the
        caller prefer to use counted notation or dated notation?  and (2) if the
        output shows the timestamp, what format should be used.

The message "today's update I sent" refers to is this.

I suspect that the original thread came from a message cross-posted to
magit list whose participants somehow seem to mangle e-mail addresses when
interacting with gmane, and it ended up mangling our e-mail addresses but
the e-mail relay at gmane probably dropped the messages.

-- >8 --

Junio C Hamano <gitster-e+AXbWqSrlAAvxtiuMwx3w-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org> writes:

    Administrivia: these gmane-mangled e-mail addresses are extremely
    annoying.  Please do not cross post with any insane list that choose
    to turn that feature on when sending message to the git list; thanks.

> Jeff King <peff-AdEPDUrAXsQ-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org> writes:
>
>> This patch flips the rules to:
>>
>>   1. if the user asked for ref@{0}, always show the index
>>
>>   2. if the user asked for ref@{now}, always show the date
>>
>>   3. otherwise, we have just "ref"; show them counted by
>>      default, but respect the presence of "--date" as a clue
>>      that the user wanted them date-based
>
> The revision.c parser for "git log --date=default -g master" would flip
> the "explicit" bit, revs->date_mode is set to DATE_NORMAL, and that value
> will eventually come as dmode here.
> ...
> But DATE_NORMAL happens to be zero ;-) "git log --date=default -g master"
> would still show the counted version.
>
> I personally do not care about that behaviour, but I know that I will
> later later have to deal with people who do care, which is annoying.

How about doing it this way?  After all, we are internally holding a bit
but the problem is that bit is lost near the tip of the callchain.

The two integer arguments to get_reflog_selector() and the two integer
arguments to show_reflog_message() might want to become two bits in one
"unsigned flags", but the former wants "do we want date?  do we want a
short output?" two bits, while the latter wants "do we want date?  do we
want a oneline output?", so I didn't bother squashing them into one flag
word with three-bit assigned.  Perhaps we should, but I dunno.

-- >8 --
Subject: [PATCH] reflog-walk: tell --date=default from not having --date at all

Introduction of opt->date_mode_explicit was a step in the right direction,
but lost that crucial bit at the very end of the callchain, and the callee
could not tell an explicitly specified "I want *date* but in default format"
from the built-in default value passed when there was no --date specified.

Signed-off-by: Junio C Hamano <gitster-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 log-tree.c             | 7 +++----
 pretty.c               | 5 ++---
 reflog-walk.c          | 8 ++++----
 reflog-walk.h          | 4 ++--
 t/t1411-reflog-show.sh | 8 ++++----
 5 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 5f9e59a..588117e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -493,10 +493,9 @@ void show_log(struct rev_info *opt)
 			 * graph info here.
 			 */
 			show_reflog_message(opt->reflog_info,
-				    opt->commit_format == CMIT_FMT_ONELINE,
-				    opt->date_mode_explicit ?
-					opt->date_mode :
-					DATE_NORMAL);
+					    opt->commit_format == CMIT_FMT_ONELINE,
+					    opt->date_mode,
+					    opt->date_mode_explicit);
 			if (opt->commit_format == CMIT_FMT_ONELINE)
 				return;
 		}
diff --git a/pretty.c b/pretty.c
index efd62e8..25944de 100644
--- a/pretty.c
+++ b/pretty.c
@@ -956,9 +956,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 			if (c->pretty_ctx->reflog_info)
 				get_reflog_selector(sb,
 						    c->pretty_ctx->reflog_info,
-						    c->pretty_ctx->date_mode_explicit ?
-						      c->pretty_ctx->date_mode :
-						      DATE_NORMAL,
+						    c->pretty_ctx->date_mode,
+						    c->pretty_ctx->date_mode_explicit,
 						    (placeholder[1] == 'd'));
 			return 2;
 		case 's':	/* reflog message */
diff --git a/reflog-walk.c b/reflog-walk.c
index b84e80f..0c904fb 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -252,7 +252,7 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 
 void get_reflog_selector(struct strbuf *sb,
 			 struct reflog_walk_info *reflog_info,
-			 enum date_mode dmode,
+			 enum date_mode dmode, int force_date,
 			 int shorten)
 {
 	struct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;
@@ -273,7 +273,7 @@ void get_reflog_selector(struct strbuf *sb,
 
 	strbuf_addf(sb, "%s@{", printed_ref);
 	if (commit_reflog->selector == SELECTOR_DATE ||
-	    (commit_reflog->selector == SELECTOR_NONE && dmode)) {
+	    (commit_reflog->selector == SELECTOR_NONE && force_date)) {
 		info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
 		strbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode));
 	} else {
@@ -302,7 +302,7 @@ void get_reflog_message(struct strbuf *sb,
 }
 
 void show_reflog_message(struct reflog_walk_info *reflog_info, int oneline,
-	enum date_mode dmode)
+			 enum date_mode dmode, int force_date)
 {
 	if (reflog_info && reflog_info->last_commit_reflog) {
 		struct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;
@@ -310,7 +310,7 @@ void show_reflog_message(struct reflog_walk_info *reflog_info, int oneline,
 		struct strbuf selector = STRBUF_INIT;
 
 		info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
-		get_reflog_selector(&selector, reflog_info, dmode, 0);
+		get_reflog_selector(&selector, reflog_info, dmode, force_date, 0);
 		if (oneline) {
 			printf("%s: %s", selector.buf, info->message);
 		}
diff --git a/reflog-walk.h b/reflog-walk.h
index 7bd2cd4..3adccb0 100644
--- a/reflog-walk.h
+++ b/reflog-walk.h
@@ -11,12 +11,12 @@ extern int add_reflog_for_walk(struct reflog_walk_info *info,
 extern void fake_reflog_parent(struct reflog_walk_info *info,
 		struct commit *commit);
 extern void show_reflog_message(struct reflog_walk_info *info, int,
-		enum date_mode);
+				enum date_mode, int force_date);
 extern void get_reflog_message(struct strbuf *sb,
 		struct reflog_walk_info *reflog_info);
 extern void get_reflog_selector(struct strbuf *sb,
 		struct reflog_walk_info *reflog_info,
-		enum date_mode dmode,
+		enum date_mode dmode, int force_date,
 		int shorten);
 
 #endif
diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh
index 7d9b5e3..9a105fe 100755
--- a/t/t1411-reflog-show.sh
+++ b/t/t1411-reflog-show.sh
@@ -73,20 +73,20 @@ test_expect_success 'using @{now} syntax shows reflog date (format=%gd)' '
 '
 
 cat >expect <<'EOF'
-Reflog: HEAD@{1112911993 -0700} (C O Mitter <committer-hcDgGtZH8xNBDgjK7y7TUQ@public.gmane.org>)
+Reflog: HEAD@{Thu Apr 7 15:13:13 2005 -0700} (C O Mitter <committer-hcDgGtZH8xNBDgjK7y7TUQ@public.gmane.org>)
 Reflog message: commit (initial): one
 EOF
 test_expect_success 'using --date= shows reflog date (multiline)' '
-	git log -g -1 --date=raw >tmp &&
+	git log -g -1 --date=default >tmp &&
 	grep ^Reflog <tmp >actual &&
 	test_cmp expect actual
 '
 
 cat >expect <<'EOF'
-e46513e HEAD@{1112911993 -0700}: commit (initial): one
+e46513e HEAD@{Thu Apr 7 15:13:13 2005 -0700}: commit (initial): one
 EOF
 test_expect_success 'using --date= shows reflog date (oneline)' '
-	git log -g -1 --oneline --date=raw >actual &&
+	git log -g -1 --oneline --date=default >actual &&
 	test_cmp expect actual
 '
 
-- 
1.7.10.1.500.g37b1e9a