Subject: [PATCH v6 6.5/6] fixup! worktree: teach "add" to check out existing branches

From: Thomas Gummerer <[email-masked]>
Date: Sun, 1 Apr 2018 14:11:38 +0100

Signed-off-by: Thomas Gummerer <[email-masked]>
---

So while playing with it a bit more I found one case where the new UI
is not ideal and a bit confusing.  Namely when the new check out dwim
kicks in, but there is already a file/directory at the path we're
giving to 'git worktree add'.

In that case something like the following would be printed:

    $ g worktree add ../next
    Checking out branch 'next'
    fatal: '../next' already exists

Instead I think we'd just want the error without the "Checking out
branch" message, which is what this fixup here does.

One thing that gets a bit strange is that the "Checking out branch"
message and the "Creating branch" messages are printed from different
places.  But without doing quite some refactoring I don't think
there's a good way to do that, and I think having the UI do the right
thing is more important.

One thing I also noticed is that if a branch is created by 'git
worktree add', but we fail, we never clean up that branch again, which
I'm not sure is ideal.  As a pre-existing problem I'd like to keep
fixing that out of the scope of this series though (at least after
this series the user would get some output showing that this happened,
even when the branch is not set up to track a remote), so I'd like to
keep fixing that out of the scope of this series.

Junio: could you please squash this in in 6/6 while queuing?  I'd
prefer not to re-send the whole series just for fixing this up, but
obviously can if that makes your life easier :)

Thanks!

 builtin/worktree.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 511d0aa370..ccc2e63e0f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -27,6 +27,7 @@ struct add_opts {
 	int detach;
 	int checkout;
 	int keep_locked;
+	int checkout_existing_branch;
 };
 
 static int show_only;
@@ -316,6 +317,8 @@ static int add_worktree(const char *path, const char *refname,
 	if (ret)
 		goto done;
 
+	if (opts->checkout_existing_branch)
+		  fprintf_ln(stderr, _("Checking out branch '%s'"), refname);
 	if (opts->checkout) {
 		cp.argv = NULL;
 		argv_array_clear(&cp.args);
@@ -397,7 +400,6 @@ static int add(int ac, const char **av, const char *prefix)
 	const char *branch;
 	const char *new_branch = NULL;
 	const char *opt_track = NULL;
-	int checkout_existing_branch = 0;
 	struct option options[] = {
 		OPT__FORCE(&opts.force, N_("checkout <branch> even if already checked out in other worktree")),
 		OPT_STRING('b', NULL, &new_branch, N_("branch"),
@@ -443,7 +445,7 @@ static int add(int ac, const char **av, const char *prefix)
 
 	if (ac < 2 && !new_branch && !opts.detach) {
 		const char *s = dwim_branch(path, &new_branch,
-					    &checkout_existing_branch);
+					    &opts.checkout_existing_branch);
 		if (s)
 			branch = s;
 	}
@@ -478,8 +480,6 @@ static int add(int ac, const char **av, const char *prefix)
 		if (run_command(&cp))
 			return -1;
 		branch = new_branch;
-	} else if (checkout_existing_branch) {
-		  fprintf_ln(stderr, _("Checking out branch '%s'"), branch);
 	} else if (opt_track) {
 		die(_("--[no-]track can only be used if a new branch is created"));
 	}
-- 
2.16.1.78.g71f731ae26.dirty