threads / bug / 32781
Subject: [BUG] `git branch --contains ID name` creates branch "name"
Peter WuHi,
I was trying to check whether a certain branch contained a commit and ran:
git branch --contains ddc150f7a33ae0c9cb16eaac3641abc00f56316f master
This resulted in:
fatal: A branch named 'master' already exists. When "name" does not exist, this command creates a branch. I expect this command to search the mentioned branch, not trying to create it. The manual page of git-branch(1) does not mention such special behavior either.
Git version 1.8.1.2
Regards, Peter
Jeff King[PATCH 0/2] improve "git branch --contains=<commit> <pattern>"
On Wed, Jan 30, 2013 at 07:57:03PM +0100, Peter Wu wrote:
> Hi, > > I was trying to check whether a certain branch contained a commit and ran: > > git branch --contains ddc150f7a33ae0c9cb16eaac3641abc00f56316f master > > This resulted in: > > fatal: A branch named 'master' already exists. > When "name" does not exist, this command creates a branch. I expect this > command to search the mentioned branch, not trying to create it. The manual > page of git-branch(1) does not mention such special behavior either.
Yeah, it's sort-of a bug. It is a syntactic ambiguity that an argument to git-branch could be a listing pattern or a new branch name. When using a listing pattern, you need to explicitly specify that you want list mode with `--list`. This is documented in git-branch under --list, but it should be more prominent, in the section that covers the various invocation modes. The first patch below fixes that.
That being said, we could be much more helpful. It seems like --contains should imply listing mode, since it is nonsensical in other modes. The second patch below adjusts that, and makes the command above do what you expect.
[1/2]: docs: clarify git-branch --list behavior [2/2]: branch: let branch filters imply --list
-Peff
Jeff King[PATCH 1/2] docs: clarify git-branch --list behavior
It was not clear from the "description" section of git-branch(1) that using a <pattern> meant that you _had_ to use the --list option. Let's clarify that, and while we're at it, reword some clunky and ambiguous sentences.
Signed-off-by: Jeff King <peff@peff.net> --- Documentation/git-branch.txt | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index 45a225e..01aa87f 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -22,13 +22,15 @@ DESCRIPTION DESCRIPTION ----------- -With no arguments, existing branches are listed and the current branch will -be highlighted with an asterisk. Option `-r` causes the remote-tracking -branches to be listed, and option `-a` shows both. This list mode is also -activated by the `--list` option (see below). -<pattern> restricts the output to matching branches, the pattern is a shell -wildcard (i.e., matched using fnmatch(3)). -Multiple patterns may be given; if any of them matches, the branch is shown. +If `--list` is given, or if there are no non-option arguments, existing +branches are listed; the current branch will be highlighted with an +asterisk. Option `-r` causes the remote-tracking branches to be listed, +and option `-a` shows both local and remote branches. If a `<pattern>` +is given, it is used as a shell wildcard to restrict the output to +matching branches. If multiple patterns are given, a branch is shown if +any it is matched by any of the patterns. Note that when providing a +`<pattern>`, you must use `--list`; otherwise the command is interpreted +as branch creation. With `--contains`, shows only the branches that contain the named commit (in other words, the branches whose tip commits are descendants of the
-- 1.8.1.2.5.g1cb3f73
Eric SunshineRe: [PATCH 1/2] docs: clarify git-branch --list behavior
On Thu, Jan 31, 2013 at 1:45 AM, Jeff King <peff@peff.net> wrote:
> +If `--list` is given, or if there are no non-option arguments, existing > +branches are listed; the current branch will be highlighted with an > +asterisk. Option `-r` causes the remote-tracking branches to be listed, > +and option `-a` shows both local and remote branches. If a `<pattern>` > +is given, it is used as a shell wildcard to restrict the output to > +matching branches. If multiple patterns are given, a branch is shown if > +any it is matched by any of the patterns.
s/if any it is/if it is/
-- ES
Junio C HamanoRe: [PATCH 1/2] docs: clarify git-branch --list behavior
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Thu, Jan 31, 2013 at 1:45 AM, Jeff King <peff@peff.net> wrote: >> +If `--list` is given, or if there are no non-option arguments, existing >> +branches are listed; the current branch will be highlighted with an >> +asterisk. Option `-r` causes the remote-tracking branches to be listed, >> +and option `-a` shows both local and remote branches. If a `<pattern>` >> +is given, it is used as a shell wildcard to restrict the output to >> +matching branches. If multiple patterns are given, a branch is shown if >> +any it is matched by any of the patterns. > > s/if any it is/if it is/
Thanks; I'll squash this in. Peff, no need to resend.
Documentation/git-branch.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index 01aa87f..2635dee 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -28,7 +28,7 @@ asterisk. Option `-r` causes the remote-tracking branches to be listed, and option `-a` shows both local and remote branches. If a `<pattern>` is given, it is used as a shell wildcard to restrict the output to matching branches. If multiple patterns are given, a branch is shown if -any it is matched by any of the patterns. Note that when providing a +it matches any of the patterns. Note that when providing a `<pattern>`, you must use `--list`; otherwise the command is interpreted as branch creation.
Jeff KingRe: [PATCH 1/2] docs: clarify git-branch --list behavior
On Thu, Jan 31, 2013 at 04:37:01PM -0800, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes: > > > On Thu, Jan 31, 2013 at 1:45 AM, Jeff King <peff@peff.net> wrote: > >> +If `--list` is given, or if there are no non-option arguments, existing > >> +branches are listed; the current branch will be highlighted with an > >> +asterisk. Option `-r` causes the remote-tracking branches to be listed, > >> +and option `-a` shows both local and remote branches. If a `<pattern>` > >> +is given, it is used as a shell wildcard to restrict the output to > >> +matching branches. If multiple patterns are given, a branch is shown if > >> +any it is matched by any of the patterns. > > > > s/if any it is/if it is/ > > Thanks; I'll squash this in. Peff, no need to resend.
Thanks. No matter how many times I proofread a doc change, I always manage to slip an error into the final version. Hooray for many eyes.
-Peff
Junio C HamanoRe: [PATCH 1/2] docs: clarify git-branch --list behavior
Jeff King <peff@peff.net> writes:
> Thanks. No matter how many times I proofread a doc change, I always > manage to slip an error into the final version.
I think everybody shares that trait.
> Hooray for many eyes.
Indeed.
Jeff King[PATCH 2/2] branch: let branch filters imply --list
Currently, a branch filter like `--contains`, `--merged`, or `--no-merged` is ignored when we are not in listing mode. For example:
git branch --contains=foo bar
will create the branch "bar" from the current HEAD, ignoring the `--contains` argument entirely. This is not very helpful. There are two reasonable behaviors for git here:
1. Flag an error; the arguments do not make sense.
2. Implicitly go into `--list` mode
This patch chooses the latter, as it is more convenient, and there should not be any ambiguity with attempting to create a branch; using `--contains` and not wanting to list is nonsensical.
That leaves the case where an explicit modification option like `-d` is given. We already catch the case where `--list` is given alongside `-d` and flag an error. With this patch, we will also catch the use of `--contains` and other filter options alongside `-d`.
Signed-off-by: Jeff King <peff@peff.net> --- Documentation/git-branch.txt | 6 +++--- builtin/branch.c | 3 +++ t/t3201-branch-contains.sh | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index 01aa87f..07ef5af 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -195,15 +195,15 @@ start-point is either a local or remote-tracking branch. --contains [<commit>]:: Only list branches which contain the specified commit (HEAD - if not specified). + if not specified). Implies `--list`. --merged [<commit>]:: Only list branches whose tips are reachable from the - specified commit (HEAD if not specified). + specified commit (HEAD if not specified). Implies `--list`. --no-merged [<commit>]:: Only list branches whose tips are not reachable from the - specified commit (HEAD if not specified). + specified commit (HEAD if not specified). Implies `--list`. <branchname>:: The name of the branch to create or delete. diff --git a/builtin/branch.c b/builtin/branch.c index 873f624..4aa3d4e 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -825,6 +825,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix) if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0) list = 1; + if (with_commit || merge_filter != NO_FILTER) + list = 1; + if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1) usage_with_options(builtin_branch_usage, options); diff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh index f86f4bc..141b061 100755 --- a/t/t3201-branch-contains.sh +++ b/t/t3201-branch-contains.sh @@ -55,6 +55,16 @@ test_expect_success 'branch --contains=side' ' ' +test_expect_success 'branch --contains with pattern implies --list' ' + + git branch --contains=master master >actual && + { + echo " master" + } >expect && + test_cmp expect actual + +' + test_expect_success 'side: branch --merged' ' git branch --merged >actual && @@ -66,6 +76,16 @@ test_expect_success 'side: branch --merged' ' ' +test_expect_success 'branch --merged with pattern implies --list' ' + + git branch --merged=side master >actual && + { + echo " master" + } >expect && + test_cmp expect actual + +' + test_expect_success 'side: branch --no-merged' ' git branch --no-merged >actual && @@ -95,4 +115,19 @@ test_expect_success 'master: branch --no-merged' ' ' +test_expect_success 'branch --no-merged with pattern implies --list' ' + + git branch --no-merged=master master >actual && + >expect && + test_cmp expect actual + +' + +test_expect_success 'implicit --list conflicts with modification options' ' + + test_must_fail git branch --contains=master -d && + test_must_fail git branch --contains=master -m foo + +' + test_done
-- 1.8.1.2.5.g1cb3f73
Peter WuRe: [PATCH 2/2] branch: let branch filters imply --list
On Thursday 31 January 2013 01:46:11 Jeff King wrote:
> Currently, a branch filter like `--contains`, `--merged`, or > `--no-merged` is ignored when we are not in listing mode. > For example: > > git branch --contains=foo bar > > will create the branch "bar" from the current HEAD, ignoring > the `--contains` argument entirely. This is not very > helpful. There are two reasonable behaviors for git here: > > 1. Flag an error; the arguments do not make sense. > > 2. Implicitly go into `--list` mode > > This patch chooses the latter, as it is more convenient, and > there should not be any ambiguity with attempting to create > a branch; using `--contains` and not wanting to list is > nonsensical. > > That leaves the case where an explicit modification option > like `-d` is given. We already catch the case where > `--list` is given alongside `-d` and flag an error. With > this patch, we will also catch the use of `--contains` and > other filter options alongside `-d`. > > Signed-off-by: Jeff King <peff@peff.net>
Tested-by: Peter Wu <lekensteyn@gmail.com>
I have tested this patch on top of 1.8.1.2 and it seems to work.
One note, the following command spits out master without complaining about the non-existing branch name:
git branch --contains <id> master <non-existant branch name>
(the order of branches doesn't affect the result.)
Regards, Peter
Junio C HamanoRe: [PATCH 2/2] branch: let branch filters imply --list
Peter Wu <lekensteyn@gmail.com> writes:
> One note, the following command spits out master without complaining about the > non-existing branch name: > > git branch --contains <id> master <non-existant branch name> > > (the order of branches doesn't affect the result.)
That is perfectly normal.
What you gave after "--contains <id>" are *not* branch names. They are patterns against branch names that fits the given criteria (in this case "--contains <id>") are matched, and the branches that do not match any of the patterns will not appear in the result.
Junio C HamanoRe: [PATCH 0/2] improve "git branch --contains=<commit> <pattern>"
Jeff King <peff@peff.net> writes:
> That being said, we could be much more helpful. It seems like --contains > should imply listing mode, since it is nonsensical in other modes. The > second patch below adjusts that, and makes the command above do what you > expect. > > [1/2]: docs: clarify git-branch --list behavior > [2/2]: branch: let branch filters imply --list
Thanks. Looks good.