On Wed, Aug 06, 2025 at 04:49:30PM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
> > On Tue, Jul 29, 2025 at 08:57:44PM +0200, Toon Claes wrote:
> >> diff --git a/diff.c b/diff.c
> >> index dca87e164f..c03a59ac3b 100644
> >> --- a/diff.c
> >> +++ b/diff.c
> >> @@ -5894,6 +5909,10 @@ struct option *add_diff_options(const struct option *opts,
> >> OPT_CALLBACK_F(0, "diff-filter", options, N_("[(A|C|D|M|R|T|U|X|B)...[*]]"),
> >> N_("select files by diff type"),
> >> PARSE_OPT_NONEG, diff_opt_diff_filter),
> >> + OPT_CALLBACK_F(0, "max-depth", options, N_("<depth>"),
> >> + N_("maximum tree depth to recurse"),
> >> + PARSE_OPT_NONEG, diff_opt_max_depth),
> >> +
> >> {
> >> .type = OPTION_CALLBACK,
> >> .long_name = "output",
> >
> > Okay. We don't use `OPT_UNSIGNED()` because we also want to impliy the
> > `recursive` flag. Wouldn't it be simpler though to use `OPT_UNSIGNED()`
> > and then set the flag in `diff_setup_done()` like we already do for a
> > couple of other options?
>
> But how would you determine `max_depth_valid`?
Ideally we wouldn't need it as we could just initialize with a default
value.
> And, without realizing, you touch a good point here. Looking at the
> git-grep(1) docs, it says:
>
> For each <pathspec> given on command line, descend at most <depth>
> levels of directories. A value of -1 means no limit.
>
> And:
>
> -r::
> --recursive::
> Same as `--max-depth=-1`; this is the default.
>
> We should handle -1 the same, that's currently not the case.
True. Do we have the same default? If so, couldn't we drop
`max_depth_valid` and initialize it with -1 from the start?
If that's not easily possible we can just stick with what you have.
Patrick
