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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> Eli Barzilay <eli@xxxxxxxxxxxx> 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.

> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]