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

From: Junio C Hamano <[email-masked]>
Date: Tue, 25 Jul 2023 12:27:46 -0700

Junio C Hamano <[email-masked]> writes:

> Kousik Sanagavarapu <[email-masked]> writes:
>
>> 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...?

Having said all that, let's move things forward by merging this
version.  Anybody (it could be you) interested in cleaning up by
unifying these two very similar implementations of the same thing
can do so on top.

Thanks.