Thanks Eric for the review of the previous round and Duy and Junio for
additional comments.

Previous rounds are at <20180121120208.12760-1-t.gumme...@gmail.com>,
<20180204221305.28300-1-t.gumme...@gmail.com>,
<20180317220830.30963-1-t.gumme...@gmail.com> and
<20180317222219.4940-1-t.gumme...@gmail.com>.

This round should address all of Eric's comments from the previous round.

As explained in more detail in a reply to the review comment directly,
I did not add an enum to 'struct add_opts', for 'force_new_branch' and
'checkout_existing_branch', but instead removed 'force_new_branch'
from the struct as it's not required.

The rest of the updates are mainly in the user facing messages,
documentation and one added test.

Interdiff below:

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 98731b71a7..eaa6bf713f 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -67,7 +67,7 @@ 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.
+worktree (unless `--force` is used).
 
 list::
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index df5c0427ba..895838b943 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -28,7 +28,6 @@ struct add_opts {
        int checkout;
        int keep_locked;
        const char *new_branch;
-       int force_new_branch;
        int checkout_existing_branch;
 };
 
@@ -320,12 +319,12 @@ static int add_worktree(const char *path, const char 
*refname,
                goto done;
 
        if (opts->checkout_existing_branch)
-               fprintf(stderr, _("checking out branch '%s'"),
-                       refname);
+               fprintf_ln(stderr, _("checking out branch '%s'"),
+                          refname);
        else if (opts->new_branch)
-               fprintf(stderr, _("creating branch '%s'"), opts->new_branch);
+               fprintf_ln(stderr, _("creating branch '%s'"), opts->new_branch);
 
-       fprintf(stderr, _("worktree HEAD is now at %s"),
+       fprintf(stderr, _("new worktree HEAD is now at %s"),
                find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
 
        strbuf_reset(&sb);
@@ -434,8 +433,7 @@ static int add(int ac, const char **av, const char *prefix)
        if (!strcmp(branch, "-"))
                branch = "@{-1}";
 
-       opts.force_new_branch = !!new_branch_force;
-       if (opts.force_new_branch) {
+       if (new_branch_force) {
                struct strbuf symref = STRBUF_INIT;
 
                opts.new_branch = new_branch_force;
@@ -472,7 +470,7 @@ static int add(int ac, const char **av, const char *prefix)
                struct child_process cp = CHILD_PROCESS_INIT;
                cp.git_cmd = 1;
                argv_array_push(&cp.args, "branch");
-               if (opts.force_new_branch)
+               if (new_branch_force)
                        argv_array_push(&cp.args, "--force");
                argv_array_push(&cp.args, opts.new_branch);
                argv_array_push(&cp.args, branch);
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 721b0e4c26..fb99f4c46f 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -198,15 +198,15 @@ test_expect_success '"add" with <branch> omitted' '
        test_cmp_rev HEAD bat
 '
 
-test_expect_success '"add" auto-vivify checks out existing branch' '
+test_expect_success '"add" checks out existing branch of dwimd name' '
        test_commit c1 &&
        test_commit c2 &&
-       git branch precious HEAD~1 &&
-       git worktree add precious &&
-       test_cmp_rev HEAD~1 precious &&
+       git branch dwim HEAD~1 &&
+       git worktree add dwim &&
+       test_cmp_rev HEAD~1 dwim &&
        (
-               cd precious &&
-               test_cmp_rev precious HEAD
+               cd dwim &&
+               test_cmp_rev dwim HEAD
        )
 '
 
@@ -216,6 +216,10 @@ test_expect_success '"add" auto-vivify fails with checked 
out branch' '
        test_path_is_missing test-branch
 '
 
+test_expect_success '"add --force" with existing dwimd name doesnt die' '
+       git worktree add --force test-branch
+'
+
 test_expect_success '"add" no auto-vivify with --detach and <branch> omitted' '
        git worktree add --detach mish/mash &&
        test_must_fail git rev-parse mash -- &&

Thomas Gummerer (6):
  worktree: improve message when creating a new worktree
  worktree: be clearer when "add" dwim-ery kicks in
  worktree: remove force_new_branch from struct add_opts
  worktree: factor out dwim_branch function
  worktree: teach "add" to check out existing branches
  t2025: rename now outdated branch name

 Documentation/git-worktree.txt |  9 ++++--
 builtin/worktree.c             | 64 +++++++++++++++++++++++++++++++-----------
 t/t2025-worktree-add.sh        | 23 +++++++++++----
 3 files changed, 72 insertions(+), 24 deletions(-)

-- 
2.16.1.77.g8685934aa2

Reply via email to