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

Denton Liu [off-list ref] writes:

quoted

quoted

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