Re: [PATCH 1/4] refspec: treat 'fetch' as a Boolean value

From: Jeff King <peff@peff.net>
Date: Mar 18, 2025, 00:24 UTC
Message-ID: <20250318002436.GC1470172@coredump.intra.peff.net>
In-Reply-To: <7e662acb5ac50b778917cbf2f9f791d35e95e31d.1742250259.git.me@ttaylorr.com>

On Mon, Mar 17, 2025 at 06:24:24PM -0400, Taylor Blau wrote:

> Since 6d4c057859 (refspec: introduce struct refspec, 2018-05-16), we
> have constants called REFSPEC_FETCH and REFSPEC_PUSH. This misleadingly
> suggests that we might introduce other modes in the future.

I don't know that I'd call it misleading. We _could_ introduce new modes if we had new operations. But I do agree it's unlikely (even if we had other operations like git-archive, cat-file, etc, they would probably not have refspecs).

So it seems like a reasonable direction to me.

The one thing I don't like is:

Show 10 quoted lines

> diff --git a/builtin/pull.c b/builtin/pull.c
> index 9c4a00620a..8bbfcce729 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -738,7 +738,7 @@ static const char *get_tracking_branch(const char *remote, const char *refspec)
>  	const char *spec_src;
>  	const char *merge_branch;
>  
> -	refspec_item_init_or_die(&spec, refspec, REFSPEC_FETCH);
> +	refspec_item_init_or_die(&spec, refspec, 1);

The third argument here (and elsewhere) becomes much more mysterious to the reader. Maybe not a big deal, though.

> diff --git a/git-diff-pairs b/git-diff-pairs

Hmm.... :)

-Peff