Re: [PATCH v2 1/2] fetch-prune: optimize dangling-ref reporting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 23, 2025 at 04:43:26PM -0700, Phil Hord wrote:

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 40a0e8d24434..65d606c6de08 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1383,9 +1383,13 @@ static int prune_refs(struct display_state *display_state,
>  	int result = 0;
>  	struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map);
>  	struct strbuf err = STRBUF_INIT;
> +	struct string_list refnames = STRING_LIST_INIT_NODUP;
>  	const char *dangling_msg = dry_run
> -		? _("   (%s will become dangling)")
> -		: _("   (%s has become dangling)");
> +		? _("   %s will become dangling after %s is deleted")
> +		: _("   %s has become dangling after %s was deleted");

This approach seems reasonable. It is a little ugly that
refs_warn_dangling_symrefs() takes a printf-formatted string that must
contain the correct number of "%s" fields (and that we get no compiler
warnings if we get it wrong).

But that is not really new in your series. Given that there are two
callers and they use (almost) the same string, I wonder if we could
refactor the interface. We'd need to pass in the indentation level, and
the dry-run flag.

I guess alternatively, we could have a function which passes back a
strvec or similar of danglers, but then both call sites would have more
printing boilerplate. I dunno. Maybe we should just avert our eyes and
live with it. ;)

> diff --git a/refs.c b/refs.c
> index dce5c49ca2ba..e2075a98c844 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -461,7 +461,9 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU
>  		return 0;
>  	}
>  
> -	fprintf(d->fp, d->msg_fmt, refname);
> +	skip_prefix(refname, "refs/remotes/", &refname);
> +	skip_prefix(resolves_to, "refs/remotes/", &resolves_to);
> +	fprintf(d->fp, d->msg_fmt, refname, resolves_to);
>  	fputc('\n', d->fp);
>  	return 0;

This prefix handling feels kind of ad-hoc. Should we use something like
refs_shorten_unambiguous_ref() to follow the usual rules?

This is also shortening the symref name, which didn't happen before.
Arguably that should happen in a separate patch, but I can live with it
either way.

-Peff




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux