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

From: Jeff King <peff@peff.net>
Date: May 1, 2012, 13:42 UTC
Message-ID: <20120501134254.GA11900@sigill.intra.peff.net>
In-Reply-To: <20381.49180.329586.983166@winooski.ccs.neu.edu>

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

Show 19 quoted lines

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

Show 5 quoted lines

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

Show 6 quoted lines

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