All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rafael Silva <rafaeloliveira.cs@gmail.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Git List <git@vger.kernel.org>, Phillip Wood <phillip.wood123@gmail.com>
Subject: Re: [PATCH v3 5/7] worktree: teach `list --porcelain` to annotate locked worktree
Date: Sun, 24 Jan 2021 11:20:36 +0100	[thread overview]
Message-ID: <gohp6kim7mocul.fsf@gmail.com> (raw)
In-Reply-To: <CAPig+cRBDSWCmrV+6w0gXaSH+xWEX1354NhfyjhCouDdsfEAGA@mail.gmail.com>


Eric Sunshine writes:

> On Tue, Jan 19, 2021 at 4:28 PM Rafael Silva
> <rafaeloliveira.cs@gmail.com> 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 <rafaeloliveira.cs@gmail.com>
>> ---
>> 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

  reply	other threads:[~2021-01-24 10:29 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-04 16:21 [PATCH 0/7] teach `worktree list` verbose mode and prunable annotations Rafael Silva
2021-01-04 16:21 ` [PATCH 1/7] worktree: move should_prune_worktree() to worktree.c Rafael Silva
2021-01-06  5:58   ` Eric Sunshine
2021-01-08  7:40     ` Rafael Silva
2021-01-06  6:55   ` Eric Sunshine
2021-01-07  7:24     ` Eric Sunshine
2021-01-08  7:41     ` Rafael Silva
2021-01-04 16:21 ` [PATCH 2/7] worktree: implement worktree_prune_reason() wrapper Rafael Silva
2021-01-06  7:08   ` Eric Sunshine
2021-01-08  7:42     ` Rafael Silva
2021-01-04 16:21 ` [PATCH 3/7] worktree: teach worktree_lock_reason() to gently handle main worktree Rafael Silva
2021-01-06  7:29   ` Eric Sunshine
2021-01-08  7:43     ` Rafael Silva
2021-01-04 16:21 ` [PATCH 4/7] worktree: teach `list` prunable annotation and verbose Rafael Silva
2021-01-06  8:31   ` Eric Sunshine
2021-01-08  7:45     ` Rafael Silva
2021-01-04 16:21 ` [PATCH 5/7] worktree: `list` escape lock reason in --porcelain Rafael Silva
2021-01-05 10:29   ` Phillip Wood
2021-01-05 11:02     ` [PATCH] worktree: add -z option for list subcommand Phillip Wood
2021-01-07  3:34       ` Eric Sunshine
2021-01-08 10:33         ` Phillip Wood
2021-01-10  7:27           ` Eric Sunshine
2021-01-06  9:07     ` [PATCH 5/7] worktree: `list` escape lock reason in --porcelain Eric Sunshine
2021-01-08  7:47     ` Rafael Silva
2021-01-06  8:59   ` Eric Sunshine
2021-01-04 16:21 ` [PATCH 6/7] worktree: add tests for `list` verbose and annotations Rafael Silva
2021-01-06  9:39   ` Eric Sunshine
2021-01-07  4:09     ` Eric Sunshine
2021-01-08  7:49     ` Rafael Silva
2021-01-04 16:21 ` [PATCH 7/7] worktree: document `list` verbose and prunable annotations Rafael Silva
2021-01-06  9:57   ` Eric Sunshine
2021-01-08  7:49     ` Rafael Silva
2021-01-06  5:36 ` [PATCH 0/7] teach `worktree list` verbose mode " Eric Sunshine
2021-01-08  7:38   ` Rafael Silva
2021-01-08  8:19     ` Eric Sunshine
2021-01-17 23:42 ` [PATCH v2 0/6] " Rafael Silva
2021-01-17 23:42   ` [PATCH v2 1/6] worktree: libify should_prune_worktree() Rafael Silva
2021-01-17 23:42   ` [PATCH v2 2/6] worktree: teach worktree to lazy-load "prunable" reason Rafael Silva
2021-01-18  2:57     ` Eric Sunshine
2021-01-19  7:57       ` Rafael Silva
2021-01-17 23:42   ` [PATCH v2 3/6] worktree: teach worktree_lock_reason() to gently handle main worktree Rafael Silva
2021-01-17 23:42   ` [PATCH v2 4/6] worktree: teach `list --porcelain` to annotate locked worktree Rafael Silva
2021-01-18  3:55     ` Eric Sunshine
2021-01-19  8:20       ` Rafael Silva
2021-01-19 17:16         ` Eric Sunshine
2021-01-17 23:42   ` [PATCH v2 5/6] worktree: teach `list` to annotate prunable worktree Rafael Silva
2021-01-18  4:45     ` Eric Sunshine
2021-01-19 10:26       ` Rafael Silva
2021-01-19 17:23         ` Eric Sunshine
2021-01-17 23:42   ` [PATCH v2 6/6] worktree: teach `list` verbose mode Rafael Silva
2021-01-18  5:15     ` Eric Sunshine
2021-01-18 19:40       ` Eric Sunshine
2021-01-18  5:33   ` [PATCH v2 0/6] teach `worktree list` verbose mode and prunable annotations Eric Sunshine
2021-01-19 16:44     ` Rafael Silva
2021-01-19 21:27   ` [PATCH v3 0/7] " Rafael Silva
2021-01-19 21:27     ` [PATCH v3 1/7] worktree: libify should_prune_worktree() Rafael Silva
2021-01-19 21:27     ` [PATCH v3 2/7] worktree: teach worktree to lazy-load "prunable" reason Rafael Silva
2021-01-19 21:27     ` [PATCH v3 3/7] worktree: teach worktree_lock_reason() to gently handle main worktree Rafael Silva
2021-01-19 21:27     ` [PATCH v3 4/7] t2402: ensure locked worktree is properly cleaned up Rafael Silva
2021-01-24  7:50       ` Eric Sunshine
2021-01-24 10:19         ` Rafael Silva
2021-01-19 21:27     ` [PATCH v3 5/7] worktree: teach `list --porcelain` to annotate locked worktree Rafael Silva
2021-01-20 11:00       ` Phillip Wood
2021-01-21  3:18         ` Junio C Hamano
2021-01-21 15:25         ` Rafael Silva
2021-01-24  8:24         ` Eric Sunshine
2021-01-24  8:10       ` Eric Sunshine
2021-01-24 10:20         ` Rafael Silva [this message]
2021-01-19 21:27     ` [PATCH v3 6/7] worktree: teach `list` to annotate prunable worktree Rafael Silva
2021-01-21  3:28       ` Junio C Hamano
2021-01-21 15:09         ` Rafael Silva
2021-01-21 22:18           ` Junio C Hamano
2021-01-19 21:27     ` [PATCH v3 7/7] worktree: teach `list` verbose mode Rafael Silva
2021-01-24  8:42       ` Eric Sunshine
2021-01-24 10:21         ` Rafael Silva
2021-01-24  8:51     ` [PATCH v3 0/7] teach `worktree list` verbose mode and prunable annotations Eric Sunshine
2021-01-27  8:08       ` Rafael Silva
2021-01-27  8:03     ` Rafael Silva
2021-01-27  8:03       ` [PATCH v4 1/7] worktree: libify should_prune_worktree() Rafael Silva
2021-01-27  8:03       ` [PATCH v4 2/7] worktree: teach worktree to lazy-load "prunable" reason Rafael Silva
2021-01-27  8:03       ` [PATCH v4 3/7] worktree: teach worktree_lock_reason() to gently handle main worktree Rafael Silva
2021-01-27  8:03       ` [PATCH v4 4/7] t2402: ensure locked worktree is properly cleaned up Rafael Silva
2021-01-27  8:03       ` [PATCH v4 5/7] worktree: teach `list --porcelain` to annotate locked worktree Rafael Silva
2021-01-27  8:03       ` [PATCH v4 6/7] worktree: teach `list` to annotate prunable worktree Rafael Silva
2021-01-27  8:03       ` [PATCH v4 7/7] worktree: teach `list` verbose mode Rafael Silva
2021-01-30  7:04       ` [PATCH v3 0/7] teach `worktree list` verbose mode and prunable annotations Eric Sunshine
2021-01-30  9:42         ` Rafael Silva
2021-01-30 17:50         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=gohp6kim7mocul.fsf@gmail.com \
    --to=rafaeloliveira.cs@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood123@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.