Re: [PATCH v5 2/6] worktree: be clearer when "add" dwim-ery kicks in

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 27, 2018, 08:59 UTC
Message-ID: <CAPig+cSNYdJwBS1me+bkELDmrPHgDZJDwjA=o=O4CzQBT=jpzQ@mail.gmail.com>
In-Reply-To: <20180325134947.25828-3-t.gummerer@gmail.com>

On Sun, Mar 25, 2018 at 9:49 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:

Show 5 quoted lines

> Currently there is no indication in the "git worktree add" output that
> a new branch was created.  This would be especially useful information
> in the case where the dwim of "git worktree add <path>" kicks in, as the
> user didn't explicitly ask for a new branch, but we create one from
> them.

Failed to notice this last time around: s/from/for/

Perhaps Junio can tweak it when queuing.

> Print some additional output showing that a branch was created and the
> branch name to help the user.
>
> This will also be useful in the next commit, which introduces a new kind

It's no longer the _next_ commit which does this. Perhaps say instead "a subsequent commit". (Again, perhaps Junio can tweak it since it's not worth a re-roll.)

Show 10 quoted lines

> of dwim-ery of checking out the branch if it exists instead of refusing
> to create a new worktree in that case, and where it's nice to tell the
> user which kind of dwim-ery kicked in.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -318,6 +318,9 @@ static int add_worktree(const char *path, const char *refname,
> +       if (opts->new_branch)
> +               fprintf_ln(stderr, _("creating branch '%s'"), opts->new_branch);

I didn't think of this before, but what happens with the original dwim-ery you added which checks out a remote branch matching the worktree name if no such local branch exists? I was wondering if we'd want to see a distinct and more informative message in that case, such as "creating branch '%s' from remote '%s'" or something. However, I see that we're already getting a message:

    Branch 'foo' set up to track remote branch 'foo' from 'origin'.
    creating branch 'foo'
    new worktree HEAD is now at d3adb33f boople

which is a bit weird since tracking appears to be set up before the branch itself is created. I thought that this was another UI regression of the sort I mentioned in [1], however, the messages were out of order even before the current patch series:

    Branch 'foo' set up to track remote branch 'foo' from 'origin'.
    Preparing foo (identifier foo)
    Checking out files: 100% (9999/9999), done.
    HEAD is now at d3adb33f boople

This highlights another regression introduced by this series. Patch 1/6 suppresses the "Checking out files:" message, which is a bit unfortunate for decent sized worktrees. I'm not sure I'm entirely happy about that loss. Thoughts?

[1]: https://public-inbox.org/git/CAPig+cT8i9L9kbhx=b0sG4_QYNdoEDPW-1xypM9xzBqPmqR__Q@mail.gmail.com/

>         fprintf(stderr, _("new worktree HEAD is now at %s"),
>                 find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));