[PATCH 0/3] Expand and enhance git-last-modified(1) documentation

[PATCH 0/3] Expand and enhance git-last-modified(1) documentation

[Toon Claes]

Option `-z` and `--max-depth` are not documented in git-last-modified(1)
while they are pretty crucial. In these patches documentation is added
in the man page and the `-h` output.

---
Toon Claes (3):
      last-modified: handle and document NUL termination
      last-modified: document option --max-depth
      last-modified: better document how depth in handled

 Documentation/git-last-modified.adoc | 71 +++++++++++++++++++++++++++++++++++-
 builtin/last-modified.c              | 23 ++++++++++--
 2 files changed, 90 insertions(+), 4 deletions(-)



---
base-commit: 6ab38b7e9cc7adafc304f3204616a4debd49c6e9
change-id: 20251114-toon-last-modified-zzzz-af9c1be74fc4

[PATCH 1/3] last-modified: handle and document NUL termination

[Toon Claes]

When option `-z` is provided to git-last-modified(1), each line is
separated with a NUL instead of a newline. Document this properly and
handle parsing of the option in the builtin itself.

Signed-off-by: Toon Claes <toon@iotcl.com>
---
 Documentation/git-last-modified.adoc | 21 ++++++++++++++++++++-
 builtin/last-modified.c              | 13 ++++++++++---
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-last-modified.adoc b/Documentation/git-last-modified.adoc
index 602843e095..cd4a5040b0 100644
--- a/Documentation/git-last-modified.adoc
+++ b/Documentation/git-last-modified.adoc
@@ -9,7 +9,7 @@ git-last-modified - EXPERIMENTAL: Show when files were last modified
 SYNOPSIS
 --------
 [synopsis]
-git last-modified [--recursive] [--show-trees] [<revision-range>] [[--] <path>...]
+git last-modified [--recursive] [--show-trees] [-z] [<revision-range>] [[--] <path>...]
 
 DESCRIPTION
 -----------
@@ -32,6 +32,9 @@ OPTIONS
 	Show tree entries even when recursing into them. It has no effect
 	without `--recursive`.
 
+`-z`::
+	Terminate each line with a _NUL_ rather than a newline.
+
 `<revision-range>`::
 	Only traverse commits in the specified revision range. When no
 	`<revision-range>` is specified, it defaults to `HEAD` (i.e. the whole
@@ -44,6 +47,22 @@ OPTIONS
 	Without an optional path parameter, all files and subdirectories
 	in path traversal the are included in the output.
 
+OUTPUT
+------
+
+The output is in the format:
+
+------------
+ <oid> TAB <path> LF
+------------
+
+If a path contains any special characters, the path is C-style quoted. To
+avoid quoting, pass option `-z` to terminate each line with a NUL.
+
+------------
+ <oid> TAB <path> NUL
+------------
+
 SEE ALSO
 --------
 linkgit:git-blame[1],
diff --git a/builtin/last-modified.c b/builtin/last-modified.c
index b0ecbdc540..9206bbdc1d 100644
--- a/builtin/last-modified.c
+++ b/builtin/last-modified.c
@@ -23,6 +23,10 @@
 #define PARENT1 (1u<<16) /* used instead of SEEN */
 #define PARENT2 (1u<<17) /* used instead of BOTTOM, BOUNDARY */
 
+#define LAST_MODIFIED_INIT { \
+	.line_termination = '\n', \
+}
+
 struct last_modified_entry {
 	struct hashmap_entry hashent;
 	struct object_id oid;
@@ -55,6 +59,7 @@ struct last_modified {
 	struct rev_info rev;
 	bool recursive;
 	bool show_trees;
+	int line_termination;
 
 	const char **all_paths;
 	size_t all_paths_nr;
@@ -165,7 +170,7 @@ static void last_modified_emit(struct last_modified *lm,
 		putchar('^');
 	printf("%s\t", oid_to_hex(&commit->object.oid));
 
-	if (lm->rev.diffopt.line_termination)
+	if (lm->line_termination)
 		write_name_quoted(path, stdout, '\n');
 	else
 		printf("%s%c", path, '\0');
@@ -507,10 +512,10 @@ int cmd_last_modified(int argc, const char **argv, const char *prefix,
 		      struct repository *repo)
 {
 	int ret;
-	struct last_modified lm = { 0 };
+	struct last_modified lm = LAST_MODIFIED_INIT;
 
 	const char * const last_modified_usage[] = {
-		N_("git last-modified [--recursive] [--show-trees] "
+		N_("git last-modified [--recursive] [--show-trees] [-z] "
 		   "[<revision-range>] [[--] <path>...]"),
 		NULL
 	};
@@ -520,6 +525,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_SET_INT('z', NULL, &lm.line_termination,
+			N_("lines are separated with NUL character"), '\0'),
 		OPT_END()
 	};
 

-- 
2.51.2

[PATCH 2/3] last-modified: document option --max-depth

[Toon Claes]

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).

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;
 
 	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;
+	}
+
 	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

[PATCH 3/3] last-modified: better document how depth in handled

[Toon Claes]

By default git-last-modified(1) only shows information about paths at
the root level. This can be confusing. Clarify the command's behavior in
the documentation.

Signed-off-by: Toon Claes <toon@iotcl.com>
---
 Documentation/git-last-modified.adoc | 43 ++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/Documentation/git-last-modified.adoc b/Documentation/git-last-modified.adoc
index 8409daebe9..36f72954a5 100644
--- a/Documentation/git-last-modified.adoc
+++ b/Documentation/git-last-modified.adoc
@@ -27,6 +27,7 @@ OPTIONS
 `--recursive`::
 	Instead of showing tree entries, step into subtrees and show all entries
 	inside them recursively.
+	See the section "NOTES ABOUT DEPTH" below for more details.
 
 `-t`::
 `--show-trees`::
@@ -38,6 +39,7 @@ OPTIONS
 	levels of directories. A negative value means no limit.
 	Setting a positive value implies `--recursive`.
 	Cannot be combined with wildcards in the pathspec.
+	See the section "NOTES ABOUT DEPTH" below for more details.
 
 `-z`::
 	Terminate each line with a _NUL_ rather than a newline.
@@ -70,6 +72,47 @@ avoid quoting, pass option `-z` to terminate each line with a NUL.
  <oid> TAB <path> NUL
 ------------
 
+NOTES ABOUT DEPTH
+-----------------
+
+By default this command only shows information about paths at the root level.
+When a path that lives in a subtree is provided, information about the top-level
+subtree is printed. For example:
+
+------------
+$ git last-modified -- sub/file
+
+abcd1234abcd1234abcd1234abcd1234abcd1234 sub
+------------
+
+To get details about the exact path in a subtree, add option `--recursive`:
+
+------------
+$ git last-modified --recursive -- sub/file
+
+5678abca5678abca5678abca5678abca5678abca sub/file
+------------
+
+This comes with a downside. When the path provided is a tree itself, with
+option `--recursive` all paths in that subtree are printed too:
+
+------------
+$ git last-modified --recursive -- sub/subsub
+
+1234cdef1234cdef1234cdef1234cdef1234cdef sub/subsub/a
+3456cdef3456cdef3456cdef3456cdef3456cdef sub/subsub/b
+5678abcd5678abcd5678abcd5678abcd5678abcd sub/subsub/c
+------------
+
+To stop this command from traversing deeper into trees, add option
+`--max-depth=0`:
+
+------------
+$ git last-modified --recursive --max-depth=0 -- sub/subsub
+
+3456def3456def3456def3456def3456def3456b sub/subsub
+------------
+
 SEE ALSO
 --------
 linkgit:git-blame[1],

-- 
2.51.2

Re: [PATCH 1/3] last-modified: handle and document NUL termination

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 4173 bytes --]

Toon Claes <toon@iotcl.com> writes:

> When option `-z` is provided to git-last-modified(1), each line is
> separated with a NUL instead of a newline.

This line make it seem like the option already exists..

> Document this properly and
> handle parsing of the option in the builtin itself.
>

But this line says we add the option now. Perhaps this should be
clearer.

> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
>  Documentation/git-last-modified.adoc | 21 ++++++++++++++++++++-
>  builtin/last-modified.c              | 13 ++++++++++---
>  2 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-last-modified.adoc b/Documentation/git-last-modified.adoc
> index 602843e095..cd4a5040b0 100644
> --- a/Documentation/git-last-modified.adoc
> +++ b/Documentation/git-last-modified.adoc
> @@ -9,7 +9,7 @@ git-last-modified - EXPERIMENTAL: Show when files were last modified
>  SYNOPSIS
>  --------
>  [synopsis]
> -git last-modified [--recursive] [--show-trees] [<revision-range>] [[--] <path>...]
> +git last-modified [--recursive] [--show-trees] [-z] [<revision-range>] [[--] <path>...]
>

This is a bit long now, let's wrap it.

>  DESCRIPTION
>  -----------
> @@ -32,6 +32,9 @@ OPTIONS
>  	Show tree entries even when recursing into them. It has no effect
>  	without `--recursive`.
>
> +`-z`::
> +	Terminate each line with a _NUL_ rather than a newline.
> +

Nit: perhaps it is just me, but it would nicer to read if it said 'a
_NUL_ character'

>  `<revision-range>`::
>  	Only traverse commits in the specified revision range. When no
>  	`<revision-range>` is specified, it defaults to `HEAD` (i.e. the whole
> @@ -44,6 +47,22 @@ OPTIONS
>  	Without an optional path parameter, all files and subdirectories
>  	in path traversal the are included in the output.
>
> +OUTPUT
> +------
> +
> +The output is in the format:
> +
> +------------
> + <oid> TAB <path> LF
> +------------
> +
> +If a path contains any special characters, the path is C-style quoted. To
> +avoid quoting, pass option `-z` to terminate each line with a NUL.
> +
> +------------
> + <oid> TAB <path> NUL
> +------------
> +
>  SEE ALSO
>  --------
>  linkgit:git-blame[1],
> diff --git a/builtin/last-modified.c b/builtin/last-modified.c
> index b0ecbdc540..9206bbdc1d 100644
> --- a/builtin/last-modified.c
> +++ b/builtin/last-modified.c
> @@ -23,6 +23,10 @@
>  #define PARENT1 (1u<<16) /* used instead of SEEN */
>  #define PARENT2 (1u<<17) /* used instead of BOTTOM, BOUNDARY */
>
> +#define LAST_MODIFIED_INIT { \
> +	.line_termination = '\n', \
> +}
> +
>  struct last_modified_entry {
>  	struct hashmap_entry hashent;
>  	struct object_id oid;
> @@ -55,6 +59,7 @@ struct last_modified {
>  	struct rev_info rev;
>  	bool recursive;
>  	bool show_trees;
> +	int line_termination;
>

Wouldn't 'line_terminator' be a better name?

>  	const char **all_paths;
>  	size_t all_paths_nr;
> @@ -165,7 +170,7 @@ static void last_modified_emit(struct last_modified *lm,
>  		putchar('^');
>  	printf("%s\t", oid_to_hex(&commit->object.oid));
>
> -	if (lm->rev.diffopt.line_termination)
> +	if (lm->line_termination)

So it did exist before. But this was parsed as part of diff_options, why
make the change than?

>  		write_name_quoted(path, stdout, '\n');
>  	else
>  		printf("%s%c", path, '\0');
> @@ -507,10 +512,10 @@ int cmd_last_modified(int argc, const char **argv, const char *prefix,
>  		      struct repository *repo)
>  {
>  	int ret;
> -	struct last_modified lm = { 0 };
> +	struct last_modified lm = LAST_MODIFIED_INIT;
>
>  	const char * const last_modified_usage[] = {
> -		N_("git last-modified [--recursive] [--show-trees] "
> +		N_("git last-modified [--recursive] [--show-trees] [-z] "
>  		   "[<revision-range>] [[--] <path>...]"),
>  		NULL
>  	};
> @@ -520,6 +525,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_SET_INT('z', NULL, &lm.line_termination,
> +			N_("lines are separated with NUL character"), '\0'),
>  		OPT_END()
>  	};
>
>
> --
> 2.51.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 2/3] last-modified: document option --max-depth

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 4159 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 1/3] last-modified: handle and document NUL termination

[Junio C Hamano]

Toon Claes <toon@iotcl.com> writes:

> When option `-z` is provided to git-last-modified(1), each line is
> separated with a NUL instead of a newline. Document this properly and
> handle parsing of the option in the builtin itself.

I think documenting does make sense, but it is not clear from the
description why it is better to handle the option in the builtin
itself, instead of letting the setup_revisions() take care of it.

Is it because after the command lets setup_revisions() to parse out
the revision range, the command does not really let the revision
machinery drive diffs and let it output anything (hence, even though
rev.diffopt.line_termination is set from the command line, the calling
builtin is the only one that pays attention, and the revision machinery
and the diff machinery called from there does not pay attention to it?

And assuming that this new division of labor between the revision
machinery and the subcommand makes sense (it needs to be explained
better), the updated code does make sense to me.

But it looks suboptimal.  See below.

> +#define LAST_MODIFIED_INIT { \
> +	.line_termination = '\n', \
> +}

You have to introduce such a non-zero initialization, only because
you pretend to accept _any_ byte here, and use it as the line
termination character.  If you were porting Git to ancient Macintosh,
you could set this to '\r' and it would follow their text file
convention there ;-)

But ...

>  struct last_modified_entry {
>  	struct hashmap_entry hashent;
>  	struct object_id oid;
> @@ -55,6 +59,7 @@ struct last_modified {
>  	struct rev_info rev;
>  	bool recursive;
>  	bool show_trees;
> +	int line_termination;
>  
>  	const char **all_paths;
>  	size_t all_paths_nr;
> @@ -165,7 +170,7 @@ static void last_modified_emit(struct last_modified *lm,
>  		putchar('^');
>  	printf("%s\t", oid_to_hex(&commit->object.oid));
>  
> -	if (lm->rev.diffopt.line_termination)
> +	if (lm->line_termination)
>  		write_name_quoted(path, stdout, '\n');
>  	else
>  		printf("%s%c", path, '\0');

... you use hardcoded '\n' here, without allowing the value of
line_termination to affect the termination character.

This is way suboptimal.  Instead, would it work if you add

	bool null_termination;

to the last_modified structure, and do

	if (!lm->null_termination)
		write_name_quoted(path, stdout, '\n');
	else
		printf("%s%c", path, '\0');

here?   Then

> @@ -507,10 +512,10 @@ int cmd_last_modified(int argc, const char **argv, const char *prefix,
>  		      struct repository *repo)
>  {
>  	int ret;
> -	struct last_modified lm = { 0 };
> +	struct last_modified lm = LAST_MODIFIED_INIT;

You do not need this change, and

>  	const char * const last_modified_usage[] = {
> -		N_("git last-modified [--recursive] [--show-trees] "
> +		N_("git last-modified [--recursive] [--show-trees] [-z] "
>  		   "[<revision-range>] [[--] <path>...]"),
>  		NULL
>  	};
> @@ -520,6 +525,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_SET_INT('z', NULL, &lm.line_termination,
> +			N_("lines are separated with NUL character"), '\0'),

This will become OPT_BOOL() to set the &lm.null_termination.

>  		OPT_END()
>  	};

Re: [PATCH 3/3] last-modified: better document how depth in handled

[Eric Sunshine]

On Wed, Nov 26, 2025 at 1:10 AM Toon Claes <toon@iotcl.com> wrote:
> last-modified: better document how depth in handled

s/in/is/

> By default git-last-modified(1) only shows information about paths at
> the root level. This can be confusing. Clarify the command's behavior in
> the documentation.
>
> Signed-off-by: Toon Claes <toon@iotcl.com>

Re: [PATCH 2/3] last-modified: document option --max-depth

[Junio C Hamano]

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).
>
> 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(-)

Does this step pass t0450?

    fixup! last-modified: document option --max-depth

diff --git a/builtin/last-modified.c b/builtin/last-modified.c
index ccb7ff66d4..857554e70d 100644
--- a/builtin/last-modified.c
+++ b/builtin/last-modified.c
@@ -523,8 +523,8 @@ 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] [--max-depth=<depth>] [-z] "
-		   "[<revision-range>] [[--] <path>...]"),
+		N_("git last-modified [--recursive] [--show-trees] [--max-depth=<depth>] [-z]\n"
+		   "    [<revision-range>] [[--] <path>...]"),
 		NULL
 	};
 

Re: [PATCH 1/3] last-modified: handle and document NUL termination

[Toon Claes]

Junio C Hamano <gitster@pobox.com> writes:

> Toon Claes <toon@iotcl.com> writes:
>
>> When option `-z` is provided to git-last-modified(1), each line is
>> separated with a NUL instead of a newline. Document this properly and
>> handle parsing of the option in the builtin itself.
>
> I think documenting does make sense, but it is not clear from the
> description why it is better to handle the option in the builtin
> itself, instead of letting the setup_revisions() take care of it.

I know it's silly, but I wanted to feed these options to
parse_options(). Doing this would make them show up in `git
last-modified -h`.

> Is it because after the command lets setup_revisions() to parse out
> the revision range, the command does not really let the revision
> machinery drive diffs and let it output anything (hence, even though
> rev.diffopt.line_termination is set from the command line, the calling
> builtin is the only one that pays attention, and the revision machinery
> and the diff machinery called from there does not pay attention to it?

That too, at least for this option. Not the other patch.

> And assuming that this new division of labor between the revision
> machinery and the subcommand makes sense (it needs to be explained
> better), the updated code does make sense to me.
>
> But it looks suboptimal.  See below.
>
>> +#define LAST_MODIFIED_INIT { \
>> +	.line_termination = '\n', \
>> +}
>
> You have to introduce such a non-zero initialization, only because
> you pretend to accept _any_ byte here, and use it as the line
> termination character.  If you were porting Git to ancient Macintosh,
> you could set this to '\r' and it would follow their text file
> convention there ;-)
>
> But ...
>
>>  struct last_modified_entry {
>>  	struct hashmap_entry hashent;
>>  	struct object_id oid;
>> @@ -55,6 +59,7 @@ struct last_modified {
>>  	struct rev_info rev;
>>  	bool recursive;
>>  	bool show_trees;
>> +	int line_termination;
>>  
>>  	const char **all_paths;
>>  	size_t all_paths_nr;
>> @@ -165,7 +170,7 @@ static void last_modified_emit(struct last_modified *lm,
>>  		putchar('^');
>>  	printf("%s\t", oid_to_hex(&commit->object.oid));
>>  
>> -	if (lm->rev.diffopt.line_termination)
>> +	if (lm->line_termination)
>>  		write_name_quoted(path, stdout, '\n');
>>  	else
>>  		printf("%s%c", path, '\0');
>
> ... you use hardcoded '\n' here, without allowing the value of
> line_termination to affect the termination character.
>
> This is way suboptimal.  Instead, would it work if you add
>
> 	bool null_termination;
>
> to the last_modified structure, and do
>
> 	if (!lm->null_termination)
> 		write_name_quoted(path, stdout, '\n');
> 	else
> 		printf("%s%c", path, '\0');
>
> here?   Then
>
>> @@ -507,10 +512,10 @@ int cmd_last_modified(int argc, const char **argv, const char *prefix,
>>  		      struct repository *repo)
>>  {
>>  	int ret;
>> -	struct last_modified lm = { 0 };
>> +	struct last_modified lm = LAST_MODIFIED_INIT;
>
> You do not need this change, and
>
>>  	const char * const last_modified_usage[] = {
>> -		N_("git last-modified [--recursive] [--show-trees] "
>> +		N_("git last-modified [--recursive] [--show-trees] [-z] "
>>  		   "[<revision-range>] [[--] <path>...]"),
>>  		NULL
>>  	};
>> @@ -520,6 +525,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_SET_INT('z', NULL, &lm.line_termination,
>> +			N_("lines are separated with NUL character"), '\0'),
>
> This will become OPT_BOOL() to set the &lm.null_termination.
>
>>  		OPT_END()
>>  	};
>

I actually agree this is better. But I was following the pattern of
`line_termination` other in various other places. I'll adapt it to use a
bool instead.

-- 
Cheers,
Toon

Re: [PATCH 2/3] last-modified: document option --max-depth

[Toon Claes]

Junio C Hamano <gitster@pobox.com> writes:

> 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).
>>
>> 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(-)
>
> Does this step pass t0450?
>
>     fixup! last-modified: document option --max-depth
>
> diff --git a/builtin/last-modified.c b/builtin/last-modified.c
> index ccb7ff66d4..857554e70d 100644
> --- a/builtin/last-modified.c
> +++ b/builtin/last-modified.c
> @@ -523,8 +523,8 @@ 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] [--max-depth=<depth>] [-z] "
> -		   "[<revision-range>] [[--] <path>...]"),
> +		N_("git last-modified [--recursive] [--show-trees] [--max-depth=<depth>] [-z]\n"
> +		   "    [<revision-range>] [[--] <path>...]"),
>  		NULL
>  	};

Derp, I should have checked that. I knew it could be a problem, but by
the time I was about to send the patch I forgot. Sorry about that.

-- 
Cheers,
Toon

Re: [PATCH 1/3] last-modified: handle and document NUL termination

[Patrick Steinhardt]

On Fri, Nov 28, 2025 at 07:50:30PM +0100, Toon Claes wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Toon Claes <toon@iotcl.com> writes:
> >
> >> When option `-z` is provided to git-last-modified(1), each line is
> >> separated with a NUL instead of a newline. Document this properly and
> >> handle parsing of the option in the builtin itself.
> >
> > I think documenting does make sense, but it is not clear from the
> > description why it is better to handle the option in the builtin
> > itself, instead of letting the setup_revisions() take care of it.
> 
> I know it's silly, but I wanted to feed these options to
> parse_options(). Doing this would make them show up in `git
> last-modified -h`.

I think that reasoning makes sense, but it's certainly non-obvious from
the commit message. So if you expand the commit message with an
explanation the change becomes much more sensible.

Patrick

Re: [PATCH 3/3] last-modified: better document how depth in handled

[Patrick Steinhardt]

On Wed, Nov 26, 2025 at 07:09:45AM +0100, Toon Claes wrote:
> By default git-last-modified(1) only shows information about paths at
> the root level. This can be confusing. Clarify the command's behavior in
> the documentation.

Hm, that's confusing indeed. Is it possible for git-last-modified(1) to
do the "right thing" automatically? That is, given "sub/file", show when
that specific file has been last modified? Or is there a good
(non-technical) reason it behaves the way it does?

Patrick

Re: [PATCH 3/3] last-modified: better document how depth in handled

[Toon Claes]

Patrick Steinhardt <ps@pks.im> writes:

> Hm, that's confusing indeed. Is it possible for git-last-modified(1) to
> do the "right thing" automatically? That is, given "sub/file", show when
> that specific file has been last modified? Or is there a good
> (non-technical) reason it behaves the way it does?

You bring up a good point. In the version of git-blame-tree(1) that
GitHub did share, the `--recursive` flag is enabled by default. So if
you pass a file path to the command, you'll get the "right thing". But
as I am pointing out in this patch, if you pass a subtree, everything in
that subtree is shown too. You could argue this is the "right thing".

Anyhow, in the version of git-last-modified(1) I submitted upstream,
recursive is not enabled by default. My reason, at the time option
--max-depth wasn't implemented yet. I submitted those changes in a
separate series (these patches also originate from GitHub by the way).
If those patches wouldn't land, I think always-on recursive behavior for
git-last-modified(1) would be quite annoying.

So long story short, as git-last-modified(1) is still marked as
"EXPERIMENTAL", shall we make recursive always-on?

-- 
Cheers,
Toon

Re: [PATCH 3/3] last-modified: better document how depth in handled

[Patrick Steinhardt]

On Tue, Dec 02, 2025 at 12:01:18PM +0100, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Hm, that's confusing indeed. Is it possible for git-last-modified(1) to
> > do the "right thing" automatically? That is, given "sub/file", show when
> > that specific file has been last modified? Or is there a good
> > (non-technical) reason it behaves the way it does?
> 
> You bring up a good point. In the version of git-blame-tree(1) that
> GitHub did share, the `--recursive` flag is enabled by default. So if
> you pass a file path to the command, you'll get the "right thing". But
> as I am pointing out in this patch, if you pass a subtree, everything in
> that subtree is shown too. You could argue this is the "right thing".
> 
> Anyhow, in the version of git-last-modified(1) I submitted upstream,
> recursive is not enabled by default. My reason, at the time option
> --max-depth wasn't implemented yet. I submitted those changes in a
> separate series (these patches also originate from GitHub by the way).
> If those patches wouldn't land, I think always-on recursive behavior for
> git-last-modified(1) would be quite annoying.
> 
> So long story short, as git-last-modified(1) is still marked as
> "EXPERIMENTAL", shall we make recursive always-on?

I think that is a good idea, as it sounds like it would make common use
cases way more intuitive.

An alternative would be to default to a max-depth of 0 and stick to the
non-recursive default. In that case, the command would work intuitively
to the user, right? At least it would work intuitively if the user
doesn't want a recursive listing.

So maybe we should combine these two improvements. That is, we use:

  - An infinite max-depth by default with the recursive bahviour.

  - A max-depth of 0 in the non-recursive case.

In this case, we'd always recursively list all entries by default, but
in case the user explicitly doesn't want recursive behaviour we'd know
to show the last modification date of all provided files.

I was briefly thinking that a max-depth of 1 might be a more obvious
default for the non-recursive case. In that case, doing e.g. `git
last-modified --no-recursive t` would list the contents of "t/", but
nothing more. I'm a bit torn there though whether that really is a
sufficient improvement over a max-depth of 0.

Patrick