Re: [PATCH v3 4/6] log: refactor "rev.pending" code in cmd_show()

From: Jeff King <peff@peff.net>
Date: Aug 3, 2022, 17:32 UTC
Message-ID: <YuqxIFkw64zcv+3c@coredump.intra.peff.net>
In-Reply-To: <patch-v3-4.6-fd474666e7c-20220802T152925Z-avarab@gmail.com>

On Tue, Aug 02, 2022 at 05:33:14PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 7 quoted lines

>  * We don't need the "count" and "objects" variables introduced in
>    5d7eeee2ac6 (git-show: grok blobs, trees and tags, too, 2006-12-14).
> 
>    They were originally added since we'd clobber rev.pending in the
>    loop without restoring it. Since the preceding commit we are
>    restoring it when we handle OBJ_COMMIT, so the main for-loop can
>    refer to "rev.pending" didrectly.

I think this is accurate, though it does feel a bit weird that we are iterating over rev.pending, and we clobber and restore it mid-loop.

It's correct because of the restore, but I think that's why my gut feeling favored the earlier approach to completely dissociate the iteration variables from "rev.pending" before the loop even starts.

That said, it seems like we're spending a lot more time going back and forth on this topic than it is really worth, so I can live with any of the versions.

-Peff