From: Karthik Nayak <karthik.188@gmail.com>
Date: Wed, 26 Nov 2025 13:03:06 GMT
Subject: Re: [PATCH 1/3] last-modified: handle and document NUL termination
Message-ID: <CAOLa=ZRn45fDriKvwVgLV5yXbKUr7bwLadHjemPjb+Mw2tHVqQ@mail.gmail.com>
In-Reply-To: <20251126-toon-last-modified-zzzz-v1-1-608350df0caa@iotcl.com>

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