On Sat, Mar 17, 2018 at 6:22 PM, Thomas Gummerer <t.gummerer@xxxxxxxxx> 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.
> [...]
> We will still 'die()' if the branch is checked out in another worktree,
> unless the --force flag is passed.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@xxxxxxxxx>
> ---
> 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 new branch based at HEAD is created automatically,
> -as if `-b $(basename <path>)` was specified.
> +then, as a convenience, a worktree with a branch named after
> +`$(basename <path>)` (call it `<branch>`) is created. 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,
> +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.
Should this mention --force?
... refuse to create the worktree (unless --force is used).
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -29,6 +29,7 @@ struct add_opts {
> int keep_locked;
> const char *new_branch;
> int force_new_branch;
> + int checkout_existing_branch;
As 'force_new_branch' and 'checkout_existing_branch' are mutually
exclusive, I wonder if they should be combined into an enum to make it
easier to reason about.
> @@ -318,8 +319,11 @@ static int add_worktree(const char *path, const char *refname,
> - if (opts->new_branch)
> - fprintf(stderr, _("creating new branch '%s'"), opts->new_branch);
> + if (opts->checkout_existing_branch)
> + fprintf(stderr, _("checking out branch '%s'"),
> + refname);
As with "creating branch" in 2/4, "checking out branch..." here is
missing a newline.
> + else if (opts->new_branch)
> + fprintf(stderr, _("creating branch '%s'"), opts->new_branch);
>
> fprintf(stderr, _("worktree HEAD is now at %s"),
> find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> @@ -198,13 +198,22 @@ test_expect_success '"add" with <branch> omitted' '
> -test_expect_success '"add" auto-vivify does not clobber existing branch' '
> +test_expect_success '"add" auto-vivify checks out existing branch' '
> 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
> + )
> +'
This test is no longer checking auto-vivification ("bringing a new
branch to life automatically"), but rather branch name inference, so
the title is now misleading. Perhaps retitle it to '"add" checks out
existing branch of dwim'd name' or something.
(The name "precious" is also now misleading since it's no longer
checking that a precious branch does not get clobbered, however,
changing the name would make the diff unnecessarily noisy, so it's
probably okay as is.)
> +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
> '
Should we also have a corresponding test that this "fail" can be
overridden by --force? (I realize that --force is tested elsewhere,
but only with an explicitly-provided branch name, whereas this dwims
the branch name.)
