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

Hi Phillip,

Thank you for reviewing the patch and the suggestions.

Phillip Wood writes:

On 04/01/2021 16:21, Rafael Silva wrote:

quoted

"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