Re: [PATCH v2 5/5] maintenance: allow custom refspecs during prefetch

From: Emily Shaffer <emilyshaffer@google.com>
Date: Apr 7, 2021, 23:37 UTC
Message-ID: <YG5CTFMIfSYcqZkJ@google.com>
In-Reply-To: <9592224e3d428762c6f9b38a0bcc2cee5c3dff6c.1617734871.git.gitgitgadget@gmail.com>

On Tue, Apr 06, 2021 at 06:47:50PM +0000, Derrick Stolee via GitGitGadget wrote:

> @@ -877,6 +878,7 @@ static int fetch_remote(struct remote *remote, void *cbdata)

[snip]

Show 8 quoted lines

> +		/*
> +		 * If a refspec dst starts with "refs/" at the start,
> +		 * then we will replace "refs/" with "refs/prefetch/".
> +		 * Otherwise, we will prepend the dst string with
> +		 * "refs/prefetch/".
> +		 */
> +		if (!strncmp(replace.dst, "refs/", 5))
> +			ignore_len = 5;

Using a literal string plus the literal value of the string length, twice, doesn't sit great with me...

> +
> +		strbuf_addstr(&new_dst, "refs/prefetch/");
> +		strbuf_addstr(&new_dst, replace.dst + ignore_len);

...plus with some ugly array pointer math. :) Why not use git-compat-util.h:skip_prefix() instead of doing your own math? (In fact, the doc comment on skip_prefix() talks about using it exactly for stripping "refs/" off the beginning of a string :) )

 - Emily