{"thread":{"id":"14343","subject":"What should \"git branch --merged master\" do?","startedAt":"2008-07-08T06:49:13Z","lastAt":"2008-07-09T09:13:12Z","messageCount":9,"participants":["Junio C Hamano","Pierre Habouzit"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"82561","messageId":"7v8wwcx446.fsf@gitster.siamese.dyndns.org","threadId":"14343","inReplyTo":null,"subject":"What should \"git branch --merged master\" do?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-08T06:49:13Z","receivedAt":"2008-07-08T06:49:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"e8b404c (git-branch: add support for --merged and --no-merged, 2008-04-17)\nintroduced \"git branch --merged\" to show the branches that are contained\nwithin the current HEAD.  I.e. the ones that you can say \"git branch -d\"\nand do not have to say \"-D\" to delete.\n\nCurrently \"git branch --merged master\" fails with a message:\n\n\tfatal: A branch named 'master' already exists.\n\nbut --merged and its opposite --no-merged are in spirit very similar to\nanother option --contains in that it is meant to be used as a way to\ninfluence which branches are _reported_, and not about creating a new\nbranch.\n\nPerhaps we should change them to default to HEAD (to retain the current\nbehaviour) but take a commit, and show branches that are merged to the\ncommit or not yet fully merged to the commit, respectively?\n\nIncidentally, \"git branch --with\" fails without the mandatory commit\nargument, and if we are going to do the above, we probably should default\nthe argument to HEAD as well.\n\nHere is an attempt to update --with but I am not happy with it.\n\nThe patch makes\n\n\t$ git branch --contains\n\nwork as expected, but breaks\n\n\t$ git branch --contains master\n\nYou need to spell \"git branch --contains=master\" instead.  Which means it\nis a regression and cannot be applied.  I suspect I am not using\nparse_options() in an optimal way, but I did not find a way for my\ncallback to tell \"I consumed the next parameter and it was mine\" or \"The\nnext one is not my optional parameter\" back to the parse_options(), so\nprobably parse_options() need to be fixed to update this without\nregression, I suspect.\n\n builtin-branch.c |   12 +++++++-----\n 1 files changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex d279702..ee722a2 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -411,7 +411,7 @@ static int opt_parse_with_commit(const struct option *opt, const char *arg, int\n \tstruct commit *commit;\n \n \tif (!arg)\n-\t\treturn -1;\n+\t\targ = \"HEAD\";\n \tif (get_sha1(arg, sha1))\n \t\tdie(\"malformed object name %s\", arg);\n \tcommit = lookup_commit_reference(sha1);\n@@ -438,13 +438,15 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOLEAN( 0 , \"color\",  &branch_use_color, \"use colored output\"),\n \t\tOPT_SET_INT('r', NULL,     &kinds, \"act on remote-tracking branches\",\n \t\t\tREF_REMOTE_BRANCH),\n-\t\tOPT_CALLBACK(0, \"contains\", &with_commit, \"commit\",\n-\t\t\t     \"print only branches that contain the commit\",\n-\t\t\t     opt_parse_with_commit),\n+\t\t{\n+\t\t\tOPTION_CALLBACK, 0, \"contains\", &with_commit, \"commit\",\n+\t\t\t\"print only branches that contain the commit\",\n+\t\t\tPARSE_OPT_OPTARG, opt_parse_with_commit,\n+\t\t},\n \t\t{\n \t\t\tOPTION_CALLBACK, 0, \"with\", &with_commit, \"commit\",\n \t\t\t\"print only branches that contain the commit\",\n-\t\t\tPARSE_OPT_HIDDEN, opt_parse_with_commit,\n+\t\t\tPARSE_OPT_HIDDEN | PARSE_OPT_OPTARG, opt_parse_with_commit,\n \t\t},\n \t\tOPT__ABBREV(&abbrev),\n \n"},{"id":"82579","messageId":"20080708101452.GB19202@artemis.madism.org","threadId":"14343","inReplyTo":"7v8wwcx446.fsf@gitster.siamese.dyndns.org","subject":"Re: What should \"git branch --merged master\" do?","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T10:14:52Z","receivedAt":"2008-07-08T10:14:52Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Tue, Jul 08, 2008 at 06:49:13AM +0000, Junio C Hamano wrote:\n> e8b404c (git-branch: add support for --merged and --no-merged, 2008-04-17)\n> introduced \"git branch --merged\" to show the branches that are contained\n> within the current HEAD.  I.e. the ones that you can say \"git branch -d\"\n> and do not have to say \"-D\" to delete.\n> \n> Currently \"git branch --merged master\" fails with a message:\n> \n> \tfatal: A branch named 'master' already exists.\n> \n> but --merged and its opposite --no-merged are in spirit very similar to\n> another option --contains in that it is meant to be used as a way to\n> influence which branches are _reported_, and not about creating a new\n> branch.\n> \n> Perhaps we should change them to default to HEAD (to retain the current\n> behaviour) but take a commit, and show branches that are merged to the\n> commit or not yet fully merged to the commit, respectively?\n> \n> Incidentally, \"git branch --with\" fails without the mandatory commit\n> argument, and if we are going to do the above, we probably should default\n> the argument to HEAD as well.\n> \n> Here is an attempt to update --with but I am not happy with it.\n> \n> The patch makes\n> \n> \t$ git branch --contains\n> \n> work as expected, but breaks\n> \n> \t$ git branch --contains master\n> \n> You need to spell \"git branch --contains=master\" instead.  Which means it\n> is a regression and cannot be applied.  I suspect I am not using\n> parse_options() in an optimal way, but I did not find a way for my\n> callback to tell \"I consumed the next parameter and it was mine\" or \"The\n> next one is not my optional parameter\" back to the parse_options(), so\n> probably parse_options() need to be fixed to update this without\n> regression, I suspect.\n\n  I actually see three ways.\n\n  (1) quick and dirty: if we see at argv[argc - 1]\n      --contains/--merge/--no-merge/... we do:\n\n      argv[argc] = \"HEAD\" and use parse-options with a mandatory\n      argument for --merge/--contains/... Okay this leaves the issue of\n      the fact that we must NULL-terminate argv, but that's trivial\n      (even if quite despisable).\n\n  (2) big update of parse-options: we refactor callbacks so that they\n      can decide if they take their argument or not. This removes the\n      need for --long-opt=foo for options with optional arguments, but\n      the refactor will be heavy. That's more or less what you suggest\n      in your last § but I don't like it for many reasons so I wont\n      enter the details.\n\n  (3) inspired from (1) and (2), have a flag for options that says \"I do\n      take an argument, but if I'm the last option on the command line,\n      please fake this argument for me.\n\n  I really like (3) more FWIW as it doesn't generate ambiguous parsers\nlike (2) would, and it's not horrible like (1). And cherry on top it's\npretty trivial to implement I think.\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"82580","messageId":"20080708103408.GC19202@artemis.madism.org","threadId":"14343","inReplyTo":"20080708101452.GB19202@artemis.madism.org","subject":"[PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T10:34:08Z","receivedAt":"2008-07-08T10:34:08Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"If you set this for a given flag, and the flag appears without a value on\nthe command line, then the `defval' is used to fake a new argument.\n\nNote that this flag is meaningless in presence of OPTARG or NOARG flags.\n(in the current implementation it will be ignored, but don't rely on it).\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n\n    >   (3) inspired from (1) and (2), have a flag for options that says\n    >       \"I do take an argument, but if I'm the last option on the\n    >       command line, please fake this argument for me.\n    >\n    > I really like (3) more FWIW as it doesn't generate ambiguous\n    > parsers like (2) would, and it's not horrible like (1). And cherry\n    > on top it's pretty trivial to implement I think.\n\n  And here it is, untested though (in the sense that I didn't test the\n  new feature, but git is not broken with the patch).\n\n\n parse-options.c |   55 +++++++++++++++++++++++++++----------------------------\n parse-options.h |    1 +\n 2 files changed, 28 insertions(+), 28 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex a90b336..a63485c 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -5,17 +5,6 @@\n #define OPT_SHORT 1\n #define OPT_UNSET 2\n \n-static inline const char *get_arg(struct parse_opt_ctx_t *p)\n-{\n-\tif (p->opt) {\n-\t\tconst char *res = p->opt;\n-\t\tp->opt = NULL;\n-\t\treturn res;\n-\t}\n-\tp->argc--;\n-\treturn *++p->argv;\n-}\n-\n static int opterror(const struct option *opt, const char *reason, int flags)\n {\n \tif (flags & OPT_SHORT)\n@@ -25,8 +14,24 @@ static int opterror(const struct option *opt, const char *reason, int flags)\n \treturn error(\"option `%s' %s\", opt->long_name, reason);\n }\n \n+static inline int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,\n+\t\t\t  int flags, const char **arg)\n+{\n+\tif (p->opt) {\n+\t\t*arg = p->opt;\n+\t\tp->opt = NULL;\n+\t} else if (p->argc) {\n+\t\tp->argc--;\n+\t\t*arg = *++p->argv;\n+\t} else if (opt->flags & PARSE_OPT_FAKELASTARG) {\n+\t\t*arg = (const char *)opt->defval;\n+\t} else\n+\t\treturn opterror(opt, \"requires a value\", flags);\n+\treturn 0;\n+}\n+\n static int get_value(struct parse_opt_ctx_t *p,\n-                     const struct option *opt, int flags)\n+\t\t     const struct option *opt, int flags)\n {\n \tconst char *s, *arg;\n \tconst int unset = flags & OPT_UNSET;\n@@ -52,7 +57,6 @@ static int get_value(struct parse_opt_ctx_t *p,\n \t\t}\n \t}\n \n-\targ = p->opt ? p->opt : (p->argc > 1 ? p->argv[1] : NULL);\n \tswitch (opt->type) {\n \tcase OPTION_BIT:\n \t\tif (unset)\n@@ -74,17 +78,12 @@ static int get_value(struct parse_opt_ctx_t *p,\n \t\treturn 0;\n \n \tcase OPTION_STRING:\n-\t\tif (unset) {\n+\t\tif (unset)\n \t\t\t*(const char **)opt->value = NULL;\n-\t\t\treturn 0;\n-\t\t}\n-\t\tif (opt->flags & PARSE_OPT_OPTARG && !p->opt) {\n+\t\telse if (opt->flags & PARSE_OPT_OPTARG && !p->opt)\n \t\t\t*(const char **)opt->value = (const char *)opt->defval;\n-\t\t\treturn 0;\n-\t\t}\n-\t\tif (!arg)\n-\t\t\treturn opterror(opt, \"requires a value\", flags);\n-\t\t*(const char **)opt->value = get_arg(p);\n+\t\telse\n+\t\t\treturn get_arg(p, opt, flags, (const char **)opt->value);\n \t\treturn 0;\n \n \tcase OPTION_CALLBACK:\n@@ -94,9 +93,9 @@ static int get_value(struct parse_opt_ctx_t *p,\n \t\t\treturn (*opt->callback)(opt, NULL, 0) ? (-1) : 0;\n \t\tif (opt->flags & PARSE_OPT_OPTARG && !p->opt)\n \t\t\treturn (*opt->callback)(opt, NULL, 0) ? (-1) : 0;\n-\t\tif (!arg)\n-\t\t\treturn opterror(opt, \"requires a value\", flags);\n-\t\treturn (*opt->callback)(opt, get_arg(p), 0) ? (-1) : 0;\n+\t\tif (get_arg(p, opt, flags, &arg))\n+\t\t\treturn -1;\n+\t\treturn (*opt->callback)(opt, arg, 0) ? (-1) : 0;\n \n \tcase OPTION_INTEGER:\n \t\tif (unset) {\n@@ -107,9 +106,9 @@ static int get_value(struct parse_opt_ctx_t *p,\n \t\t\t*(int *)opt->value = opt->defval;\n \t\t\treturn 0;\n \t\t}\n-\t\tif (!arg)\n-\t\t\treturn opterror(opt, \"requires a value\", flags);\n-\t\t*(int *)opt->value = strtol(get_arg(p), (char **)&s, 10);\n+\t\tif (get_arg(p, opt, flags, &arg))\n+\t\t\treturn -1;\n+\t\t*(int *)opt->value = strtol(arg, (char **)&s, 10);\n \t\tif (*s)\n \t\t\treturn opterror(opt, \"expects a numerical value\", flags);\n \t\treturn 0;\ndiff --git a/parse-options.h b/parse-options.h\nindex c5f0b4b..6e9edd1 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -28,6 +28,7 @@ enum parse_opt_option_flags {\n \tPARSE_OPT_NOARG   = 2,\n \tPARSE_OPT_NONEG   = 4,\n \tPARSE_OPT_HIDDEN  = 8,\n+\tPARSE_OPT_FAKELASTARG = 16,\n };\n \n struct option;\n-- \n1.5.6.2.398.g3c3f1.dirty\n"},{"id":"82664","messageId":"7vvdzfoo1s.fsf@gitster.siamese.dyndns.org","threadId":"14343","inReplyTo":"20080708103408.GC19202@artemis.madism.org","subject":"Re: [PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-09T01:15:43Z","receivedAt":"2008-07-09T01:15:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n> If you set this for a given flag, and the flag appears without a value on\n> the command line, then the `defval' is used to fake a new argument.\n>\n> Note that this flag is meaningless in presence of OPTARG or NOARG flags.\n> (in the current implementation it will be ignored, but don't rely on it).\n>\n> Signed-off-by: Pierre Habouzit <madcoder@debian.org>\n> ---\n>\n>     >   (3) inspired from (1) and (2), have a flag for options that says\n>     >       \"I do take an argument, but if I'm the last option on the\n>     >       command line, please fake this argument for me.\n>     >\n>     > I really like (3) more FWIW as it doesn't generate ambiguous\n>     > parsers like (2) would, and it's not horrible like (1). And cherry\n>     > on top it's pretty trivial to implement I think.\n\nYeah, I do not particularly want a major rewrite that only introduces\npossible ambiguity to the option parser (even though arguably it would add\nto the usability very much, just like we accept revs and paths when\nunambiguous), so I think this is a reasonable compromise.\n\nIt feels more like LASTARG_DEFAULT but that is bikeshedding ;-)\n\nBut I see one thinko (fix below) and another issue I am not sure what the\nbest fix would be.\n\n---\ndiff --git a/parse-options.c b/parse-options.c\nindex b6735a5..cba20d7 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -26,11 +26,11 @@ static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,\n \tif (p->opt) {\n \t\t*arg = p->opt;\n \t\tp->opt = NULL;\n+\t} else if (p->argc == 1 && (opt->flags & PARSE_OPT_FAKELASTARG)) {\n+\t\t*arg = (const char *)opt->defval;\n \t} else if (p->argc) {\n \t\tp->argc--;\n \t\t*arg = *++p->argv;\n-\t} else if (opt->flags & PARSE_OPT_FAKELASTARG) {\n-\t\t*arg = (const char *)opt->defval;\n \t} else\n \t\treturn opterror(opt, \"requires a value\", flags);\n \treturn 0;\n"},{"id":"82665","messageId":"7vr6a3onyr.fsf_-_@gitster.siamese.dyndns.org","threadId":"14343","inReplyTo":"7vvdzfoo1s.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 1/2] branch --contains: default to HEAD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-09T01:17:32Z","receivedAt":"2008-07-09T01:17:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We used to require the name of the commit to limit the branches shown to\nthe --contains option, but more recent --merged/--no-meregd defaults to\nHEAD (and they do not allow arbitrary commit, which is a separate issue).\n\nThis teaches --contains to default to HEAD when no parameter is given.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n * This comes on top of the FAKELASTARG patch\n\n builtin-branch.c |   12 ++++++++----\n 1 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex d279702..375e5e0 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -438,13 +438,17 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOLEAN( 0 , \"color\",  &branch_use_color, \"use colored output\"),\n \t\tOPT_SET_INT('r', NULL,     &kinds, \"act on remote-tracking branches\",\n \t\t\tREF_REMOTE_BRANCH),\n-\t\tOPT_CALLBACK(0, \"contains\", &with_commit, \"commit\",\n-\t\t\t     \"print only branches that contain the commit\",\n-\t\t\t     opt_parse_with_commit),\n+\t\t{\n+\t\t\tOPTION_CALLBACK, 0, \"contains\", &with_commit, \"commit\",\n+\t\t\t\"print only branches that contain the commit\",\n+\t\t\tPARSE_OPT_FAKELASTARG,\n+\t\t\topt_parse_with_commit, (intptr_t)\"HEAD\",\n+\t\t},\n \t\t{\n \t\t\tOPTION_CALLBACK, 0, \"with\", &with_commit, \"commit\",\n \t\t\t\"print only branches that contain the commit\",\n-\t\t\tPARSE_OPT_HIDDEN, opt_parse_with_commit,\n+\t\t\tPARSE_OPT_HIDDEN | PARSE_OPT_FAKELASTARG,\n+\t\t\topt_parse_with_commit, (intptr_t) \"HEAD\",\n \t\t},\n \t\tOPT__ABBREV(&abbrev),\n \n-- \n1.5.6.2.255.gbed62\n"},{"id":"82666","messageId":"7vmykronr1.fsf_-_@gitster.siamese.dyndns.org","threadId":"14343","inReplyTo":"7vvdzfoo1s.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 2/2] branch --merged/--not-merged: allow specifying arbitrary commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-09T01:22:10Z","receivedAt":"2008-07-09T01:22:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git-branch --merged\" is a handy way to list all the branches that have\nalready been merged to the current branch, but it did not allow checking\nagainst anything but the current branch.  Having to check out only for\nthat purpose made the command practically useless.\n\nThis updates the option parser so that \"git branch --merged next\" is\naccepted when you are on 'master' branch.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This does have an issue.  --no-<option>=<value> is often nonsense and\n   parse-options does not accept it (and I do not think we would want to\n   change it).  The use of \"--no-merged\" was a mistake, but nobody has\n   perfect foresight.\n\n   This adds --not-merged <commit> and allows the <commit> to default to\n   HEAD if not given to work it around.  This and the previous one are not\n   for application but primarily meant for discussion on what further\n   flexibility we may want to have in parse-options.\n\n builtin-branch.c |   50 ++++++++++++++++++++++++++++++++++++++++++--------\n 1 files changed, 42 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 375e5e0..151f519 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -46,7 +46,12 @@ enum color_branch {\n \tCOLOR_BRANCH_CURRENT = 4,\n };\n \n-static int mergefilter = -1;\n+static enum merge_filter {\n+\tNO_FILTER = 0,\n+\tSHOW_NOT_MERGED,\n+\tSHOW_MERGED,\n+} merge_filter;\n+static unsigned char merge_filter_ref[20];\n \n static int parse_branch_color_slot(const char *var, int ofs)\n {\n@@ -234,13 +239,15 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,\n \tif ((kind & ref_list->kinds) == 0)\n \t\treturn 0;\n \n-\tif (mergefilter > -1) {\n+\tif (merge_filter != NO_FILTER) {\n \t\tbranch.item = lookup_commit_reference_gently(sha1, 1);\n \t\tif (!branch.item)\n \t\t\tdie(\"Unable to lookup tip of branch %s\", refname);\n-\t\tif (mergefilter == 0 && has_commit(head_sha1, &branch))\n+\t\tif (merge_filter == SHOW_NOT_MERGED &&\n+\t\t    has_commit(merge_filter_ref, &branch))\n \t\t\treturn 0;\n-\t\tif (mergefilter == 1 && !has_commit(head_sha1, &branch))\n+\t\tif (merge_filter == SHOW_MERGED &&\n+\t\t    !has_commit(merge_filter_ref, &branch))\n \t\t\treturn 0;\n \t}\n \n@@ -421,6 +428,20 @@ static int opt_parse_with_commit(const struct option *opt, const char *arg, int\n \treturn 0;\n }\n \n+static int opt_parse_merge_filter(const struct option *opt, const char *arg, int unset)\n+{\n+\tmerge_filter = ((opt->long_name[0] == 'n')\n+\t\t\t? SHOW_NOT_MERGED\n+\t\t\t: SHOW_MERGED);\n+\tif (unset)\n+\t\tmerge_filter = SHOW_NOT_MERGED; /* b/c for --no-merged */\n+\tif (!arg)\n+\t\targ = \"HEAD\";\n+\tif (get_sha1(arg, merge_filter_ref))\n+\t\tdie(\"malformed object name %s\", arg);\n+\treturn 0;\n+}\n+\n int cmd_branch(int argc, const char **argv, const char *prefix)\n {\n \tint delete = 0, rename = 0, force_create = 0;\n@@ -461,7 +482,18 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('M', NULL, &rename, \"move/rename a branch, even if target exists\", 2),\n \t\tOPT_BOOLEAN('l', NULL, &reflog, \"create the branch's reflog\"),\n \t\tOPT_BOOLEAN('f', NULL, &force_create, \"force creation (when already exists)\"),\n-\t\tOPT_SET_INT(0, \"merged\", &mergefilter, \"list only merged branches\", 1),\n+\t\t{\n+\t\t\tOPTION_CALLBACK, 0, \"merged\", &merge_filter_ref,\n+\t\t\t\"commit\", \"print only merged branches\",\n+\t\t\tPARSE_OPT_FAKELASTARG,\n+\t\t\topt_parse_merge_filter, (intptr_t) \"HEAD\",\n+\t\t},\n+\t\t{\n+\t\t\tOPTION_CALLBACK, 0, \"not-merged\", &merge_filter_ref,\n+\t\t\t\"commit\", \"print only not merged branches\",\n+\t\t\tPARSE_OPT_FAKELASTARG,\n+\t\t\topt_parse_merge_filter, (intptr_t) \"HEAD\",\n+\t\t},\n \t\tOPT_END(),\n \t};\n \n@@ -471,9 +503,6 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tbranch_use_color = git_use_color_default;\n \n \ttrack = git_branch_track;\n-\targc = parse_options(argc, argv, options, builtin_branch_usage, 0);\n-\tif (!!delete + !!rename + !!force_create > 1)\n-\t\tusage_with_options(builtin_branch_usage, options);\n \n \thead = resolve_ref(\"HEAD\", head_sha1, 0, NULL);\n \tif (!head)\n@@ -486,6 +515,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"HEAD not found below refs/heads!\");\n \t\thead += 11;\n \t}\n+\thashcpy(merge_filter_ref, head_sha1);\n+\n+\targc = parse_options(argc, argv, options, builtin_branch_usage, 0);\n+\tif (!!delete + !!rename + !!force_create > 1)\n+\t\tusage_with_options(builtin_branch_usage, options);\n \n \tif (delete)\n \t\treturn delete_branches(argc, argv, delete > 1, kinds);\n-- \n1.5.6.2.255.gbed62\n"},{"id":"82693","messageId":"20080709074516.GA23482@artemis.madism.org","threadId":"14343","inReplyTo":"7vmykronr1.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] branch --merged/--not-merged: allow specifying arbitrary commit","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-09T07:45:16Z","receivedAt":"2008-07-09T07:45:16Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Jul 09, 2008 at 01:22:10AM +0000, Junio C Hamano wrote:\n> \"git-branch --merged\" is a handy way to list all the branches that have\n> already been merged to the current branch, but it did not allow checking\n> against anything but the current branch.  Having to check out only for\n> that purpose made the command practically useless.\n> \n> This updates the option parser so that \"git branch --merged next\" is\n> accepted when you are on 'master' branch.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> \n>  * This does have an issue.  --no-<option>=<value> is often nonsense and\n>    parse-options does not accept it (and I do not think we would want to\n>    change it).  The use of \"--no-merged\" was a mistake, but nobody has\n>    perfect foresight.\n> \n>    This adds --not-merged <commit> and allows the <commit> to default to\n>    HEAD if not given to work it around.  This and the previous one are not\n>    for application but primarily meant for discussion on what further\n>    flexibility we may want to have in parse-options.\n\n  There's a way: declare --merge as (PARSE_OPT_)NONEG to tell parse-opt\nnot to generate --no-merge by itself, and declare a --no-merge option as\nwell.  I think it works, and if not, we should make parse-opt work with\nthat. This is a quite disgusting hack, and no *new* options should be\nwritten this way, but we have to be flexible enough for backward\ncompatibility's sake.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"82694","messageId":"20080709074724.GB23482@artemis.madism.org","threadId":"14343","inReplyTo":"7vvdzfoo1s.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-09T07:47:24Z","receivedAt":"2008-07-09T07:47:24Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Jul 09, 2008 at 01:15:43AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit <madcoder@debian.org> writes:\n> \n> > If you set this for a given flag, and the flag appears without a value on\n> > the command line, then the `defval' is used to fake a new argument.\n> >\n> > Note that this flag is meaningless in presence of OPTARG or NOARG flags.\n> > (in the current implementation it will be ignored, but don't rely on it).\n> >\n> > Signed-off-by: Pierre Habouzit <madcoder@debian.org>\n> > ---\n> >\n> >     >   (3) inspired from (1) and (2), have a flag for options that says\n> >     >       \"I do take an argument, but if I'm the last option on the\n> >     >       command line, please fake this argument for me.\n> >     >\n> >     > I really like (3) more FWIW as it doesn't generate ambiguous\n> >     > parsers like (2) would, and it's not horrible like (1). And cherry\n> >     > on top it's pretty trivial to implement I think.\n> \n> Yeah, I do not particularly want a major rewrite that only introduces\n> possible ambiguity to the option parser (even though arguably it would add\n> to the usability very much, just like we accept revs and paths when\n> unambiguous), so I think this is a reasonable compromise.\n> \n> It feels more like LASTARG_DEFAULT but that is bikeshedding ;-)\n\n  I absolutely don't like this FAKELASTARG name, so really, use what you\nlike.\n\n> But I see one thinko (fix below) and another issue I am not sure what the\n> best fix would be.\n\n  Like I said it was just a draft, I did not test the new feature, so\nI'm not really surprised it's partly broken ;)\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"82699","messageId":"7v8wwbl8t3.fsf@gitster.siamese.dyndns.org","threadId":"14343","inReplyTo":"20080709074516.GA23482@artemis.madism.org","subject":"Re: [PATCH 2/2] branch --merged/--not-merged: allow specifying arbitrary commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-09T09:13:12Z","receivedAt":"2008-07-09T09:13:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n>   There's a way: declare --merge as (PARSE_OPT_)NONEG to tell parse-opt\n> not to generate --no-merge by itself, and declare a --no-merge option as\n> well.  I think it works, and if not, we should make parse-opt work with\n> that. This is a quite disgusting hack, and no *new* options should be\n> written this way, but we have to be flexible enough for backward\n> compatibility's sake.\n\nNice, thanks.\n\nWith the obvious s/_FAKELASTARG/_LASTARG_DEFAULT/g to the first one, this\nreplaces the second one (with documentation updates for a change ;-), and\nit seems to work Ok.\n\n-- >8 --\nSubject: branch --merged/--no-merged: allow specifying arbitrary commit\n\n\"git-branch --merged\" is a handy way to list all the branches that have\nalready been merged to the current branch, but it did not allow checking\nagainst anything but the current branch.  Having to switch branches only\nto list the branches that are merged with another branch made the feature\npractically useless.\n\nThis updates the option parser so that \"git branch --merged next\" is\naccepted when you are on 'master' branch.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-branch.txt |   27 ++++++++++++----------\n builtin-branch.c             |   50 +++++++++++++++++++++++++++++++++++------\n 2 files changed, 57 insertions(+), 20 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex 0fd5808..450f903 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -8,24 +8,27 @@ git-branch - List, create, or delete branches\n SYNOPSIS\n --------\n [verse]\n-'git-branch' [--color | --no-color] [-r | -a] [--merged | --no-merged]\n-\t   [-v [--abbrev=<length> | --no-abbrev]]\n-\t   [--contains <commit>]\n+'git-branch' [--color | --no-color] [-r | -a]\n+\t[-v [--abbrev=<length> | --no-abbrev]]\n+\t[(--merged | --no-merged | --contains) [<commit>]]\n 'git-branch' [--track | --no-track] [-l] [-f] <branchname> [<start-point>]\n 'git-branch' (-m | -M) [<oldbranch>] <newbranch>\n 'git-branch' (-d | -D) [-r] <branchname>...\n \n DESCRIPTION\n -----------\n-With no arguments given a list of existing branches\n-will be shown, the current branch will be highlighted with an asterisk.\n-Option `-r` causes the remote-tracking branches to be listed,\n-and option `-a` shows both.\n-With `--contains <commit>`, shows only the branches that\n-contains the named commit (in other words, the branches whose\n-tip commits are descendant of the named commit).\n-With `--merged`, only branches merged into HEAD will be listed, and\n-with `--no-merged` only branches not merged into HEAD will be listed.\n+\n+With no arguments, existing branches are listed, the current branch will\n+be highlighted with an asterisk.  Option `-r` causes the remote-tracking\n+branches to be listed, and option `-a` shows both.\n+\n+With `--contains`, shows only the branches that contains the named commit\n+(in other words, the branches whose tip commits are descendant of the\n+named commit).  With `--merged`, only branches merged into the named\n+commit (i.e. the branches whose tip commits are reachable from the named\n+commit) will be listed.  With `--no-merged` only branches not merged into\n+the named commit will be listed.  Missing <commit> argument defaults to\n+'HEAD' (i.e. the tip of the current branch).\n \n In its second form, a new branch named <branchname> will be created.\n It will start out with a head equal to the one given as <start-point>.\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 50cbc9c..1926c47 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -46,7 +46,12 @@ enum color_branch {\n \tCOLOR_BRANCH_CURRENT = 4,\n };\n \n-static int mergefilter = -1;\n+static enum merge_filter {\n+\tNO_FILTER = 0,\n+\tSHOW_NOT_MERGED,\n+\tSHOW_MERGED,\n+} merge_filter;\n+static unsigned char merge_filter_ref[20];\n \n static int parse_branch_color_slot(const char *var, int ofs)\n {\n@@ -234,13 +239,15 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,\n \tif ((kind & ref_list->kinds) == 0)\n \t\treturn 0;\n \n-\tif (mergefilter > -1) {\n+\tif (merge_filter != NO_FILTER) {\n \t\tbranch.item = lookup_commit_reference_gently(sha1, 1);\n \t\tif (!branch.item)\n \t\t\tdie(\"Unable to lookup tip of branch %s\", refname);\n-\t\tif (mergefilter == 0 && has_commit(head_sha1, &branch))\n+\t\tif (merge_filter == SHOW_NOT_MERGED &&\n+\t\t    has_commit(merge_filter_ref, &branch))\n \t\t\treturn 0;\n-\t\tif (mergefilter == 1 && !has_commit(head_sha1, &branch))\n+\t\tif (merge_filter == SHOW_MERGED &&\n+\t\t    !has_commit(merge_filter_ref, &branch))\n \t\t\treturn 0;\n \t}\n \n@@ -421,6 +428,20 @@ static int opt_parse_with_commit(const struct option *opt, const char *arg, int\n \treturn 0;\n }\n \n+static int opt_parse_merge_filter(const struct option *opt, const char *arg, int unset)\n+{\n+\tmerge_filter = ((opt->long_name[0] == 'n')\n+\t\t\t? SHOW_NOT_MERGED\n+\t\t\t: SHOW_MERGED);\n+\tif (unset)\n+\t\tmerge_filter = SHOW_NOT_MERGED; /* b/c for --no-merged */\n+\tif (!arg)\n+\t\targ = \"HEAD\";\n+\tif (get_sha1(arg, merge_filter_ref))\n+\t\tdie(\"malformed object name %s\", arg);\n+\treturn 0;\n+}\n+\n int cmd_branch(int argc, const char **argv, const char *prefix)\n {\n \tint delete = 0, rename = 0, force_create = 0;\n@@ -461,7 +482,18 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('M', NULL, &rename, \"move/rename a branch, even if target exists\", 2),\n \t\tOPT_BOOLEAN('l', NULL, &reflog, \"create the branch's reflog\"),\n \t\tOPT_BOOLEAN('f', NULL, &force_create, \"force creation (when already exists)\"),\n-\t\tOPT_SET_INT(0, \"merged\", &mergefilter, \"list only merged branches\", 1),\n+\t\t{\n+\t\t\tOPTION_CALLBACK, 0, \"no-merged\", &merge_filter_ref,\n+\t\t\t\"commit\", \"print only not merged branches\",\n+\t\t\tPARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG,\n+\t\t\topt_parse_merge_filter, (intptr_t) \"HEAD\",\n+\t\t},\n+\t\t{\n+\t\t\tOPTION_CALLBACK, 0, \"merged\", &merge_filter_ref,\n+\t\t\t\"commit\", \"print only merged branches\",\n+\t\t\tPARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG,\n+\t\t\topt_parse_merge_filter, (intptr_t) \"HEAD\",\n+\t\t},\n \t\tOPT_END(),\n \t};\n \n@@ -471,9 +503,6 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tbranch_use_color = git_use_color_default;\n \n \ttrack = git_branch_track;\n-\targc = parse_options(argc, argv, options, builtin_branch_usage, 0);\n-\tif (!!delete + !!rename + !!force_create > 1)\n-\t\tusage_with_options(builtin_branch_usage, options);\n \n \thead = resolve_ref(\"HEAD\", head_sha1, 0, NULL);\n \tif (!head)\n@@ -486,6 +515,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"HEAD not found below refs/heads!\");\n \t\thead += 11;\n \t}\n+\thashcpy(merge_filter_ref, head_sha1);\n+\n+\targc = parse_options(argc, argv, options, builtin_branch_usage, 0);\n+\tif (!!delete + !!rename + !!force_create > 1)\n+\t\tusage_with_options(builtin_branch_usage, options);\n \n \tif (delete)\n \t\treturn delete_branches(argc, argv, delete > 1, kinds);\n-- \n1.5.6.2.276.gbb293\n"}]}