From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sun, 08 Apr 2018 10:09:49 GMT
Subject: Re: [PATCH v6 6/6] worktree: teach "add" to check out existing branches
Message-ID: <CAPig+cSRa=HQkBKXoNNf2tW+c_j5mPKu2NHTUGXxyd6qsLzcnQ@mail.gmail.com>
In-Reply-To: <20180331151804.30380-7-t.gummerer@gmail.com>

On Sat, Mar 31, 2018 at 11:18 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> [...]
> However we can do a little better than that, and check the branch out if
> it is not checked out anywhere else.  This will help users who just want
> to check an existing branch out into a new worktree, and save a few
> keystrokes.
> [...]
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> ---
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -61,8 +61,13 @@ $ git worktree add --track -b <branch> <path> <remote>/<branch>
>  If `<commit-ish>` is omitted and neither `-b` nor `-B` nor `--detach` used,
> +then, as a convenience, a worktree with a branch named after
> +`$(basename <path>)` (call it `<branch>`) is created.

I had a hard time digesting this. I _think_ it wants to say:

    If `<commit-ish>` is omitted and neither `-b` nor `-B` nor
    `--detach` is used, then, as a convenience, the new worktree is
    associated with a branch (call it `<branch>`) named after
    `$(basename <path>)`.

>  If `<branch>`
> +doesn't exist, a new branch based on HEAD is automatically created as
> +if `-b <branch>` was given.  If `<branch>` exists in the repository,

Maybe: s/exists in the repository/does exist/
Or: s/.../is a local branch/

Though, the latter may be getting too pedantic.

> +it will be checked out in the new worktree, if it's not checked out
> +anywhere else, otherwise the command will refuse to create the
> +worktree (unless `--force` is used).
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> @@ -198,13 +198,24 @@ test_expect_success '"add" with <branch> omitted' '
> +test_expect_success '"add" checks out existing branch of dwimd name' '
> +       git branch dwim HEAD~1 &&
> +       git worktree add dwim &&
> +       test_cmp_rev HEAD~1 dwim &&
> +       (
> +               cd dwim &&
> +               test_cmp_rev dwim HEAD

Nit: Argument order of the two test_cmp_rev() invocations differs.

> +       )
> +'
> +
> +test_expect_success '"add <path>" dwim fails with checked out branch' '
> +       git checkout -b test-branch &&
> +       test_must_fail git worktree add test-branch &&
> +       test_path_is_missing test-branch
> +'
> +
> +test_expect_success '"add --force" with existing dwimd name doesnt die' '
> +       git worktree add --force test-branch
>  '

Maybe make this last test slightly more robust by having it "git
checkout test-branch" before "git worktree add ..." to protect against
someone inserting a new test (which checks out some other branch)
between these two. Probably not worth a re-roll.