Re: [PATCH] refs: allow @{n} to work with n-sized reflog

[Junio C Hamano <gitster@pobox.com>]

Denton Liu <liu.denton@gmail.com> writes:

>> > -	if (timestamp <= cb->at_time || cb->cnt == 0) {
>> > +	if (cb->cnt > 0)
>> > +		cb->cnt--;
>> > +	at_indexed_ent = cb->cnt == 0 && !is_null_oid(ooid);
>> 
>> The code treats two cases identically (i.e. the case where cb->cnt
>> was originally zero, and one).  Is that intended?
>
> It shouldn't be possible for cb->cnt == 0 on the first iteration
> because there's a special-case check at [0]. As a result, it can only be
> -1 or >= 1 on the first iteration.
>
> The -1 case happens when we're doing date-based lookup and that's what
> this if is intended to handle.

I knew about -1; it wasn't apparent that the caller won't call us
with cnt==0.  Perhaps it deserves a mention in an in-code comment.

> "at_indexed_ent" is meant to signal when we are indexing the reflog
> numerically (as opposed to by date), we have arrived at the correct
> entry. If you have a more fitting name, I'm open to suggestions.

When querying for <ref>@{24}, all the entries are indexed
numerically (counted), not just the 24th one, and that contributed
to my puzzlement.

I offhand do not think of a "name", but "at target", "found",
"reached count", are phrases that come to my mind as starting
points.

Thanks.