Re: Bug in git-stash(.sh) ?

From: Jeff King <peff-adepduraxsq@public.gmane.org>
Date: May 1, 2012, 15:02 UTC
Message-ID: <20120501150211.GA14185@sigill.intra.peff.net>
In-Reply-To: <7vlilexkcq.fsf-s2KvWo2KEQL18tm6hw+yZpy9Z0UEorGK@public.gmane.org>

On Sun, Apr 29, 2012 at 03:07:49PM -0700, Junio C Hamano wrote:

Show 21 quoted lines

> Eli Barzilay <eli-oSK4jVRJLyZg9hUCZPvPmw@public.gmane.org> writes:
> 
> > ...  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.

Right. My argument is that the context in your third point was always intended to be about command-line options. Respecting the log.date config there is a bug (and not just in breaking intent; it also breaks scriptability). It was fixed for the regular pretty-print code path, but was broken again when the "%gd" code path was added.

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

If that is the case (and I haven't checked either way, but it does not surprise me at all), then I believe that is a separate bug. And we should fix that, too.

Show 6 quoted lines

> 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.
> [...]
> -	git log --format="%gd: %gs" -g "$@" $ref_stash --
> +	git log --format="%gd: %gs" -g "$@" "$ref_stash@{0}" --

That will solve the problem for stash, but the config bug would remain for every _other_ user of "git log -g --format=%gd". So that needs fixed either way.

However, I really wonder if this is the right thing. If I do:

  git stash list --date=relative

isn't it a feature that I get to see the date at which each stash was made? Why are we taking it away? I can see if it were the only way to fix the problem with log.date, but that has another solution. Are people really calling "stash list" with a date on the command line and getting confused by the output? My understanding was that the observed problem was purely a bad interaction with log.date, which should not be respected at all.

-Peff