Re: [PATCH] worktree: add -z option for list subcommand

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 25, 2022, 17:59 UTC
Message-ID: <xmqqh78mesj6.fsf@gitster.g>
In-Reply-To: <pull.1164.git.1645801727732.gitgitgadget@gmail.com>

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +-z::
> +	When `--porcelain` is specified with `list` terminate each line with a
> +	NUL rather than a newline. This makes it possible to parse the output
> +	when a worktree path contains a newline character.

This makes it sound as if it were impossible. As the changes this patch makes to the documentation and the code indicate, we were already doing quote_c_style(), so it is not quite correct to say so.

Perhaps "makes it easier" is more accurate?

Unless the original wasn't using quote_c_style() correctly and wasn't quoting its output, that is?

Show 17 quoted lines

> @@ -411,7 +417,8 @@ working tree itself.
>  
>  Porcelain Format
>  ~~~~~~~~~~~~~~~~
> -The porcelain format has a line per attribute.  Attributes are listed with a
> +The porcelain format has a line per attribute.  If `-z` is given then the lines
> +are terminated with NUL rather than a newline.  Attributes are listed with a
>  label and value separated by a single space.  Boolean attributes (like `bare`
>  and `detached`) are listed as a label only, and are present only
>  if the value is true.  Some attributes (like `locked`) can be listed as a label
> @@ -449,7 +456,7 @@ prunable gitdir file points to non-existent location
>  
>  ------------
>  
> -If the lock reason contains "unusual" characters such as newline, they
> +Unless `-z` is used any "unusual" characters in the lock reason such as newlines
>  are escaped and the entire reason is quoted as explained for the

OK.  That is sensible.

Show 10 quoted lines

>  	reason = worktree_lock_reason(wt);
>  	if (reason && *reason) {
>  		struct strbuf sb = STRBUF_INIT;
> -		quote_c_style(reason, &sb, NULL, 0);
> -		printf("locked %s\n", sb.buf);
> +		if (line_terminator) {
> +			quote_c_style(reason, &sb, NULL, 0);
> +			reason = sb.buf;
> +		}
> +		printf("locked %s%c", reason, line_terminator);

OK. I suspect write_name_quoted() may be a better fit that does not require us to have our own strbuf, but this should be OK.

>  		strbuf_release(&sb);
>  	} else if (reason)
> -		printf("locked\n");
> +		printf("locked%c", line_terminator);

It is a shame that we need a special code path for an empty string given as the reason, only for the single SP after "locked", but we have to live with it, I guess.

Show 8 quoted lines

>  	reason = worktree_prune_reason(wt, expire);
>  	if (reason)
> -		printf("prunable %s\n", reason);
> +		printf("prunable %s%c", reason, line_terminator);
>  
> -	printf("\n");
> +	fputc(line_terminator, stdout);
>  }

All code changes looked sensible.

Show 15 quoted lines

> +	else if (!line_terminator && !porcelain)
> +		die(_("'-z' requires '--porcelain'"));
>  	else {
>  		struct worktree **worktrees = get_worktrees();
>  		int path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;
> @@ -708,7 +716,8 @@ static int list(int ac, const char **av, const char *prefix)
>  
>  		for (i = 0; worktrees[i]; i++) {
>  			if (porcelain)
> -				show_worktree_porcelain(worktrees[i]);
> +				show_worktree_porcelain(worktrees[i],
> +							line_terminator);
>  			else
>  				show_worktree(worktrees[i], path_maxlen, abbrev);
>  		}