Re: [PATCH v4 1/2] ref-filter: add multiple-option parsing functions

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 24, 2023, 20:39 UTC
Message-ID: <xmqqmszlnixe.fsf@gitster.g>
In-Reply-To: <ZL6_DlDIE8Hfl_T6@five231003>

Kousik Sanagavarapu <five231003@gmail.com> writes:

Show 13 quoted lines

> What do you mean by "share code"?
>
> They are similar in their functionality, that is parsing the option and
> grabbing the value (if the option has a value, otherwise we do what we
> did here). The difference is the way we do such a parsing.
>
> In pretty, we directly skip_prefix() the placeholder. So we check for ')'
> to see if we have reached the end of "to_parse".
>
> In ref-filter (the current patches), we deal directly with the options
> ("arg" here), that is we can't do a check for ')' to see if we have
> exhausted our option list. So we can't really use the same functions, but
> there is the possiblity that we can modify them to be used here too.

That is the kind of "sharing" to reduce repetition I had in mind.

I haven't checked the callers, but another way would be to update the caller of for-each-ref's side to match the calling convention of how pretty calls the parser, wouldn't it? After all, they parse the same "%(token:key=val,key=val,...)" so...?