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

[Emily Shaffer <emilyshaffer@google.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]
> +		/*
> +		 * 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