Re: [PATCH v5 5/6] worktree: teach "add" to check out existing branches
From: Eric Sunshine <hidden>
Date: 2018-03-27 09:04:24
On Sun, Mar 25, 2018 at 9:49 AM, Thomas Gummerer [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted> ---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.
quoted hunk ↗ jump to hunk
+ 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 '