* [PATCH 1/3] branch: introduce --set-upstream-to
2012-08-20 13:47 [PATCH 0/3] Improve branch UI for setting upstream information Carlos Martín Nieto
@ 2012-08-20 13:47 ` Carlos Martín Nieto
2012-08-20 13:47 ` [PATCH 2/3] branch: add --unset-upstream option Carlos Martín Nieto
2012-08-20 13:47 ` [PATCH 3/3] branch: suggest how to undo a --set-upstream when given one branch Carlos Martín Nieto
2 siblings, 0 replies; 33+ messages in thread
From: Carlos Martín Nieto @ 2012-08-20 13:47 UTC (permalink / raw)
To: git
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 <cmn@elego.de>
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 2/3] branch: add --unset-upstream option
2012-08-20 13:47 [PATCH 0/3] Improve branch UI for setting upstream information Carlos Martín Nieto
2012-08-20 13:47 ` [PATCH 1/3] branch: introduce --set-upstream-to Carlos Martín Nieto
@ 2012-08-20 13:47 ` Carlos Martín Nieto
2012-08-23 21:20 ` Junio C Hamano
2012-08-20 13:47 ` [PATCH 3/3] branch: suggest how to undo a --set-upstream when given one branch Carlos Martín Nieto
2 siblings, 1 reply; 33+ messages in thread
From: Carlos Martín Nieto @ 2012-08-20 13:47 UTC (permalink / raw)
To: git
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 <cmn@elego.de>
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] branch: add --unset-upstream option
2012-08-20 13:47 ` [PATCH 2/3] branch: add --unset-upstream option Carlos Martín Nieto
@ 2012-08-23 21:20 ` Junio C Hamano
2012-08-27 17:30 ` Carlos Martín Nieto
0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2012-08-23 21:20 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git
Carlos Martín Nieto <cmn@elego.de> writes:
> 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'
> +
What should happen when you say "--unset-upstream" on a branch B
that does not have B@{upstream}? Should it fail? Should it be
silently ignored? Is it undefined that we do not want to test?
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] branch: add --unset-upstream option
2012-08-23 21:20 ` Junio C Hamano
@ 2012-08-27 17:30 ` Carlos Martín Nieto
2012-08-27 18:01 ` Junio C Hamano
0 siblings, 1 reply; 33+ messages in thread
From: Carlos Martín Nieto @ 2012-08-27 17:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> Carlos Martín Nieto <cmn@elego.de> writes:
>
>> 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'
>> +
>
> What should happen when you say "--unset-upstream" on a branch B
> that does not have B@{upstream}? Should it fail? Should it be
> silently ignored? Is it undefined that we do not want to test?
I'd say it should be ignored, as the end result we want is for there to
be no upstream information. What we do underneath is remove a couple of
config options, wich doesn't fail if they didn't insist in the first
place.
Would you like a test that makes sure two --unset-upstream in a row
don't cause any errors?
cmn
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] branch: add --unset-upstream option
2012-08-27 17:30 ` Carlos Martín Nieto
@ 2012-08-27 18:01 ` Junio C Hamano
2012-08-27 18:14 ` Junio C Hamano
0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2012-08-27 18:01 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git
cmn@elego.de (Carlos Martín Nieto) writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Carlos Martín Nieto <cmn@elego.de> writes:
>>
>>> 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'
>>> +
>>
>> What should happen when you say "--unset-upstream" on a branch B
>> that does not have B@{upstream}? Should it fail? Should it be
>> silently ignored? Is it undefined that we do not want to test?
>
> I'd say it should be ignored, as the end result we want is for there to
> be no upstream information. What we do underneath is remove a couple of
> config options, wich doesn't fail if they didn't insist in the first
> place.
I am not sure about that reasoning.
$ git config --unset core.nosuchvariable ; echo $?
5
looks like a failure to me.
More importantly, wouldn't we want to catch typo in
git branch --unset-upstream mext
which admittedly should come from a different codepath (I do not
think your patch checks if "mext" branch exists before going ahead
to the config--unset dance)?
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] branch: add --unset-upstream option
2012-08-27 18:01 ` Junio C Hamano
@ 2012-08-27 18:14 ` Junio C Hamano
2012-08-27 21:33 ` Carlos Martín Nieto
0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2012-08-27 18:14 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> cmn@elego.de (Carlos Martín Nieto) writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Carlos Martín Nieto <cmn@elego.de> writes:
>>>
>>>> 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'
>>>> +
>>>
>>> What should happen when you say "--unset-upstream" on a branch B
>>> that does not have B@{upstream}? Should it fail? Should it be
>>> silently ignored? Is it undefined that we do not want to test?
>>
>> I'd say it should be ignored, as the end result we want is for there to
>> be no upstream information. What we do underneath is remove a couple of
>> config options, wich doesn't fail if they didn't insist in the first
>> place.
>
> I am not sure about that reasoning.
>
> $ git config --unset core.nosuchvariable ; echo $?
> 5
>
> looks like a failure to me.
>
> More importantly, wouldn't we want to catch typo in
>
> git branch --unset-upstream mext
>
> which admittedly should come from a different codepath (I do not
> think your patch checks if "mext" branch exists before going ahead
> to the config--unset dance)?
Sorry, the last paragraph was incomplete.
In general, we should detect errors and allow the user to choose to
ignore.
A script that wants to make sure that B does not have an upstream,
without knowing if it already has one, can say "--unset-upstream B"
and choose to ignore the error if B does not have an upstream.
If the script is carefully written, it would try to check if B has
one and call "--unset-upstream B" only when it doesn't. A casually
written loose script would say "--unset-upstream B 2>/dev/null"
and keep going (it would not notice other kinds of errors, but that
is what makes it "casual and loose").
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] branch: add --unset-upstream option
2012-08-27 18:14 ` Junio C Hamano
@ 2012-08-27 21:33 ` Carlos Martín Nieto
0 siblings, 0 replies; 33+ messages in thread
From: Carlos Martín Nieto @ 2012-08-27 21:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> cmn@elego.de (Carlos Martín Nieto) writes:
>>
>>> Junio C Hamano <gitster@pobox.com> writes:
>>>
>>>> Carlos Martín Nieto <cmn@elego.de> writes:
>>>>
>>>>> 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'
>>>>> +
>>>>
>>>> What should happen when you say "--unset-upstream" on a branch B
>>>> that does not have B@{upstream}? Should it fail? Should it be
>>>> silently ignored? Is it undefined that we do not want to test?
>>>
>>> I'd say it should be ignored, as the end result we want is for there to
>>> be no upstream information. What we do underneath is remove a couple of
>>> config options, wich doesn't fail if they didn't insist in the first
>>> place.
>>
>> I am not sure about that reasoning.
>>
>> $ git config --unset core.nosuchvariable ; echo $?
>> 5
>>
>> looks like a failure to me.
>>
>> More importantly, wouldn't we want to catch typo in
>>
>> git branch --unset-upstream mext
>>
>> which admittedly should come from a different codepath (I do not
>> think your patch checks if "mext" branch exists before going ahead
>> to the config--unset dance)?
>
> Sorry, the last paragraph was incomplete.
>
> In general, we should detect errors and allow the user to choose to
> ignore.
>
> A script that wants to make sure that B does not have an upstream,
> without knowing if it already has one, can say "--unset-upstream B"
> and choose to ignore the error if B does not have an upstream.
>
> If the script is carefully written, it would try to check if B has
> one and call "--unset-upstream B" only when it doesn't. A casually
> written loose script would say "--unset-upstream B 2>/dev/null"
> and keep going (it would not notice other kinds of errors, but that
> is what makes it "casual and loose").
OK, that's a good point, I'll update the patch.
cmn
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 3/3] branch: suggest how to undo a --set-upstream when given one branch
2012-08-20 13:47 [PATCH 0/3] Improve branch UI for setting upstream information Carlos Martín Nieto
2012-08-20 13:47 ` [PATCH 1/3] branch: introduce --set-upstream-to Carlos Martín Nieto
2012-08-20 13:47 ` [PATCH 2/3] branch: add --unset-upstream option Carlos Martín Nieto
@ 2012-08-20 13:47 ` Carlos Martín Nieto
2012-08-20 18:50 ` Junio C Hamano
2 siblings, 1 reply; 33+ messages in thread
From: Carlos Martín Nieto @ 2012-08-20 13:47 UTC (permalink / raw)
To: git
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 <cmn@elego.de>
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 3/3] branch: suggest how to undo a --set-upstream when given one branch
2012-08-20 13:47 ` [PATCH 3/3] branch: suggest how to undo a --set-upstream when given one branch Carlos Martín Nieto
@ 2012-08-20 18:50 ` Junio C Hamano
2012-08-22 1:26 ` Carlos Martín Nieto
2012-08-23 18:56 ` [PATCHv2 3/3] branch: deprecate --set-upstream and show help if we detect possible mistaken use Carlos Martín Nieto
0 siblings, 2 replies; 33+ messages in thread
From: Junio C Hamano @ 2012-08-20 18:50 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git
Carlos Martín Nieto <cmn@elego.de> writes:
> 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
In "git branch --set-upstream $A", if $A did not exist (i.e. we
ended up creating $A that is forked from the current branch), and if
refs/remotes/$A exists, I agree it is very likely that the user
wanted to set the upstream of the current branch to remotes/$A
instead.
But I am not sure about other cases. If $A already existed, it is
equally likely that "git branch --set-upstream $A" wanted to make
the existing $A track our current branch, or make our branch track
that existing $A, isn't it? We would end up giving unwanted advice
that is *wrong* for the user's situation 50% of the time, which does
not sound like an acceptable thing to do.
So I would really prefer to see the condition this advice is offered
limited to very specific cases (namely, only one parameter $A was
given to cause us use "HEAD" as the other branch, refs/heads/$A did
not exist and refs/remotes/$A did; there may be others but I think
this is the one we most care about) in which we know the advice is
correct with certainty.
> While we're at it, add a notice that the --set-upstream flag is
> deprecated and will be removed at some point.
This part is unquestionably good.
> Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
>
> ---
>
> 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.
The literal meaning of what the user did was to create B and then
made A that existed (and used to build upon B) to build upon B,
which is a no-op. And undoing can be done with the same command.
Which is funny.
I am sure you will get complaint from the real users. To avoid it,
you would need to know what the old value was (if any), and skip the
"undo" part, but I think it is probably worth it. You already have
code to learn what the old value was anyway, so the ideal is just a
single comparison away, no?
By the way, are you assuming that what the user wanted to do was to
make B build on top of A (i.e. set branch.B.up to A) in this
example? For that setting to make sense, perhaps B should already
be a descendant of A, shouldn't it? If that is the case, that is
another clue you can use in your logic to decide if you want to tell
the user "you told me to make $A build on the current branch, but I
think you meant the other way around" with more confidence.
> 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);
I would suggest strongly against using the verb "track" here, as it
is overused and has caused confusion "is it tracking branch, remote
tracking branch, do I merge in one but fetch to update the other?".
Regardless of the verb, I think there should be a line _before_ the
above to tell the user what the command actually _did_, to make the
distinction stand out to help the user decide.
That is, tell the user "I set upstream for that other branch to the
current branch, making that other branch build on top of the current
branch. If that was a mistake, and what you really wanted to do was
to make the current branch build on top of the other branch, here
are the things you need to do to recover". Without saying anything
before "If that was a mistake, " part and giving "to do X, do Y"
would leave the user confused why he is being given a way to do X in
the first place, whether what he really wanted to do was X or
something else.
> + 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);
> +}
And I suspect the logic to call into this function needs to be
tightened a lot. It may even turn out that we may not need messages
when "branch_existed" is true.
> 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);
> +
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/3] branch: suggest how to undo a --set-upstream when given one branch
2012-08-20 18:50 ` Junio C Hamano
@ 2012-08-22 1:26 ` Carlos Martín Nieto
2012-08-23 18:56 ` [PATCHv2 3/3] branch: deprecate --set-upstream and show help if we detect possible mistaken use Carlos Martín Nieto
1 sibling, 0 replies; 33+ messages in thread
From: Carlos Martín Nieto @ 2012-08-22 1:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Mon, 2012-08-20 at 11:50 -0700, Junio C Hamano wrote:
> Carlos Martín Nieto <cmn@elego.de> writes:
>
> > 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
>
> In "git branch --set-upstream $A", if $A did not exist (i.e. we
> ended up creating $A that is forked from the current branch), and if
> refs/remotes/$A exists, I agree it is very likely that the user
> wanted to set the upstream of the current branch to remotes/$A
> instead.
>
> But I am not sure about other cases. If $A already existed, it is
> equally likely that "git branch --set-upstream $A" wanted to make
> the existing $A track our current branch, or make our branch track
> that existing $A, isn't it? We would end up giving unwanted advice
> that is *wrong* for the user's situation 50% of the time, which does
> not sound like an acceptable thing to do.
>
> So I would really prefer to see the condition this advice is offered
> limited to very specific cases (namely, only one parameter $A was
> given to cause us use "HEAD" as the other branch, refs/heads/$A did
> not exist and refs/remotes/$A did; there may be others but I think
> this is the one we most care about) in which we know the advice is
> correct with certainty.
I'm not sure about the 50%, I've personally never used the one-argument
version (correctly), but the restriction does make sense as
--set-upstream origin/master is the main thing we want to "protect"
against.
>
> > While we're at it, add a notice that the --set-upstream flag is
> > deprecated and will be removed at some point.
>
> This part is unquestionably good.
>
> > Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
> >
> > ---
> >
> > 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.
>
> The literal meaning of what the user did was to create B and then
> made A that existed (and used to build upon B) to build upon B,
> which is a no-op. And undoing can be done with the same command.
>
> Which is funny.
>
> I am sure you will get complaint from the real users. To avoid it,
> you would need to know what the old value was (if any), and skip the
> "undo" part, but I think it is probably worth it. You already have
> code to learn what the old value was anyway, so the ideal is just a
> single comparison away, no?
Yes, but this comparison is already two or three levels deep in ifs,
which is why I didn't put it in this version, as it was unlikely to be
triggered anyway. As per the next paragraph, this probably won't be an
issue.
>
> By the way, are you assuming that what the user wanted to do was to
> make B build on top of A (i.e. set branch.B.up to A) in this
> example? For that setting to make sense, perhaps B should already
> be a descendant of A, shouldn't it? If that is the case, that is
> another clue you can use in your logic to decide if you want to tell
> the user "you told me to make $A build on the current branch, but I
> think you meant the other way around" with more confidence.
I am pretty much assuming that every use of --set-upstream with a single
argument was meant to use the --set-upstream-to semantics. This
particular examples is an edge-case I found when trying to figure out
the possible combinations, not a sane one I expect to see in the wild.
>
> > 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);
>
> I would suggest strongly against using the verb "track" here, as it
> is overused and has caused confusion "is it tracking branch, remote
> tracking branch, do I merge in one but fetch to update the other?".
>
> Regardless of the verb, I think there should be a line _before_ the
> above to tell the user what the command actually _did_, to make the
> distinction stand out to help the user decide.
I don't like the word either, but that's what 'branch --set-upstream'
and 'checkout -t' use (or anything that ends up calling
install_branch_config()). There is such a message (see below) and that's
how it says it. I worry that changing the wording might be even more
confusing, but I'll play with some wording (unless you want to change
the message in install_branch_config()).
>
> That is, tell the user "I set upstream for that other branch to the
> current branch, making that other branch build on top of the current
> branch. If that was a mistake, and what you really wanted to do was
> to make the current branch build on top of the other branch, here
> are the things you need to do to recover". Without saying anything
> before "If that was a mistake, " part and giving "to do X, do Y"
> would leave the user confused why he is being given a way to do X in
> the first place, whether what he really wanted to do was X or
> something else.
This message is always printed as a result of create_branch() at the
bottom of cmd_branch() and it's printed to stdout. The rest is printed
to stderr and that's what the tests are interested in checking, so you
can't really tell from there. The whole output looks like
% ./bin-wrappers/git branch --set-upstream master
The --set-upstream flag is deprecated and will be removed. Consider using --track or --set-upstream-to
Branch master set up to track local branch set-upstream-to.
If you wanted to make 'set-upstream-to' track 'master', do this:
git branch --set-upstream master set-upstream-to
git branch --set-upstream-to master
It may need an extra LF after the deprecation warning for readability.
>
> > + 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);
> > +}
>
> And I suspect the logic to call into this function needs to be
> tightened a lot. It may even turn out that we may not need messages
> when "branch_existed" is true.
Without that check, this function will be much simpler, yeah. I'll go
for showing the help only for a new branch in the next iteration and
then we can see if we want to check how existing branches relate.
>
> > 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);
> > +
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCHv2 3/3] branch: deprecate --set-upstream and show help if we detect possible mistaken use
2012-08-20 18:50 ` Junio C Hamano
2012-08-22 1:26 ` Carlos Martín Nieto
@ 2012-08-23 18:56 ` Carlos Martín Nieto
2012-08-23 21:16 ` Junio C Hamano
1 sibling, 1 reply; 33+ messages in thread
From: Carlos Martín Nieto @ 2012-08-23 18:56 UTC (permalink / raw)
To: gitster; +Cc: git
This interface is error prone, and a better one (--set-upstream-to)
exists. Add a message listing the alternatives and suggest how to fix
a --set-upstream invocation in case the user only gives one argument
which causes a local branch with the same name as a remote-tracking
one to be created. The typical case is
git branch --set-upstream origin/master
when the user meant
git branch --set-upstream master origin/master
assuming that the current branch is master. Show a message telling the
user how to undo their action and get what they wanted. For the
command above, the message would be
The --set-upstream flag is deprecated and will be removed. Consider using --track or --set-upstream-to
Branch origin/master set up to track local branch master.
If you wanted to make 'master' track 'origin/master', do this:
git branch -d origin/master
git branch --set-upstream-to origin/master
Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
The 'track' in the message is still not great, but it does fit with
the one above. Maybe if we make it say "If youw wanted [...] track the
remote-tracking branch 'origin/master'" it would be clearer?
I've simplified and tightened the logic. Now it will only show the
undo message if the branch didn't exist locally and there is a
remote-tracking branch of the same name.
builtin/branch.c | 26 ++++++++++++++++++++++++++
t/t3200-branch.sh | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/builtin/branch.c b/builtin/branch.c
index 08068f7..a0302a2 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -877,10 +877,36 @@ 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]);
+ int branch_existed = 0, remote_tracking = 0;
+ struct strbuf buf = STRBUF_INIT;
+
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"));
+
+ strbuf_addf(&buf, "refs/remotes/%s", branch->name);
+ remote_tracking = ref_exists(buf.buf);
+ strbuf_release(&buf);
+
+ branch_existed = ref_exists(branch->refname);
create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
force_create, reflog, 0, quiet, track);
+
+ /*
+ * We only show the instructions if the user gave us
+ * one branch which doesn't exist locally, but is the
+ * name of a remote-tracking branch.
+ */
+ if (argc == 1 && track == BRANCH_TRACK_OVERRIDE &&
+ !branch_existed && remote_tracking) {
+ fprintf(stderr, _("\nIf you wanted to make '%s' track '%s', do this:\n\n"), head, branch->name);
+ fprintf(stderr, _(" git branch -d %s\n"), branch->name);
+ fprintf(stderr, _(" git branch --set-upstream-to %s\n"), branch->name);
+ }
+
} else
usage_with_options(builtin_branch_usage, options);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 93e5d6e..e9e11cf 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -399,6 +399,40 @@ 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 '--set-upstream shows message when creating a new branch that exists as remote-tracking' \
+ 'git update-ref refs/remotes/origin/master HEAD &&
+ git branch --set-upstream origin/master 2>actual &&
+ test_when_finished git update-ref -d refs/remotes/origin/master &&
+ 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 two args only shows 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
+'
+
+test_expect_success '--set-upstream with one arg only shows the deprecation message if the branch existed' \
+ 'git branch --set-upstream 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.12
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCHv2 3/3] branch: deprecate --set-upstream and show help if we detect possible mistaken use
2012-08-23 18:56 ` [PATCHv2 3/3] branch: deprecate --set-upstream and show help if we detect possible mistaken use Carlos Martín Nieto
@ 2012-08-23 21:16 ` Junio C Hamano
0 siblings, 0 replies; 33+ messages in thread
From: Junio C Hamano @ 2012-08-23 21:16 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git
Carlos Martín Nieto <cmn@elego.de> writes:
> The 'track' in the message is still not great, but it does fit with
> the one above. Maybe if we make it say "If youw wanted [...] track the
> remote-tracking branch 'origin/master'" it would be clearer?
The verb "track" in the phrase "remote-tracking branch" means "keep
track of the branch at the remote, by storing a copy of the last
observed state of it". In the same sentence, the verb "track"
elsewhere is used to describe what the branch B whose upstream is
set to B@{upstream} does against B@{upstream}, but that is not
"keeping a copy"---it is doing an entirely different thing.
If we say that the branch B whose upstream is set to B@{upstream} is
"building on top of B@{upstream}", "integrating with B@{upstream}",
"forked from B@{upstream}", etc., without using the verb "track"
that already means something else (i.e. keeps track of the copy of
last observed state), it would reduce the confusion, but I do not
think it would clarify anything if the verb "track" is used for
that.
As usual, because I am not the best source of phrasing, others may
find a better verb than "builds", "forks", or "integrates", though.
> I've simplified and tightened the logic. Now it will only show the
> undo message if the branch didn't exist locally and there is a
> remote-tracking branch of the same name.
The updated and simplified logic reads quite straight-forward, and
looks good. It is likely that the message will be reworded and also
localized in the future, so it would make sense to use test_i18ncmp
from the day one, though.
Thanks. Will queue.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/3] branch: introduce --set-upstream-to
2012-08-30 17:23 [PATCHv2 0/3] Improve branch UI for setting upstream information Carlos Martín Nieto
@ 2012-08-30 17:23 ` Carlos Martín Nieto
2012-08-30 17:51 ` Ralf Thielow
0 siblings, 1 reply; 33+ messages in thread
From: Carlos Martín Nieto @ 2012-08-30 17:23 UTC (permalink / raw)
To: git; +Cc: gitster
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 <cmn@elego.de>
---
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.12.3.g0dd8ef6
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-08-30 17:23 ` [PATCH 1/3] branch: introduce --set-upstream-to Carlos Martín Nieto
@ 2012-08-30 17:51 ` Ralf Thielow
2012-08-31 15:22 ` Carlos Martín Nieto
0 siblings, 1 reply; 33+ messages in thread
From: Ralf Thielow @ 2012-08-30 17:51 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git, gitster
On Thu, Aug 30, 2012 at 7:23 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
> 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.
>
Could you please also add this new option to the
"contrib/completion/git-completion.bash"
script?
Thanks!
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-08-30 17:51 ` Ralf Thielow
@ 2012-08-31 15:22 ` Carlos Martín Nieto
2012-08-31 15:30 ` Ralf Thielow
0 siblings, 1 reply; 33+ messages in thread
From: Carlos Martín Nieto @ 2012-08-31 15:22 UTC (permalink / raw)
To: Ralf Thielow; +Cc: git, gitster
Ralf Thielow <ralf.thielow@gmail.com> writes:
> On Thu, Aug 30, 2012 at 7:23 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
>> 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.
>>
>
> Could you please also add this new option to the
> "contrib/completion/git-completion.bash"
> script?
If I knew how those things work... Is this enough?
cmn
--8<--
Subject: [PATCH] completion: add --set-upstream-to and --unset-upstream
---
contrib/completion/git-completion.bash | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ffedce7..4f46357 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -880,6 +880,7 @@ _git_branch ()
--color --no-color --verbose --abbrev= --no-abbrev
--track --no-track --contains --merged --no-merged
--set-upstream --edit-description --list
+ --unset-upstream --set-upstream-to=
"
;;
*)
--
1.7.12.3.g0dd8ef6
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-08-31 15:22 ` Carlos Martín Nieto
@ 2012-08-31 15:30 ` Ralf Thielow
2012-08-31 17:09 ` Junio C Hamano
0 siblings, 1 reply; 33+ messages in thread
From: Ralf Thielow @ 2012-08-31 15:30 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git, gitster
On Fri, Aug 31, 2012 at 5:22 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
> Ralf Thielow <ralf.thielow@gmail.com> writes:
>
>> On Thu, Aug 30, 2012 at 7:23 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
>>> 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.
>>>
>>
>> Could you please also add this new option to the
>> "contrib/completion/git-completion.bash"
>> script?
>
> If I knew how those things work... Is this enough?
>
Yes, Thanks.
> cmn
>
> --8<--
> Subject: [PATCH] completion: add --set-upstream-to and --unset-upstream
>
> ---
> contrib/completion/git-completion.bash | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index ffedce7..4f46357 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -880,6 +880,7 @@ _git_branch ()
> --color --no-color --verbose --abbrev= --no-abbrev
> --track --no-track --contains --merged --no-merged
> --set-upstream --edit-description --list
> + --unset-upstream --set-upstream-to=
> "
> ;;
> *)
> --
> 1.7.12.3.g0dd8ef6
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-08-31 15:30 ` Ralf Thielow
@ 2012-08-31 17:09 ` Junio C Hamano
2012-09-01 15:13 ` Carlos Martín Nieto
0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2012-08-31 17:09 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: Ralf Thielow, git
Ralf Thielow <ralf.thielow@gmail.com> writes:
> On Fri, Aug 31, 2012 at 5:22 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
>> Ralf Thielow <ralf.thielow@gmail.com> writes:
>>
>>> On Thu, Aug 30, 2012 at 7:23 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
>>>> 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.
>>>>
>>>
>>> Could you please also add this new option to the
>>> "contrib/completion/git-completion.bash"
>>> script?
>>
>> If I knew how those things work... Is this enough?
>>
>
> Yes, Thanks.
While you are at it, perhaps you may want to unadvertise --set-upstream?
>
>> cmn
>>
>> --8<--
>> Subject: [PATCH] completion: add --set-upstream-to and --unset-upstream
>>
>> ---
>> contrib/completion/git-completion.bash | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index ffedce7..4f46357 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -880,6 +880,7 @@ _git_branch ()
>> --color --no-color --verbose --abbrev= --no-abbrev
>> --track --no-track --contains --merged --no-merged
>> --set-upstream --edit-description --list
>> + --unset-upstream --set-upstream-to=
>> "
>> ;;
>> *)
>> --
>> 1.7.12.3.g0dd8ef6
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-08-31 17:09 ` Junio C Hamano
@ 2012-09-01 15:13 ` Carlos Martín Nieto
0 siblings, 0 replies; 33+ messages in thread
From: Carlos Martín Nieto @ 2012-09-01 15:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ralf Thielow, git
Junio C Hamano <gitster@pobox.com> writes:
> Ralf Thielow <ralf.thielow@gmail.com> writes:
>
>> On Fri, Aug 31, 2012 at 5:22 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
>>> Ralf Thielow <ralf.thielow@gmail.com> writes:
>>>
>>>> On Thu, Aug 30, 2012 at 7:23 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
>>>>> 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.
>>>>>
>>>>
>>>> Could you please also add this new option to the
>>>> "contrib/completion/git-completion.bash"
>>>> script?
>>>
>>> If I knew how those things work... Is this enough?
>>>
>>
>> Yes, Thanks.
>
> While you are at it, perhaps you may want to unadvertise --set-upstream?
>
Yeah, that'd be good.
cmn
--8<--
Subject: [PATCH] completion: add --set-upstream-to and --unset-upstream
Remove --set-upstream as it's deprecated now.
---
contrib/completion/git-completion.bash | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ffedce7..4a4d30a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -879,7 +879,8 @@ _git_branch ()
__gitcomp "
--color --no-color --verbose --abbrev= --no-abbrev
--track --no-track --contains --merged --no-merged
- --set-upstream --edit-description --list
+ --set-upstream-to= --edit-description --list
+ --unset-upstream
"
;;
*)
--
1.7.12.3.g0dd8ef6
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-10 16:52 [PATCH 0/3] A better way of handling upstream information in git-branch Carlos Martín Nieto
@ 2012-07-10 16:52 ` Carlos Martín Nieto
2012-07-10 17:08 ` Matthieu Moy
` (2 more replies)
0 siblings, 3 replies; 33+ messages in thread
From: Carlos Martín Nieto @ 2012-07-10 16:52 UTC (permalink / raw)
To: git; +Cc: jrnieder, gitster
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.
---
Documentation/git-branch.txt | 9 ++++++++-
builtin/branch.c | 15 +++++++++++++--
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 47235be..0f33fc7 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 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 0e060f2..c886fc0 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,15 @@ 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"));
--
1.7.10.2.1.g8c77c3c
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-10 16:52 ` [PATCH 1/3] branch: introduce --set-upstream-to Carlos Martín Nieto
@ 2012-07-10 17:08 ` Matthieu Moy
2012-07-10 17:26 ` Junio C Hamano
2012-07-10 19:13 ` Jonathan Nieder
2 siblings, 0 replies; 33+ messages in thread
From: Matthieu Moy @ 2012-07-10 17:08 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git, jrnieder, gitster
Carlos Martín Nieto <cmn@elego.de> writes:
> The new options allows us to type
>
> git branch --set-upstream-to origin/master
This is cool :-).
> Documentation/git-branch.txt | 9 ++++++++-
> builtin/branch.c | 15 +++++++++++++--
I think this deserves a few new tests (probably in t/t3200-branch.sh).
> +-u <upstream>::
> +--set-upstream-to=<upstream>::
> + Set up <branchname>'s tracking information so <upstream> is
> + considered <branchname>'s upstream branch. If no branch is
> + specified it defaults to the current branch.
Perhaps "if <branchname> is not specified, then it defaults to the
current branch.". The current wording does not make it very clear if "no
branch is specified" refers to <branchname> or <upstream> (although the
second option would be plain silly).
> + } else if (new_upstream) {
> + struct branch *branch = branch_get(argv[0]);
> +
> + if (!ref_exists(branch->refname))
> + die(_("branch '%s' does not exist"), branch->name);
Indentation (2 spaces -> tab).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-10 16:52 ` [PATCH 1/3] branch: introduce --set-upstream-to Carlos Martín Nieto
2012-07-10 17:08 ` Matthieu Moy
@ 2012-07-10 17:26 ` Junio C Hamano
2012-07-10 19:13 ` Jonathan Nieder
2 siblings, 0 replies; 33+ messages in thread
From: Junio C Hamano @ 2012-07-10 17:26 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git, jrnieder
Carlos Martín Nieto <cmn@elego.de> writes:
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 0e060f2..c886fc0 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);
It probably is an error to have track and new_upstream together.
The remainder of [Patch 1/3] looked entirely sensible, including the
proposed log message (modulo missing sign-off).
Thanks.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-10 16:52 ` [PATCH 1/3] branch: introduce --set-upstream-to Carlos Martín Nieto
2012-07-10 17:08 ` Matthieu Moy
2012-07-10 17:26 ` Junio C Hamano
@ 2012-07-10 19:13 ` Jonathan Nieder
2012-07-10 19:49 ` Junio C Hamano
2 siblings, 1 reply; 33+ messages in thread
From: Jonathan Nieder @ 2012-07-10 19:13 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git, gitster, Matthieu Moy
Hi,
Carlos Martín Nieto wrote:
> 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
Thanks. A part of me really dislikes this --set-upstream-to which
is named more awkwardly than the deprecated mistake it replaces,
though.
Here's a patch on top to play with that names the new option
"--set-upstream=". Untested.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
diff --git i/Documentation/git-branch.txt w/Documentation/git-branch.txt
index f572913f..57935a64 100644
--- i/Documentation/git-branch.txt
+++ w/Documentation/git-branch.txt
@@ -49,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-to`.
+changed later using `git branch --set-upstream`.
With a `-m` or `-M` option, <oldbranch> will be renamed to <newbranch>.
If <oldbranch> had a corresponding reflog, it is renamed to match
@@ -174,11 +174,13 @@ 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-upstream=<upstream>::
Set up <branchname>'s tracking information so <upstream> is
considered <branchname>'s upstream branch. If no branch is
specified it defaults to the current branch.
++
+If no argument is attached, for historical reasons the meaning is
+different. See above.
--edit-description::
Open an editor and edit the text to explain what the branch is
diff --git i/builtin/branch.c w/builtin/branch.c
index c886fc06..0d705790 100644
--- i/builtin/branch.c
+++ w/builtin/branch.c
@@ -669,6 +669,31 @@ static int opt_parse_merge_filter(const struct option *opt, const char *arg, int
return 0;
}
+struct set_upstream_params {
+ enum branch_track *track;
+ const char **new_upstream;
+};
+static int parse_opt_set_upstream(const struct option *opt, const char *arg, int unset)
+{
+ struct set_upstream_params *o = opt->value;
+
+ if (unset) { /* --no-set-upstream */
+ *o->track = BRANCH_TRACK_NEVER;
+ *o->new_upstream = NULL;
+ return 0;
+ }
+
+ *o->track = BRANCH_TRACK_OVERRIDE;
+ if (!arg) /* --set-upstream <branchname> <start-point> */
+ *o->new_upstream = NULL;
+ else /* --set-upstream=<upstream> <branchname> */
+ *o->new_upstream = arg;
+ return 0;
+}
+#define OPT_SET_UPSTREAM(s, l, v) \
+ { OPTION_CALLBACK, (s), (l), (v), "upstream", "change upstream info", \
+ PARSE_OPT_OPTARG, &parse_opt_set_upstream }
+
static const char edit_description[] = "BRANCH_DESCRIPTION";
static int edit_branch_description(const char *branch_name)
@@ -716,6 +741,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
const char *new_upstream = NULL;
enum branch_track track;
int kinds = REF_LOCAL_BRANCH;
+ struct set_upstream_params set_upstream_args = { &track, &new_upstream };
struct commit_list *with_commit = NULL;
struct option options[] = {
@@ -725,9 +751,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
OPT__QUIET(&quiet, "suppress informational messages"),
OPT_SET_INT('t', "track", &track, "set up tracking mode (see git-pull(1))",
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_SET_UPSTREAM(0, "set-upstream", &set_upstream_args),
OPT__COLOR(&branch_use_color, "use colored output"),
OPT_SET_INT('r', "remotes", &kinds, "act on remote-tracking branches",
REF_REMOTE_BRANCH),
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-10 19:13 ` Jonathan Nieder
@ 2012-07-10 19:49 ` Junio C Hamano
2012-07-10 20:11 ` Jonathan Nieder
0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2012-07-10 19:49 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Carlos Martín Nieto, git, Matthieu Moy
Jonathan Nieder <jrnieder@gmail.com> writes:
>> 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
>
> Thanks. A part of me really dislikes this --set-upstream-to which
> is named more awkwardly than the deprecated mistake it replaces,
> though.
I am not super excited about it either, but at least it is a vast
improvement compared to the older one, with which it was entirely
unclear if we are setting the value of upstream *to* what is given
as an option, or setting the upstream *for* what is given on the
command line.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-10 19:49 ` Junio C Hamano
@ 2012-07-10 20:11 ` Jonathan Nieder
2012-07-10 20:49 ` Junio C Hamano
0 siblings, 1 reply; 33+ messages in thread
From: Jonathan Nieder @ 2012-07-10 20:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlos Martín Nieto, git, Matthieu Moy
Junio C Hamano wrote:
> I am not super excited about it either, but at least it is a vast
> improvement compared to the older one, with which it was entirely
> unclear if we are setting the value of upstream *to* what is given
> as an option, or setting the upstream *for* what is given on the
> command line.
Ah, do you mean that --set-upstream is meant to have usage like
"git remote set-url" and co?
git remote set-url <remote> <url>
git branch --set-upstream <branch> <upstream>
That's a reasonable stance, and it seems possible to get used to it.
In that case, we should just teach --set-upstream not to create
new branches, and people will get used to it.
The immediate problem that seems to trip people up is that it is very
tempting to run
git branch --set-upstream junio/master
in an attempt to change what is upstream to the current branch, and
the result is some other completely counterintuitive thing. I suspect
the order of arguments to --set-upstream is a red herring, as long as
it errors out when the arguments are switched to help people catch
mistakes.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-10 20:11 ` Jonathan Nieder
@ 2012-07-10 20:49 ` Junio C Hamano
2012-07-10 21:09 ` Jonathan Nieder
0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2012-07-10 20:49 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Carlos Martín Nieto, git, Matthieu Moy
Jonathan Nieder <jrnieder@gmail.com> writes:
> The immediate problem that seems to trip people up is that it is very
> tempting to run
>
> git branch --set-upstream junio/master
I think we have discussed this already a few days ago. See my
comment in the earlier thread before this round.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-10 20:49 ` Junio C Hamano
@ 2012-07-10 21:09 ` Jonathan Nieder
2012-07-10 23:13 ` Junio C Hamano
0 siblings, 1 reply; 33+ messages in thread
From: Jonathan Nieder @ 2012-07-10 21:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlos Martín Nieto, git, Matthieu Moy
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> The immediate problem that seems to trip people up is that it is very
>> tempting to run
>>
>> git branch --set-upstream junio/master
>
> I think we have discussed this already a few days ago. See my
> comment in the earlier thread before this round.
You wrote[*]:
| I think it was a mistake that nobody noticed that it is likely that
| the operation most often will be done for the current branch and the
| usual "give me one branch name to operate on, or I'll operate on the
| current branch" command line convention of "git branch" commannd is
| not a good fit for it, when "set upstream" feature was added
with which I completely agree. You then moved on to
| and
[someone should have]
| suggested an alternative syntax that avoids the mistake you quoted
| above, perhaps something like:
|
| git branch --set-upstream-to=origin/master [HEAD]
with which I disagree.
As far as I can tell, nobody really thought very hard about what
--set-upstream would do when passed only one argument. It should have
been made to error out and only later change if someone had an idea
about how to make it useful.
Luckily we have a way out. Any example transition plan looks like
the following.
DAY 1.
$ git branch --set-upstream origin/master
Branch origin/master set up to track local branch debian-sid.
hint: If you intended to make the current branch track
hint: origin/master, you can recover with the following commands:
hint: $ git branch -d origin/master
hint: $ git branch --set-upstream master origin/master
$
DAY 2.
$ git branch --set-upstream origin/master
Branch origin/master set up to track local branch debian-sid.
warning: using --set-upstream when creating a new branch is deprecated
hint: use --track instead
hint:
hint: If you intended to make the current branch track
hint: origin/master, you can recover with the following commands:
hint: $ git branch -d origin/master
hint: $ git branch --set-upstream master origin/master
$
DAY 3.
$ git branch --set-upstream origin/master
fatal: no such branch "origin/master"
$
DAY 4.
$ git branch --set-upstream origin/master
usage: git branch --set-upstream <branchname> <upstream>
$
[*] http://thread.gmane.org/gmane.comp.version-control.git/201040/focus=201051
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-10 21:09 ` Jonathan Nieder
@ 2012-07-10 23:13 ` Junio C Hamano
2012-07-10 23:47 ` Jonathan Nieder
0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2012-07-10 23:13 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Carlos Martín Nieto, git, Matthieu Moy
Jonathan Nieder <jrnieder@gmail.com> writes:
> [someone should have]
> | suggested an alternative syntax that avoids the mistake you quoted
> | above, perhaps something like:
> |
> | git branch --set-upstream-to=origin/master [HEAD]
>
> with which I disagree.
You can think of it this way.
"git branch" can not only _create_ a new branch (or list existing
ones, but that is another entirely different mode), but also can be
used to set attributes to an existing branch. Imagine a new option,
say --set-description, to replace branch.frotz.description, for
example. It would be used like this:
$ git branch --set-description='add frotz feature' frotz
to set the description for the 'frotz' branch (i.e. the above would
set branch.frotz.description), and we default to HEAD if 'frotz' is
missing from the command line. "git branch --option [<branch>]" is
about manipulating the branch, and we default the target of
manipulation to HEAD.
"upstream" is just another kind of attribute for the branch being
manipulated, whose value happens to be a branch name.
The mistake was that --set-upstream was coded by piggybacking the
existing --track implementation where a new branch was created, and
in that codepath, "git branch <name1> [<name2>]" creates <name1>
while defaulting a missing <name2> to HEAD.
Creating a new branch that is forked from the current HEAD is an
often useful thing to do, so defaulting a missing <name2> (aka
"start-point") to HEAD is very sensible, but reconfiguring a named
branch <name1> to integrate with the current branch is much less
useful than the other way around. One major reason why it is so is
because you would more likely set any branch to integrate with a
remote tracking branch (rather than a local branch) and by
definition your HEAD cannot be a remote tracking branch.
It makes it worse that you would often want to reconfigure the
current branch; for the purpose of reconfiguring a branch <name1> to
integrate with something else <name2>, it is much more likely that
you want a missing <name1> to default to HEAD, not the other way
around to default a missing <name2> to HEAD, which is useful for
branch creation.
But switching which missing argument gets default based on what
options are used is insane.
If the very original "create this new branch starting at that point"
were spelled like this
$ git branch [--start-point=<name2>] <name1>
and a missing <name2> defaulted to HEAD, it probably would have been
better. It would have made it very unlikely to tempt anybody to hack
the --set-upstream option into the system with the wrong parameter
order if such a command line convention was in place.
If anything, it could be a sensible longer-term direction to a more
intuitive UI to deprecate the two-name format and make the creation
to be specified with an explicit --start-point option with an
argument (which defaults to HEAD), but I think that falls into the
"if I were reinventing git without existing userbase in 2005"
category and it is too late for that.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-10 23:13 ` Junio C Hamano
@ 2012-07-10 23:47 ` Jonathan Nieder
2012-07-11 1:20 ` Junio C Hamano
0 siblings, 1 reply; 33+ messages in thread
From: Jonathan Nieder @ 2012-07-10 23:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlos Martín Nieto, git, Matthieu Moy
Junio C Hamano wrote:
> You can think of it this way.
>
> "git branch" can not only _create_ a new branch (or list existing
> ones, but that is another entirely different mode), but also can be
> used to set attributes to an existing branch. Imagine a new option,
> say --set-description, to replace branch.frotz.description, for
> example. It would be used like this:
>
> $ git branch --set-description='add frotz feature' frotz
That's the same question.
You say that it would be used like that. I say that it would be
more intuitive, given how "git remote", "git config", and other
commands other than "update-index --chmod" that set attributes already
work, for it to be used like this:
git branch --set-description frotz 'add frotz feature'
Notice how similar that is to "git remote set-head origin master".
It would just be the consistent thing to do.
The truth is that neither one of us is right. Both conventions
could work, and which one is more intuitive will vary from person
to person. The convention used for plain "git branch" is
copy(target, source)
That matches memcpy() and is the opposite of what "cp" uses. Oh
well. The convention used for "git remote add" is
method(this, args...)
It's generally pretty natural. The convention used for "git
update-index --chmod" is
action(parameters)(files...)
That matches "chmod" so it was probably a good choice.
Hoping that clarifies,
Jonathan
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-10 23:47 ` Jonathan Nieder
@ 2012-07-11 1:20 ` Junio C Hamano
2012-07-11 1:37 ` Jonathan Nieder
2012-07-12 8:41 ` Miles Bader
0 siblings, 2 replies; 33+ messages in thread
From: Junio C Hamano @ 2012-07-11 1:20 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Carlos Martín Nieto, git, Matthieu Moy
Jonathan Nieder <jrnieder@gmail.com> writes:
> The truth is that neither one of us is right. Both conventions
> could work, and which one is more intuitive will vary from person
> to person.
It is not just person-to-person, I think.
In short, you are saying that, assuming that missing <start> and
<branch> are given a sane default values (namely "HEAD"), the
syntax:
git branch <branch> [<start>]
git branch --set-upstream-jrn [<branch>] <upstream>
is easier to understand, while I think
git branch <branch> [<start>]
git branch --set-upstream-to=<upstream> [<branch>]
so that omitted things can come uniformly at the end (of course,
unless the --option=argument in the middle is omitted, that is)
makes things more consistent.
I do not think it is productive to keep agreeing that we disagree
and continuing to talk between ourselves without waiting for others
to catch up, so I'll stop here.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-11 1:20 ` Junio C Hamano
@ 2012-07-11 1:37 ` Jonathan Nieder
2012-07-12 8:41 ` Miles Bader
1 sibling, 0 replies; 33+ messages in thread
From: Jonathan Nieder @ 2012-07-11 1:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlos Martín Nieto, git, Matthieu Moy
Junio C Hamano wrote:
> In short, you are saying that, assuming that missing <start> and
> <branch> are given a sane default values (namely "HEAD"), the
> syntax:
>
> git branch <branch> [<start>]
> git branch --set-upstream-jrn [<branch>] <upstream>
>
> is easier to understand
I didn't propose allowing the branch argument to be omitted, actually.
It would be clearest, _especially_ because one argument currently
means something different, to make that error out. Sorry for the lack
of clarity.
One more detail I didn't mention before: I think a convenience feature
git branch --set-upstream-to <upstream>
that takes exactly one argument and means
git branch --set-upstream HEAD <upstream>
would be fine. Having a second command to do the same thing as
--set-upstream does (or adding new --set-other-things commands that
use this proposed convention where the value comes before the key) and
migrating awkwardly to it is what I object to.
Clearer?
Jonathan
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-11 1:20 ` Junio C Hamano
2012-07-11 1:37 ` Jonathan Nieder
@ 2012-07-12 8:41 ` Miles Bader
2012-07-12 16:58 ` Junio C Hamano
1 sibling, 1 reply; 33+ messages in thread
From: Miles Bader @ 2012-07-12 8:41 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Carlos Martín Nieto, git, Matthieu Moy
Junio C Hamano <gitster@pobox.com> writes:
> is easier to understand, while I think
>
> git branch <branch> [<start>]
> git branch --set-upstream-to=<upstream> [<branch>]
Isn't one problem with this that even if a "--set-upstream-to" option
exists, inevitably some [and I'm guessing, many] people will not be
aware of it (after all, nobody reads documentation more than they have
to), and will attempt to use "--set-upstream" with an argument
(that's the natural thing to do, after all) -- which may succeed with
weird results ...?
-miles
--
One of the lessons of history is that nothing is often a good thing to
do, and always a clever thing to say. -- Will Durant
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] branch: introduce --set-upstream-to
2012-07-12 8:41 ` Miles Bader
@ 2012-07-12 16:58 ` Junio C Hamano
0 siblings, 0 replies; 33+ messages in thread
From: Junio C Hamano @ 2012-07-12 16:58 UTC (permalink / raw)
To: Miles Bader; +Cc: Jonathan Nieder, Carlos Martín Nieto, git, Matthieu Moy
Miles Bader <miles@gnu.org> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>> is easier to understand, while I think
>>
>> git branch <branch> [<start>]
>> git branch --set-upstream-to=<upstream> [<branch>]
>
> Isn't one problem with this that even if a "--set-upstream-to" option
> exists, inevitably some [and I'm guessing, many] people will not be
> aware of it (after all, nobody reads documentation more than they have
> to), and will attempt to use "--set-upstream" with an argument
> (that's the natural thing to do, after all) -- which may succeed with
> weird results ...?
In the part you quoted in the message you are responding to in the
subthread between Jonathan and, I was expressing doubts about his
"upon seeing a single argument for operations that need two pieces
of info, sometimes the first one is assumed to be missing and gets
the default, some other times the second one is assumed to be
missing and gets the default" design, which I felt would be
unnecessarily confusing.
The issue of possible confusion you raised is real, was discussed in
the main thread of discussion of the earlier round, and has been
addressed in this round of the patch series, I think, with warnings
and/or advises.
^ permalink raw reply [flat|nested] 33+ messages in thread