Re: [PATCH 5/9] refspec_ref_prefixes(): clean up refspec_item logic

[Taylor Blau <me@ttaylorr.com>]

On Mon, Mar 17, 2025 at 04:25:07PM -0700, Junio C Hamano wrote:
> So, if we can reasonably expect that the choice will stay between
> fetch and push and we wouldn't be adding a new kind, I think
> reverting the meaning of .fetch to yes/no and getting rid of
> REFSPEC_{FETCH,PUSH} may be a better approach.  If we stil want to
> keep the descriptive CPP macro, then perhaps .transfer (or
> .direction) that lets us choose between fetch or push?  I dunno.

I suppose adding a new direction/mode/transfer/etc is always possible.
But I think that it's unlikely enough to happen any time soon (if at
all) in my view that we should simplify the code anyway. If that changes
in the future *and* those changes fit will in the design of
REFSPEC_FETCH and friends, then we can always resurrect those macros and
reinterpret this field.

But I don't think that we should carry this extra baggage around in the
meantime if we don't need to.

> > , which gives us the "default" case in the switch statement. But this
> > really is a boolean. I wonder if we should just use 0/1 constants and
> > leave the field name alone. That would turn something like:
> >
> >     if (rs->fetch == REFSPEC_FETCH) { ... }
> >
> > into:
> >
> >     if (rs->fetch) { ... }
> >
> > , which I think is cleaner. There's no reason to rename true/false to
> > FETCH and PUSH if the field name itself is already 'fetch'.
>
> Yup, that makes two of us.

Good :-). I'll send a small reroll of the topic anyway to avoid sending
a patch that has a git-diff-pairs binary in it :-<.

Thanks,
Taylor