git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Thomas Gummerer <t.gummerer@gmail.com>
To: git@vger.kernel.org
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Nguyễn Thái Ngọc Duy  <pclouds@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Thomas Gummerer <t.gummerer@gmail.com>
Subject: [PATCH v7 0/4] worktree: teach "add" to check out existing branches
Date: Sun, 15 Apr 2018 21:29:13 +0100
Message-ID: <20180415202917.4360-1-t.gummerer@gmail.com> (raw)
In-Reply-To: <20180331151804.30380-1-t.gummerer@gmail.com>

Thanks Eric for the review and all of the suggestions in the last
round.

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

The main change once again in this series is the user interface.  It
feels like we went in a complete circle here now, as this iteration is
bringing the "Preparing ..." line back (although in a slightly
different form than the original), and is moving away from printing
it's own "HEAD is now at..." line again.  This also means we don't
need the new hidden option to 'git reset' anymore, which is nice.

I do like the new UI more than what we had in the last round (which I
already liked more than the original UI) :)

Other than those changes, it also includes Eric's suggestion for a
better wording in the documentation, fixes the argument order to
test_cmd_rev in a test, and makes a test more robust.

To demonstrate the UI updates, let's compare the new UI and the old UI
again.  This is the same commands as used for the demonstration in the
last iteration, so please have a look at <20180331151804.30380-1-t.gummerer@gmail.com> 
for an example of what it looked like after the last round.

New UI:

 - guess-remote mode

    $ git worktree add --guess-remote ../next
    Preparing worktree (new branch 'next')
    Branch 'next' set up to track remote branch 'next' from 'origin'.
    HEAD is now at caa68db14 Merge branch 'sb/packfiles-in-repository' into next

 - original dwim (create a branch based on the current HEAD)

    $ git worktree add ../test
    Preparing worktree (new branch 'test')
    HEAD is now at c2a499e6c Merge branch 'jh/partial-clone'

 - new dwim (check out existing branch)

    $ git worktree add ../test
    Preparing worktree (checking out 'test')
    HEAD is now at c2a499e6c Merge branch 'jh/partial-clone'

 - detached HEAD

    $ git worktree add ../test2 origin/master
    Preparing worktree (detached HEAD c2a499e6c)
    HEAD is now at c2a499e6c Merge branch 'jh/partial-clone'

 - resetting existing branch

    $ git worktree add -B next ../test3 origin/master
    Preparing worktree (resetting branch 'next' (was at caa68db14))
    Branch 'next' set up to track remote branch 'master' from 'origin'.
    HEAD is now at c2a499e6c Merge branch 'jh/partial-clone'

    This output is new in this round and wasn't previously discussed.
    While working on the "Preparing ..." line I noticed this, and
    thought it would be a good idea.  I feel like this fits in the
    scope of the series quite well, as it's improving the UI, but I'm
    happy to split it out if that's preferred.

    It may also be worth displaying the title of the commit here, but
    at that point the line would get a bit long, so dunno.

 - large repository (with original dwim)

    $ git worktree add ../test
    Preparing worktree (new branch 'test')
    Checking out files: 100% (62915/62915), done.
    HEAD is now at c2a9838452a4 Merge tag 'for-4.16/dm-fixes-4' of git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm

 - error when directory already exists

    $ git worktree add ../test
    Preparing worktree (checking out 'test')
    fatal: '../test' already exists

Compare this to the old UI (new dwim omitted, as there's no old
version of that):

 - guess-remote mode

    $ git worktree add --guess-remote ../next
    Branch 'next' set up to track remote branch 'next' from 'origin'.
    Preparing ../next (identifier next)
    HEAD is now at caa68db14 Merge branch 'sb/packfiles-in-repository' into next

 - original dwim (create a branch based on the current HEAD)

    $ git worktree add ../test
    Preparing ../test (identifier test)
    HEAD is now at c2a499e6c Merge branch 'jh/partial-clone'

 - no new branch created

    $ git worktree add ../test2 origin/master
    Preparing ../test2 (identifier test2)
    HEAD is now at c2a499e6c Merge branch 'jh/partial-clone'

 - large repository

    $ git worktree add ../test
    Preparing ../test (identifier test)
    Checking out files: 100% (62915/62915), done.
    HEAD is now at c2a9838452a4 Merge tag 'for-4.16/dm-fixes-4' of git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm

 - error when directory already exists

    $ git worktree add ../test
    fatal: '../test' already exists

Interdiff below:

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index eaa6bf713f..5d54f36a71 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -61,13 +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 worktree with a branch named after
-`$(basename <path>)` (call it `<branch>`) is created.  If `<branch>`
+then, as a convenience, the new worktree is associated with a branch
+(call it `<branch>`) named after `$(basename <path>)`.  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 (unless `--force` is used).
+if `-b <branch>` was given.  If `<branch>` does exist, 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 (unless
+`--force` is used).
 
 list::
 
diff --git a/builtin/reset.c b/builtin/reset.c
index 54b2576449..e15f595799 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -288,7 +288,6 @@ static int git_reset_config(const char *var, const char *value, void *cb)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int reset_type = NONE, update_ref_status = 0, quiet = 0;
-	int show_new_head_line = 1;
 	int patch_mode = 0, unborn;
 	const char *rev;
 	struct object_id oid;
@@ -311,7 +310,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
 		OPT_BOOL('N', "intent-to-add", &intent_to_add,
 				N_("record only the fact that removed paths will be added later")),
-		OPT_HIDDEN_BOOL(0, "show-new-head-line", &show_new_head_line, N_("show information on the new HEAD")),
 		OPT_END()
 	};
 
@@ -405,8 +403,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		 * switched to, saving the previous head in ORIG_HEAD before. */
 		update_ref_status = reset_refs(rev, &oid);
 
-		if (reset_type == HARD && !update_ref_status && !quiet &&
-		    show_new_head_line)
+		if (reset_type == HARD && !update_ref_status && !quiet)
 			print_new_head_line(lookup_commit_reference(&oid));
 	}
 	if (!pathspec.nr)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index ccc2e63e0f..f5a5283b39 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -27,7 +27,6 @@ struct add_opts {
 	int detach;
 	int checkout;
 	int keep_locked;
-	int checkout_existing_branch;
 };
 
 static int show_only;
@@ -317,28 +316,16 @@ 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);
 		argv_array_pushl(&cp.args, "reset", "--hard", NULL);
-		argv_array_push(&cp.args, "--no-show-new-head-line");
 		cp.env = child_env.argv;
 		ret = run_command(&cp);
 		if (ret)
 			goto done;
 	}
 
-	fprintf(stderr, _("New worktree HEAD is now at %s"),
-		find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
-
-	strbuf_reset(&sb);
-	pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
-	if (sb.len > 0)
-		fprintf(stderr, " %s", sb.buf);
-	fputc('\n', stderr);
-
 	is_junk = 0;
 	FREE_AND_NULL(junk_work_tree);
 	FREE_AND_NULL(junk_git_dir);
@@ -366,6 +353,34 @@ static int add_worktree(const char *path, const char *refname,
 	return ret;
 }
 
+static void print_preparing_worktree_line(const char *branch,
+					  const char *new_branch,
+					  const char *new_branch_force,
+					  int checkout_existing_branch)
+{
+	if (checkout_existing_branch) {
+		printf_ln(_("Preparing worktree (checking out '%s')"), branch);
+	} else if (new_branch_force) {
+		struct commit *commit = lookup_commit_reference_by_name(new_branch_force);
+		if (!commit)
+			printf_ln(_("Preparing worktree (new branch '%s')"), new_branch_force);
+		else
+			printf_ln(_("Preparing worktree (resetting branch '%s' (was at %s))"),
+				  new_branch_force,
+				  find_unique_abbrev(commit->object.oid.hash,
+						     DEFAULT_ABBREV));
+	} else if (new_branch) {
+		printf_ln(_("Preparing worktree (new branch '%s')"), new_branch);
+	} else {
+		struct commit *commit = lookup_commit_reference_by_name(branch);
+		if (!commit)
+			die(_("invalid reference: %s"), branch);
+		printf_ln(_("Preparing worktree (detached HEAD %s)"),
+			  find_unique_abbrev(commit->object.oid.hash,
+					     DEFAULT_ABBREV));
+	}
+}
+
 static const char *dwim_branch(const char *path, const char **new_branch,
 			       int *checkout_existing_branch)
 {
@@ -397,6 +412,7 @@ static int add(int ac, const char **av, const char *prefix)
 	struct add_opts opts;
 	const char *new_branch_force = NULL;
 	char *path;
+	int checkout_existing_branch = 0;
 	const char *branch;
 	const char *new_branch = NULL;
 	const char *opt_track = NULL;
@@ -445,7 +461,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,
-					    &opts.checkout_existing_branch);
+					    &checkout_existing_branch);
 		if (s)
 			branch = s;
 	}
@@ -465,10 +481,11 @@ static int add(int ac, const char **av, const char *prefix)
 		}
 	}
 
+	print_preparing_worktree_line(branch, new_branch, new_branch_force,
+				      checkout_existing_branch);
+
 	if (new_branch) {
 		struct child_process cp = CHILD_PROCESS_INIT;
-
-		fprintf_ln(stderr, _("Creating branch '%s'"), new_branch);
 		cp.git_cmd = 1;
 		argv_array_push(&cp.args, "branch");
 		if (new_branch_force)
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index f72cb0eb0b..ad38507d00 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -204,7 +204,7 @@ test_expect_success '"add" checks out existing branch of dwimd name' '
 	test_cmp_rev HEAD~1 dwim &&
 	(
 		cd dwim &&
-		test_cmp_rev dwim HEAD
+		test_cmp_rev HEAD dwim
 	)
 '
 
@@ -215,6 +215,7 @@ test_expect_success '"add <path>" dwim fails with checked out branch' '
 '
 
 test_expect_success '"add --force" with existing dwimd name doesnt die' '
+	git checkout test-branch &&
 	git worktree add --force test-branch
 '
 
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index a160f78aba..95653a08ca 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,9 +568,4 @@ test_expect_success 'reset --mixed sets up work tree' '
 	test_cmp expect actual
 '
 
-test_expect_success 'reset --no-show-new-head-line suppresses "HEAD is now at" output' '
-	git reset --hard --no-show-new-head-line HEAD >actual &&
-	! grep "HEAD is now at" <actual
-'
-
 test_done

Thomas Gummerer (4):
  worktree: remove extra members from struct add_opts
  worktree: improve message when creating a new worktree
  worktree: factor out dwim_branch function
  worktree: teach "add" to check out existing branches

 Documentation/git-worktree.txt |   9 +++-
 builtin/worktree.c             | 101 ++++++++++++++++++++++++++++++-----------
 t/t2025-worktree-add.sh        |  26 ++++++++---
 3 files changed, 100 insertions(+), 36 deletions(-)

-- 
2.16.1.74.g6cd9b6cbe3


  parent reply index

Thread overview: 111+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-21 12:02 [PATCH] " Thomas Gummerer
2018-01-21 12:02 ` Robert P. J. Day
2018-01-22 11:18 ` Duy Nguyen
2018-01-22 20:17   ` Thomas Gummerer
2018-02-04 22:13 ` [PATCH v2 0/3] " Thomas Gummerer
2018-02-04 22:13   ` [PATCH v2 1/3] worktree: improve message when creating a new worktree Thomas Gummerer
2018-02-05  2:12     ` Duy Nguyen
2018-02-05 20:13       ` Thomas Gummerer
2018-02-05 20:15       ` Junio C Hamano
2018-02-07  8:51       ` Eric Sunshine
2018-02-09 11:27         ` Thomas Gummerer
2018-02-09 12:08           ` Duy Nguyen
2018-02-10 11:20             ` Duy Nguyen
2018-02-04 22:13   ` [PATCH v2 2/3] worktree: be clearer when "add" dwim-ery kicks in Thomas Gummerer
2018-02-04 22:13   ` [PATCH v2 3/3] worktree: teach "add" to check out existing branches Thomas Gummerer
2018-02-05  2:18     ` Duy Nguyen
2018-02-05 20:20       ` Junio C Hamano
2018-02-05 20:23       ` Thomas Gummerer
2018-02-06 11:53         ` Duy Nguyen
2018-02-09 11:04           ` Thomas Gummerer
2018-03-17 22:08   ` [PATCH v3 0/4] " Thomas Gummerer
2018-03-17 22:08     ` [PATCH v3 1/4] worktree: improve message when creating a new worktree Thomas Gummerer
2018-03-17 22:08     ` [PATCH v3 2/4] worktree: be clearer when "add" dwim-ery kicks in Thomas Gummerer
2018-03-17 22:08     ` [PATCH v3 3/4] worktree: factor out dwim_branch function Thomas Gummerer
2018-03-17 22:08     ` [PATCH v3 4/4] worktree: teach "add" to check out existing branches Thomas Gummerer
2018-03-17 22:22     ` [PATCH v4 0/4] " Thomas Gummerer
2018-03-17 22:22       ` [PATCH v4 1/4] worktree: improve message when creating a new worktree Thomas Gummerer
2018-03-19 17:11         ` Duy Nguyen
2018-03-19 18:09           ` Junio C Hamano
2018-03-20  6:37         ` Eric Sunshine
2018-03-24 20:34           ` Thomas Gummerer
2018-03-17 22:22       ` [PATCH v4 2/4] worktree: be clearer when "add" dwim-ery kicks in Thomas Gummerer
2018-03-20  6:40         ` Eric Sunshine
2018-03-20  7:26         ` Eric Sunshine
2018-03-20  7:32           ` Eric Sunshine
2018-03-24 20:35             ` Thomas Gummerer
2018-03-17 22:22       ` [PATCH v4 3/4] worktree: factor out dwim_branch function Thomas Gummerer
2018-03-17 22:22       ` [PATCH v4 4/4] worktree: teach "add" to check out existing branches Thomas Gummerer
2018-03-20  8:02         ` Eric Sunshine
2018-03-24 21:00           ` Thomas Gummerer
2018-03-25 13:49       ` [PATCH v5 0/6] " Thomas Gummerer
2018-03-25 13:49         ` [PATCH v5 1/6] worktree: improve message when creating a new worktree Thomas Gummerer
2018-03-25 13:49         ` [PATCH v5 2/6] worktree: be clearer when "add" dwim-ery kicks in Thomas Gummerer
2018-03-27  8:59           ` Eric Sunshine
2018-03-30 13:53             ` Thomas Gummerer
2018-03-25 13:49         ` [PATCH v5 3/6] worktree: remove force_new_branch from struct add_opts Thomas Gummerer
2018-03-27  9:00           ` Eric Sunshine
2018-03-30 13:55             ` Thomas Gummerer
2018-03-25 13:49         ` [PATCH v5 4/6] worktree: factor out dwim_branch function Thomas Gummerer
2018-03-27  9:01           ` Eric Sunshine
2018-03-25 13:49         ` [PATCH v5 5/6] worktree: teach "add" to check out existing branches Thomas Gummerer
2018-03-27  9:04           ` Eric Sunshine
2018-03-30 14:04             ` Thomas Gummerer
2018-03-25 13:49         ` [PATCH v5 6/6] t2025: rename now outdated branch name Thomas Gummerer
2018-03-27  8:58         ` [PATCH v5 0/6] worktree: teach "add" to check out existing branches Eric Sunshine
2018-03-30 14:08           ` Thomas Gummerer
2018-03-31 15:17         ` [PATCH v6 " Thomas Gummerer
2018-03-31 15:17           ` [PATCH v6 1/6] worktree: remove extra members from struct add_opts Thomas Gummerer
2018-03-31 15:18           ` [PATCH v6 2/6] reset: introduce show-new-head-line option Thomas Gummerer
2018-04-02 20:29             ` Junio C Hamano
2018-04-02 22:07               ` Thomas Gummerer
2018-04-02 22:20               ` Thomas Gummerer
2018-04-02 20:34             ` Junio C Hamano
2018-04-02 22:09               ` Thomas Gummerer
2018-03-31 15:18           ` [PATCH v6 3/6] worktree: improve message when creating a new worktree Thomas Gummerer
2018-04-08  9:27             ` Eric Sunshine
2018-03-31 15:18           ` [PATCH v6 4/6] worktree: be clearer when "add" dwim-ery kicks in Thomas Gummerer
2018-03-31 15:18           ` [PATCH v6 5/6] worktree: factor out dwim_branch function Thomas Gummerer
2018-03-31 15:18           ` [PATCH v6 6/6] worktree: teach "add" to check out existing branches Thomas Gummerer
2018-04-01 13:11             ` [PATCH v6 6.5/6] fixup! " Thomas Gummerer
2018-04-09  0:23               ` Eric Sunshine
2018-04-09 19:44                 ` Thomas Gummerer
2018-04-09 21:35                   ` Eric Sunshine
2018-04-08 10:09             ` [PATCH v6 6/6] " Eric Sunshine
2018-04-08 14:30               ` Thomas Gummerer
2018-04-08  9:08           ` [PATCH v6 0/6] " Eric Sunshine
2018-04-08 14:24             ` Thomas Gummerer
2018-04-09  0:38               ` Eric Sunshine
2018-04-09 19:47                 ` Thomas Gummerer
2018-04-09 19:30             ` Thomas Gummerer
2018-04-09 22:06               ` Eric Sunshine
2018-04-11 20:09                 ` Thomas Gummerer
2018-04-11 20:48                   ` Eric Sunshine
2018-04-11 20:50                   ` Thomas Gummerer
2018-04-11 21:14                     ` Eric Sunshine
2018-04-15 20:29           ` Thomas Gummerer [this message]
2018-04-15 20:29             ` [PATCH v7 1/4] worktree: remove extra members from struct add_opts Thomas Gummerer
2018-04-15 20:29             ` [PATCH v7 2/4] worktree: improve message when creating a new worktree Thomas Gummerer
2018-04-16  2:09               ` Junio C Hamano
2018-04-23 18:55                 ` Thomas Gummerer
2018-04-23  4:27               ` Eric Sunshine
2018-04-23 18:50                 ` Thomas Gummerer
2018-04-15 20:29             ` [PATCH v7 3/4] worktree: factor out dwim_branch function Thomas Gummerer
2018-04-15 20:29             ` [PATCH v7 4/4] worktree: teach "add" to check out existing branches Thomas Gummerer
2018-04-23  4:52             ` [PATCH v7 0/4] " Eric Sunshine
2018-04-23 19:38             ` [PATCH v8 " Thomas Gummerer
2018-04-23 19:38               ` [PATCH v8 1/4] worktree: remove extra members from struct add_opts Thomas Gummerer
2018-04-24  3:26                 ` Eric Sunshine
2018-04-23 19:38               ` [PATCH v8 2/4] worktree: improve message when creating a new worktree Thomas Gummerer
2018-04-24  3:58                 ` Eric Sunshine
2018-04-23 19:38               ` [PATCH v8 3/4] worktree: factor out dwim_branch function Thomas Gummerer
2018-04-23 19:38               ` [PATCH v8 4/4] worktree: teach "add" to check out existing branches Thomas Gummerer
2018-04-24  4:25                 ` Eric Sunshine
2018-04-24 21:56               ` [PATCH v9 0/4] " Thomas Gummerer
2018-04-24 21:56                 ` [PATCH v9 1/4] worktree: remove extra members from struct add_opts Thomas Gummerer
2018-04-24 21:56                 ` [PATCH v9 2/4] worktree: improve message when creating a new worktree Thomas Gummerer
2018-04-24 21:56                 ` [PATCH v9 3/4] worktree: factor out dwim_branch function Thomas Gummerer
2018-04-24 21:56                 ` [PATCH v9 4/4] worktree: teach "add" to check out existing branches Thomas Gummerer
2018-04-27  7:36                 ` [PATCH v9 0/4] " Eric Sunshine
2018-04-28 16:09                   ` Thomas Gummerer
2018-04-30  0:07                     ` Junio C Hamano
2018-03-18  0:24     ` [PATCH v3 " Junio C Hamano

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180415202917.4360-1-t.gummerer@gmail.com \
    --to=t.gummerer@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox