Re: [PATCH v2 6/6] rev-list: support NUL-delimited --missing option

[Patrick Steinhardt <ps@pks.im>]

On Wed, Mar 12, 2025 at 07:17:06PM -0500, Justin Tobler wrote:
> diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
> index d400d76cf2..145ded5c78 100644
> --- a/Documentation/rev-list-options.adoc
> +++ b/Documentation/rev-list-options.adoc
> @@ -371,8 +371,8 @@ ifdef::git-rev-list[]
>  <OID> NUL [<token>=<value> NUL]...
>  -----------------------------------------------------------------------
>  +
> -Additional object metadata, such as object paths or boundary objects, is
> -printed using the `<token>=<value>` form. Token values are printed as-is
> +Additional object metadata, such as object paths or boundary/missing objects,
> +is printed using the `<token>=<value>` form. Token values are printed as-is
>  without any encoding/truncation. An OID entry never contains a '=' character
>  and thus is used to signal the start of a new object record. Examples:
>  +

Nit: I don't think we need to update this paragraph here as it is
written as a non-exhaustive list anyway.

> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 7c6d4b25b0..d7b4dd48ff 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -136,24 +136,39 @@ static void print_missing_object(struct missing_objects_map_entry *entry,
>  {
>  	struct strbuf sb = STRBUF_INIT;
>  
> +	if (line_term)
> +		putchar('?');
> +
> +	printf("%s", oid_to_hex(&entry->entry.oid));
> +
> +	if (!line_term)
> +		printf("%cmissing=yes", info_term);
> +
>  	if (!print_missing_info) {
> -		printf("?%s\n", oid_to_hex(&entry->entry.oid));
> +		putchar(line_term);
>  		return;
>  	}
>  
>  	if (entry->path && *entry->path) {
>  		struct strbuf path = STRBUF_INIT;

Nit: the variable and its cleanup could be moved closer to where it's
used.

> -		strbuf_addstr(&sb, " path=");
> -		quote_path(entry->path, NULL, &path, QUOTE_PATH_QUOTE_SP);
> -		strbuf_addbuf(&sb, &path);
> +		strbuf_addf(&sb, "%cpath=", info_term);
> +
> +		if (line_term) {
> +			quote_path(entry->path, NULL, &path, QUOTE_PATH_QUOTE_SP);
> +			strbuf_addbuf(&sb, &path);
> +		} else {
> +			strbuf_addstr(&sb, entry->path);
> +		}
>  
>  		strbuf_release(&path);
>  	}
>  	if (entry->type)
> -		strbuf_addf(&sb, " type=%s", type_name(entry->type));
> +		strbuf_addf(&sb, "%ctype=%s", info_term, type_name(entry->type));
> +
> +	fwrite(sb.buf, sizeof(char), sb.len, stdout);
> +	putchar(line_term);
>  
> -	printf("?%s%s\n", oid_to_hex(&entry->entry.oid), sb.buf);
>  	strbuf_release(&sb);
>  }
>  

Patrick