Re: [PATCH v3 4/4] add-patch: add diff.context command line overrides
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jul 2, 2025, 14:07 UTC
- Message-ID
- <2ee8df78-1c8b-42e6-bf93-a5aa42c20af3@gmail.com>
- In-Reply-To
- <xmqqa55nlxdw.fsf@gitster.g>
On 01/07/2025 16:54, Junio C Hamano wrote:
Show 14 quoted lines
> 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.
There are six users of each of these macros so I think it is worthwhile. That's two more users than there are for OPT_RERERE_AUTOUPDATE() and twice as many users as OPT_CONTAINS().
Show 18 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?
That would be nice
Thanks
Phillip