Re: [PATCH 5/7] worktree: `list` escape lock reason in --porcelain

[Rafael Silva <rafaeloliveira.cs@gmail.com>]

Hi Phillip,

Thank you for reviewing the patch and the suggestions.

Phillip Wood writes:

> On 04/01/2021 16:21, Rafael Silva wrote:
>> "git worktree list" porcelain format shows one attribute per line, each
>> attribute is listed with a label and value separated by a single space.
>> If any worktree is locked, a "locked" attribute is listed followed by the
>> reason, if available, otherwise only "locked" is shown.
>> In case the lock reason contains newline characters (i.e: LF or
>> CRLF)
>> this will cause the format to break as each line should correspond to
>> one attribute. This leads to unwanted behavior, specially as the
>> porcelain is intended to be machine-readable. To address this shortcoming
>> let's teach "git worktree list" to escape any newline character before
>> outputting the locked reason for porcelain format.
>
> As the porcelain output format cannot handle worktree paths that
> contain newlines either I think it would be better to add a `-z` flag
> which uses NUL termination as we have for many other commands (diff,
> ls-files etc). This would fix the problem forever without having to
> worry about quoting each time a new field is added.
>

This is a good point and it seems to be good addition for the porcelain format.

Perhaps it make more sense to apply as separate patch though, something
that Eric Sunshine also mentioned on this message reply.

> If you really need to quote the lock reason then please use one of the
> path quoting routines (probably quote_c_style() or
> write_name_quoted()) in quote.c rather than rolling your own - the
> code below gives the same output for a string that contains the two
> characters `\` followed by `n` as it does for the single character
> `\n`. People are (hopefully) used to dequoting our path quoting and
> there are routines out there to do it (we have one git Git.pm) using a
> function in quote.c will allow them to use those routines here.
>
> Best Wishes
>
> Phillip
>

That's great, thank you for pointing this out to me. I will definitely
change this into the next revision by dropping the function written from
scratch on this patch and reuse one of the existing functions.

For what is worth, I just added the function because I didn't know about
the existing ones. So, there is no reason to add the new function.

-- 
Thanks
Rafael