Re: [PATCH 4/5] worktree: add "lock" command

From: Duy Nguyen <pclouds@gmail.com>
Date: May 22, 2016, 10:31 UTC
Message-ID: <CACsJy8CpsW+Jp=A8swDzR0+udGq8H-ampztyZwyXiK2mbcjqPA@mail.gmail.com>
In-Reply-To: <CAPig+cRMxYGx_gR9vei=6bcoXkhm2ntQSdqA3Vq9rtTzDd1RSQ@mail.gmail.com>

(the answer to rest of your questions is "yes you're right, will fix" or something along that line so I will not quote them here)

On Mon, May 16, 2016 at 7:09 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 9 quoted lines

>> +       old_reason = is_worktree_locked(wt);
>> +       if (old_reason) {
>> +               if (*old_reason)
>> +                       die(_("already locked, reason: %s"), old_reason);
>> +               die(_("already locked, no reason"));
>> +       }
>
> As a convenience, would it make sense to allow a worktree to be
> re-locked with a different reason rather than erroring out?

For now I think the user can just unlock then relock, after examining the previous reason and maybe incorporating it to the new reason. Yes there is a race but for these operations I don't think it really matters. If it's done often enough, we can lift this restriction.

Show 11 quoted lines

>> +       write_file(git_common_path("worktrees/%s/locked", wt->id),
>> +                  "%s", reason);
>> +       return 0;
>> +}
>
> This is a tangent, but it would be nice for the "git worktree list"
> command to show whether a worktree is locked, and the reason (if
> available), both in pretty and porcelain formats. (That was another
> reason why I suggested to Mike, back when he was adding the "list"
> command, that 'struct worktree' should have a 'locked' field and
> get_worktrees() should populate it automatically.)

Yes. And a good reason for get_worktrees() to read lock status automatically too. I will not do it right away though or at least until after I'm done with move/remove and the shared config file problem.

-- 
Duy