git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff-AdEPDUrAXsQ@public.gmane.org>
To: Junio C Hamano <gitster-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
Cc: Eli Barzilay <eli-oSK4jVRJLyZg9hUCZPvPmw@public.gmane.org>,
	Thomas Rast <trast-oe7qfRrRQfdfcPYw/2PL0g@public.gmane.org>,
	Yann Hodique
	<yann.hodique-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Andreas Schwab <schwab-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>,
	git-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	magit-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
Subject: Re: Bug in git-stash(.sh) ?
Date: Tue, 1 May 2012 11:02:11 -0400	[thread overview]
Message-ID: <20120501150211.GA14185@sigill.intra.peff.net> (raw)
In-Reply-To: <7vlilexkcq.fsf-s2KvWo2KEQL18tm6hw+yZpy9Z0UEorGK@public.gmane.org>

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

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

> 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

  parent reply	other threads:[~2012-05-01 15:02 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-27 22:57 Bug in git-stash(.sh) ? Eli Barzilay
2012-04-27 23:02 ` Junio C Hamano
2012-04-28  0:16   ` Eli Barzilay
     [not found]     ` <CALO-gut4csy5wef4iGPGD5jVPc1f0iFBfS3MUWrOwc2yczdviw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
     [not found]       ` <m2pqasb8mr.fsf-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
     [not found]         ` <87wr4za9mr.fsf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2012-04-28 23:59           ` Eli Barzilay
2012-04-29 22:01             ` Jeff King
     [not found]               ` <20120429220132.GB4491-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org>
2012-04-29 22:26                 ` Eli Barzilay
2012-05-01 13:42                   ` Jeff King
     [not found]                     ` <20120501134254.GA11900-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org>
2012-05-03 18:44                       ` [git] " Eli Barzilay
     [not found]                         ` <20386.53745.200846.115335-a5nvgYPMCZcx/1z6v04GWfZ8FUJU4vz8@public.gmane.org>
2012-05-04  5:21                           ` Jeff King
2012-05-04  5:26                             ` [PATCH 3/4] reflog-walk: clean up "flag" field of commit_reflog struct Jeff King
     [not found]                             ` <20120504052106.GA15970-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org>
2012-05-04  5:23                               ` [PATCH 1/4] t1411: add more selector index/date tests Jeff King
2012-05-04  5:25                               ` [PATCH 2/4] log: respect date_mode_explicit with --format:%gd Jeff King
2012-05-04  5:27                               ` [PATCH 4/4] reflog-walk: always make HEAD@{0} show indexed selectors Jeff King
     [not found]                                 ` <20120504052725.GD16107-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org>
2012-05-04 17:02                                   ` Junio C Hamano
     [not found]                                     ` <7v7gwrc212.fsf-s2KvWo2KEQL18tm6hw+yZpy9Z0UEorGK@public.gmane.org>
2012-05-07 21:37                                       ` Jeff King
     [not found]                                         ` <20120507213752.GA19911-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org>
2012-05-10 15:37                                           ` Jeff King
     [not found]                                             ` <20120510153754.GA23941-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org>
2012-05-10 16:39                                               ` Junio C Hamano
     [not found]                                                 ` <7vd36cng6n.fsf-s2KvWo2KEQL18tm6hw+yZpy9Z0UEorGK@public.gmane.org>
2012-05-10 17:19                                                   ` OT: gmane address mangling selectors Jeff King
     [not found]                                                     ` <20120510171912.GA29972-bBVMEuqLR+SYVEpFpFwlB0AkDMvbqDRI@public.gmane.org>
2012-05-10 17:35                                                       ` Eli Barzilay
2012-05-04 18:57                               ` [git] Re: Bug in git-stash(.sh) ? Eli Barzilay
     [not found]                                 ` <20388.9885.608325.489624-a5nvgYPMCZcx/1z6v04GWfZ8FUJU4vz8@public.gmane.org>
2012-05-04 22:36                                   ` Eli Barzilay
2012-04-29 22:07             ` Junio C Hamano
     [not found]               ` <7vlilexkcq.fsf-s2KvWo2KEQL18tm6hw+yZpy9Z0UEorGK@public.gmane.org>
2012-04-29 22:37                 ` Eli Barzilay
2012-05-01 15:02                 ` Jeff King [this message]
2012-04-28  7:47 ` Andreas Schwab
2012-04-28 20:23 ` Yann Hodique

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120501150211.GA14185@sigill.intra.peff.net \
    --to=peff-adepduraxsq@public.gmane.org \
    --cc=eli-oSK4jVRJLyZg9hUCZPvPmw@public.gmane.org \
    --cc=git-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=gitster-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org \
    --cc=magit-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org \
    --cc=schwab-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org \
    --cc=trast-oe7qfRrRQfdfcPYw/2PL0g@public.gmane.org \
    --cc=yann.hodique-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).