Re: [PATCH 6/8] fetch: deduplicate logic to print remote URL

From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 6/8] fetch: deduplicate logic to print remote URL
Date: Thu, 16 Mar 2023 16:06:14 +0100	[thread overview]
Message-ID: <ZBMwZvQOooFiIl6S@ncase> (raw)
In-Reply-To: <xmqqy1nxmxdx.fsf@gitster.g>

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

On Wed, Mar 15, 2023 at 04:02:18PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > When fetching from a remote, we not only print the actual references
> > that have changed, but will also print the URL from which we have
> > fetched them to standard output. The logic to handle this is duplicated
> > across two different callsites with some non-trivial logic to compute
> > the anonymized URL. Furthermore, we're using global state to track
> > whether we have already shown the URL to the user or not.
> 
> If we are certain that store_updated_refs() is called only once for
> the entire process, then storing the preprocessed url in the display
> state and passing it around does sound like a good optimization and
> clean-up.  What do we do when fetching from multiple remotes?

We execute separate git-fetch(1) processes when fetching from multiple
remotes or when fetching submodules, so we should be fine here.

> > +	display->url_len = strlen(display->url);
> > +	for (i = display->url_len - 1; display->url[i] == '/' && 0 <= i; i--)
> > +		;
> > +	display->url_len = i + 1;
> 
> This loop is inherited from the original, but we may want to use
> strrchr() or rindex() as a post clean-up after this series settles.

Yeah, that'd make sense.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]