[PATCH 0/3] Improve branch UI for setting upstream information

DORMANTno replies

4 messages, 1 author, 2016-06-15 · open the first message on its own page

[PATCH 0/3] Improve branch UI for setting upstream information

From: Carlos Martín Nieto <hidden>
Date: 2016-06-15 22:54:31

Hi all,

After way too long, here's the next iteration of the concept that
began with swapping arguments in --set-upstream like -m does.

After the feedback from the list, --set-upstream-to was born and
--set-upstream is being deprecated in favour of either --track or
--set-upstream-to depening on which of the behaviours the user wants
to have.

Using --set-upsteam with one argument now also leads to a message
explaining how to undo it. For that, branch has learnt
--unset-upstream which will remove the upstream information for the
given branch (or HEAD).

Carlos Martín Nieto (3):
  branch: introduce --set-upstream-to
  branch: add --unset-upstream option
  branch: suggest how to undo a --set-upstream when given one branch

 Documentation/git-branch.txt | 14 +++++++-
 builtin/branch.c             | 65 +++++++++++++++++++++++++++++++++--
 t/t3200-branch.sh            | 82 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 157 insertions(+), 4 deletions(-)

-- 
1.7.11.1.104.ge7b44f1

[PATCH 1/3] branch: introduce --set-upstream-to

From: Carlos Martín Nieto <hidden>
Date: 2016-06-15 22:54:31

The existing --set-uptream option can cause confusion, as it uses the
usual branch convention of assuming a starting point of HEAD if none
is specified, causing

    git branch --set-upstream origin/master

to create a new local branch 'origin/master' that tracks the current
branch. As --set-upstream already exists, we can't simply change its
behaviour. To work around this, introduce --set-upstream-to which
accepts a compulsory argument indicating what the new upstream branch
should be and one optinal argument indicating which branch to change,
defaulting to HEAD.

The new options allows us to type

    git branch --set-upstream-to origin/master

to set the current branch's upstream to be origin's master.

Signed-off-by: Carlos Martín Nieto <redacted>
---
 Documentation/git-branch.txt |  9 ++++++++-
 builtin/branch.c             | 17 +++++++++++++++--
 t/t3200-branch.sh            | 14 ++++++++++++++
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 47235be..e41c4b5 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt

@@ -13,6 +13,7 @@ SYNOPSIS
 	[--column[=<options>] | --no-column]
 	[(--merged | --no-merged | --contains) [<commit>]] [<pattern>...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
+'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
 'git branch' (-m | -M) [<oldbranch>] <newbranch>
 'git branch' (-d | -D) [-r] <branchname>...
 'git branch' --edit-description [<branchname>]

@@ -48,7 +49,7 @@ branch so that 'git pull' will appropriately merge from
 the remote-tracking branch. This behavior may be changed via the global
 `branch.autosetupmerge` configuration flag. That setting can be
 overridden by using the `--track` and `--no-track` options, and
-changed later using `git branch --set-upstream`.
+changed later using `git branch --set-upstream-to`.
 
 With a `-m` or `-M` option, <oldbranch> will be renamed to <newbranch>.
 If <oldbranch> had a corresponding reflog, it is renamed to match

@@ -173,6 +174,12 @@ start-point is either a local or remote-tracking branch.
 	like `--track` would when creating the branch, except that where
 	branch points to is not changed.
 
+-u <upstream>::
+--set-upstream-to=<upstream>::
+	Set up <branchname>'s tracking information so <upstream> is
+	considered <branchname>'s upstream branch. If no <branchname>
+	is specified, then it defaults to the current branch.
+
 --edit-description::
 	Open an editor and edit the text to explain what the branch is
 	for, to be used by various other commands (e.g. `request-pull`).

diff --git a/builtin/branch.c b/builtin/branch.c
index 0e060f2..3c978eb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -713,6 +713,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	int verbose = 0, abbrev = -1, detached = 0;
 	int reflog = 0, edit_description = 0;
 	int quiet = 0;
+	const char *new_upstream = NULL;
 	enum branch_track track;
 	int kinds = REF_LOCAL_BRANCH;
 	struct commit_list *with_commit = NULL;

@@ -726,6 +727,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			BRANCH_TRACK_EXPLICIT),
 		OPT_SET_INT( 0, "set-upstream",  &track, "change upstream info",
 			BRANCH_TRACK_OVERRIDE),
+		OPT_STRING('u', "set-upstream-to", &new_upstream, "upstream", "change the upstream info"),
 		OPT__COLOR(&branch_use_color, "use colored output"),
 		OPT_SET_INT('r', "remotes",     &kinds, "act on remote-tracking branches",
 			REF_REMOTE_BRANCH),

@@ -794,10 +796,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 			     0);
 
-	if (!delete && !rename && !edit_description && argc == 0)
+	if (!delete && !rename && !edit_description && !new_upstream && argc == 0)
 		list = 1;
 
-	if (!!delete + !!rename + !!force_create + !!list > 1)
+	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
 	if (abbrev == -1)

@@ -852,6 +854,17 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			rename_branch(argv[0], argv[1], rename > 1);
 		else
 			usage_with_options(builtin_branch_usage, options);
+	} else if (new_upstream) {
+		struct branch *branch = branch_get(argv[0]);
+
+		if (!ref_exists(branch->refname))
+			die(_("branch '%s' does not exist"), branch->name);
+
+		/*
+		 * create_branch takes care of setting up the tracking
+		 * info and making sure new_upstream is correct
+		 */
+		create_branch(head, branch->name, new_upstream, 0, 0, 0, quiet, BRANCH_TRACK_OVERRIDE);
 	} else if (argc > 0 && argc <= 2) {
 		if (kinds != REF_LOCAL_BRANCH)
 			die(_("-a and -r options to 'git branch' do not make sense with a branch name"));

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index a17f8b2..e9019ac 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh

@@ -369,6 +369,20 @@ test_expect_success \
     'git tag foobar &&
      test_must_fail git branch --track my11 foobar'
 
+test_expect_success 'use --set-upstream-to modify HEAD' \
+    'test_config branch.master.remote foo &&
+     test_config branch.master.merge foo &&
+     git branch my12
+     git branch --set-upstream-to my12 &&
+     test "$(git config branch.master.remote)" = "." &&
+     test "$(git config branch.master.merge)" = "refs/heads/my12"'
+
+test_expect_success 'use --set-upstream-to modify a particular branch' \
+    'git branch my13
+     git branch --set-upstream-to master my13 &&
+     test "$(git config branch.my13.remote)" = "." &&
+     test "$(git config branch.my13.merge)" = "refs/heads/master"'
+
 # Keep this test last, as it changes the current branch
 cat >expect <<EOF
 $_z40 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	branch: Created from master

-- 
1.7.11.1.104.ge7b44f1

[PATCH 2/3] branch: add --unset-upstream option

From: Carlos Martín Nieto <hidden>
Date: 2016-06-15 22:54:31

We have ways of setting the upstream information, but if we want to
unset it, we need to resort to modifying the configuration manually.

Teach branch an --unset-upstream option that unsets this information.

Signed-off-by: Carlos Martín Nieto <redacted>
---
 Documentation/git-branch.txt |  5 +++++
 builtin/branch.c             | 17 ++++++++++++++---
 t/t3200-branch.sh            | 16 ++++++++++++++++
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index e41c4b5..9c1d2f1 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt

@@ -14,6 +14,7 @@ SYNOPSIS
 	[(--merged | --no-merged | --contains) [<commit>]] [<pattern>...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
+'git branch' --unset-upstream [<branchname>]
 'git branch' (-m | -M) [<oldbranch>] <newbranch>
 'git branch' (-d | -D) [-r] <branchname>...
 'git branch' --edit-description [<branchname>]

@@ -180,6 +181,10 @@ start-point is either a local or remote-tracking branch.
 	considered <branchname>'s upstream branch. If no <branchname>
 	is specified, then it defaults to the current branch.
 
+--unset-upstream::
+	Remove the upstream information for <branchname>. If no branch
+	is specified it defaults to the current branch.
+
 --edit-description::
 	Open an editor and edit the text to explain what the branch is
 	for, to be used by various other commands (e.g. `request-pull`).

diff --git a/builtin/branch.c b/builtin/branch.c
index 3c978eb..08068f7 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -712,7 +712,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	int delete = 0, rename = 0, force_create = 0, list = 0;
 	int verbose = 0, abbrev = -1, detached = 0;
 	int reflog = 0, edit_description = 0;
-	int quiet = 0;
+	int quiet = 0, unset_upstream = 0;
 	const char *new_upstream = NULL;
 	enum branch_track track;
 	int kinds = REF_LOCAL_BRANCH;

@@ -728,6 +728,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT( 0, "set-upstream",  &track, "change upstream info",
 			BRANCH_TRACK_OVERRIDE),
 		OPT_STRING('u', "set-upstream-to", &new_upstream, "upstream", "change the upstream info"),
+		OPT_BOOLEAN(0, "unset-upstream", &unset_upstream, "Unset the upstream info"),
 		OPT__COLOR(&branch_use_color, "use colored output"),
 		OPT_SET_INT('r', "remotes",     &kinds, "act on remote-tracking branches",
 			REF_REMOTE_BRANCH),

@@ -796,10 +797,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 			     0);
 
-	if (!delete && !rename && !edit_description && !new_upstream && argc == 0)
+	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
 		list = 1;
 
-	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream > 1)
+	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
 	if (abbrev == -1)

@@ -865,6 +866,16 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		 * info and making sure new_upstream is correct
 		 */
 		create_branch(head, branch->name, new_upstream, 0, 0, 0, quiet, BRANCH_TRACK_OVERRIDE);
+	} else if (unset_upstream) {
+		struct branch *branch = branch_get(argv[0]);
+		struct strbuf buf = STRBUF_INIT;
+
+		strbuf_addf(&buf, "branch.%s.remote", branch->name);
+		git_config_set_multivar(buf.buf, NULL, NULL, 1);
+		strbuf_reset(&buf);
+		strbuf_addf(&buf, "branch.%s.merge", branch->name);
+		git_config_set_multivar(buf.buf, NULL, NULL, 1);
+		strbuf_release(&buf);
 	} else if (argc > 0 && argc <= 2) {
 		if (kinds != REF_LOCAL_BRANCH)
 			die(_("-a and -r options to 'git branch' do not make sense with a branch name"));

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index e9019ac..93e5d6e 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh

@@ -383,6 +383,22 @@ test_expect_success 'use --set-upstream-to modify a particular branch' \
      test "$(git config branch.my13.remote)" = "." &&
      test "$(git config branch.my13.merge)" = "refs/heads/master"'
 
+test_expect_success 'test --unset-upstream on HEAD' \
+    'git branch my14
+     test_config branch.master.remote foo &&
+     test_config branch.master.merge foo &&
+     git branch --set-upstream-to my14 &&
+     git branch --unset-upstream &&
+     test_must_fail git config branch.master.remote &&
+     test_must_fail git config branch.master.merge'
+
+test_expect_success 'test --unset-upstream on a particular branch' \
+    'git branch my15
+     git branch --set-upstream-to master my14 &&
+     git branch --unset-upstream my14 &&
+     test_must_fail git config branch.my14.remote &&
+     test_must_fail git config branch.my14.merge'
+
 # Keep this test last, as it changes the current branch
 cat >expect <<EOF
 $_z40 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	branch: Created from master

-- 
1.7.11.1.104.ge7b44f1

[PATCH 3/3] branch: suggest how to undo a --set-upstream when given one branch

From: Carlos Martín Nieto <hidden>
Date: 2016-06-15 22:54:31

This interface is error prone, and a better one (--set-upstream-to)
exists. Suggest how to fix a --set-upstream invocation in case the
user only gives one argument, which makes it likely that he meant to
do the opposite, like with

    git branch --set-upstream origin/master

when they meant one of

    git branch --set-upstream origin/master master
    git branch --set-upstream-to origin/master

While we're at it, add a notice that the --set-upstream flag is
deprecated and will be removed at some point.

Signed-off-by: Carlos Martín Nieto <redacted>

---

This produces suboptimal output in case that A tracks B and we do

    git checkout B
    git branch --set-upstream A

as it will suggest

    git branch --set-upstream A B

as a way of undoing what we just did. Avoiding it becomes a bit messy
(yet another layer of ifs), so I've left it out. Anybody reckon it's
worth recognising this?
---
 builtin/branch.c  | 35 +++++++++++++++++++++++++++++++++++
 t/t3200-branch.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+)

diff --git a/builtin/branch.c b/builtin/branch.c
index 08068f7..33641d9 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -707,6 +707,21 @@ static int edit_branch_description(const char *branch_name)
 	return status;
 }
 
+static void print_set_upstream_warning(const char *branch, int branch_existed, const char *old_upstream)
+{
+	fprintf(stderr, _("If you wanted to make '%s' track '%s', do this:\n\n"), head, branch);
+	if (branch_existed) {
+		if (old_upstream)
+			fprintf(stderr, _("    git branch --set-upstream %s %s\n"), old_upstream, branch);
+		else
+			fprintf(stderr, _("    git branch --unset-upstream %s\n"), branch);
+	} else {
+		fprintf(stderr, _("    git branch -d %s\n"), branch);
+	}
+
+	fprintf(stderr, _("    git branch --set-upstream-to %s\n"), branch);
+}
+
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, force_create = 0, list = 0;

@@ -877,10 +892,30 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		git_config_set_multivar(buf.buf, NULL, NULL, 1);
 		strbuf_release(&buf);
 	} else if (argc > 0 && argc <= 2) {
+		struct branch *branch = branch_get(argv[0]);
+		const char *old_upstream = NULL;
+		int branch_existed = 0;
+
 		if (kinds != REF_LOCAL_BRANCH)
 			die(_("-a and -r options to 'git branch' do not make sense with a branch name"));
+
+		if (track == BRANCH_TRACK_OVERRIDE)
+			fprintf(stderr, _("The --set-upstream flag is deprecated and will be removed. Consider using --track or --set-upstream-to\n"));
+
+		/*
+		 * Save what argv[0] was pointing to so we can give
+		 * the --set-upstream-to hint
+		 */
+		if (branch_has_merge_config(branch))
+			old_upstream = shorten_unambiguous_ref(branch->merge[0]->dst, 0);
+
+		branch_existed = ref_exists(branch->refname);
 		create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
 			      force_create, reflog, 0, quiet, track);
+
+		if (argc == 1 && track == BRANCH_TRACK_OVERRIDE)
+			print_set_upstream_warning(argv[0], branch_existed, old_upstream);
+
 	} else
 		usage_with_options(builtin_branch_usage, options);

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 93e5d6e..702bffa 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh

@@ -399,6 +399,58 @@ test_expect_success 'test --unset-upstream on a particular branch' \
      test_must_fail git config branch.my14.remote &&
      test_must_fail git config branch.my14.merge'
 
+test_expect_success 'test --set-upstream help message with one arg' \
+    'git branch --set-upstream origin/master 2>actual &&
+     test_when_finished git branch -d origin/master &&
+     cat >expected <<EOF &&
+The --set-upstream flag is deprecated and will be removed. Consider using --track or --set-upstream-to
+If you wanted to make '"'master'"' track '"'origin/master'"', do this:
+
+    git branch -d origin/master
+    git branch --set-upstream-to origin/master
+EOF
+     test_cmp expected actual
+'
+
+test_expect_success '--set-upstream with a branch that already has an upstream' \
+    'git branch --set-upstream-to my12 master &&
+     git branch --set-upstream-to my13 my12 &&
+     test_when_finished git branch --unset-upstream my12 &&
+     test_when_finished git branch --unset-upstream my13 &&
+     git branch --set-upstream my12 2>actual &&
+     cat actual &&
+     cat >expected <<EOF &&
+The --set-upstream flag is deprecated and will be removed. Consider using --track or --set-upstream-to
+If you wanted to make '"'master'"' track '"'my12'"', do this:
+
+    git branch --set-upstream my13 my12
+    git branch --set-upstream-to my12
+EOF
+     test_cmp expected actual
+'
+
+test_expect_success '--set-upstream with a branch that has no upstream' \
+    'git branch --set-upstream my12 2>actual &&
+     test_when_finished git branch --unset-upstream my12
+     cat >expected <<EOF &&
+The --set-upstream flag is deprecated and will be removed. Consider using --track or --set-upstream-to
+If you wanted to make '"'master'"' track '"'my12'"', do this:
+
+    git branch --unset-upstream my12
+    git branch --set-upstream-to my12
+EOF
+     test_cmp expected actual
+'
+
+test_expect_success '--set-upstream with two args should only show the deprecation message' \
+    'git branch --set-upstream master my13 2>actual &&
+     test_when_finished git branch --unset-upstream master &&
+     cat >expected <<EOF &&
+The --set-upstream flag is deprecated and will be removed. Consider using --track or --set-upstream-to
+EOF
+     test_cmp expected actual
+'
+
 # Keep this test last, as it changes the current branch
 cat >expect <<EOF
 $_z40 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	branch: Created from master

-- 
1.7.11.1.104.ge7b44f1

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help