git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Pierre Habouzit <madcoder@debian.org>, Lars Hjemli <hjemli@gmail.com>
Subject: What should "git branch --merged master" do?
Date: Mon, 07 Jul 2008 23:49:13 -0700	[thread overview]
Message-ID: <7v8wwcx446.fsf@gitster.siamese.dyndns.org> (raw)

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),
 

             reply	other threads:[~2008-07-08  6:50 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-08  6:49 Junio C Hamano [this message]
2008-07-08 10:14 ` What should "git branch --merged master" do? Pierre Habouzit
2008-07-08 10:34   ` [PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag Pierre Habouzit
2008-07-09  1:15     ` Junio C Hamano
2008-07-09  1:17       ` [PATCH 1/2] branch --contains: default to HEAD Junio C Hamano
2008-07-09  1:22       ` [PATCH 2/2] branch --merged/--not-merged: allow specifying arbitrary commit Junio C Hamano
2008-07-09  7:45         ` Pierre Habouzit
2008-07-09  9:13           ` Junio C Hamano
2008-07-09  7:47       ` [PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag Pierre Habouzit

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=7v8wwcx446.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=hjemli@gmail.com \
    --cc=madcoder@debian.org \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).