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

From: Eli Barzilay <eli-osk4jvrjlyzg9huczpvpmw@public.gmane.org>
Date: Apr 29, 2012, 22:26 UTC
Message-ID: <20381.49180.329586.983166@winooski.ccs.neu.edu>
In-Reply-To: <20120429220132.GB4491-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org>

A few minutes ago, Jeff King wrote:

Show 18 quoted lines

> On Sat, Apr 28, 2012 at 07:59:37PM -0400, Eli Barzilay wrote:
> 
> > > How exactly do you make magit generate these calls?  AFAICT, Magit
> > > should operate on whatever "git stash list" outputs, meaning
> > > stash@{N}. So I guess I'm missing something.
> > 
> > ... right: the offending configuration I had was log.date = iso.  This
> > calls for a simple chane for git-stash.sh to use `--date default':
> > 
> > 	git log --date default --format="%gd: %gs" -g "$@" $ref_stash --
> 
> I seem to remember dealing with this once a long time ago. And while
> "--date=default" works, it is papering over the symptom of a larger
> problem, which is that "log" should not use a non-commandline date
> to make the stash selector decision. Searching turned up this
> discussion:
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/128569

Ah, that looks like almost exactly the problem I started with...

Show 14 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. 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 useless
  * 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)"

With this the output has the number independent of log.date setting, and I get a --format if I want to see something else, which makes more sense than --date being explicit or not. IOW, I'd expect this:

>   git stash list --date=iso

to not have any effect.

This is not a backwards compatible change, but my guess is that existing uses of %g[dD] are suffering from a similar problem anyway. (So another option maybe making %gd use the number and something else for the date version.)

(But my opinion is of course limited to my short encounter with all of this...)

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!