What should "git branch --merged master" do?

What should "git branch --merged master" do?

[Junio C Hamano]

e8b404c (git-branch: add support for --merged and --no-merged, 2008-04-17)
introduced "git branch --merged" to show the branches that are contained
within the current HEAD.  I.e. the ones that you can say "git branch -d"
and do not have to say "-D" to delete.

Currently "git branch --merged master" fails with a message:

	fatal: A branch named 'master' already exists.

but --merged and its opposite --no-merged are in spirit very similar to
another option --contains in that it is meant to be used as a way to
influence which branches are _reported_, and not about creating a new
branch.

Perhaps we should change them to default to HEAD (to retain the current
behaviour) but take a commit, and show branches that are merged to the
commit or not yet fully merged to the commit, respectively?

Incidentally, "git branch --with" fails without the mandatory commit
argument, and if we are going to do the above, we probably should default
the argument to HEAD as well.

Here is an attempt to update --with but I am not happy with it.

The patch makes

	$ git branch --contains

work as expected, but breaks

	$ git branch --contains master

You need to spell "git branch --contains=master" instead.  Which means it
is a regression and cannot be applied.  I suspect I am not using
parse_options() in an optimal way, but I did not find a way for my
callback to tell "I consumed the next parameter and it was mine" or "The
next one is not my optional parameter" back to the parse_options(), so
probably parse_options() need to be fixed to update this without
regression, I suspect.

 builtin-branch.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index d279702..ee722a2 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -411,7 +411,7 @@ static int opt_parse_with_commit(const struct option *opt, const char *arg, int
 	struct commit *commit;
 
 	if (!arg)
-		return -1;
+		arg = "HEAD";
 	if (get_sha1(arg, sha1))
 		die("malformed object name %s", arg);
 	commit = lookup_commit_reference(sha1);
@@ -438,13 +438,15 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN( 0 , "color",  &branch_use_color, "use colored output"),
 		OPT_SET_INT('r', NULL,     &kinds, "act on remote-tracking branches",
 			REF_REMOTE_BRANCH),
-		OPT_CALLBACK(0, "contains", &with_commit, "commit",
-			     "print only branches that contain the commit",
-			     opt_parse_with_commit),
+		{
+			OPTION_CALLBACK, 0, "contains", &with_commit, "commit",
+			"print only branches that contain the commit",
+			PARSE_OPT_OPTARG, opt_parse_with_commit,
+		},
 		{
 			OPTION_CALLBACK, 0, "with", &with_commit, "commit",
 			"print only branches that contain the commit",
-			PARSE_OPT_HIDDEN, opt_parse_with_commit,
+			PARSE_OPT_HIDDEN | PARSE_OPT_OPTARG, opt_parse_with_commit,
 		},
 		OPT__ABBREV(&abbrev),
 

Re: What should "git branch --merged master" do?

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]

On Tue, Jul 08, 2008 at 06:49:13AM +0000, Junio C Hamano wrote:
> e8b404c (git-branch: add support for --merged and --no-merged, 2008-04-17)
> introduced "git branch --merged" to show the branches that are contained
> within the current HEAD.  I.e. the ones that you can say "git branch -d"
> and do not have to say "-D" to delete.
> 
> Currently "git branch --merged master" fails with a message:
> 
> 	fatal: A branch named 'master' already exists.
> 
> but --merged and its opposite --no-merged are in spirit very similar to
> another option --contains in that it is meant to be used as a way to
> influence which branches are _reported_, and not about creating a new
> branch.
> 
> Perhaps we should change them to default to HEAD (to retain the current
> behaviour) but take a commit, and show branches that are merged to the
> commit or not yet fully merged to the commit, respectively?
> 
> Incidentally, "git branch --with" fails without the mandatory commit
> argument, and if we are going to do the above, we probably should default
> the argument to HEAD as well.
> 
> Here is an attempt to update --with but I am not happy with it.
> 
> The patch makes
> 
> 	$ git branch --contains
> 
> work as expected, but breaks
> 
> 	$ git branch --contains master
> 
> You need to spell "git branch --contains=master" instead.  Which means it
> is a regression and cannot be applied.  I suspect I am not using
> parse_options() in an optimal way, but I did not find a way for my
> callback to tell "I consumed the next parameter and it was mine" or "The
> next one is not my optional parameter" back to the parse_options(), so
> probably parse_options() need to be fixed to update this without
> regression, I suspect.

  I actually see three ways.

  (1) quick and dirty: if we see at argv[argc - 1]
      --contains/--merge/--no-merge/... we do:

      argv[argc] = "HEAD" and use parse-options with a mandatory
      argument for --merge/--contains/... Okay this leaves the issue of
      the fact that we must NULL-terminate argv, but that's trivial
      (even if quite despisable).

  (2) big update of parse-options: we refactor callbacks so that they
      can decide if they take their argument or not. This removes the
      need for --long-opt=foo for options with optional arguments, but
      the refactor will be heavy. That's more or less what you suggest
      in your last § but I don't like it for many reasons so I wont
      enter the details.

  (3) inspired from (1) and (2), have a flag for options that says "I do
      take an argument, but if I'm the last option on the command line,
      please fake this argument for me.

  I really like (3) more FWIW as it doesn't generate ambiguous parsers
like (2) would, and it's not horrible like (1). And cherry on top it's
pretty trivial to implement I think.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 4482 bytes --]

If you set this for a given flag, and the flag appears without a value on
the command line, then the `defval' is used to fake a new argument.

Note that this flag is meaningless in presence of OPTARG or NOARG flags.
(in the current implementation it will be ignored, but don't rely on it).

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

    >   (3) inspired from (1) and (2), have a flag for options that says
    >       "I do take an argument, but if I'm the last option on the
    >       command line, please fake this argument for me.
    >
    > I really like (3) more FWIW as it doesn't generate ambiguous
    > parsers like (2) would, and it's not horrible like (1). And cherry
    > on top it's pretty trivial to implement I think.

  And here it is, untested though (in the sense that I didn't test the
  new feature, but git is not broken with the patch).


 parse-options.c |   55 +++++++++++++++++++++++++++----------------------------
 parse-options.h |    1 +
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index a90b336..a63485c 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -5,17 +5,6 @@
 #define OPT_SHORT 1
 #define OPT_UNSET 2
 
-static inline const char *get_arg(struct parse_opt_ctx_t *p)
-{
-	if (p->opt) {
-		const char *res = p->opt;
-		p->opt = NULL;
-		return res;
-	}
-	p->argc--;
-	return *++p->argv;
-}
-
 static int opterror(const struct option *opt, const char *reason, int flags)
 {
 	if (flags & OPT_SHORT)
@@ -25,8 +14,24 @@ static int opterror(const struct option *opt, const char *reason, int flags)
 	return error("option `%s' %s", opt->long_name, reason);
 }
 
+static inline int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
+			  int flags, const char **arg)
+{
+	if (p->opt) {
+		*arg = p->opt;
+		p->opt = NULL;
+	} else if (p->argc) {
+		p->argc--;
+		*arg = *++p->argv;
+	} else if (opt->flags & PARSE_OPT_FAKELASTARG) {
+		*arg = (const char *)opt->defval;
+	} else
+		return opterror(opt, "requires a value", flags);
+	return 0;
+}
+
 static int get_value(struct parse_opt_ctx_t *p,
-                     const struct option *opt, int flags)
+		     const struct option *opt, int flags)
 {
 	const char *s, *arg;
 	const int unset = flags & OPT_UNSET;
@@ -52,7 +57,6 @@ static int get_value(struct parse_opt_ctx_t *p,
 		}
 	}
 
-	arg = p->opt ? p->opt : (p->argc > 1 ? p->argv[1] : NULL);
 	switch (opt->type) {
 	case OPTION_BIT:
 		if (unset)
@@ -74,17 +78,12 @@ static int get_value(struct parse_opt_ctx_t *p,
 		return 0;
 
 	case OPTION_STRING:
-		if (unset) {
+		if (unset)
 			*(const char **)opt->value = NULL;
-			return 0;
-		}
-		if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
+		else if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
 			*(const char **)opt->value = (const char *)opt->defval;
-			return 0;
-		}
-		if (!arg)
-			return opterror(opt, "requires a value", flags);
-		*(const char **)opt->value = get_arg(p);
+		else
+			return get_arg(p, opt, flags, (const char **)opt->value);
 		return 0;
 
 	case OPTION_CALLBACK:
@@ -94,9 +93,9 @@ static int get_value(struct parse_opt_ctx_t *p,
 			return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
 		if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
 			return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
-		if (!arg)
-			return opterror(opt, "requires a value", flags);
-		return (*opt->callback)(opt, get_arg(p), 0) ? (-1) : 0;
+		if (get_arg(p, opt, flags, &arg))
+			return -1;
+		return (*opt->callback)(opt, arg, 0) ? (-1) : 0;
 
 	case OPTION_INTEGER:
 		if (unset) {
@@ -107,9 +106,9 @@ static int get_value(struct parse_opt_ctx_t *p,
 			*(int *)opt->value = opt->defval;
 			return 0;
 		}
-		if (!arg)
-			return opterror(opt, "requires a value", flags);
-		*(int *)opt->value = strtol(get_arg(p), (char **)&s, 10);
+		if (get_arg(p, opt, flags, &arg))
+			return -1;
+		*(int *)opt->value = strtol(arg, (char **)&s, 10);
 		if (*s)
 			return opterror(opt, "expects a numerical value", flags);
 		return 0;
diff --git a/parse-options.h b/parse-options.h
index c5f0b4b..6e9edd1 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -28,6 +28,7 @@ enum parse_opt_option_flags {
 	PARSE_OPT_NOARG   = 2,
 	PARSE_OPT_NONEG   = 4,
 	PARSE_OPT_HIDDEN  = 8,
+	PARSE_OPT_FAKELASTARG = 16,
 };
 
 struct option;
-- 
1.5.6.2.398.g3c3f1.dirty

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag.

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

> If you set this for a given flag, and the flag appears without a value on
> the command line, then the `defval' is used to fake a new argument.
>
> Note that this flag is meaningless in presence of OPTARG or NOARG flags.
> (in the current implementation it will be ignored, but don't rely on it).
>
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
>
>     >   (3) inspired from (1) and (2), have a flag for options that says
>     >       "I do take an argument, but if I'm the last option on the
>     >       command line, please fake this argument for me.
>     >
>     > I really like (3) more FWIW as it doesn't generate ambiguous
>     > parsers like (2) would, and it's not horrible like (1). And cherry
>     > on top it's pretty trivial to implement I think.

Yeah, I do not particularly want a major rewrite that only introduces
possible ambiguity to the option parser (even though arguably it would add
to the usability very much, just like we accept revs and paths when
unambiguous), so I think this is a reasonable compromise.

It feels more like LASTARG_DEFAULT but that is bikeshedding ;-)

But I see one thinko (fix below) and another issue I am not sure what the
best fix would be.

---
diff --git a/parse-options.c b/parse-options.c
index b6735a5..cba20d7 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -26,11 +26,11 @@ static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
 	if (p->opt) {
 		*arg = p->opt;
 		p->opt = NULL;
+	} else if (p->argc == 1 && (opt->flags & PARSE_OPT_FAKELASTARG)) {
+		*arg = (const char *)opt->defval;
 	} else if (p->argc) {
 		p->argc--;
 		*arg = *++p->argv;
-	} else if (opt->flags & PARSE_OPT_FAKELASTARG) {
-		*arg = (const char *)opt->defval;
 	} else
 		return opterror(opt, "requires a value", flags);
 	return 0;

[PATCH 1/2] branch --contains: default to HEAD

[Junio C Hamano]

We used to require the name of the commit to limit the branches shown to
the --contains option, but more recent --merged/--no-meregd defaults to
HEAD (and they do not allow arbitrary commit, which is a separate issue).

This teaches --contains to default to HEAD when no parameter is given.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * This comes on top of the FAKELASTARG patch

 builtin-branch.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index d279702..375e5e0 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -438,13 +438,17 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN( 0 , "color",  &branch_use_color, "use colored output"),
 		OPT_SET_INT('r', NULL,     &kinds, "act on remote-tracking branches",
 			REF_REMOTE_BRANCH),
-		OPT_CALLBACK(0, "contains", &with_commit, "commit",
-			     "print only branches that contain the commit",
-			     opt_parse_with_commit),
+		{
+			OPTION_CALLBACK, 0, "contains", &with_commit, "commit",
+			"print only branches that contain the commit",
+			PARSE_OPT_FAKELASTARG,
+			opt_parse_with_commit, (intptr_t)"HEAD",
+		},
 		{
 			OPTION_CALLBACK, 0, "with", &with_commit, "commit",
 			"print only branches that contain the commit",
-			PARSE_OPT_HIDDEN, opt_parse_with_commit,
+			PARSE_OPT_HIDDEN | PARSE_OPT_FAKELASTARG,
+			opt_parse_with_commit, (intptr_t) "HEAD",
 		},
 		OPT__ABBREV(&abbrev),
 
-- 
1.5.6.2.255.gbed62

[PATCH 2/2] branch --merged/--not-merged: allow specifying arbitrary commit

[Junio C Hamano]

"git-branch --merged" is a handy way to list all the branches that have
already been merged to the current branch, but it did not allow checking
against anything but the current branch.  Having to check out only for
that purpose made the command practically useless.

This updates the option parser so that "git branch --merged next" is
accepted when you are on 'master' branch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This does have an issue.  --no-<option>=<value> is often nonsense and
   parse-options does not accept it (and I do not think we would want to
   change it).  The use of "--no-merged" was a mistake, but nobody has
   perfect foresight.

   This adds --not-merged <commit> and allows the <commit> to default to
   HEAD if not given to work it around.  This and the previous one are not
   for application but primarily meant for discussion on what further
   flexibility we may want to have in parse-options.

 builtin-branch.c |   50 ++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 375e5e0..151f519 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -46,7 +46,12 @@ enum color_branch {
 	COLOR_BRANCH_CURRENT = 4,
 };
 
-static int mergefilter = -1;
+static enum merge_filter {
+	NO_FILTER = 0,
+	SHOW_NOT_MERGED,
+	SHOW_MERGED,
+} merge_filter;
+static unsigned char merge_filter_ref[20];
 
 static int parse_branch_color_slot(const char *var, int ofs)
 {
@@ -234,13 +239,15 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,
 	if ((kind & ref_list->kinds) == 0)
 		return 0;
 
-	if (mergefilter > -1) {
+	if (merge_filter != NO_FILTER) {
 		branch.item = lookup_commit_reference_gently(sha1, 1);
 		if (!branch.item)
 			die("Unable to lookup tip of branch %s", refname);
-		if (mergefilter == 0 && has_commit(head_sha1, &branch))
+		if (merge_filter == SHOW_NOT_MERGED &&
+		    has_commit(merge_filter_ref, &branch))
 			return 0;
-		if (mergefilter == 1 && !has_commit(head_sha1, &branch))
+		if (merge_filter == SHOW_MERGED &&
+		    !has_commit(merge_filter_ref, &branch))
 			return 0;
 	}
 
@@ -421,6 +428,20 @@ static int opt_parse_with_commit(const struct option *opt, const char *arg, int
 	return 0;
 }
 
+static int opt_parse_merge_filter(const struct option *opt, const char *arg, int unset)
+{
+	merge_filter = ((opt->long_name[0] == 'n')
+			? SHOW_NOT_MERGED
+			: SHOW_MERGED);
+	if (unset)
+		merge_filter = SHOW_NOT_MERGED; /* b/c for --no-merged */
+	if (!arg)
+		arg = "HEAD";
+	if (get_sha1(arg, merge_filter_ref))
+		die("malformed object name %s", arg);
+	return 0;
+}
+
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, force_create = 0;
@@ -461,7 +482,18 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('M', NULL, &rename, "move/rename a branch, even if target exists", 2),
 		OPT_BOOLEAN('l', NULL, &reflog, "create the branch's reflog"),
 		OPT_BOOLEAN('f', NULL, &force_create, "force creation (when already exists)"),
-		OPT_SET_INT(0, "merged", &mergefilter, "list only merged branches", 1),
+		{
+			OPTION_CALLBACK, 0, "merged", &merge_filter_ref,
+			"commit", "print only merged branches",
+			PARSE_OPT_FAKELASTARG,
+			opt_parse_merge_filter, (intptr_t) "HEAD",
+		},
+		{
+			OPTION_CALLBACK, 0, "not-merged", &merge_filter_ref,
+			"commit", "print only not merged branches",
+			PARSE_OPT_FAKELASTARG,
+			opt_parse_merge_filter, (intptr_t) "HEAD",
+		},
 		OPT_END(),
 	};
 
@@ -471,9 +503,6 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		branch_use_color = git_use_color_default;
 
 	track = git_branch_track;
-	argc = parse_options(argc, argv, options, builtin_branch_usage, 0);
-	if (!!delete + !!rename + !!force_create > 1)
-		usage_with_options(builtin_branch_usage, options);
 
 	head = resolve_ref("HEAD", head_sha1, 0, NULL);
 	if (!head)
@@ -486,6 +515,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			die("HEAD not found below refs/heads!");
 		head += 11;
 	}
+	hashcpy(merge_filter_ref, head_sha1);
+
+	argc = parse_options(argc, argv, options, builtin_branch_usage, 0);
+	if (!!delete + !!rename + !!force_create > 1)
+		usage_with_options(builtin_branch_usage, options);
 
 	if (delete)
 		return delete_branches(argc, argv, delete > 1, kinds);
-- 
1.5.6.2.255.gbed62

Re: [PATCH 2/2] branch --merged/--not-merged: allow specifying arbitrary commit

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1632 bytes --]

On Wed, Jul 09, 2008 at 01:22:10AM +0000, Junio C Hamano wrote:
> "git-branch --merged" is a handy way to list all the branches that have
> already been merged to the current branch, but it did not allow checking
> against anything but the current branch.  Having to check out only for
> that purpose made the command practically useless.
> 
> This updates the option parser so that "git branch --merged next" is
> accepted when you are on 'master' branch.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  * This does have an issue.  --no-<option>=<value> is often nonsense and
>    parse-options does not accept it (and I do not think we would want to
>    change it).  The use of "--no-merged" was a mistake, but nobody has
>    perfect foresight.
> 
>    This adds --not-merged <commit> and allows the <commit> to default to
>    HEAD if not given to work it around.  This and the previous one are not
>    for application but primarily meant for discussion on what further
>    flexibility we may want to have in parse-options.

  There's a way: declare --merge as (PARSE_OPT_)NONEG to tell parse-opt
not to generate --no-merge by itself, and declare a --no-merge option as
well.  I think it works, and if not, we should make parse-opt work with
that. This is a quite disgusting hack, and no *new* options should be
written this way, but we have to be flexible enough for backward
compatibility's sake.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1790 bytes --]

On Wed, Jul 09, 2008 at 01:15:43AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > If you set this for a given flag, and the flag appears without a value on
> > the command line, then the `defval' is used to fake a new argument.
> >
> > Note that this flag is meaningless in presence of OPTARG or NOARG flags.
> > (in the current implementation it will be ignored, but don't rely on it).
> >
> > Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> > ---
> >
> >     >   (3) inspired from (1) and (2), have a flag for options that says
> >     >       "I do take an argument, but if I'm the last option on the
> >     >       command line, please fake this argument for me.
> >     >
> >     > I really like (3) more FWIW as it doesn't generate ambiguous
> >     > parsers like (2) would, and it's not horrible like (1). And cherry
> >     > on top it's pretty trivial to implement I think.
> 
> Yeah, I do not particularly want a major rewrite that only introduces
> possible ambiguity to the option parser (even though arguably it would add
> to the usability very much, just like we accept revs and paths when
> unambiguous), so I think this is a reasonable compromise.
> 
> It feels more like LASTARG_DEFAULT but that is bikeshedding ;-)

  I absolutely don't like this FAKELASTARG name, so really, use what you
like.

> But I see one thinko (fix below) and another issue I am not sure what the
> best fix would be.

  Like I said it was just a draft, I did not test the new feature, so
I'm not really surprised it's partly broken ;)

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/2] branch --merged/--not-merged: allow specifying arbitrary commit

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

>   There's a way: declare --merge as (PARSE_OPT_)NONEG to tell parse-opt
> not to generate --no-merge by itself, and declare a --no-merge option as
> well.  I think it works, and if not, we should make parse-opt work with
> that. This is a quite disgusting hack, and no *new* options should be
> written this way, but we have to be flexible enough for backward
> compatibility's sake.

Nice, thanks.

With the obvious s/_FAKELASTARG/_LASTARG_DEFAULT/g to the first one, this
replaces the second one (with documentation updates for a change ;-), and
it seems to work Ok.

-- >8 --
Subject: branch --merged/--no-merged: allow specifying arbitrary commit

"git-branch --merged" is a handy way to list all the branches that have
already been merged to the current branch, but it did not allow checking
against anything but the current branch.  Having to switch branches only
to list the branches that are merged with another branch made the feature
practically useless.

This updates the option parser so that "git branch --merged next" is
accepted when you are on 'master' branch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-branch.txt |   27 ++++++++++++----------
 builtin-branch.c             |   50 +++++++++++++++++++++++++++++++++++------
 2 files changed, 57 insertions(+), 20 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 0fd5808..450f903 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -8,24 +8,27 @@ git-branch - List, create, or delete branches
 SYNOPSIS
 --------
 [verse]
-'git-branch' [--color | --no-color] [-r | -a] [--merged | --no-merged]
-	   [-v [--abbrev=<length> | --no-abbrev]]
-	   [--contains <commit>]
+'git-branch' [--color | --no-color] [-r | -a]
+	[-v [--abbrev=<length> | --no-abbrev]]
+	[(--merged | --no-merged | --contains) [<commit>]]
 'git-branch' [--track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git-branch' (-m | -M) [<oldbranch>] <newbranch>
 'git-branch' (-d | -D) [-r] <branchname>...
 
 DESCRIPTION
 -----------
-With no arguments given a list of existing branches
-will be shown, the current branch will be highlighted with an asterisk.
-Option `-r` causes the remote-tracking branches to be listed,
-and option `-a` shows both.
-With `--contains <commit>`, shows only the branches that
-contains the named commit (in other words, the branches whose
-tip commits are descendant of the named commit).
-With `--merged`, only branches merged into HEAD will be listed, and
-with `--no-merged` only branches not merged into HEAD will be listed.
+
+With no 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.
+
+With `--contains`, shows only the branches that contains the named commit
+(in other words, the branches whose tip commits are descendant of the
+named commit).  With `--merged`, only branches merged into the named
+commit (i.e. the branches whose tip commits are reachable from the named
+commit) will be listed.  With `--no-merged` only branches not merged into
+the named commit will be listed.  Missing <commit> argument defaults to
+'HEAD' (i.e. the tip of the current branch).
 
 In its second form, a new branch named <branchname> will be created.
 It will start out with a head equal to the one given as <start-point>.
diff --git a/builtin-branch.c b/builtin-branch.c
index 50cbc9c..1926c47 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -46,7 +46,12 @@ enum color_branch {
 	COLOR_BRANCH_CURRENT = 4,
 };
 
-static int mergefilter = -1;
+static enum merge_filter {
+	NO_FILTER = 0,
+	SHOW_NOT_MERGED,
+	SHOW_MERGED,
+} merge_filter;
+static unsigned char merge_filter_ref[20];
 
 static int parse_branch_color_slot(const char *var, int ofs)
 {
@@ -234,13 +239,15 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,
 	if ((kind & ref_list->kinds) == 0)
 		return 0;
 
-	if (mergefilter > -1) {
+	if (merge_filter != NO_FILTER) {
 		branch.item = lookup_commit_reference_gently(sha1, 1);
 		if (!branch.item)
 			die("Unable to lookup tip of branch %s", refname);
-		if (mergefilter == 0 && has_commit(head_sha1, &branch))
+		if (merge_filter == SHOW_NOT_MERGED &&
+		    has_commit(merge_filter_ref, &branch))
 			return 0;
-		if (mergefilter == 1 && !has_commit(head_sha1, &branch))
+		if (merge_filter == SHOW_MERGED &&
+		    !has_commit(merge_filter_ref, &branch))
 			return 0;
 	}
 
@@ -421,6 +428,20 @@ static int opt_parse_with_commit(const struct option *opt, const char *arg, int
 	return 0;
 }
 
+static int opt_parse_merge_filter(const struct option *opt, const char *arg, int unset)
+{
+	merge_filter = ((opt->long_name[0] == 'n')
+			? SHOW_NOT_MERGED
+			: SHOW_MERGED);
+	if (unset)
+		merge_filter = SHOW_NOT_MERGED; /* b/c for --no-merged */
+	if (!arg)
+		arg = "HEAD";
+	if (get_sha1(arg, merge_filter_ref))
+		die("malformed object name %s", arg);
+	return 0;
+}
+
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, force_create = 0;
@@ -461,7 +482,18 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('M', NULL, &rename, "move/rename a branch, even if target exists", 2),
 		OPT_BOOLEAN('l', NULL, &reflog, "create the branch's reflog"),
 		OPT_BOOLEAN('f', NULL, &force_create, "force creation (when already exists)"),
-		OPT_SET_INT(0, "merged", &mergefilter, "list only merged branches", 1),
+		{
+			OPTION_CALLBACK, 0, "no-merged", &merge_filter_ref,
+			"commit", "print only not merged branches",
+			PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG,
+			opt_parse_merge_filter, (intptr_t) "HEAD",
+		},
+		{
+			OPTION_CALLBACK, 0, "merged", &merge_filter_ref,
+			"commit", "print only merged branches",
+			PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG,
+			opt_parse_merge_filter, (intptr_t) "HEAD",
+		},
 		OPT_END(),
 	};
 
@@ -471,9 +503,6 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		branch_use_color = git_use_color_default;
 
 	track = git_branch_track;
-	argc = parse_options(argc, argv, options, builtin_branch_usage, 0);
-	if (!!delete + !!rename + !!force_create > 1)
-		usage_with_options(builtin_branch_usage, options);
 
 	head = resolve_ref("HEAD", head_sha1, 0, NULL);
 	if (!head)
@@ -486,6 +515,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			die("HEAD not found below refs/heads!");
 		head += 11;
 	}
+	hashcpy(merge_filter_ref, head_sha1);
+
+	argc = parse_options(argc, argv, options, builtin_branch_usage, 0);
+	if (!!delete + !!rename + !!force_create > 1)
+		usage_with_options(builtin_branch_usage, options);
 
 	if (delete)
 		return delete_branches(argc, argv, delete > 1, kinds);
-- 
1.5.6.2.276.gbb293