Re: [RFC/PATCH 0/5] remote: eliminate remote->{fetch,push}_refspec and lazy parsing of refspecs

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 19, 2017, 20:07 UTC
Message-ID: <xmqqmv93spvb.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170617115522.lbke5wnle5irbunj@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 13 quoted lines

> If we forget the "storing it twice" argument, would it make sense to
> convert the parallel arrays of items into a single array-of-struct?
> I.e.:
>
>   struct configured_refspec {
> 	const char *string;
> 	struct refspec refspec;
> 	unsigned parsed:1;
>   }
>
> I guess that may run into problems where we really need an
> array-of-refspec to pass into sub-functions. So going further, could we
> just have "struct refspec" store the text form it was parsed from?

I find this a sensible suggestion.

I think the original "parallel" structure was anticipating that a textual input we get from the user (either from the command line or from the configuration) could expand to multiple parsed refspecs for easier use by the code, but it appears that we hadn't seen a need for that, so I think it is safe to unify them into a single struct.

Of course, that still anticipates that a parsed refspec may not be unambiguously turned back to textual original input form; if that will never be the case, then the approach taken by the posted patches should also be OK.