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

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Feb 28, 2022, 16:00 UTC
Message-ID: <e90340b0-09da-bf3a-f8eb-47fa408c51f1@gmail.com>
In-Reply-To: <xmqqh78mesj6.fsf@gitster.g>

On 25/02/2022 17:59, Junio C Hamano wrote:

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

Show 23 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.

We could have

	if (reason) {
		fputs("locked", stdout);
		if (*reason) {
			fputc(" ", stdout)
			write_name_quoted(reason, stdout, line_terminator);
		} else {
			fputc(line_terminator, stdout)
		
	}

which shares the code to print "locked" but I'm not sure it is any bettor overall though especially as write_name_quoted() means we only want to output a terminator when there is no reason text.

Best Wishes

Phillip