On 30/06/2025 18:03, Junio C Hamano wrote:
"Leon Michalak via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
From: Leon Michalak <leonmichalak6@xxxxxxxxx>
This patch compliments the previous commit, where builtins that use
add-patch infrastructure now respect diff.context and
diff.interHunkContext file configurations.
In particular, this patch helps users who don't want to set persistent
context configurations or just want a way to override them on a one-time
basis, by allowing the relevant builtins to accept corresponding command
line options that override the file configurations.
This mimics commands such as diff and log, which allow for both context
file configuration and command line overrides.
I skimmed the patch briefly. I am not sure if it is a good idea to
* 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.
* 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.
but other than that, looked reasonable to me.
I've left a couple of comments on the tests but the code changes look
reasonable to me too
Thanks
Phillip