Re: [PATCH v3 5/7] worktree: teach `list --porcelain` to annotate locked worktree

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

Hi Phillip,

Phillip Wood writes:

> Hi Rafael
>
> Thanks for reworking this to use c_quote_path(). I have a couple of
> comments below.
>

Thanks for reviewing this patch.

> On 19/01/2021 21:27, Rafael Silva wrote:
>> Commit c57b3367be (worktree: teach `list` to annotate locked worktree,
>> 2020-10-11) taught "git worktree list" to annotate locked worktrees by
>> appending "locked" text to its output, however, this is not listed in
>> the --porcelain format.
>> Teach "list --porcelain" to do the same and add a "locked" attribute
>> followed by its reason, thus making both default and porcelain format
>> consistent. If the locked reason is not available then only "locked"
>> is shown.
>> The output of the "git worktree list --porcelain" becomes like so:
>>      $ git worktree list --porcelain
>>      ...
>>      worktree /path/to/locked
>>      HEAD 123abcdea123abcd123acbd123acbda123abcd12
>>      detached
>>      locked
>>      worktree /path/to/locked-with-reason
>>      HEAD abc123abc123abc123abc123abc123abc123abc1
>>      detached
>>      locked reason why it is locked
>>      ...
>> In porcelain mode, if the lock reason contains special characters
>> such as newlines, they are escaped with backslashes and the entire
>> reason is enclosed in double quotes. For example:
>>     $ git worktree list --porcelain
>>     ...
>>     locked "worktree's path mounted in\nremovable device"
>>     ...
>> Furthermore, let's update the documentation to state that some
>> attributes in the porcelain format might be listed alone or together
>> with its value depending whether the value is available or not. Thus
>> documenting the case of the new "locked" attribute.
>> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
>> Signed-off-by: Rafael Silva <rafaeloliveira.cs@gmail.com>
>> ---
>>   Documentation/git-worktree.txt | 16 ++++++++++++++--
>>   builtin/worktree.c             | 13 +++++++++++++
>>   t/t2402-worktree-list.sh       | 30 ++++++++++++++++++++++++++++++
>>   3 files changed, 57 insertions(+), 2 deletions(-)
>> diff --git a/Documentation/git-worktree.txt
>> b/Documentation/git-worktree.txt
>> index 02a706c4c0..7cb8124f28 100644
>> --- a/Documentation/git-worktree.txt
>> +++ b/Documentation/git-worktree.txt
>> @@ -377,8 +377,10 @@ Porcelain Format
>>   The porcelain format has a line per attribute.  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.  The first attribute of a working tree is always
>> -`worktree`, an empty line indicates the end of the record.  For example:
>> +if the value is true.  Some attributes (like `locked`) can be listed as a label
>> +only or with a value depending upon whether a reason is available.  The first
>> +attribute of a working tree is always `worktree`, an empty line indicates the
>> +end of the record.  For example:
>
> I think it would be helpful to document that the reasons are quoted
> according core.quotePath.
>

Good point. I'll include this addition on the next revision.

-- 
Thanks
Rafael