Re: Bug in git-stash(.sh) ?
- From
- Junio C Hamano <gitster-e+axbwqsrlaavxtiumwx3w@public.gmane.org>
- Date
- Apr 29, 2012, 22:07 UTC
- Message-ID
- <7vlilexkcq.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20380.33897.666338.766096@winooski.ccs.neu.edu>
Eli Barzilay <eli-oSK4jVRJLyZg9hUCZPvPmw@public.gmane.org> writes:
Show 9 quoted lines
> ... In any case,
> it is also questionable -- reading the documentation for %gd:
>
> · %gD: reflog selector, e.g., refs/stash@{1}
> · %gd: shortened reflog selector, e.g., stash@{1}
>
> makes it look like the problem is there -- in get_reflog_selector() --
> which has explicit code for showing the dates. (This was done in
> 8f8f5476.)I think the root cause of the bug is that there are three cases:
- If we ask for "log -g ref@{0}", we should show them counted no matter what. - If we ask for "log -g ref@{now}", we should show them timed no matter what.- If we ask for "log -g ref" without specifier, we show them counted by default, but we try to be nice and show them timed when we can infer from other context that the user wanted to see them timed.
An ancient 4e244cb (log --reflog: honour --relative-date, 2007-02-08) was what introduced the "explicit code for showing the dates", but it was done somewhat poorly---it does not differentiate the first and third case.
Once we fix *that* bug, to disable the "timed" codepath altogether when the caller gives "ref@{0}" to explicitly ask for counted output, we can fix it a lot easily.
And the patch to do so should look like this; I'll leave it to the readers to add whatever tests that are appropriate.
git-stash.sh | 2 +- reflog-walk.c | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh index fe4ab28..590c1f3 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -265,7 +265,7 @@ have_stash () { list_stash () { have_stash || return 0 - git log --format="%gd: %gs" -g "$@" $ref_stash -- + git log --format="%gd: %gs" -g "$@" "$ref_stash@{0}" -- } show_stash () { diff --git a/reflog-walk.c b/reflog-walk.c index 86d1884..6fe60a8 100644 --- a/reflog-walk.c +++ b/reflog-walk.c @@ -126,7 +126,10 @@ static void add_commit_info(struct commit *commit, void *util, } struct commit_reflog { - int flag, recno; + int recno; +#define REFLOG_COUNTED 01 +#define REFLOG_TIMED 02 + unsigned flags; struct complete_reflogs *reflogs; }; @@ -150,6 +153,7 @@ int add_reflog_for_walk(struct reflog_walk_info *info, struct complete_reflogs *reflogs; char *branch, *at = strchr(name, '@'); struct commit_reflog *commit_reflog; + unsigned flags = 0; if (commit->object.flags & UNINTERESTING) die ("Cannot walk reflogs for %s", name); @@ -162,6 +166,9 @@ int add_reflog_for_walk(struct reflog_walk_info *info, if (*ep != '}') { recno = -1; timestamp = approxidate(at + 2); + flags = REFLOG_TIMED; + } else { + flags = REFLOG_COUNTED; } } else recno = 0; @@ -199,8 +206,8 @@ int add_reflog_for_walk(struct reflog_walk_info *info, } commit_reflog = xcalloc(sizeof(struct commit_reflog), 1); - if (recno < 0) { - commit_reflog->flag = 1; + commit_reflog->flags = flags; + if (flags & REFLOG_TIMED) { commit_reflog->recno = get_reflog_recno_by_time(reflogs, timestamp); if (commit_reflog->recno < 0) { free(branch); @@ -267,7 +274,8 @@ void get_reflog_selector(struct strbuf *sb, } strbuf_addf(sb, "%s@{", printed_ref); - if (commit_reflog->flag || dmode) { + if ((! (commit_reflog->flags && (REFLOG_COUNTED | REFLOG_TIMED)) && dmode) || + (commit_reflog->flags & REFLOG_TIMED)) { info = &commit_reflog->reflogs->items[commit_reflog->recno+1]; strbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode)); } else {