From: Karthik Nayak <karthik.188@gmail.com>
Date: Wed, 26 Nov 2025 13:31:32 GMT
Subject: Re: [PATCH 2/3] last-modified: document option --max-depth
Message-ID: <CAOLa=ZSbV6SeJ9orOz0T+oh3PVhYhTsaxrsYVkr+5q7i_tsVCw@mail.gmail.com>
In-Reply-To: <20251126-toon-last-modified-zzzz-v1-2-608350df0caa@iotcl.com>

Toon Claes <toon@iotcl.com> writes:

> Option --max-depth is supported by git-last-modified(1), because it was
> added to the diff machinery in a1dfa5448d (diff: teach tree-diff a
> max-depth parameter, 2025-08-07).
>

At this point, does it make more sense to link the respective sections
within 'Documentation/diff-options.adoc' as done by many other commands?
This would ensure that we don't have to repeat the documentation.

> This option is useful for everyday use of the git-last-modified(1)
> command, so document it's existence in the man page and `-h` output.
>
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
>  Documentation/git-last-modified.adoc |  9 ++++++++-
>  builtin/last-modified.c              | 12 +++++++++++-
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-last-modified.adoc b/Documentation/git-last-modified.adoc
> index cd4a5040b0..8409daebe9 100644
> --- a/Documentation/git-last-modified.adoc
> +++ b/Documentation/git-last-modified.adoc
> @@ -9,7 +9,8 @@ git-last-modified - EXPERIMENTAL: Show when files were last modified
>  SYNOPSIS
>  --------
>  [synopsis]
> -git last-modified [--recursive] [--show-trees] [-z] [<revision-range>] [[--] <path>...]
> +git last-modified [--recursive] [--show-trees] [--max-depth=<depth>] [-z]
> +	[<revision-range>] [[--] <path>...]
>
>  DESCRIPTION
>  -----------
> @@ -32,6 +33,12 @@ OPTIONS
>  	Show tree entries even when recursing into them. It has no effect
>  	without `--recursive`.
>
> +`--max-depth=<depth>`::
> +	For each pathspec given on the command line, descend at most `<depth>`
> +	levels of directories. A negative value means no limit.
> +	Setting a positive value implies `--recursive`.
> +	Cannot be combined with wildcards in the pathspec.
> +
>  `-z`::
>  	Terminate each line with a _NUL_ rather than a newline.
>
> diff --git a/builtin/last-modified.c b/builtin/last-modified.c
> index 9206bbdc1d..ccb7ff66d4 100644
> --- a/builtin/last-modified.c
> +++ b/builtin/last-modified.c
> @@ -25,6 +25,7 @@
>
>  #define LAST_MODIFIED_INIT { \
>  	.line_termination = '\n', \
> +	.max_depth = -1, \
>  }
>
>  struct last_modified_entry {
> @@ -60,6 +61,7 @@ struct last_modified {
>  	bool recursive;
>  	bool show_trees;
>  	int line_termination;
> +	int max_depth;
>

Should this be signed?

>  	const char **all_paths;
>  	size_t all_paths_nr;
> @@ -487,6 +489,12 @@ static int last_modified_init(struct last_modified *lm, struct repository *r,
>  	lm->rev.diffopt.flags.recursive = lm->recursive;
>  	lm->rev.diffopt.flags.tree_in_recursive = lm->show_trees;
>
> +	if (lm->max_depth >= 0) {
> +		lm->rev.diffopt.flags.recursive = 1;
> +		lm->rev.diffopt.max_depth = lm->max_depth;
> +		lm->rev.diffopt.max_depth_valid = 1;
> +	}
> +

Or if our goal is to actually handle them within the
'git-last-modified(1)' command, shouldn't we ensure we don't allow any
additional flags from being parsed as diffopt?

Currently other diffopts flags such as '--no-prefix', '--cc' and so on,
are parsed even if they don't affect the output of
'git-last-modified(1)'. Shouldn't we disallow such behavior?

>  	argc = setup_revisions(argc, argv, &lm->rev, NULL);
>  	if (argc > 1) {
>  		error(_("unknown last-modified argument: %s"), argv[1]);
> @@ -515,7 +523,7 @@ int cmd_last_modified(int argc, const char **argv, const char *prefix,
>  	struct last_modified lm = LAST_MODIFIED_INIT;
>
>  	const char * const last_modified_usage[] = {
> -		N_("git last-modified [--recursive] [--show-trees] [-z] "
> +		N_("git last-modified [--recursive] [--show-trees] [--max-depth=<depth>] [-z] "
>  		   "[<revision-range>] [[--] <path>...]"),
>  		NULL
>  	};
> @@ -525,6 +533,8 @@ int cmd_last_modified(int argc, const char **argv, const char *prefix,
>  			 N_("recurse into subtrees")),
>  		OPT_BOOL('t', "show-trees", &lm.show_trees,
>  			 N_("show tree entries when recursing into subtrees")),
> +		OPT_INTEGER_F(0, "max-depth", &lm.max_depth,
> +			N_("maximum tree depth to recurse"), PARSE_OPT_NONEG),
>  		OPT_SET_INT('z', NULL, &lm.line_termination,
>  			N_("lines are separated with NUL character"), '\0'),
>  		OPT_END()
>
> --
> 2.51.2