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

[Junio C Hamano <gitster@pobox.com>]

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

>>   * 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.

>>   * 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?