git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/3] diff: teach tree-diff a max-depth parameter

From
Patrick Steinhardt <ps@pks.im>
Date
Aug 7, 2025, 05:55 UTC
Message-ID
<aJQ_x787aceaudef@pks.im>
In-Reply-To
<87jz3gtshx.fsf@iotcl.com>
On Wed, Aug 06, 2025 at 04:49:30PM +0200, Toon Claes wrote:
Show 24 quoted lines
> Patrick Steinhardt <ps@pks.im> 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.

Show 13 quoted lines
> 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
Previous: Toon ClaesNext: Toon Claes
Message 10 of 16 in “Teach git-diff-tree(1) option --max-depth”
  1. 0/3 Teach git-diff-tree(1) option --max-depthToon Claes, Jul 29, 2025
  2. 1/3 combine-diff: zero memory used for callback filepairsToon Claes, Jul 29, 2025
  3. 2/3 within_depth: fix return for empty pathToon Claes, Jul 29, 2025
  4. Patrick SteinhardtJul 30, 2025
  5. Toon ClaesAug 6, 2025
  6. Patrick SteinhardtAug 7, 2025
  7. 3/3 diff: teach tree-diff a max-depth parameterToon Claes, Jul 29, 2025
  8. Patrick SteinhardtJul 30, 2025
  9. Toon ClaesAug 6, 2025
  10. Patrick SteinhardtAug 7, 2025
  11. 0/3 Teach git-diff-tree(1) option --max-depthToon Claes, Aug 7, 2025
  12. 1/3 combine-diff: zero memory used for callback filepairsToon Claes, Aug 7, 2025
  13. 2/3 within_depth: fix return for empty pathToon Claes, Aug 7, 2025
  14. 3/3 diff: teach tree-diff a max-depth parameterToon Claes, Aug 7, 2025
  15. Junio C HamanoAug 14, 2025
  16. Patrick SteinhardtAug 15, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.