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

From: Jeff King <peff@peff.net>
To: Eli Barzilay <eli@barzilay.org>
Cc: Yann Hodique <yann.hodique@gmail.com>,
	Andreas Schwab <schwab@linux-m68k.org>,
	Junio C Hamano <gitster@pobox.com>,
	Eli Barzilay <public-eli-oSK4jVRJLyZg9hUCZPvPmw@plane.gmane.org>,
	git@vger.kernel.org,
	Eli Barzilay <public-eli-oSK4jVRJLyZg9hUCZPvPmw@plane.gmane.org>,
	magit@googlegroups.com
Subject: Re: Bug in git-stash(.sh) ?
Date: Tue, 1 May 2012 09:42:55 -0400	[thread overview]
Message-ID: <20120501134254.GA11900@sigill.intra.peff.net> (raw)
In-Reply-To: <20381.49180.329586.983166@winooski.ccs.neu.edu>

On Sun, Apr 29, 2012 at 06:26:36PM -0400, Eli Barzilay wrote:

> > which led to f4ea32f (improve reflog date/number heuristic,
> > 2009-09-24).  That fixed the case of:
> > 
> >   git config log.date iso
> >   git log -g --oneline
> > 
> > But later, 8f8f547 (Introduce new pretty formats %g[sdD] for reflog
> > information, 2009-10-19) added another way to show selectors, and it
> > did not respect the date_mode_explicit flag from f4ea32f. Which I
> > think is a bug.
> > 
> > So the right solution is to pass the date_mode_explicit flag through
> > to the pretty-print --format code, and then pass it along to the
> > reflog code.
> 
> Assuming that I followed all of that correctly, it still seems bogus
> to do that, given that %gd and %gD are described as producing reflog
> selector, and given that Junio's note that stash operations are really
> intended to be used only with these selectos.

Keep in mind this bug is not about stash at all; it is about showing
reflog selectors. Those are a more general mechanism, and are used for
more than just stash. The fact that user config affects the format of
"%gd" is a bug; it should follow the same rules as the regular reflog
pretty-printing (and the behavior of neither should be affected by user
config, as scripts rely on the output being consistent).

Once that is fixed, then we can consider whether something more should
happen for stash (though I am inclined to say that is enough; it is a
feature that you can do "git stash list --date=relative" to see the
stash timestamps).

> What looks more sensible to me given the necessity of %gd (and the
> fact that it's different from %cd/%ad) is to change things as follows:
> 
>   * %gd produces only the date, with the "default" having the same
>     meaning as elsewhere (so it doesn't show the index numbers)

%gd is part of the public interface and will not change its semantics
(or at least not without a long deprecation period).  It's a shame that
"d" is taken for the selector, when it would be better to mean "date" as
it does for author and committer. But I don't know if it's worth
changing at this point.

We could add new placeholders with different semantics, though. When I
added reflog identity placeholders a few months ago, there was a brief
discussion on adding a date placeholder:

  http://article.gmane.org/gmane.comp.version-control.git/185043

but the related work hasn't progressed.

>   * Some new %gi uses the index number: stash@{1}, and %gI produces
>     refs/stash@{1}, unrelated to any date setting
>   * git-stash.sh uses %gi so the output has the numbers
>   * Some new option for "stash list" for the format string, so it's
>     possible to show the dates if you want to with something like
>     git stash list --format:"%gi: %gs (%gd)"

I don't have a huge problem with that. But what issue is it really
solving? Are people using "git stash list --date=iso" and then getting
confused by the output? Or is it simply a matter of mistakenly applying
the config when it should not be? The latter needs fixed in either case.

-Peff