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

[Eric Sunshine <sunshine@sunshineco.com>]

On Sun, May 22, 2016 at 6:31 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Mon, May 16, 2016 at 7:09 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> +       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.

Agreed. It's easy to imagine someone wanting to change the lock
message to correct a typo, but that's likely a rare occurrence, and
other conceivable reasons for wanting to change the message are
probably even more unusual, thus not worth worrying about now.

>>> +       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.

For the record (for future readers of this thread), [1] itemized
various useful bits of information that would be handy as fields in
'struct worktree' and populated automatically by get_worktrees(), all
of which (I think) could be nice to have in the output of "git
worktree list". Excerpt:

    For git-worktree commands such as "lock", "mv", "remove", it
    likely will be nice to allow people to specify the linked
    worktree not only by path, but also by tag, and possibly even by
    $(basename $path) if not ambiguous. Therefore, to support such
    usage, at minimum, I think you also want to show the worktree's
    tag (d->d_name) in addition to the path.

    Other information which would be nice to display for each
    worktree (possibly controlled by a --verbose flag):

       * the checked out branch or detached head
       * whether it is locked
            - the lock reason (if available)
            - and whether the worktree is currently accessible
        * whether it can be pruned
            - and the prune reason if so

    The prune reason could be obtained by factoring out the
    reason-determination code from worktree.c:prune_worktree() to
    make it re-usable.

[1]: http://article.gmane.org/gmane.comp.version-control.git/275528