Re: RFH: unexpected reflog behavior with --since=

On Thu, Nov 10, 2011 at 12:04:38AM -0800, Eric Raible wrote:

quoted

I think I am leaning towards the latter. It seems to me to be the more
likely guess for what the user would want. And there is real benefit to
doing it in git, since we can stop the traversal early. In the
"grep-like" case, doing it inside git is not really any more efficient
than filtering in a pipeline, like:

  git log -g --format='%ct %H' |
  awk '{ print $2 if $1 < SOME_TIMESTAMP }'

And then the sha would have to be fed back into git to be useful, eh?

It's just illustrative. You could replace "%H" with the actual
information you're interested in.

quoted

Of course we could still offer both (with a "--reflog-since" type of
option). We'd also need to turn off the optimization for "--since", and
then check whether "--until" has a similar bug (and offer
"--reflog-until").

I don't see the point of --reflog-since.  If the user specifies 'reflog'
(either directly or with -g), then can't we just use the reflog's timestamp?
Note: there might be good reasons, as my use of the reflog (and --since, for
that matter), has been very simplistic so far.

The only point would be to leave "--since" to act on the commit
timestamps, so that you don't have to resort to the external grepping I
mentioned above. However, I'm not convinced anybody even cares about
that use case.

I think the behavior you want is much more sensible.

quoted

diff --git a/reflog-walk.c b/reflog-walk.c
index 5d81d39..2e5b270 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c

@@ -231,6 +231,7 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 	reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
 	info->last_commit_reflog = commit_reflog;
 	commit_reflog->recno--;
+	commit->date = reflog->timestamp;
 	commit_info->commit = (struct commit *)parse_object(reflog->osha1);
 	if (!commit_info->commit) {
 		commit->parents = NULL;

Is this something you'd be willing to turn into a real patch?
I'm certainly not qualified.

Yes. We're in release freeze now, so I didn't even bother with sending
it to Junio. But also, I'd like to gather more opinions on whether the
design is the right thing (hopefully the implementation is Obviously
Correct. :) ).

-Peff