Re: [PATCH v3 4/4] add-patch: add diff.context command line overrides

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 1, 2025, 15:54 UTC
Message-ID: <xmqqa55nlxdw.fsf@gitster.g>
In-Reply-To: <7686a5e0-23c3-4e3a-b5ea-3b8c80ba54fa@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 8 quoted lines

>>   * add OPT_DIFF_*() macros to parse-options API, as its utility is
>>     very narrow, and forces those who are learning parse-options API
>>     to learn one more thing.
>
> It means that we have consistent help for all the commands with these
> options which I think is valuable. We have a number of other macros
> that define options that are shared between commands and I think that
> works quite well.

I understand that principe. What I was wondering was if there are enough places to use these particular ones to make it worthwhile to enlarge the set of OPT_* macros.

Show 7 quoted lines

>>   * validation of the value range to be duplicated for each and
>> every
>>     users of the new OPT_DIFF_*() macros.
>
> Yes the validation is awkward. If we changed the OPT_DIFF_* to use a
> callback that rejected negative values that would reduce the
> duplication.

Yeah, I was wondering about that approach, too. Another benefit with the "validate just after we parse the value before we assign the result to a variable or a struct member" approach is that we can also complain about -1 that is given from the command line (which the current code ignores, if I am not mistaken, because it needs to be silent if that -1 is there merely because it is the "not set yet" sentinel value).

Or perhaps the valid value range Réne has been workingon canbe used here?