On Sun, Mar 25, 2018 at 9:49 AM, Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
> Currently 'git worktree add <path>' creates a new branch named after the
> basename of the path by default. If a branch with that name already
> exists, the command refuses to do anything, unless the '--force' option
> is given.
>
> 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@xxxxxxxxx>
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -317,7 +318,10 @@ static int add_worktree(const char *path, const char *refname,
> - if (opts->new_branch)
> + if (opts->checkout_existing_branch)
> + fprintf_ln(stderr, _("checking out branch '%s'"),
> + refname);
This fprintf_ln() can easily fit on one line; no need to wrap
'refname' to the next one.
Not worth a re-roll, though.
> + else if (opts->new_branch)
> fprintf_ln(stderr, _("creating branch '%s'"), opts->new_branch);
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> @@ -198,13 +198,26 @@ test_expect_success '"add" with <branch> omitted' '
> -test_expect_success '"add" auto-vivify does not clobber existing branch' '
> +test_expect_success '"add" checks out existing branch of dwimd name' '
> test_commit c1 &&
> test_commit c2 &&
> git branch precious HEAD~1 &&
> - test_must_fail git worktree add precious &&
> + git worktree add precious &&
> test_cmp_rev HEAD~1 precious &&
> - test_path_is_missing precious
> + (
> + cd precious &&
> + test_cmp_rev precious HEAD
> + )
> +'
Looking at this more closely, once it stops being a "don't clobber
precious branch" test, it's doing more than it needs to do, thus is
confusing for future readers. The setup -- creating two commits and
making "precious" point one commit back -- was very intentional and
allowed the test to verify not only that the worktree wasn't created
but that "precious" didn't change to reference a different commit.
However, _none_ of this matters once it becomes a dwim'ing test; the
unnecessary code is confusing since it doesn't make sense in the
context of dwim'ing. I _think_ the entire test can collapse to:
git branch existing &&
git worktree add existing &&
(
cd existing &&
test_cmp_rev existing HEAD
)
In other words, this patch should drop the "precious" test altogether
and just introduce a new dwim'ing test (and drop patch 6/6).
I do think that with the potential confusion to future readers, this
does deserve a re-roll.
> +test_expect_success '"add" auto-vivify 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
> '
