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

From: Rafael Silva <[email-masked]>
Date: Sun, 24 Jan 2021 11:20:36 +0100

Eric Sunshine writes:

> On Tue, Jan 19, 2021 at 4:28 PM Rafael Silva
> <[email-masked]> wrote:
>> [...]
>> 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.
>> [...]
>> Signed-off-by: Rafael Silva <[email-masked]>
>> ---
>> diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
>> @@ -72,6 +72,36 @@ test_expect_success '"list" all worktrees with locked annotation' '
>> +test_expect_success '"list" all worktrees --porcelain with locked' '
>> +       test_when_finished "rm -rf locked1 locked2 unlocked out actual expect && git worktree prune" &&
>> +       echo "locked" >expect &&
>> +       echo "locked with reason" >>expect &&
>> +       git worktree add --detach locked1 &&
>> +       git worktree add --detach locked2 &&
>> +       # unlocked worktree should not be annotated with "locked"
>> +       git worktree add --detach unlocked &&
>> +       git worktree lock locked1 &&
>> +       git worktree lock locked2 --reason "with reason" &&
>> +       test_when_finished "git worktree unlock locked1 && git worktree unlock locked2" &&
>
> There's a minor problem here. If the second `git worktree lock`
> command fails, test_when_finished() will never be invoked, which means
> that the first lock won't get cleaned up, thus the worktree won't get
> pruned. To fix, you'd want:
>
>     git worktree lock locked1 &&
>     test_when_finished "git worktree unlock locked1" &&
>     git worktree lock locked2 --reason "with reason" &&
>     test_when_finished "git worktree unlock locked2" &&
>

Excellent point. This case didn't occur to me when I was working on v3, I
will make this change in the next revision.

>> +       git worktree list --porcelain >out &&
>> +       grep "^locked" out >actual &&
>> +       test_cmp expect actual
>> +'
>> +
>> +test_expect_success '"list" all worktrees --porcelain with locked reason newline escaped' '
>> +       test_when_finished "rm -rf locked_lf locked_crlf out actual expect && git worktree prune" &&
>> +       printf "locked \"locked\\\\r\\\\nreason\"\n" >expect &&
>> +       printf "locked \"locked\\\\nreason\"\n" >>expect &&
>> +       git worktree add --detach locked_lf &&
>> +       git worktree add --detach locked_crlf &&
>> +       git worktree lock locked_lf --reason "$(printf "locked\nreason")" &&
>> +       git worktree lock locked_crlf --reason "$(printf "locked\r\nreason")" &&
>> +       test_when_finished "git worktree unlock locked_lf && git worktree unlock locked_crlf" &&
>
> Same issue as above.
>
>> +       git worktree list --porcelain >out &&
>> +       grep "^locked" out >actual &&
>> +       test_cmp expect actual
>> +'

-- 
Thanks
Rafael