Re: [PATCH 2/2] builtin/show-branch: detect empty reflogs

From: Jeff King <peff@peff.net>
Date: Feb 21, 2024, 17:35 UTC
Message-ID: <20240221173545.GB634809@coredump.intra.peff.net>
In-Reply-To: <6107efeffa3a9e87ed95f4fe592d9b480887c510.1708509190.git.ps@pks.im>

On Wed, Feb 21, 2024 at 10:56:40AM +0100, Patrick Steinhardt wrote:

Show 13 quoted lines

> diff --git a/builtin/show-branch.c b/builtin/show-branch.c
> index b01ec761d2..8837415031 100644
> --- a/builtin/show-branch.c
> +++ b/builtin/show-branch.c
> @@ -785,6 +785,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
>  			if (read_ref_at(get_main_ref_store(the_repository),
>  					ref, flags, 0, base + i, &oid, &logmsg,
>  					&timestamp, &tz, NULL)) {
> +				if (!i)
> +					die(_("log for %s is empty"), ref);
>  				reflog = i;
>  				break;
>  			}

There's another call to read_ref_at() earlier in the function that does not check the return code. I think it is OK, though, because it always passes a timestamp rather than a count, so it never hits the special-case added by 6436a20284.

But I think this highlights my confusion with the first patch; most of the time read_ref_at() will print the appropriate message for us and die, but for this one weird case we have to do it ourselves. It makes more sense to me for it to behave consistently for an empty reflog, whether we were looking for branch@{0} or branch@{1}.

-Peff