{"thread":{"id":"11270","subject":"[RFH] convert shortlog to use parse_options","startedAt":"2007-12-13T05:52:26Z","lastAt":"2007-12-17T23:14:59Z","messageCount":51,"participants":["Jeff King","Pierre Habouzit","Wincent Colaiuta","Junio C Hamano","Kristian Høgsberg","Andreas Ericsson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"62982","messageId":"20071213055226.GA3636@coredump.intra.peff.net","threadId":"11270","inReplyTo":null,"subject":"[RFH] convert shortlog to use parse_options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-13T05:52:26Z","receivedAt":"2007-12-13T05:52:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Pierre,\n\nI am converting shortlog to use parse_options, but I have hit a behavior\nI can't seem to represent.\n\nThe \"-w\" option has an optional argument, and we used to accept only\n\"-w\" or \"-wX,Y,Z\". However, parse_options allows \"-w X,Y,Z\".\n\nThis means that \"-w HEAD\" used to mean \"turn on wrapping\nwith default parameters, look at HEAD\" and now translates to\n\"-w with parameter HEAD\".\n\nIs there a way to do this currently, or can we add an option flag?\n\n---\n builtin-shortlog.c | 111 +++++++++++++++++++++++++++++-----------------------\n 1 files changed, 62 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin-shortlog.c b/builtin-shortlog.c\nindex 3d8d709..37fdb45 100644\n--- a/builtin-shortlog.c\n+++ b/builtin-shortlog.c\n@@ -6,9 +6,12 @@\n #include \"revision.h\"\n #include \"utf8.h\"\n #include \"mailmap.h\"\n+#include \"parse-options.h\"\n \n-static const char shortlog_usage[] =\n-\"git-shortlog [-n] [-s] [... ]\";\n+static const char * const shortlog_usage[] = {\n+\t\"git-shortlog [-n] [-s] [-e] [... ]\",\n+\tNULL\n+};\n \n static char *common_repo_prefix;\n static int email;\n@@ -181,34 +184,45 @@ static int parse_uint(char const **arg, int comma)\n }\n \n static const char wrap_arg_usage[] = \"-w[[,[,]]]\";\n+struct wrap_arg {\n+\tint enabled;\n+\tint len;\n+\tint in1;\n+\tint in2;\n+};\n #define DEFAULT_WRAPLEN 76\n #define DEFAULT_INDENT1 6\n #define DEFAULT_INDENT2 9\n \n-static void parse_wrap_args(const char *arg, int *in1, int *in2, int *wrap)\n+static int parse_wrap(const struct option *opt, const char *arg, int unset)\n {\n-\targ += 2; /* skip -w */\n+\tstruct wrap_arg *wrap = opt->value;\n \n-\t*wrap = parse_uint(&arg, ',');\n-\tif (*wrap < 0)\n+\twrap->enabled = 1;\n+\n+\tif (!arg)\n+\t\treturn 0;\n+\twrap->len = parse_uint(&arg, ',');\n+\tif (wrap->len < 0)\n \t\tdie(wrap_arg_usage);\n-\t*in1 = parse_uint(&arg, ',');\n-\tif (*in1 < 0)\n+\twrap->in1 = parse_uint(&arg, ',');\n+\tif (wrap->in1 < 0)\n \t\tdie(wrap_arg_usage);\n-\t*in2 = parse_uint(&arg, '\\0');\n-\tif (*in2 < 0)\n+\twrap->in2 = parse_uint(&arg, '\\0');\n+\tif (wrap->in2 < 0)\n \t\tdie(wrap_arg_usage);\n \n-\tif (!*wrap)\n-\t\t*wrap = DEFAULT_WRAPLEN;\n-\tif (!*in1)\n-\t\t*in1 = DEFAULT_INDENT1;\n-\tif (!*in2)\n-\t\t*in2 = DEFAULT_INDENT2;\n-\tif (*wrap &&\n-\t ((*in1 && *wrap <= *in1) ||\n-\t (*in2 && *wrap <= *in2)))\n+\tif (!wrap->len)\n+\t\twrap->len = DEFAULT_WRAPLEN;\n+\tif (!wrap->in1)\n+\t\twrap->in1 = DEFAULT_INDENT1;\n+\tif (!wrap->in2)\n+\t\twrap->in2 = DEFAULT_INDENT2;\n+\tif (wrap->len &&\n+\t ((wrap->in1 && wrap->len <= wrap->in1) ||\n+\t (wrap->in2 && wrap->len <= wrap->in2)))\n \t\tdie(wrap_arg_usage);\n+\treturn 0;\n }\n \n int cmd_shortlog(int argc, const char **argv, const char *prefix)\n@@ -216,34 +230,31 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \tstruct rev_info rev;\n \tstruct path_list list = { NULL, 0, 0, 1 };\n \tint i, j, sort_by_number = 0, summary = 0;\n-\tint wrap_lines = 0;\n-\tint wrap = DEFAULT_WRAPLEN;\n-\tint in1 = DEFAULT_INDENT1;\n-\tint in2 = DEFAULT_INDENT2;\n-\n-\t/* since -n is a shadowed rev argument, parse our args first */\n-\twhile (argc > 1) {\n-\t\tif (!strcmp(argv[1], \"-n\") || !strcmp(argv[1], \"--numbered\"))\n-\t\t\tsort_by_number = 1;\n-\t\telse if (!strcmp(argv[1], \"-s\") ||\n-\t\t\t\t!strcmp(argv[1], \"--summary\"))\n-\t\t\tsummary = 1;\n-\t\telse if (!strcmp(argv[1], \"-e\") ||\n-\t\t\t !strcmp(argv[1], \"--email\"))\n-\t\t\temail = 1;\n-\t\telse if (!prefixcmp(argv[1], \"-w\")) {\n-\t\t\twrap_lines = 1;\n-\t\t\tparse_wrap_args(argv[1], &in1, &in2, &wrap);\n-\t\t}\n-\t\telse if (!strcmp(argv[1], \"-h\") || !strcmp(argv[1], \"--help\"))\n-\t\t\tusage(shortlog_usage);\n-\t\telse\n-\t\t\tbreak;\n-\t\targv++;\n-\t\targc--;\n-\t}\n+\tstruct wrap_arg wrap = {\n+\t\t0,\n+\t\tDEFAULT_WRAPLEN,\n+\t\tDEFAULT_INDENT1,\n+\t\tDEFAULT_INDENT2\n+\t};\n+\tstruct option options[] = {\n+\t\tOPT_BOOLEAN('n', \"numbered\", &sort_by_number, \"sort by number\"),\n+\t\tOPT_BOOLEAN('s', \"summary\", &summary,\n+\t\t\t\t\"show commit count summary\"),\n+\t\tOPT_BOOLEAN('e', \"email\", &email, \"show email addresses\"),\n+\t\t{\n+\t\t\tOPTION_CALLBACK, 'w', NULL, &wrap, \"wrap\",\n+\t\t\t\"line-wrap commit message\", PARSE_OPT_OPTARG,\n+\t\t\tparse_wrap\n+\t\t},\n+\t};\n+\n+\targc = parse_options(argc, argv, options, shortlog_usage, 0);\n+\n \tinit_revisions(&rev, prefix);\n-\targc = setup_revisions(argc, argv, &rev, NULL);\n+\t/* setup_revisions expects to have the program name in argv[0],\n+\t * but parse_options has removed it. We have to do parse_options\n+\t * before setup_revisions, though, to claim \"-n\". */\n+\targc = setup_revisions(argc+1, argv-1, &rev, NULL);\n \tif (argc > 1)\n \t\tdie (\"unrecognized argument: %s\", argv[1]);\n \n@@ -272,9 +283,11 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \t\t\tfor (j = onelines->nr - 1; j >= 0; j--) {\n \t\t\t\tconst char *msg = onelines->items[j].path;\n \n-\t\t\t\tif (wrap_lines) {\n-\t\t\t\t\tint col = print_wrapped_text(msg, in1, in2, wrap);\n-\t\t\t\t\tif (col != wrap)\n+\t\t\t\tif (wrap.enabled) {\n+\t\t\t\t\tint col = print_wrapped_text(msg,\n+\t\t\t\t\t\t\twrap.in1, wrap.in2,\n+\t\t\t\t\t\t\twrap.len);\n+\t\t\t\t\tif (col != wrap.len)\n \t\t\t\t\t\tputchar('\\n');\n \t\t\t\t}\n \t\t\t\telse\n-- \n1.5.4.rc0.1088.gfa79c-dirty\n"},{"id":"62994","messageId":"20071213090604.GA12398@artemis.madism.org","threadId":"11270","inReplyTo":"20071213055226.GA3636@coredump.intra.peff.net","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-13T09:06:04Z","receivedAt":"2007-12-13T09:06:04Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Thu, Dec 13, 2007 at 05:52:26AM +0000, Jeff King wrote:\n> Pierre,\n> \n> I am converting shortlog to use parse_options, but I have hit a behavior\n> I can't seem to represent.\n> \n> The \"-w\" option has an optional argument, and we used to accept only\n> \"-w\" or \"-wX,Y,Z\". However, parse_options allows \"-w X,Y,Z\".\n> \n> This means that \"-w HEAD\" used to mean \"turn on wrapping\n> with default parameters, look at HEAD\" and now translates to\n> \"-w with parameter HEAD\".\n> \n> Is there a way to do this currently, or can we add an option flag?\n\nNo we can't. And I believe that such a thing is definitely bad practice\n:/ So if you really need to, we will have to add some PARSE_OPT_STICKARG\nor sth alike that would check that the argument was \"sticked\" to the\noption either with `-wA,B,C` or `--long-opt=A,B,C` depending on the fact\nthat an option is short or long.\n\nThough: `git shortlog -w -- HEAD` will work properly because option\narguments don't take the next token as an argument thing if it starts\nwith a dash.\n\nThough note that you can't migrate things that use init_revisions and so\non to parseoptions yet, because revisions also have dashed tokens (--not\ne.g.) and that the first run of parse_options will just hate it and\nfail. Maybe we can do parse_options work in multiple passes though, but\nthat would require a quite extensive rethought of the module, the\nintroduction of a parseopt context to be freed after the last pass\n(because we will have a lot of small allocation stuff going on). I'll\ntry to see what I can do in that direction.\n\nFor that we must migrate diff and revisions option parser as big macros\n(I started[0] it but didn't had the time to complete it yet, and it's\nquite a huge task, because there is no incremental upgrade path here,\nand there are commands using both diff and revisions options so we must\nmigrate both at once) and use aggregated parseoptions specifiers.\n\n [0] http://git.madism.org/?p=git.git;a=commitdiff;h=059cfb6d4cfbdff68d81577d00c9dbce6fed443e\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"62995","messageId":"20071213091055.GA5674@coredump.intra.peff.net","threadId":"11270","inReplyTo":"20071213090604.GA12398@artemis.madism.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-13T09:10:56Z","receivedAt":"2007-12-13T09:10:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 13, 2007 at 10:06:04AM +0100, Pierre Habouzit wrote:\n\n> No we can't. And I believe that such a thing is definitely bad practice\n> :/ So if you really need to, we will have to add some PARSE_OPT_STICKARG\n> or sth alike that would check that the argument was \"sticked\" to the\n> option either with `-wA,B,C` or `--long-opt=A,B,C` depending on the fact\n> that an option is short or long.\n\nYes, I am not sure if the right solution is to just say \"we are changing\nhow -w works\". Because it either must change, or it must be inconsistent\nwith the rest of the option parsing for the rest of eternity.\n\n> Though note that you can't migrate things that use init_revisions and so\n> on to parseoptions yet, because revisions also have dashed tokens (--not\n> e.g.) and that the first run of parse_options will just hate it and\n\nAh, yes. I hadn't really considered that angle yet (my basic testing had\njust been on non-dashed revision parameters).\n\nI will table this patch for now, then, since it obviously is too complex\nfor v1.5.4.\n\nThanks for your input.\n\n-Peff\n"},{"id":"62996","messageId":"20071213092942.GB12398@artemis.madism.org","threadId":"11270","inReplyTo":"20071213090604.GA12398@artemis.madism.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-13T09:29:43Z","receivedAt":"2007-12-13T09:29:43Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On jeu, déc 13, 2007 at 09:06:04 +0000, Pierre Habouzit wrote:\n> fail. Maybe we can do parse_options work in multiple passes though, but\n> that would require a quite extensive rethought of the module, the\n\n In fact it's not doable because of short options. I knew I already had\nthat idea but dropped it, the reason is that if you have the atom:\n\n -ab\n\n That the first pass knows about a -b, and that the second pass knows\nabout -a, then you have a problem. Because at soon you meet a flag that\nyou don't know about, you cannot parse any further. So we're back at\nwhat I said, we must migrate revs and diff options parsing first, and\nthat's a huge task.\n\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"62997","messageId":"20071213093536.GC12398@artemis.madism.org","threadId":"11270","inReplyTo":"20071213091055.GA5674@coredump.intra.peff.net","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-13T09:35:36Z","receivedAt":"2007-12-13T09:35:36Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Thu, Dec 13, 2007 at 09:10:56AM +0000, Jeff King wrote:\n> On Thu, Dec 13, 2007 at 10:06:04AM +0100, Pierre Habouzit wrote:\n> \n> > No we can't. And I believe that such a thing is definitely bad practice\n> > :/ So if you really need to, we will have to add some PARSE_OPT_STICKARG\n> > or sth alike that would check that the argument was \"sticked\" to the\n> > option either with `-wA,B,C` or `--long-opt=A,B,C` depending on the fact\n> > that an option is short or long.\n> \n> Yes, I am not sure if the right solution is to just say \"we are changing\n> how -w works\". Because it either must change, or it must be inconsistent\n> with the rest of the option parsing for the rest of eternity.\n\nIn fact we have kind of the issue for every single optional argument out\nthere:\n\n$ git describe --abbrev HEAD\nerror: option `abbrev' expects a numerical value\n[...]\n\n *ouch*\n\nSo I believe that with optional arguments we must change the way we do\nthings, and that we _must_ enforce the argument to be sticked in that\ncase. Because this kind of backward incompatibility I totally missed in\nthe first place is unacceptable. Patch on its way.\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63000","messageId":"20071213102636.GD12398@artemis.madism.org","threadId":"11270","inReplyTo":"20071213093536.GC12398@artemis.madism.org","subject":"[PATCH 1/2] parseopt: Enforce the use of the sticked form for optional arguments.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-13T10:26:36Z","receivedAt":"2007-12-13T10:26:36Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Signed-off-by: Pierre Habouzit \n---\n parse-options.c | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex e12b428..7a08a0c 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -89,7 +89,7 @@ static int get_value(struct optparse_t *p,\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 && (!arg || *arg == '-')) {\n+\t\tif (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@@ -103,7 +103,7 @@ static int get_value(struct optparse_t *p,\n \t\t\treturn (*opt->callback)(opt, NULL, 1);\n \t\tif (opt->flags & PARSE_OPT_NOARG)\n \t\t\treturn (*opt->callback)(opt, NULL, 0);\n-\t\tif (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-'))\n+\t\tif (opt->flags & PARSE_OPT_OPTARG && !p->opt)\n \t\t\treturn (*opt->callback)(opt, NULL, 0);\n \t\tif (!arg)\n \t\t\treturn opterror(opt, \"requires a value\", flags);\n@@ -114,7 +114,7 @@ static int get_value(struct optparse_t *p,\n \t\t\t*(int *)opt->value = 0;\n \t\t\treturn 0;\n \t\t}\n-\t\tif (opt->flags & PARSE_OPT_OPTARG && (!arg || !isdigit(*arg))) {\n+\t\tif (opt->flags & PARSE_OPT_OPTARG && !p->opt) {\n \t\t\t*(int *)opt->value = opt->defval;\n \t\t\treturn 0;\n \t\t}\n-- \n1.5.3.7.2249.g89c99-dirty\n"},{"id":"63001","messageId":"20071213102724.GE12398@artemis.madism.org","threadId":"11270","inReplyTo":"20071213102636.GD12398@artemis.madism.org","subject":"[PATCH 2/2] parseopt: Add a gitcli(5) man page.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-13T10:27:24Z","receivedAt":"2007-12-13T10:27:24Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"This page should hold every information about the git ways to parse command\nlines, and best practices to be used for scripting.\n\nSigned-off-by: Pierre Habouzit \n---\n Documentation/Makefile | 2 +-\n Documentation/gitcli.txt | 104 ++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 105 insertions(+), 1 deletions(-)\n create mode 100644 Documentation/gitcli.txt\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 37ec355..f58f947 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -2,7 +2,7 @@ MAN1_TXT= \\\n \t$(filter-out $(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \\\n \t\t$(wildcard git-*.txt)) \\\n \tgitk.txt\n-MAN5_TXT=gitattributes.txt gitignore.txt gitmodules.txt\n+MAN5_TXT=gitattributes.txt gitignore.txt gitcli.txt gitmodules.txt\n MAN7_TXT=git.txt\n \n MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)\ndiff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt\nnew file mode 100644\nindex 0000000..3b3f5e4\n--- /dev/null\n+++ b/Documentation/gitcli.txt\n@@ -0,0 +1,104 @@\n+gitcli(5)\n+=========\n+\n+NAME\n+----\n+gitcli - git command line interface and its usual conventions\n+\n+SYNOPSIS\n+--------\n+gitcli\n+\n+\n+DESCRIPTION\n+-----------\n+This manual intends to describe best practice in how to use git CLI. Here are\n+the rules that you should follow when you are scripting git:\n+\n+ * it's preferred to use the non dashed form of git commands, which means that\n+ you should prefer `\"git foo\"` to `\"git-foo\"`.\n+\n+ * splitting short option switches in separate atoms (prefer `\"git foo -a -b\"`\n+ to `\"git foo -ab\"`, the latter may not even work).\n+\n+ * when a command line switch takes an argument, use the 'sticked' form, which\n+ means that you must prefer `\"git foo -oArg\"` to `\"git foo -o Arg\"` for short\n+ option switches, and `\"git foo --long-opt=Arg\"` to `\"git foo --long-opt Arg\"`\n+ for long switches.\n+\n+\n+ENHANCED CLI\n+------------\n+From the git 1.5.4 series and further, git commands (not all of them at the\n+time of the writing though) come with an enhanced option parser with nice\n+facilities. Here is an exhaustive list of them\n+\n+Magic Options\n+~~~~~~~~~~~~~\n+Commands which have the enhanced option parser activated all understand a\n+couple of magic command line switches:\n+\n+-h::\n+\tgives a pretty printed usage of the command.\n++\n+---------------------------------------------\n+$ git describe -h\n+usage: git-describe [options] *\n+\n+ --contains find the tag that comes after the commit\n+ --debug debug search strategy on stderr\n+ --all use any ref in .git/refs\n+ --tags use any tag in .git/refs/tags\n+ --abbrev [] use digits to display SHA-1s\n+ --candidates consider most recent tags (default: 10)\n+---------------------------------------------\n+\n+--help-all::\n+\tSome git commands takes options that are only used for plumbing or that\n+\tare deprecated, and such options are hidden from the default usage. This\n+\tswitch gives the full list of options.\n+\n+\n+Negating options\n+~~~~~~~~~~~~~~~~\n+Another things to keep in mind is that long options can be negated. For\n+example, `\"git branch\"` has the option `\"--track\"` which is 'on' by default. You\n+can use `\"--no-track\"` to override that behaviour. The same goes for `\"--color\"`\n+and `\"--no-color\"`.\n+\n+\n+Aggregating short options\n+~~~~~~~~~~~~~~~~~~~~~~~~~\n+Commands that support the enhanced option parser allow you to aggregate short\n+options. This means that you can for example use `\"git rm -rf\"` or\n+`\"git clean -fdx\"`.\n+\n+\n+Separating argument from the switch\n+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n+Also for option switches that take a mandatory argument, you can separate it\n+from the switch. That means that all the following uses are correct:\n+\n+----------------------------\n+$ git foo --long-opt=Arg\n+$ git foo --long-opt Arg\n+$ git foo -oArg\n+$ git foo -o Arg\n+----------------------------\n+\n+However, this is *NOT* possible for switches with an optionnal value, where the\n+'sticked' form must be used:\n+----------------------------\n+$ git describe --abbrev HEAD # correct\n+$ git describe --abbrev=10 HEAD # correct\n+$ git describe --abbrev 10 HEAD # NOT WHAT YOU MEANT\n+----------------------------\n+\n+\n+Documentation\n+-------------\n+Documentation by Pierre Habouzit.\n+\n+GIT\n+---\n+Part of the gitlink:git[7] suite\n-- \n1.5.3.7.2249.g89c99-dirty\n"},{"id":"63002","messageId":"A78DF383-1BD5-4AED-9DE8-60B8E96E54B5@wincent.com","threadId":"11270","inReplyTo":"20071213102724.GE12398@artemis.madism.org","subject":"Re: [PATCH 2/2] parseopt: Add a gitcli(5) man page.","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-13T11:04:08Z","receivedAt":"2007-12-13T11:04:08Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 13/12/2007, a las 11:27, Pierre Habouzit escribió:\n\n> This page should hold every information about the git ways to parse \n> command\n> lines, and best practices to be used for scripting.\n\nSome feedback from a native English speaker follows...\n\n> @@ -0,0 +1,104 @@\n> +gitcli(5)\n> +=========\n> +\n> +NAME\n> +----\n> +gitcli - git command line interface and its usual conventions\n\n\"git command line interface and conventions\" sounds better; the \n\"usual\" is redundant.\n\nOr did you mean \"*usage* conventions\"? If that is the case, \"git \ncommand line interface and usage\" is better, but just \"git command \nline interface\" is enough.\n\n> +DESCRIPTION\n> +-----------\n> +This manual intends to describe best practice in how to use git \n> CLI. Here are\n> +the rules that you should follow when you are scripting git:\n\nSuggest \"how to use the git CLI\".\n\n> + * it's preferred to use the non dashed form of git commands, which \n> means that\n> + you should prefer `\"git foo\"` to `\"git-foo\"`.\n\n\"non-dashed\", and in any case, this could be more concise. How about:\n\n* the non-dashed form of git commands is preferred; use `\"git foo\"` \nrather than\n `\"git-foo\"`\n\n> + * splitting short option switches in separate atoms (prefer `\"git \n> foo -a -b\"`\n> + to `\"git foo -ab\"`, the latter may not even work).\n\n\"*split* short option switches *into* separate atoms\"\n\nAnd the comma before \"the latter may not even work\" should be a semi- \ncolon.\n\n> + * when a command line switch takes an argument, use the 'sticked' \n> form, which\n> + means that you must prefer `\"git foo -oArg\"` to `\"git foo -o \n> Arg\"` for short\n> + option switches, and `\"git foo --long-opt=Arg\"` to `\"git foo -- \n> long-opt Arg\"`\n> + for long switches.\n\nAgain this could be more concise. Instead of:\n\n\t\"which means that you must prefer .... to ...\"\n\nyou could just say:\n\n\t\"use ... instead of ...\"\n\n> +ENHANCED CLI\n> +------------\n> +From the git 1.5.4 series and further, git commands (not all of \n> them at the\n> +time of the writing though) come with an enhanced option parser \n> with nice\n> +facilities. Here is an exhaustive list of them\n\nHow about:\n\n\"From git 1.5.4 onwards, many git commands come with an enhanced \noption parser...\"\n\n> +Magic Options\n> +~~~~~~~~~~~~~\n> +Commands which have the enhanced option parser activated all \n> understand a\n> +couple of magic command line switches:\n\n\"Commands which use the enhanced option parser all understand...\"\n\n> +\n> +-h::\n> +\tgives a pretty printed usage of the command.\n\n\"pretty-printed\"\n\n> +--help-all::\n> +\tSome git commands takes options that are only used for plumbing or \n> that\n> +\tare deprecated, and such options are hidden from the default \n> usage. This\n> +\tswitch gives the full list of options.\n\n\"Some git commands *take* options that are deprecated or used only \n*by* plumbing\"\n\nAnd:\n\n\"such options are *not included in* the default usage\" (\"hidden from\" \nsounds awkward).\n\n> +Negating options\n> +~~~~~~~~~~~~~~~~\n> +Another things to keep in mind is that long options can be negated.\n\n\"Another *thing*\"\n\nBut you could replace the whole sentence with just:\n\n\"Long options can be negated.\"\n\n> +Separating argument from the switch\n> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n\n\"Separating *the* argument from the switch\"\n\nOr if you prefer\n\n\"Separating *arguments* from the *switches*\"\n\n> +However, this is *NOT* possible for switches with an optionnal \n> value, where the\n> +'sticked' form must be used:\n\nTypo: \"optional\"\n\n>\nCheers,\nWincent\n"},{"id":"63015","messageId":"20071213135658.GA13802@artemis.madism.org","threadId":"11270","inReplyTo":"A78DF383-1BD5-4AED-9DE8-60B8E96E54B5@wincent.com","subject":"Re: [PATCH 2/2] parseopt: Add a gitcli(5) man page.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-13T13:56:58Z","receivedAt":"2007-12-13T13:56:58Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On jeu, déc 13, 2007 at 11:04:08 +0000, Wincent Colaiuta wrote:\n> El 13/12/2007, a las 11:27, Pierre Habouzit escribió:\n> \n> >This page should hold every information about the git ways to parse \n> >command\n> >lines, and best practices to be used for scripting.\n> \n> Some feedback from a native English speaker follows...\n\n FWIW, like always when it comes to documentation, you're welcome to\nsend a patch superseding my 2/2. I'm not a good English writer, I won't\nbe offended at all. I care about the page being here, not about it being\nwritten by me.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63039","messageId":"7vbq8u4ho8.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071213093536.GC12398@artemis.madism.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-13T17:40:23Z","receivedAt":"2007-12-13T17:40:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> In fact we have kind of the issue for every single optional argument out\n> there:\n>\n> $ git describe --abbrev HEAD\n> error: option `abbrev' expects a numerical value\n> [...]\n>\n> *ouch*\n>\n> So I believe that with optional arguments we must change the way we do\n> things, and that we _must_ enforce the argument to be sticked in that\n> case.\n\nI think \"Must\" is a bit too strong an expression.\n\n\tgit describe --abbrev 7 HEAD\n git describe --abbrev HEAD\n git describe --abbrev=HEAD\n\tgit describe --abbrev=7 HEAD\n\tgit describe --abbrev\n\nThe --abbrev parser in this case could be asked with this question: \"You\nare on the command line. There is a token after you. Is it your\nparameter?\".\n\nAmong the above cases, the third one through the last one will get slightly\ndifferent questions. The third and fourth ones get \"You are given this\nparameter and it must be yours\", and the last one gets \"You are on the\ncommand line, and were not given any parameter.\"\n\nThe parser can do one of these things:\n\n * Inspect the token, if exists, and see if it is appropriate for it.\n\n * If not\n\n - if it is optional, then take the default value, and answer \"I\n handled myself Ok, but that HEAD is not mine\";\n\n - if it \"must be yours\" (the third case), barf.\n\n * If so\n\n - Use that given value and answer \"I handled myself Ok, and that\n parameter 7 is mine\"; this includes the fourth case as well.\n\nAnd this does not have to be callback for common types like integers.\n"},{"id":"63040","messageId":"20071213180347.GE1224@artemis.madism.org","threadId":"11270","inReplyTo":"7vbq8u4ho8.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-13T18:03:47Z","receivedAt":"2007-12-13T18:03:47Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Thu, Dec 13, 2007 at 05:40:23PM +0000, Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n> > In fact we have kind of the issue for every single optional argument out\n> > there:\n> >\n> > $ git describe --abbrev HEAD\n> > error: option `abbrev' expects a numerical value\n> > [...]\n> >\n> > *ouch*\n> >\n> > So I believe that with optional arguments we must change the way we do\n> > things, and that we _must_ enforce the argument to be sticked in that\n> > case.\n> \n> I think \"Must\" is a bit too strong an expression.\n> \n> \tgit describe --abbrev 7 HEAD\n> git describe --abbrev HEAD\n> git describe --abbrev=HEAD\n> \tgit describe --abbrev=7 HEAD\n> \tgit describe --abbrev\n> \n> The --abbrev parser in this case could be asked with this question: \"You\n> are on the command line. There is a token after you. Is it your\n> parameter?\".\n\n I thought of that, but it's really convoluted and can definitely lead\nto very subtle issues. The number of git commands with optional\narguments is quite low, mostly due to legacy, I don't expect _new_\ncommands to take optional arguments. I don't really like the ambiguity\nit creates, and in some cases you just won't be able to disambiguate at\nall. Here it looks nice because --abbrev takes an integer argument, and\nit's likely that no branch nor reference names will be only made of\ndigits. Though for commands taking an optional string[0] argument this is\nway more fishy.\n\n *I* (and it's my opinion, maybe other don't see it that way) see the\nparse-option module as a convenience given to people using the CLI UI in\nan interactive shell. So it tries to achieve a good balance between\nbrevity and error detection. Here I think it's quite error prone and\ngives almost no help to the user: if there is a gain to type git repack\n-afd vs. git repack -a -f -d, I see no real gain in --abbrev 10 vs.\n--abbrev=10.\n\n\n [0] OTOH I'm not sure there will ever be optional arguments that\n aren't integers in git, but I may be wrong.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63042","messageId":"20071213180701.GF1224@artemis.madism.org","threadId":"11270","inReplyTo":"20071213180347.GE1224@artemis.madism.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-13T18:07:01Z","receivedAt":"2007-12-13T18:07:01Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Thu, Dec 13, 2007 at 06:03:47PM +0000, Pierre Habouzit wrote:\n> On Thu, Dec 13, 2007 at 05:40:23PM +0000, Junio C Hamano wrote:\n> > Pierre Habouzit writes:\n> > \n> > > In fact we have kind of the issue for every single optional argument out\n> > > there:\n> > >\n> > > $ git describe --abbrev HEAD\n> > > error: option `abbrev' expects a numerical value\n> > > [...]\n> > >\n> > > *ouch*\n> > >\n> > > So I believe that with optional arguments we must change the way we do\n> > > things, and that we _must_ enforce the argument to be sticked in that\n> > > case.\n> > \n> > I think \"Must\" is a bit too strong an expression.\n> > \n> > \tgit describe --abbrev 7 HEAD\n> > git describe --abbrev HEAD\n> > git describe --abbrev=HEAD\n> > \tgit describe --abbrev=7 HEAD\n> > \tgit describe --abbrev\n> > \n> > The --abbrev parser in this case could be asked with this question: \"You\n> > are on the command line. There is a token after you. Is it your\n> > parameter?\".\n\nThe other issue is that when you had --abbrev=foo or --abbrev foo, the\nfirst one has to generate an error, whereas the second one should just\nsay \"foo\" is not for me. The point being that the callback is not really\naware of how the argument got assigned to it.\n\nThat means that we will have to pass more flags to callabacks, making it\nless easy to write them. Again, I'm not sure that the coding hassle is\nworth of the small gain.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63044","messageId":"1197570521.28742.0.camel@hinata.boston.redhat.com","threadId":"11270","inReplyTo":"20071213180347.GE1224@artemis.madism.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-12-13T18:28:41Z","receivedAt":"2007-12-13T18:28:41Z","isPatch":false,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Thu, 2007-12-13 at 19:03 +0100, Pierre Habouzit wrote:\n> On Thu, Dec 13, 2007 at 05:40:23PM +0000, Junio C Hamano wrote:\n> > Pierre Habouzit writes:\n> > \n> > > In fact we have kind of the issue for every single optional argument out\n> > > there:\n> > >\n> > > $ git describe --abbrev HEAD\n> > > error: option `abbrev' expects a numerical value\n> > > [...]\n> > >\n> > > *ouch*\n> > >\n> > > So I believe that with optional arguments we must change the way we do\n> > > things, and that we _must_ enforce the argument to be sticked in that\n> > > case.\n> > \n> > I think \"Must\" is a bit too strong an expression.\n> > \n> > \tgit describe --abbrev 7 HEAD\n> > git describe --abbrev HEAD\n> > git describe --abbrev=HEAD\n> > \tgit describe --abbrev=7 HEAD\n> > \tgit describe --abbrev\n> > \n> > The --abbrev parser in this case could be asked with this question: \"You\n> > are on the command line. There is a token after you. Is it your\n> > parameter?\".\n> \n> I thought of that, but it's really convoluted and can definitely lead\n> to very subtle issues. The number of git commands with optional\n> arguments is quite low, mostly due to legacy, I don't expect _new_\n> commands to take optional arguments. I don't really like the ambiguity\n> it creates, and in some cases you just won't be able to disambiguate at\n> all. Here it looks nice because --abbrev takes an integer argument, and\n> it's likely that no branch nor reference names will be only made of\n> digits. Though for commands taking an optional string[0] argument this is\n> way more fishy.\n\nMy \n"},{"id":"63045","messageId":"1197571656.28742.13.camel@hinata.boston.redhat.com","threadId":"11270","inReplyTo":"1197570521.28742.0.camel@hinata.boston.redhat.com","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-12-13T18:47:36Z","receivedAt":"2007-12-13T18:47:36Z","isPatch":false,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Thu, 2007-12-13 at 13:28 -0500, Kristian Høgsberg wrote:\n> On Thu, 2007-12-13 at 19:03 +0100, Pierre Habouzit wrote:\n> > On Thu, Dec 13, 2007 at 05:40:23PM +0000, Junio C Hamano wrote:\n> > > Pierre Habouzit writes:\n> > > \n> > > > In fact we have kind of the issue for every single optional argument out\n> > > > there:\n> > > >\n> > > > $ git describe --abbrev HEAD\n> > > > error: option `abbrev' expects a numerical value\n> > > > [...]\n> > > >\n> > > > *ouch*\n> > > >\n> > > > So I believe that with optional arguments we must change the way we do\n> > > > things, and that we _must_ enforce the argument to be sticked in that\n> > > > case.\n> > > \n> > > I think \"Must\" is a bit too strong an expression.\n> > > \n> > > \tgit describe --abbrev 7 HEAD\n> > > git describe --abbrev HEAD\n> > > git describe --abbrev=HEAD\n> > > \tgit describe --abbrev=7 HEAD\n> > > \tgit describe --abbrev\n> > > \n> > > The --abbrev parser in this case could be asked with this question: \"You\n> > > are on the command line. There is a token after you. Is it your\n> > > parameter?\".\n> > \n> > I thought of that, but it's really convoluted and can definitely lead\n> > to very subtle issues. The number of git commands with optional\n> > arguments is quite low, mostly due to legacy, I don't expect _new_\n> > commands to take optional arguments. I don't really like the ambiguity\n> > it creates, and in some cases you just won't be able to disambiguate at\n> > all. Here it looks nice because --abbrev takes an integer argument, and\n> > it's likely that no branch nor reference names will be only made of\n> > digits. Though for commands taking an optional string[0] argument this is\n> > way more fishy.\n> \n> My \n\nOops, sorry about that. I just wanted to say we shouldn't jump through\nall these hoops to make the option parser support every type of option\nthere ever was in the git command line ui. A lot of these were probably\ndecided somewhat arbitrarily by whoever implemented the command.\nInstead it's an opportunity to retroactively enforce some consistency\nand predictability to the various option-styles that have been\nhand-rolled over time in different git commands.\n\nKristian\n"},{"id":"63046","messageId":"7vzlwe1jeb.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071213180347.GE1224@artemis.madism.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-13T19:31:24Z","receivedAt":"2007-12-13T19:31:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> I thought of that, but it's really convoluted and can definitely lead\n> to very subtle issues. The number of git commands with optional\n> arguments is quite low, mostly due to legacy, I don't expect _new_\n> commands to take optional arguments.\n\nI do not think we want to assume anything like that for a generic API.\n\n> I don't really like the ambiguity\n> it creates, and in some cases you just won't be able to disambiguate at\n> all. Here it looks nice because --abbrev takes an integer argument, and\n> it's likely that no branch nor reference names will be only made of\n> digits. Though for commands taking an optional string[0] argument this is\n> way more fishy.\n\nI thought about ambiguity issues and I was only 70% sure about my\nsuggestion. If you have an object, the initial unique part of whose\nname consists only of decimal digits, you could get:\n\n\tgit describe --abbrev 7 538538538\n\tgit describe --abbrev 538538538 HEAD\n\tgit describe --abbrev 538538538\n\nand the last case needs to be disambiguated (either show HEAD with\nabbreviation of 540 million digits, or show that object abbreviated to\nthe default length). Because we are discussing a generic \"optional\ninteger with default value\" parser, let's not argue that only a small\ninteger between 0..40 makes sense to --abbrev flag in this context and\nwe should use that knowledge to further disambiguate [*1*].\n\nYou could say \"If a flag takes an optional parameter and has a default,\nan empty string means use the default\", and disambiguate the lat example\nin the above this way:\n\n\tgit describe --abbrev 538538538\n\tgit describe --abbrev '' 538538538\n\nBut it is not prettier than always requiring the optional parameter to\nbe sticked with the flag, as you suggest, like this;\n\n\tgit describe --abbrev 538538538\n\tgit describe --abbrev=538538538\n\nSo I am not entirely opposed to your version, nor I am claiming my\nsuggestion is better. However, I just thought that \"some parameters you\nMUST stick to the flag\" might create confusion to the end users, and I\nwanted to see if others can come up with a less confusing alternative.\nAnd the way I did so was to keep the discussion going by stirring the\npot a bit.\n\n> [0] OTOH I'm not sure there will ever be optional arguments that\n> aren't integers in git, but I may be wrong.\n\nWe could make HEAD as the default for \"git branch --contains\", if you\nwant an example.\n\n[Footnote]\n\n*1* We could introduce \"optional integer with valid range with default\",\nand that would fit naturally into the scheme I outlined. Even if the\nnext token parses as an integer, if it is out of range, you can say it\nis not yours, and if it has to be yours, you can barf, saying \"that's\nout of range\".\n\n\"The valid range\" would be useful regardless of disambiguation. I wished\nfor \"only valid is 1 or more\" when I adjusted one of the commands to\nparse_options().\n"},{"id":"63048","messageId":"7vve721ikb.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071213180701.GF1224@artemis.madism.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-13T19:49:24Z","receivedAt":"2007-12-13T19:49:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> The other issue is that when you had --abbrev=foo or --abbrev foo, the\n> first one has to generate an error, whereas the second one should just\n> say \"foo\" is not for me. The point being that the callback is not really\n> aware of how the argument got assigned to it.\n\nThat obviously needs to be fixed (\"this is the next token that could be\nyours\" vs \"this must be yours\"), if we were to go the route I suggested.\n"},{"id":"63050","messageId":"7vk5ni1fwh.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"1197571656.28742.13.camel@hinata.boston.redhat.com","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-13T20:46:54Z","receivedAt":"2007-12-13T20:46:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristian Høgsberg writes:\n\n> Oops, sorry about that. I just wanted to say we shouldn't jump through\n> all these hoops to make the option parser support every type of option\n> there ever was in the git command line ui. A lot of these were probably\n> decided somewhat arbitrarily by whoever implemented the command.\n> Instead it's an opportunity to retroactively enforce some consistency\n> and predictability to the various option-styles that have been\n> hand-rolled over time in different git commands.\n\nThat principle is fine, but I do not think it is relevant to what is\nbeing discussed. The issue is what to do with a flag that can\noptionally take a parameter but works fine without because it has a\ndefault.\n\n * You can obviously disallow such a flag, and call the result\n \"consistent\". I do not think we want to go that route. --abbrev, -M\n (to diff) and -w (to shortlog) are good examples why this is a good\n thing. You want to use the default most of the time, but want to be\n able to tweak the value sometimes.\n\n * You can alternatively require parameters to such a flag always stuck\n with the flag, which is what Pierre did. I suspected that is\n introducing an inconsistency and may be confusing to the users (\"I\n can write -n=1 or -n 2, but why not --abbrev 7???\") and wanted to see\n if somebody can come up with a better alternative.\n\n * You can try to make the parser a bit more context sensitive by\n looking at the possible parameter and see if it is plausible, which\n hopefully would work for most of the real life cases (e.g. \"--abbrev\n 7 HEAD\" vs \"--abbrev HEAD\"). I however agree with Pierre that \"DWIM\n works most of the time\" is not good enough if there is no way to\n disambiguate in cases that fall outside.\n"},{"id":"63051","messageId":"20071213205305.GA5423@artemis.madism.org","threadId":"11270","inReplyTo":"7vzlwe1jeb.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-13T20:53:05Z","receivedAt":"2007-12-13T20:53:05Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Thu, Dec 13, 2007 at 07:31:24PM +0000, Junio C Hamano wrote:\n> So I am not entirely opposed to your version, nor I am claiming my\n> suggestion is better. However, I just thought that \"some parameters you\n> MUST stick to the flag\" might create confusion to the end users, and I\n> wanted to see if others can come up with a less confusing alternative.\n> And the way I did so was to keep the discussion going by stirring the\n> pot a bit.\n\nI understand that, and I hope I wasn't sounding harsh at all, I was just\ndebating. Note that I don't like the asymmetry of some options needing a\nspecific syntax whereas all the rest is lax. It's cumbersome at the very\nleast. Though, to me there is one gain: when the user uses the\n--long-opt=foo version you are _sure_ about what he meant. When you have\n--long-opt foo you're not.\n\nYour proposal tries hard to do what the user meant, with a reasonable\nchance of being wrong. My proposal is on the conservative side, but\ngenerate totally incomprehensible errors: if you do\n $ git describe --abbrev 10 HEAD\nwith my patch you get a not very nice\n fatal: Not a valid object name 10\nas an answer. which is kind of going back to the kind of situations\nparse-options is trying to avoid in the first place. I wouldn't be\nreally annoyed by my solution if we were able to generate an\nintelligible error message instead. But guess what, if we were able to\ndo so, we would be able to fix the ambiguity in the first place *sigh*.\n\n\nSo maybe the thing would be to sleep on a it a bit and not taking any of\nmy patches yet (not even in pu) and let people think. It's not a major\nproblem, though it's one that must be fixed before 1.5.4 because we\ndon't want a broken option parser ever be released.\n\n\nNote that there is another path, that would be to disallow mixing of\noptions with real arguments, and have an option separator (though with\n`--` having a distinct meaning in git, that wouldn't be that easy). But\nsadly some git commands already allowed such a thing, so imposing it in\nparse-options would be backward incompatible for those. And I believe\nmany people are attached to this feature anyways.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63078","messageId":"20071214040803.GA10169@sigill.intra.peff.net","threadId":"11270","inReplyTo":"1197571656.28742.13.camel@hinata.boston.redhat.com","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-14T04:08:03Z","receivedAt":"2007-12-14T04:08:03Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 13, 2007 at 01:47:36PM -0500, Kristian Høgsberg wrote:\n\n> Oops, sorry about that. I just wanted to say we shouldn't jump through\n> all these hoops to make the option parser support every type of option\n> there ever was in the git command line ui. A lot of these were probably\n> decided somewhat arbitrarily by whoever implemented the command.\n> Instead it's an opportunity to retroactively enforce some consistency\n> and predictability to the various option-styles that have been\n> hand-rolled over time in different git commands.\n\nI agree. I am already a little bit uncomfortable with the \"--abbrev 10\"\nwon't work but \"--foo 10\" will, because it requires that the user\nremember which arguments are optional and which are required. But\nswitching it to \"--abbrev 10 works, but --abbrev $foo does not, unless\nof course $foo is an integer, in which case you must use --abbrev=$foo\"\nis just a little bit too DWIM. E.g., if you are scripting, it's just one\nmore source of error (if I have $foo, how must I write --abbrev $foo for\nit to ensure that I _don't_ trigger the optional argument?).\n\n-Peff\n"},{"id":"63090","messageId":"7vir31vmsn.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071214040803.GA10169@sigill.intra.peff.net","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-14T05:59:52Z","receivedAt":"2007-12-14T05:59:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I agree. I am already a little bit uncomfortable with the \"--abbrev 10\"\n> won't work but \"--foo 10\" will, because it requires that the user\n> remember which arguments are optional and which are required. But\n> switching it to \"--abbrev 10 works, but --abbrev $foo does not, unless\n> of course $foo is an integer, in which case you must use --abbrev=$foo\"\n> is just a little bit too DWIM. E.g., if you are scripting, it's just one\n> more source of error (if I have $foo, how must I write --abbrev $foo for\n> it to ensure that I _don't_ trigger the optional argument?).\n\nOh, there is no question that scripted callers need a way to safely\ndisambiguate, and be able to call \"git cmd\" with always the default\nabbreviation for whatever $foo it gets from the user unambiguously.\n\nI do not disagree with that.\n\nBut that is different from making it harder for end users (not scripters\nbut command line users).\n\nAgain, I am not saying that my suggested alternative is superiour; I\njust threw it out to as an example of a different approach to achieve\ndisambiguation than the \"some flags must have its parameter stuck with\nit, some don't\" behaviour, which will surely be confusing to the command\nline end users.\n\nTo contrast the two, with Pierre's, if you want $n-digit abbreviations\nand $cnt-lines of output in your script, your script would say:\n\n\tgit cmd --abbrev=$n -n $cnt $foo\n\nbecause you as a script writer are required to know --abbrev is such a\nmagic flag that take optional parameter and cannot be fed as two options\n(you can also write \"-n=$cnt\", but I am talking about --abbrev part).\n\nIf you are accepting the default abbreviation, on the other hand:\n\n\tgit cmd --abbrev -n $cnt $foo\n\nwhich looks nice. The latter needs to be written in a funky way:\n\n\tgit cmd --abbrev= $foo\n\nif we take \"empty parameter to a flag that take optional parameter means\nuse the default setting\"; we could introduce a magic 'default' token to\nread it slightly better:\n\n\tgit cmd --abbrev=default $foo\n\nbut that does not change the fact that it makes it harder for\nscripters. I do not disagree with that.\n\nThe command line end users, who want to do the same, but has a bit more\nconcrete than unknown $n, $cnt, or $foo, can do\n\n\tgit cmd --abbrev HEAD~4\n\nin either approach. However, with Pierre's, the command line end users\ncan say either:\n\n\tgit cmd --abbrev=10 -n=4\n\tgit cmd --abbrev=10 -n 4\n\tgit cmd --abbrev=10 -n4\n\nbut they cannot say:\n\n\tgit cmd --abbrev 10 -n 4\n\nThey need to learn the difference between --abbrev and -n, because you\navoid DWIMmery for the sake of script writers. I have a slight\nsuspicion that it is backwards.\n\nIf there is _no_ existing users and previous versions of git, one\nplausible alternative that would be much cleaner than anything we\ndiscussed so far would be to always require '-n=4' (or \"-n4\") form and\nnever accept \"-n 4\", even for a flag that takes mandatory parameter.\nThen there is no room for confusion. Users (both command line end users\nand script writers) need to learn only one rule: flag parameters are\nalways with the flag, not given as two words.\n\nI really wish the world were that simple, but I do no think that would\nfly well with the existing users' fingers.\n"},{"id":"63125","messageId":"47623FF3.1050601@op5.se","threadId":"11270","inReplyTo":"7vir31vmsn.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-12-14T08:33:55Z","receivedAt":"2007-12-14T08:33:55Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Jeff King writes:\n> \n>> I agree. I am already a little bit uncomfortable with the \"--abbrev 10\"\n>> won't work but \"--foo 10\" will, because it requires that the user\n>> remember which arguments are optional and which are required. But\n>> switching it to \"--abbrev 10 works, but --abbrev $foo does not, unless\n>> of course $foo is an integer, in which case you must use --abbrev=$foo\"\n>> is just a little bit too DWIM. E.g., if you are scripting, it's just one\n>> more source of error (if I have $foo, how must I write --abbrev $foo for\n>> it to ensure that I _don't_ trigger the optional argument?).\n> \n> Oh, there is no question that scripted callers need a way to safely\n> disambiguate, and be able to call \"git cmd\" with always the default\n> abbreviation for whatever $foo it gets from the user unambiguously.\n> \n> I do not disagree with that.\n> \n> But that is different from making it harder for end users (not scripters\n> but command line users).\n> \n> Again, I am not saying that my suggested alternative is superiour; I\n> just threw it out to as an example of a different approach to achieve\n> disambiguation than the \"some flags must have its parameter stuck with\n> it, some don't\" behaviour, which will surely be confusing to the command\n> line end users.\n> \n> To contrast the two, with Pierre's, if you want $n-digit abbreviations\n> and $cnt-lines of output in your script, your script would say:\n> \n> \tgit cmd --abbrev=$n -n $cnt $foo\n> \n> because you as a script writer are required to know --abbrev is such a\n> magic flag that take optional parameter and cannot be fed as two options\n> (you can also write \"-n=$cnt\", but I am talking about --abbrev part).\n> \n> If you are accepting the default abbreviation, on the other hand:\n> \n> \tgit cmd --abbrev -n $cnt $foo\n> \n> which looks nice. The latter needs to be written in a funky way:\n> \n> \tgit cmd --abbrev= $foo\n> \n> if we take \"empty parameter to a flag that take optional parameter means\n> use the default setting\"; we could introduce a magic 'default' token to\n> read it slightly better:\n> \n> \tgit cmd --abbrev=default $foo\n> \n> but that does not change the fact that it makes it harder for\n> scripters. I do not disagree with that.\n> \n> The command line end users, who want to do the same, but has a bit more\n> concrete than unknown $n, $cnt, or $foo, can do\n> \n> \tgit cmd --abbrev HEAD~4\n> \n> in either approach. However, with Pierre's, the command line end users\n> can say either:\n> \n> \tgit cmd --abbrev=10 -n=4\n> \tgit cmd --abbrev=10 -n 4\n> \tgit cmd --abbrev=10 -n4\n> \n> but they cannot say:\n> \n> \tgit cmd --abbrev 10 -n 4\n> \n> They need to learn the difference between --abbrev and -n, because you\n> avoid DWIMmery for the sake of script writers. I have a slight\n> suspicion that it is backwards.\n> \n> If there is _no_ existing users and previous versions of git, one\n> plausible alternative that would be much cleaner than anything we\n> discussed so far would be to always require '-n=4' (or \"-n4\") form and\n> never accept \"-n 4\", even for a flag that takes mandatory parameter.\n> Then there is no room for confusion. Users (both command line end users\n> and script writers) need to learn only one rule: flag parameters are\n> always with the flag, not given as two words.\n> \n> I really wish the world were that simple, but I do no think that would\n> fly well with the existing users' fingers.\n\nI think you're right in that last sentence. It would also be terribly\ninconsistent with the other 3000 or so command-line programs one has\nin ones path.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"63126","messageId":"20071214083943.GA24475@artemis.madism.org","threadId":"11270","inReplyTo":"7vir31vmsn.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-14T08:39:43Z","receivedAt":"2007-12-14T08:39:43Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Fri, Dec 14, 2007 at 05:59:52AM +0000, Junio C Hamano wrote:\n> \tgit cmd --abbrev=10 -n=4\n\n actually -n=4 isn't understood atm, only -n4 and -n 4 are.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63203","messageId":"7v63z1qakt.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071214083943.GA24475@artemis.madism.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-14T20:34:58Z","receivedAt":"2007-12-14T20:34:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> On Fri, Dec 14, 2007 at 05:59:52AM +0000, Junio C Hamano wrote:\n>> \tgit cmd --abbrev=10 -n=4\n>\n> actually -n=4 isn't understood atm, only -n4 and -n 4 are.\n\nAh, my mistake. And I do not think accepting -n=4 is a good idea (it is\nnot historically done).\n\nAfter thinking about it a bit more, I think I was worried too much about\nburdening the users to remember the differences between options with,\nwithout and optional option-arguments [*1*]. They need to know the\ndifference between options with and without option-arguments already\nbecause single letter options can be combined if they are without\noption-arguments, and they have to write \"shortlog -new72\" but not\n\"shortlog -wen72\". If they want to be extra sure, they can be more\nexplicit and say \"shortlog -n -e -w72\".\n\nSo let's go with the version you outlined --- options that take optional\noption-arguments must get their option-arguments stuck to them, but\notherwise option-arguments can also be given as a separate word that\nfollows the option.\n\n[Footnote]\n\n*1* The fact some of our commands support options with optional\noption-arguments is already against Guideline #7 in \"12.2 Utility Syntax\nGuidelines\", so other POSIX guidelines are not useful for us in deciding\nwhat behaviour to model after.\n"},{"id":"63244","messageId":"20071215110311.GA11321@artemis.madism.org","threadId":"11270","inReplyTo":"7v63z1qakt.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-15T11:03:11Z","receivedAt":"2007-12-15T11:03:11Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On ven, déc 14, 2007 at 08:34:58 +0000, Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n> > On Fri, Dec 14, 2007 at 05:59:52AM +0000, Junio C Hamano wrote:\n> >> \tgit cmd --abbrev=10 -n=4\n> >\n> > actually -n=4 isn't understood atm, only -n4 and -n 4 are.\n> \n> Ah, my mistake. And I do not think accepting -n=4 is a good idea (it is\n> not historically done).\n\n> After thinking about it a bit more, I think I was worried too much about\n> burdening the users to remember the differences between options with,\n> without and optional option-arguments [*1*]. They need to know the\n> difference between options with and without option-arguments already\n> because single letter options can be combined if they are without\n> option-arguments, and they have to write \"shortlog -new72\" but not\n> \"shortlog -wen72\". If they want to be extra sure, they can be more\n> explicit and say \"shortlog -n -e -w72\".\n\n Yep, let's do it then. Note that's the reason why I felt we need a\nmanual page about this, because we should give some guidelines of what\nis safe for scripting.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63368","messageId":"7vir2xeq76.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071215110311.GA11321@artemis.madism.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-17T07:27:25Z","receivedAt":"2007-12-17T07:27:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> Yep, let's do it then. Note that's the reason why I felt we need a\n> manual page about this, because we should give some guidelines of what\n> is safe for scripting.\n\nThere are some fallouts from the series, though. I've fixed up git-tag\nbut I strongly suspect all of parseopt users now need careful auditing.\nIf we cannot be confident with the parseoptified commands within\nreasonably short timeframe by -rc1, we may end up having to revert the\nparseopt changes from them, which I'd rather avoid, but if you look at\nthe git-tag change (especially for -l) you would understand it. The\n\"must stick\" restriction feels Ok on paper but in practice it looks\nrather draconian and very user unfriendly.\n\nI'll be sending out two patches shortly. The result will sit in 'pu'\nwaiting for an Ack and/or improvement, but that will happen only when I\ngot around pushing things out, which may come later than you see the\npatches.\n"},{"id":"63369","messageId":"7vd4t5eq52.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071213102636.GD12398@artemis.madism.org","subject":"[PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-17T07:28:41Z","receivedAt":"2007-12-17T07:28:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The command freely used optional option-argments for its -l and -n options.\nI think allowing \"git tag -n xxx\" without barfing was an error to begin with,\nbut not supporting \"git tag -l pattern\" form is a serious regression.\n\nSo this fixes the handling of -l to reinstate the original behaviour while\ndetecting a user error \"git tag -l pattern garbage\", and adjusts tests that\nuse \"-n param\" form to use \"-nparam\".\n\nSigned-off-by: Junio C Hamano \n---\n builtin-tag.c | 15 +++++++-\n t/t7004-tag.sh | 110 +++++++++++++++++++++++++------------------------------\n 2 files changed, 63 insertions(+), 62 deletions(-)\n\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 274901a..5213c62 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -371,7 +371,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \n \tint annotate = 0, sign = 0, force = 0, lines = 0,\n \t\t\t\t\tdelete = 0, verify = 0;\n-\tchar *list = NULL, *msgfile = NULL, *keyid = NULL;\n+\tconst char *list = NULL, *msgfile = NULL, *keyid = NULL;\n \tconst char *no_pattern = \"NO_PATTERN\";\n \tstruct msg_arg msg = { 0, STRBUF_INIT };\n \tstruct option options[] = {\n@@ -407,8 +407,19 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (sign)\n \t\tannotate = 1;\n \n-\tif (list)\n+\tif (list) {\n+\t\t/*\n+\t\t * This is unfortunate but requiring \"git tag -lpattern\" and not\n+\t\t * allowing \"git tag -l pattern\" is a serious regression.\n+\t\t */\n+\t\tif (argc && list == no_pattern) {\n+\t\t\tlist = argv[0];\n+\t\t\targc--;\n+\t\t}\n+\t\tif (argc)\n+\t\t\tdie(\"extra argument after -l[pattern]: %s\", argv[0]);\n \t\treturn list_tags(list == no_pattern ? NULL : list, lines);\n+\t}\n \tif (delete)\n \t\treturn for_each_tag_name(argv, delete_tag);\n \tif (verify)\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 09d56e0..7f58038 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -491,25 +491,19 @@ test_expect_success \\\n \techo \"tag-one-line\" >expect &&\n \tgit-tag -l | grep \"^tag-one-line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 0 -l | grep \"^tag-one-line\" >actual &&\n+\tgit-tag -n0 -l | grep \"^tag-one-line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 0 -l tag-one-line >actual &&\n+\tgit-tag -n0 -l tag-one-line >actual &&\n \tgit diff expect actual &&\n \n \techo \"tag-one-line A msg\" >expect &&\n-\tgit-tag -n xxx -l | grep \"^tag-one-line\" >actual &&\n-\tgit diff expect actual &&\n-\tgit-tag -n \"\" -l | grep \"^tag-one-line\" >actual &&\n-\tgit diff expect actual &&\n-\tgit-tag -n 1 -l | grep \"^tag-one-line\" >actual &&\n-\tgit diff expect actual &&\n \tgit-tag -n -l | grep \"^tag-one-line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 1 -l tag-one-line >actual &&\n+\tgit-tag -n1 -l | grep \"^tag-one-line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 2 -l tag-one-line >actual &&\n+\tgit-tag -n2 -l tag-one-line >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 999 -l tag-one-line >actual &&\n+\tgit-tag -n999 -l tag-one-line >actual &&\n \tgit diff expect actual\n '\n \n@@ -520,21 +514,21 @@ test_expect_success \\\n \techo \"tag-zero-lines\" >expect &&\n \tgit-tag -l | grep \"^tag-zero-lines\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 0 -l | grep \"^tag-zero-lines\" >actual &&\n+\tgit-tag -n0 -l | grep \"^tag-zero-lines\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 0 -l tag-zero-lines >actual &&\n+\tgit-tag -n0 -l tag-zero-lines >actual &&\n \tgit diff expect actual &&\n \n \techo \"tag-zero-lines \" >expect &&\n-\tgit-tag -n 1 -l | grep \"^tag-zero-lines\" >actual &&\n+\tgit-tag -n1 -l | grep \"^tag-zero-lines\" >actual &&\n \tgit diff expect actual &&\n \tgit-tag -n -l | grep \"^tag-zero-lines\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 1 -l tag-zero-lines >actual &&\n+\tgit-tag -n1 -l tag-zero-lines >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 2 -l tag-zero-lines >actual &&\n+\tgit-tag -n2 -l tag-zero-lines >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 999 -l tag-zero-lines >actual &&\n+\tgit-tag -n999 -l tag-zero-lines >actual &&\n \tgit diff expect actual\n '\n \n@@ -548,37 +542,37 @@ test_expect_success \\\n \techo \"tag-lines\" >expect &&\n \tgit-tag -l | grep \"^tag-lines\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 0 -l | grep \"^tag-lines\" >actual &&\n+\tgit-tag -n0 -l | grep \"^tag-lines\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 0 -l tag-lines >actual &&\n+\tgit-tag -n0 -l tag-lines >actual &&\n \tgit diff expect actual &&\n \n \techo \"tag-lines tag line one\" >expect &&\n-\tgit-tag -n 1 -l | grep \"^tag-lines\" >actual &&\n+\tgit-tag -n1 -l | grep \"^tag-lines\" >actual &&\n \tgit diff expect actual &&\n \tgit-tag -n -l | grep \"^tag-lines\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 1 -l tag-lines >actual &&\n+\tgit-tag -n1 -l tag-lines >actual &&\n \tgit diff expect actual &&\n \n \techo \" tag line two\" >>expect &&\n-\tgit-tag -n 2 -l | grep \"^ *tag.line\" >actual &&\n+\tgit-tag -n2 -l | grep \"^ *tag.line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 2 -l tag-lines >actual &&\n+\tgit-tag -n2 -l tag-lines >actual &&\n \tgit diff expect actual &&\n \n \techo \" tag line three\" >>expect &&\n-\tgit-tag -n 3 -l | grep \"^ *tag.line\" >actual &&\n+\tgit-tag -n3 -l | grep \"^ *tag.line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 3 -l tag-lines >actual &&\n+\tgit-tag -n3 -l tag-lines >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 4 -l | grep \"^ *tag.line\" >actual &&\n+\tgit-tag -n4 -l | grep \"^ *tag.line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 4 -l tag-lines >actual &&\n+\tgit-tag -n4 -l tag-lines >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 99 -l | grep \"^ *tag.line\" >actual &&\n+\tgit-tag -n99 -l | grep \"^ *tag.line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 99 -l tag-lines >actual &&\n+\tgit-tag -n99 -l tag-lines >actual &&\n \tgit diff expect actual\n '\n \n@@ -906,25 +900,21 @@ test_expect_success \\\n \techo \"stag-one-line\" >expect &&\n \tgit-tag -l | grep \"^stag-one-line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 0 -l | grep \"^stag-one-line\" >actual &&\n+\tgit-tag -n0 -l | grep \"^stag-one-line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 0 -l stag-one-line >actual &&\n+\tgit-tag -n0 -l stag-one-line >actual &&\n \tgit diff expect actual &&\n \n \techo \"stag-one-line A message line signed\" >expect &&\n-\tgit-tag -n xxx -l | grep \"^stag-one-line\" >actual &&\n-\tgit diff expect actual &&\n-\tgit-tag -n \"\" -l | grep \"^stag-one-line\" >actual &&\n-\tgit diff expect actual &&\n-\tgit-tag -n 1 -l | grep \"^stag-one-line\" >actual &&\n-\tgit diff expect actual &&\n \tgit-tag -n -l | grep \"^stag-one-line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 1 -l stag-one-line >actual &&\n+\tgit-tag -n1 -l | grep \"^stag-one-line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 2 -l stag-one-line >actual &&\n+\tgit-tag -n1 -l stag-one-line >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 999 -l stag-one-line >actual &&\n+\tgit-tag -n2 -l stag-one-line >actual &&\n+\tgit diff expect actual &&\n+\tgit-tag -n999 -l stag-one-line >actual &&\n \tgit diff expect actual\n '\n \n@@ -935,21 +925,21 @@ test_expect_success \\\n \techo \"stag-zero-lines\" >expect &&\n \tgit-tag -l | grep \"^stag-zero-lines\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 0 -l | grep \"^stag-zero-lines\" >actual &&\n+\tgit-tag -n0 -l | grep \"^stag-zero-lines\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 0 -l stag-zero-lines >actual &&\n+\tgit-tag -n0 -l stag-zero-lines >actual &&\n \tgit diff expect actual &&\n \n \techo \"stag-zero-lines \" >expect &&\n-\tgit-tag -n 1 -l | grep \"^stag-zero-lines\" >actual &&\n-\tgit diff expect actual &&\n \tgit-tag -n -l | grep \"^stag-zero-lines\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 1 -l stag-zero-lines >actual &&\n+\tgit-tag -n1 -l | grep \"^stag-zero-lines\" >actual &&\n+\tgit diff expect actual &&\n+\tgit-tag -n1 -l stag-zero-lines >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 2 -l stag-zero-lines >actual &&\n+\tgit-tag -n2 -l stag-zero-lines >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 999 -l stag-zero-lines >actual &&\n+\tgit-tag -n999 -l stag-zero-lines >actual &&\n \tgit diff expect actual\n '\n \n@@ -963,37 +953,37 @@ test_expect_success \\\n \techo \"stag-lines\" >expect &&\n \tgit-tag -l | grep \"^stag-lines\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 0 -l | grep \"^stag-lines\" >actual &&\n+\tgit-tag -n0 -l | grep \"^stag-lines\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 0 -l stag-lines >actual &&\n+\tgit-tag -n0 -l stag-lines >actual &&\n \tgit diff expect actual &&\n \n \techo \"stag-lines stag line one\" >expect &&\n-\tgit-tag -n 1 -l | grep \"^stag-lines\" >actual &&\n-\tgit diff expect actual &&\n \tgit-tag -n -l | grep \"^stag-lines\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 1 -l stag-lines >actual &&\n+\tgit-tag -n1 -l | grep \"^stag-lines\" >actual &&\n+\tgit diff expect actual &&\n+\tgit-tag -n1 -l stag-lines >actual &&\n \tgit diff expect actual &&\n \n \techo \" stag line two\" >>expect &&\n-\tgit-tag -n 2 -l | grep \"^ *stag.line\" >actual &&\n+\tgit-tag -n2 -l | grep \"^ *stag.line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 2 -l stag-lines >actual &&\n+\tgit-tag -n2 -l stag-lines >actual &&\n \tgit diff expect actual &&\n \n \techo \" stag line three\" >>expect &&\n-\tgit-tag -n 3 -l | grep \"^ *stag.line\" >actual &&\n+\tgit-tag -n3 -l | grep \"^ *stag.line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 3 -l stag-lines >actual &&\n+\tgit-tag -n3 -l stag-lines >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 4 -l | grep \"^ *stag.line\" >actual &&\n+\tgit-tag -n4 -l | grep \"^ *stag.line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 4 -l stag-lines >actual &&\n+\tgit-tag -n4 -l stag-lines >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 99 -l | grep \"^ *stag.line\" >actual &&\n+\tgit-tag -n99 -l | grep \"^ *stag.line\" >actual &&\n \tgit diff expect actual &&\n-\tgit-tag -n 99 -l stag-lines >actual &&\n+\tgit-tag -n99 -l stag-lines >actual &&\n \tgit diff expect actual\n '\n \n-- \n1.5.4.rc0.54.g3eb2\n"},{"id":"63370","messageId":"7v7ijdeq4w.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071213102724.GE12398@artemis.madism.org","subject":"[PATCH] (squashme) gitcli documentation fixups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-17T07:28:47Z","receivedAt":"2007-12-17T07:28:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This comes directly on top of gitcli documentation patch and is intended\nto be squashed into it.\n\nI looked around to see if there is a good place to add gitlink:gitcli[5]\nbut unfortunately I did not find any. One possibility, once this\ndocument is enhanced enough to be usable as \"introduction to scripting\nusing plumbing\", we could add it to near the top of git(7) where we\nrefer to the tutorial, user manual and the everyday document, but in the\ncurrent form it is too sketchy and does not cover enough. But we have\nto start somewhere.\n\nI think we should add the first rule in the bulletted list.\n\n * avoid reinventing the wheel.\n\nbut it needs a bit more explanation. Quite a few people seem to try to\nreinvent \"git rev-parse --verify HEAD\" in their scripts using much\nhigher level \"git show -s -1 --pretty=format:xxx\", which is unfortunate\nand disgusting at the same time.\n\n---\n\n Documentation/gitcli.txt | 45 +++++++++++++++++++++++++++------------------\n Makefile | 1 +\n 2 files changed, 28 insertions(+), 18 deletions(-)\n\ndiff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt\nindex 3b3f5e4..f790b78 100644\n--- a/Documentation/gitcli.txt\n+++ b/Documentation/gitcli.txt\n@@ -3,7 +3,7 @@ gitcli(5)\n \n NAME\n ----\n-gitcli - git command line interface and its usual conventions\n+gitcli - git command line interface and conventions\n \n SYNOPSIS\n --------\n@@ -12,31 +12,40 @@ gitcli\n \n DESCRIPTION\n -----------\n-This manual intends to describe best practice in how to use git CLI. Here are\n+\n+This manual describes best practice in how to use git CLI. Here are\n the rules that you should follow when you are scripting git:\n \n * it's preferred to use the non dashed form of git commands, which means that\n you should prefer `\"git foo\"` to `\"git-foo\"`.\n \n- * splitting short option switches in separate atoms (prefer `\"git foo -a -b\"`\n+ * splitting short options to separate words (prefer `\"git foo -a -b\"`\n to `\"git foo -ab\"`, the latter may not even work).\n \n- * when a command line switch takes an argument, use the 'sticked' form, which\n- means that you must prefer `\"git foo -oArg\"` to `\"git foo -o Arg\"` for short\n- option switches, and `\"git foo --long-opt=Arg\"` to `\"git foo --long-opt Arg\"`\n- for long switches.\n+ * when a command line option takes an argument, use the 'sticked' form. In\n+ other words, write `\"git foo -oArg\"` instead of `\"git foo -o Arg\"` for short\n+ options, and `\"git foo --long-opt=Arg\"` instead of `\"git foo --long-opt Arg\"`\n+ for long options. An option that takes optional option-argument must be\n+ written in the 'sticked' form.\n+\n+ * when you give a revision parameter to a command, make sure the parameter is\n+ not ambiguous with a name of a file in the work tree. E.g. do not write\n+ `\"git log -1 HEAD\"` but write `\"git log -1 HEAD --\"`; the former will not work\n+ if you happen to have a file called `HEAD` in the work tree.\n \n \n ENHANCED CLI\n ------------\n-From the git 1.5.4 series and further, git commands (not all of them at the\n-time of the writing though) come with an enhanced option parser with nice\n-facilities. Here is an exhaustive list of them\n+From the git 1.5.4 series and further, many git commands (not all of them at the\n+time of the writing though) come with an enhanced option parser.\n+\n+Here is an exhaustive list of the facilities provided by this option parser.\n+\n \n Magic Options\n ~~~~~~~~~~~~~\n Commands which have the enhanced option parser activated all understand a\n-couple of magic command line switches:\n+couple of magic command line options:\n \n -h::\n \tgives a pretty printed usage of the command.\n@@ -54,14 +63,14 @@ usage: git-describe [options] *\n ---------------------------------------------\n \n --help-all::\n-\tSome git commands takes options that are only used for plumbing or that\n+\tSome git commands take options that are only used for plumbing or that\n \tare deprecated, and such options are hidden from the default usage. This\n-\tswitch gives the full list of options.\n+\toption gives the full list of options.\n \n \n Negating options\n ~~~~~~~~~~~~~~~~\n-Another things to keep in mind is that long options can be negated. For\n+Boolean options with long option names can be negated by prefixing `\"--no-\"`. For\n example, `\"git branch\"` has the option `\"--track\"` which is 'on' by default. You\n can use `\"--no-track\"` to override that behaviour. The same goes for `\"--color\"`\n and `\"--no-color\"`.\n@@ -74,10 +83,10 @@ options. This means that you can for example use `\"git rm -rf\"` or\n `\"git clean -fdx\"`.\n \n \n-Separating argument from the switch\n+Separating argument from the option\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n-Also for option switches that take a mandatory argument, you can separate it\n-from the switch. That means that all the following uses are correct:\n+You can write the mandatory option parameter to an option as a separate\n+word on the command line. That means that all the following uses work:\n \n ----------------------------\n $ git foo --long-opt=Arg\n@@ -86,7 +95,7 @@ $ git foo -oArg\n $ git foo -o Arg\n ----------------------------\n \n-However, this is *NOT* possible for switches with an optionnal value, where the\n+However, this is *NOT* allowed for switches with an optionnal value, where the\n 'sticked' form must be used:\n ----------------------------\n $ git describe --abbrev HEAD # correct\ndiff --git a/Makefile b/Makefile\nindex 617e5f5..fd9b939 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1172,6 +1172,7 @@ check-docs::\n \t\tdocumented,gitattributes | \\\n \t\tdocumented,gitignore | \\\n \t\tdocumented,gitmodules | \\\n+\t\tdocumented,gitcli | \\\n \t\tdocumented,git-tools | \\\n \t\tsentinel,not,matching,is,ok ) continue ;; \\\n \t\tesac; \\\n-- \n1.5.4.rc0.54.g3eb2\n"},{"id":"63371","messageId":"47662715.9070200@op5.se","threadId":"11270","inReplyTo":"7vir2xeq76.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-12-17T07:36:53Z","receivedAt":"2007-12-17T07:36:53Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n>> Yep, let's do it then. Note that's the reason why I felt we need a\n>> manual page about this, because we should give some guidelines of what\n>> is safe for scripting.\n> \n> There are some fallouts from the series, though. I've fixed up git-tag\n> but I strongly suspect all of parseopt users now need careful auditing.\n> If we cannot be confident with the parseoptified commands within\n> reasonably short timeframe by -rc1, we may end up having to revert the\n> parseopt changes from them, which I'd rather avoid, but if you look at\n> the git-tag change (especially for -l) you would understand it. The\n> \"must stick\" restriction feels Ok on paper but in practice it looks\n> rather draconian and very user unfriendly.\n> \n\nUsually, optional arguments warrant adding a second parameter. This can\noften even improve usability, as it's never unclear or ambiguous what's\nhappening. For the 'git tag -l' case, I'd use something like\n'git tag -l --match=\"regex\"' or some such, or perhaps make \"-l\" its own\nsubcommand (\"list\") with a built-in alias of -l. That means \"-l\" has to\nbe the first argument after \"git tag\" on the command-line, but I suspect\nit doesn't make much sense to use it along with other options anyway, so\nperhaps that's not much of an issue.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"63374","messageId":"7v3au1eopr.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"47662715.9070200@op5.se","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-17T07:59:28Z","receivedAt":"2007-12-17T07:59:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Ericsson writes:\n\n> Junio C Hamano wrote:\n> \n>> ... The\n>> \"must stick\" restriction feels Ok on paper but in practice it looks\n>> rather draconian and very user unfriendly.\n>\n> Usually, optional arguments warrant adding a second parameter. This can\n> often even improve usability, as it's never unclear or ambiguous what's\n> happening. For the 'git tag -l' case, I'd use something like\n> 'git tag -l --match=\"regex\"' or some such,...\n\nThat is essentially arguing for POSIXly correct \"do not allow optional\noption-arguments\" (utility syntax guidelines #7). That position might\nbe politically correct, but I am already discussing beyond that:\nusability.\n\nFor \"git tag -l\", the fix was rather simple, as the option would either\nhave taken a zero pattern (list all) or a single pattern (list matching\nthis pattern), and the command itself did not take any extra arguments,\nso that was what I did in the patch. Compare your POSIXly correct\nversion:\n\n git tag -l\t\t\t(ok)\n git tag -l pattern\t\t(not ok)\n\tgit tag -l --match=pattern\t(ok)\n\nwith the traditional (and fixed):\n\n git tag -l\t\t\t(ok)\n git tag -l pattern\t\t(ok)\n\tgit tag -l pattern garbage\t(not ok)\n\nWhich one is easier for the user?\n"},{"id":"63382","messageId":"20071217085106.GA7453@artemis.madism.org","threadId":"11270","inReplyTo":"7v7ijdeq4w.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] (squashme) gitcli documentation fixups","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T08:51:06Z","receivedAt":"2007-12-17T08:51:06Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 07:28:47AM +0000, Junio C Hamano wrote:\n> This comes directly on top of gitcli documentation patch and is intended\n> to be squashed into it.\n\n I obviously ack.\n> -Another things to keep in mind is that long options can be negated. For\n> +Boolean options with long option names can be negated by prefixing `\"--no-\"`. For\n ^^^^^^^\nThough this isn't correct: you can negate any kind of option, even one\nwith strings arguments, and it does makes sense. E.g. if you have some:\n\n foo.stringOpt = \"value\"\n\nin your gitconfig file, then it's very handy to be able to write:\n\n $ git foo --no-string-opt\n\nto be sure the gitconfig from the user won't mess with what you intend\nto do. The negation of commands can be disabled (in the recent\niterations of parseopt) using a flag I don't recall the name, but it's\non by default even for non boolean options. It may make sense to do a\nre-read pass of all options and see which ones it makes sense to negate\nand which not.\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63383","messageId":"20071217085734.GB7453@artemis.madism.org","threadId":"11270","inReplyTo":"7v7ijdeq4w.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] (squashme) gitcli documentation fixups","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T08:57:34Z","receivedAt":"2007-12-17T08:57:34Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 07:28:47AM +0000, Junio C Hamano wrote:\n> * avoid reinventing the wheel.\n> \n> but it needs a bit more explanation. Quite a few people seem to try to\n> reinvent \"git rev-parse --verify HEAD\" in their scripts using much\n> higher level \"git show -s -1 --pretty=format:xxx\", which is unfortunate\n> and disgusting at the same time.\n\nOh and about that, the point is, users don't always know the wheel\nexists because they don't know where to look in the first place. Maybe\ngitcli(5) will become the right place to explain this kind of usual\ntricks under a \"git scripting idioms\" section.\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63385","messageId":"20071217090749.GC7453@artemis.madism.org","threadId":"11270","inReplyTo":"7vd4t5eq52.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T09:07:49Z","receivedAt":"2007-12-17T09:07:49Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 07:28:41AM +0000, Junio C Hamano wrote:\n> The command freely used optional option-argments for its -l and -n options.\n> I think allowing \"git tag -n xxx\" without barfing was an error to begin with,\n> but not supporting \"git tag -l pattern\" form is a serious regression.\n> \n> So this fixes the handling of -l to reinstate the original behaviour while\n> detecting a user error \"git tag -l pattern garbage\", and adjusts tests that\n> use \"-n param\" form to use \"-nparam\".\n\n> -\tif (list)\n> +\tif (list) {\n> +\t\t/*\n> +\t\t * This is unfortunate but requiring \"git tag -lpattern\" and not\n> +\t\t * allowing \"git tag -l pattern\" is a serious regression.\n> +\t\t */\n> +\t\tif (argc && list == no_pattern) {\n> +\t\t\tlist = argv[0];\n> +\t\t\targc--;\n> +\t\t}\n> +\t\tif (argc)\n> +\t\t\tdie(\"extra argument after -l[pattern]: %s\", argv[0]);\n> \t\treturn list_tags(list == no_pattern ? NULL : list, lines);\n> +\t}\n\n Okay this is kind of disgusting, and I'm absolutely not pleased with\nit (I mean I'm not pleased that parse_opt forces us to write things like\nthat). This hack allows:\n\n git tag -l -n10 \n\nand will then attach the to the '-l' switch, and I find it\nnowhere near acceptable. I believe the fix is worse than the disease.\nI'll try to think harder about what we can do about it. Though for now,\nwe will have to go for it for a while.\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63389","messageId":"476643A0.6020700@op5.se","threadId":"11270","inReplyTo":"7v3au1eopr.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-12-17T09:38:40Z","receivedAt":"2007-12-17T09:38:40Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Andreas Ericsson writes:\n> \n>> Junio C Hamano wrote:\n>>\n>>> ... The\n>>> \"must stick\" restriction feels Ok on paper but in practice it looks\n>>> rather draconian and very user unfriendly.\n>> Usually, optional arguments warrant adding a second parameter. This can\n>> often even improve usability, as it's never unclear or ambiguous what's\n>> happening. For the 'git tag -l' case, I'd use something like\n>> 'git tag -l --match=\"regex\"' or some such,...\n> \n> That is essentially arguing for POSIXly correct \"do not allow optional\n> option-arguments\" (utility syntax guidelines #7). That position might\n> be politically correct, but I am already discussing beyond that:\n> usability.\n> \n> For \"git tag -l\", the fix was rather simple, as the option would either\n> have taken a zero pattern (list all) or a single pattern (list matching\n> this pattern), and the command itself did not take any extra arguments,\n> so that was what I did in the patch. Compare your POSIXly correct\n> version:\n> \n> git tag -l\t\t\t(ok)\n> git tag -l pattern\t\t(not ok)\n> \tgit tag -l --match=pattern\t(ok)\n> \n> with the traditional (and fixed):\n> \n> git tag -l\t\t\t(ok)\n> git tag -l pattern\t\t(ok)\n> \tgit tag -l pattern garbage\t(not ok)\n> \n> Which one is easier for the user?\n\n\ngit tag -l pattern, although I suspect newcomers often write it as\n\"git tag -l | grep pattern\" anyways.\n\nIf -l was a short-hand for \"git tag list\", and \"list\" was a subcommand\nto git tag, the whole business would be explainable. The fact that\n\"git tag\" lists all tag but doesn't take a patter, while \"git tag -l\"\ndoes exactly the same thing, but *does* take an (optional) pattern\nmeans the --match functionality could just as well be written as\n\"git tag -m pattern\" (and let '-m' imply '-l', which is sensible).\n\"-l\" can then be deprecated, as its syntax doesn't match that of\nthe other way to list tags.\n\nOtoh, I don't care all that deeply about it. It's just nicer to explain\nthe UI if there are no optional arguments, primarily because it involves\nexceptions, and secondarily because many programs follow the posixly\ncorrect thing of \"no optional arguments\", so they're a definitive\nminority in the program argument jungle.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"63395","messageId":"7vir2xa8z7.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071217090749.GC7453@artemis.madism.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-17T10:53:00Z","receivedAt":"2007-12-17T10:53:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> Okay this is kind of disgusting, and I'm absolutely not pleased with\n> it (I mean I'm not pleased that parse_opt forces us to write things like\n> that).\n> ...\n> I'll try to think harder about what we can do about it. Though for now,\n> we will have to go for it for a while.\n\nThis is just a quick idea before I go back to sleep, but your earlier\ncomment on \"--no-\" made me realize\nthat the alternative I was suggesting earlier would actually work much\nnicer, if you introduce \"---default\"\nmagic.\n\nThen, normal users who know what the value of $foo is (iow, not scripts)\ncan say:\n\n\tgit cmd --abbrev 10\n git cmd --abbrev HEAD\n git cmd --abbrev=10 HEAD\n\nand scripts that want to have $foo to be treated as rev, even when it\nconsists entirely of digits, can say:\n\n\tgit cmd --abbrev-default $foo\n\nto disambiguate (i.e. like \"--no-\" magic, \"-default\" is a magic, and it\ntells the parser that \"there is no option-argument given to this\").\n\nTo make sure $foo is treated as the precision, the script can say:\n\n\tgit cmd --abbrev=$foo\n\nIf the script wants DWIM just like human users want, it can do:\n\n\tgit cmd --abbrev $foo\n\nThere of course is a little details called coding, but I think this is\nprobably the most user friendly to the users and the scripts alike. It\ncertainly is nicer than what the current parse_options() does, and/or\nthe git-tag workaround does.\n"},{"id":"63397","messageId":"20071217105834.GG7453@artemis.madism.org","threadId":"11270","inReplyTo":"7vir2xa8z7.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T10:58:34Z","receivedAt":"2007-12-17T10:58:34Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 10:53:00AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n> > Okay this is kind of disgusting, and I'm absolutely not pleased with\n> > it (I mean I'm not pleased that parse_opt forces us to write things like\n> > that).\n> > ...\n> > I'll try to think harder about what we can do about it. Though for now,\n> > we will have to go for it for a while.\n> \n> This is just a quick idea before I go back to sleep, but your earlier\n> comment on \"--no-\" made me realize\n> that the alternative I was suggesting earlier would actually work much\n> nicer, if you introduce \"---default\"\n> magic.\n\n meeeow I love the idea !\n\n> Then, normal users who know what the value of $foo is (iow, not scripts)\n> can say:\n> \n> \tgit cmd --abbrev 10\n> git cmd --abbrev HEAD\n> git cmd --abbrev=10 HEAD\n> \n> and scripts that want to have $foo to be treated as rev, even when it\n> consists entirely of digits, can say:\n> \n> \tgit cmd --abbrev-default $foo\n> \n> to disambiguate (i.e. like \"--no-\" magic, \"-default\" is a magic, and it\n> tells the parser that \"there is no option-argument given to this\").\n> \n> To make sure $foo is treated as the precision, the script can say:\n> \n> \tgit cmd --abbrev=$foo\n> \n> If the script wants DWIM just like human users want, it can do:\n> \n> \tgit cmd --abbrev $foo\n> \n> There of course is a little details called coding, but I think this is\n> probably the most user friendly to the users and the scripts alike. It\n> certainly is nicer than what the current parse_options() does, and/or\n> the git-tag workaround does.\n\nI like the idea, this way we can do the \"let's pass the argument as an\noption to the callback and let it say if it likes it or not, and have a\nquite not so bad way to help the guy scripting this disambiguate. I like\nit a lot, and it shouldn't be that hard to deal with. I'll work on it,\nand propose new patches ASAP.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63403","messageId":"7v7ijda81h.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071217090749.GC7453@artemis.madism.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-17T11:13:14Z","receivedAt":"2007-12-17T11:13:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> Okay this is kind of disgusting, and I'm absolutely not pleased with\n> it (I mean I'm not pleased that parse_opt forces us to write things like\n> that). This hack allows:\n>\n> git tag -l -n10 \n>\n> and will then attach the to the '-l' switch,...\n\nHeh, it turns out that we were both stupid and blind.\n\nLook at contrib/examples/git-tag.sh again.\n\nThe original addition of \"-n \" was suboptimal and did not allow\n\"git tag -l -n10 \", but I would actually call that a bug of the -n\n implementation (it wanted to affect working of other option, so\nat that point it should have restructured the loop and made parsing of\noptions and carrying out of actions separate steps).\n\nThe -l option tells \"git tag\" to work in \"list tags\" mode, and in that\nmode, the command itself takes zero or one (we could have taken more but\nwe didn't) glob to limit the listing. The is not even an option\nargument to option -l, but the arguments to \"git tag\" command itself.\n\nSo \"git-tag -l\" was a bad example of an option that takes optional\noption-argument, and you can see that the conversion to parse_options()\ndone in 396865859918e9c7bf8ce74aae137c57da134610 (Make builtin-tag.c use\nparse_options.) broke it.\n\nIOW, the fixup I posted was not a workaround but happens to be a bugfix\nto the above commit that did parse_options() conversion.\n"},{"id":"63404","messageId":"7v1w9la7o8.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071217105834.GG7453@artemis.madism.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-17T11:21:11Z","receivedAt":"2007-12-17T11:21:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> On Mon, Dec 17, 2007 at 10:53:00AM +0000, Junio C Hamano wrote:\n> ...\n>> This is just a quick idea before I go back to sleep, but your earlier\n>> comment on \"--no-\" made me realize\n>> that the alternative I was suggesting earlier would actually work much\n>> nicer, if you introduce \"---default\"\n>> magic.\n>\n> meeeow I love the idea !\n\nThere is a bit more serious issue than coding, actually.\n\nShort options.\n\nA script wants to use default rename detection threshold for unknown\ncommit $foo whose name might look like a number. IOW, this\n\n\tgit diff -M $foo\n\ncould be ambiguous. Obviously, \"git diff -M-default $foo\" would not fly\nvery well.\n"},{"id":"63407","messageId":"20071217115648.GI7453@artemis.madism.org","threadId":"11270","inReplyTo":"7v7ijda81h.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T11:56:48Z","receivedAt":"2007-12-17T11:56:48Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 11:13:14AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n> > Okay this is kind of disgusting, and I'm absolutely not pleased with\n> > it (I mean I'm not pleased that parse_opt forces us to write things like\n> > that). This hack allows:\n> >\n> > git tag -l -n10 \n> >\n> > and will then attach the to the '-l' switch,...\n> \n> Heh, it turns out that we were both stupid and blind.\n\n [...]\n\n indeed, but then this happens to be a better patch than yours IMHO:\n\n\n>From 5a3cdd255f17c7d7bc9245881f0d50146413113f Mon Sep 17 00:00:00 2001\nFrom: Pierre Habouzit \nDate: Mon, 17 Dec 2007 12:54:55 +0100\nSubject: [PATCH] git-tag: fix -l switch handling regression.\n\nSigned-off-by: Pierre Habouzit \n---\n builtin-tag.c | 12 +++++-------\n 1 files changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 274901a..219633d 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -16,7 +16,7 @@\n static const char * const git_tag_usage[] = {\n \t\"git-tag [-a|-s|-u ] [-f] [-m |-F ] []\",\n \t\"git-tag -d ...\",\n-\t\"git-tag [-n []] -l []\",\n+\t\"git-tag -l [-n []] []\",\n \t\"git-tag -v ...\",\n \tNULL\n };\n@@ -370,13 +370,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstruct ref_lock *lock;\n \n \tint annotate = 0, sign = 0, force = 0, lines = 0,\n-\t\t\t\t\tdelete = 0, verify = 0;\n-\tchar *list = NULL, *msgfile = NULL, *keyid = NULL;\n-\tconst char *no_pattern = \"NO_PATTERN\";\n+\t\tlist = 0, delete = 0, verify = 0;\n+\tchar *msgfile = NULL, *keyid = NULL;\n \tstruct msg_arg msg = { 0, STRBUF_INIT };\n \tstruct option options[] = {\n-\t\t{ OPTION_STRING, 'l', NULL, &list, \"pattern\", \"list tag names\",\n-\t\t\tPARSE_OPT_OPTARG, NULL, (intptr_t) no_pattern },\n+\t\tOPT_INTEGER('l', NULL, &list, \"list tag names\"),\n \t\t{ OPTION_INTEGER, 'n', NULL, &lines, NULL,\n \t\t\t\t\"print n lines of each tag message\",\n \t\t\t\tPARSE_OPT_OPTARG, NULL, 1 },\n@@ -408,7 +406,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tannotate = 1;\n \n \tif (list)\n-\t\treturn list_tags(list == no_pattern ? NULL : list, lines);\n+\t\treturn list_tags(argv[0], lines);\n \tif (delete)\n \t\treturn for_each_tag_name(argv, delete_tag);\n \tif (verify)\n-- \ndebian.1.5.3.7.1-dirty\n"},{"id":"63409","messageId":"20071217115939.GJ7453@artemis.madism.org","threadId":"11270","inReplyTo":"20071217115648.GI7453@artemis.madism.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T11:59:39Z","receivedAt":"2007-12-17T11:59:39Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 11:56:48AM +0000, Pierre Habouzit wrote:\n> On Mon, Dec 17, 2007 at 11:13:14AM +0000, Junio C Hamano wrote:\n> > Pierre Habouzit writes:\n> > \n> > > Okay this is kind of disgusting, and I'm absolutely not pleased with\n> > > it (I mean I'm not pleased that parse_opt forces us to write things like\n> > > that). This hack allows:\n> > >\n> > > git tag -l -n10 \n> > >\n> > > and will then attach the to the '-l' switch,...\n> > \n> > Heh, it turns out that we were both stupid and blind.\n> \n> [...]\n> \n> indeed, but then this happens to be a better patch than yours IMHO:\n> \n> \n> From 5a3cdd255f17c7d7bc9245881f0d50146413113f Mon Sep 17 00:00:00 2001\n> From: Pierre Habouzit \n> Date: Mon, 17 Dec 2007 12:54:55 +0100\n> Subject: [PATCH] git-tag: fix -l switch handling regression.\n> \n> Signed-off-by: Pierre Habouzit \n> ---\n> builtin-tag.c | 12 +++++-------\n> 1 files changed, 5 insertions(+), 7 deletions(-)\n> \n> diff --git a/builtin-tag.c b/builtin-tag.c\n> index 274901a..219633d 100644\n> --- a/builtin-tag.c\n> +++ b/builtin-tag.c\n> @@ -16,7 +16,7 @@\n> static const char * const git_tag_usage[] = {\n> \t\"git-tag [-a|-s|-u ] [-f] [-m |-F ] []\",\n> \t\"git-tag -d ...\",\n> -\t\"git-tag [-n []] -l []\",\n> +\t\"git-tag -l [-n []] []\",\n> \t\"git-tag -v ...\",\n> \tNULL\n> };\n> @@ -370,13 +370,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n> \tstruct ref_lock *lock;\n> \n> \tint annotate = 0, sign = 0, force = 0, lines = 0,\n> -\t\t\t\t\tdelete = 0, verify = 0;\n> -\tchar *list = NULL, *msgfile = NULL, *keyid = NULL;\n> -\tconst char *no_pattern = \"NO_PATTERN\";\n> +\t\tlist = 0, delete = 0, verify = 0;\n> +\tchar *msgfile = NULL, *keyid = NULL;\n> \tstruct msg_arg msg = { 0, STRBUF_INIT };\n> \tstruct option options[] = {\n> -\t\t{ OPTION_STRING, 'l', NULL, &list, \"pattern\", \"list tag names\",\n> -\t\t\tPARSE_OPT_OPTARG, NULL, (intptr_t) no_pattern },\n> +\t\tOPT_INTEGER('l', NULL, &list, \"list tag names\"),\n ^^^^^^^^^^^\n that should obviously be OPT_BOOLEAN\n\n> \t\t{ OPTION_INTEGER, 'n', NULL, &lines, NULL,\n> \t\t\t\t\"print n lines of each tag message\",\n> \t\t\t\tPARSE_OPT_OPTARG, NULL, 1 },\n> @@ -408,7 +406,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n> \t\tannotate = 1;\n> \n> \tif (list)\n> -\t\treturn list_tags(list == no_pattern ? NULL : list, lines);\n> +\t\treturn list_tags(argv[0], lines);\n> \tif (delete)\n> \t\treturn for_each_tag_name(argv, delete_tag);\n> \tif (verify)\n> -- \n> debian.1.5.3.7.1-dirty\n> \n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"63417","messageId":"20071217123307.GK7453@artemis.madism.org","threadId":"11270","inReplyTo":"7v1w9la7o8.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T12:33:07Z","receivedAt":"2007-12-17T12:33:07Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 11:21:11AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n> > On Mon, Dec 17, 2007 at 10:53:00AM +0000, Junio C Hamano wrote:\n> > ...\n> >> This is just a quick idea before I go back to sleep, but your earlier\n> >> comment on \"--no-\" made me realize\n> >> that the alternative I was suggesting earlier would actually work much\n> >> nicer, if you introduce \"---default\"\n> >> magic.\n> >\n> > meeeow I love the idea !\n> \n> There is a bit more serious issue than coding, actually.\n> \n> Short options.\n> \n> A script wants to use default rename detection threshold for unknown\n> commit $foo whose name might look like a number. IOW, this\n> \n> \tgit diff -M $foo\n> \n> could be ambiguous. Obviously, \"git diff -M-default $foo\" would not fly\n> very well.\n\nYes, I thought about that too actually.\n\nAfter having written this mail 4 time already, I came up with an idea I\nkind of like: like find, we could make {} be a placeholder for the\n\"default\" argument. For example:\n\n $ git foo --abbrev {} 10\n $ git log -M {} 1\n ...\n\n{} would have the same semantics as your --long-opt-default. It tells the\noption parser that \"no there isn't anything to grok for that command thank you\nvery much\". Of course if for some reason you really want to pass \"{}\" to the\ncommand, the stuck form holds:\n\n $ git foo --long-opt={}\n $ git foo -o{}\n\nWhat do you think ?\n\nPS: I know that in some shells {} needs escaping, which isn't nice. I chose it\n because it's the same as find(1) but we could e.g. use '_' that is less\n \"conventional\" (if it even makes sense) but is a bit easier to type than \\{}.\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63435","messageId":"1197908463.8463.9.camel@hinata.boston.redhat.com","threadId":"11270","inReplyTo":"7v3au1eopr.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] convert shortlog to use parse_options","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-12-17T16:21:03Z","receivedAt":"2007-12-17T16:21:03Z","isPatch":false,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Sun, 2007-12-16 at 23:59 -0800, Junio C Hamano wrote:\n> Andreas Ericsson writes:\n> \n> > Junio C Hamano wrote:\n> > \n> >> ... The\n> >> \"must stick\" restriction feels Ok on paper but in practice it looks\n> >> rather draconian and very user unfriendly.\n> >\n> > Usually, optional arguments warrant adding a second parameter. This can\n> > often even improve usability, as it's never unclear or ambiguous what's\n> > happening. For the 'git tag -l' case, I'd use something like\n> > 'git tag -l --match=\"regex\"' or some such,...\n> \n> That is essentially arguing for POSIXly correct \"do not allow optional\n> option-arguments\" (utility syntax guidelines #7). That position might\n> be politically correct, but I am already discussing beyond that:\n> usability.\n> \n> For \"git tag -l\", the fix was rather simple, as the option would either\n> have taken a zero pattern (list all) or a single pattern (list matching\n> this pattern), and the command itself did not take any extra arguments,\n> so that was what I did in the patch. Compare your POSIXly correct\n> version:\n> \n> git tag -l\t\t\t(ok)\n> git tag -l pattern\t\t(not ok)\n> \tgit tag -l --match=pattern\t(ok)\n> \n> with the traditional (and fixed):\n> \n> git tag -l\t\t\t(ok)\n> git tag -l pattern\t\t(ok)\n> \tgit tag -l pattern garbage\t(not ok)\n> \n> Which one is easier for the user?\n\ngit tag now lists tags by default so there's a couple of other options\nto consider:\n\n git tag\t\t\t\t(ok)\n git tag --match pattern\t\t(ok)\n\tgit tag --match=pattern \t(ok)\n\tgit tag --match=pattern -l\t(ok)\n\tgit tag -l pattern\t\t(not ok)\n\nOr we could repurpose -l as the match option:\n\n\tgit tag\t\t\t(ok)\n\tgit tag -l\t\t(not ok)\n\tgit tag -l pattern\t(ok)\n\nAnd that was the point I was trying to make earlier with my rather\nabstract sounding post about jumping through hoops. If we can't break\nthe options interface to make git sane, we'll be stuck with a broken\ncommand line interface and must write complicated documents on what option\nsticking is and how it works.\n\ncheers,\nKristian\n"},{"id":"63466","messageId":"7vy7bt6qv6.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071217123307.GK7453@artemis.madism.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-17T19:52:29Z","receivedAt":"2007-12-17T19:52:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> After having written this mail 4 time already, I came up with an idea I\n> kind of like: like find, we could make {} be a placeholder for the\n> \"default\" argument. For example:\n>\n> $ git foo --abbrev {} 10\n> $ git log -M {} 1\n> ...\n>\n> {} would have the same semantics as your --long-opt-default. It tells the\n> option parser that \"no there isn't anything to grok for that command thank you\n> very much\". Of course if for some reason you really want to pass \"{}\" to the\n> command, the stuck form holds:\n>\n> $ git foo --long-opt={}\n> $ git foo -o{}\n>\n> What do you think ?\n\n1. {} means a completely different thing to find (\"place the real value\n here\"); there is no similarity. I would strongly oppose to it. If\n you want to invoke opt with default but still want to pass \"{}\" as an\n argument unrelated to that opt, you would do \"--opt={} {}\". That's\n double ugly.\n\n2. For a long option with optional option-argument, --abbrev-default (or\n in the other order, --default-abbrev) to mark \"there is no option\n argument, do not do your context sensitive parsing\" and using an\n explicit '=' (e.g. --abbrev=) to mark \"this is the argument,\n do not do your context sensitive parsing\" is much more readable.\n\n3. There are only handful options with optional option-argument that\n does not have long format. I think it is reasonable to require\n \"stuck argument\" to them. For most of the short options that take\n optional option-argument, traditionally we did not allow them to be\n spelled as separate words, so there is no regression to introduce\n such a behaviour. -B/-M/-C options to diff family would be handled\n sanely this way.\n\n Another possibility, which I do not like very much, is to add long\n format to them, if only for paranoid scripters who want rename\n detection with the default threshold and cannot say \"diff -M $foo\".\n They can say \"diff --detect-rename-default $foo\" instead (\"-M\" is a\n bad example here, as giving a single path never makes sense for -M so\n $foo cannot be a file whose name is e.g. \"20\", and default number of\n abbreviated commit object name is longer than 2 which means it would\n make it longer than \"percentage\" form of threshold).\n\nSo in short, for an option that takes optional option-argument:\n\n - if it is given as \"---default\", there is no optional\n argument, period.\n\n - if it is given as \"--long-name\" but there is no next word, there is\n no optional argument, either.\n\n - if it is given as \"--long-name=value\", that \"value\" is the\n argument. Barf if it does not validate.\n\n - if it is given as \"--long-name\", and there is a next word, see if\n that is plausible as its argument. Get it and signal the caller\n you consumed it, if it is. Ignore it and signal the caller you\n didn't, if it isn't.\n\n - if it is given as \"-svalue\", that \"value\" is the argument. Barf if\n it does not validate.\n\n - if it is given as \"-s\", and there is a next word, and if the option\n has long format counterpart as well, then see if the next word is\n plausible as its argument. Get it and signal the caller you\n consumed it, if it is. Ignore it and signal the caller you didn't,\n if it isn't.\n\n - if it is given as \"-s\" but the previous rule did not trigger, there\n is no optional argument.\n"},{"id":"63473","messageId":"20071217203143.GA2105@coredump.intra.peff.net","threadId":"11270","inReplyTo":"7vy7bt6qv6.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-17T20:31:43Z","receivedAt":"2007-12-17T20:31:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 17, 2007 at 11:52:29AM -0800, Junio C Hamano wrote:\n\n> So in short, for an option that takes optional option-argument:\n\nI agree with everything you said, except...\n\n> - if it is given as \"--long-name\", and there is a next word, see if\n> that is plausible as its argument. Get it and signal the caller\n> you consumed it, if it is. Ignore it and signal the caller you\n> didn't, if it isn't.\n\nThis \"plausible\" makes me a little nervous, and I wonder why we want to\nsupport this at all. Is it\n\n 1. We have traditionally supported \"--abbrev 10\"? I don't think this\n is the case.\n 2. Consistency with \"--non-optional-arg foo\"? Do we have any such\n non-optional long arguments? I didn't see any; I think we stick\n with --non-optional-arg=foo everywhere.\n 3. More convenience to the user? I don't see how \" \" is easier than\n \"=\".\n\n> - if it is given as \"-s\", and there is a next word, and if the option\n> has long format counterpart as well, then see if the next word is\n> plausible as its argument. Get it and signal the caller you\n> consumed it, if it is. Ignore it and signal the caller you didn't,\n> if it isn't.\n\nSimilarly, what is the goal here?\n\n 1. Have we ever supported \"-s foo\"? Not for -B/-M/-C, nor for\n shortlog's -w.\n 2. This would add consistency to non-optional arguments.\n 3. It's longer to type.\n\nSo I see a slight case for \"-s foo\", but none at all for \"--long foo\".\n\n-Peff\n"},{"id":"63475","messageId":"20071217204204.GA13004@artemis.madism.org","threadId":"11270","inReplyTo":"20071217203143.GA2105@coredump.intra.peff.net","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T20:42:04Z","receivedAt":"2007-12-17T20:42:04Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 08:31:43PM +0000, Jeff King wrote:\n> On Mon, Dec 17, 2007 at 11:52:29AM -0800, Junio C Hamano wrote:\n> \n> > So in short, for an option that takes optional option-argument:\n> \n> I agree with everything you said, except...\n> \n> > - if it is given as \"--long-name\", and there is a next word, see if\n> > that is plausible as its argument. Get it and signal the caller\n> > you consumed it, if it is. Ignore it and signal the caller you\n> > didn't, if it isn't.\n> \n> This \"plausible\" makes me a little nervous, and I wonder why we want to\n> support this at all. Is it\n> \n> 1. We have traditionally supported \"--abbrev 10\"? I don't think this\n> is the case.\n\n Yes, that's why the restriction bugs me a bit too.\n\n> 2. Consistency with \"--non-optional-arg foo\"? Do we have any such\n> non-optional long arguments? I didn't see any; I think we stick\n> with --non-optional-arg=foo everywhere.\n\n there are some, I don't recall the exact commands, but option parsing\nwas quite inconsistent in git (well still is), there are the very simple\nloops that just do strcmp and look for the '=', there are the loops that\nallow interleaving of options and arguments (and that rewrite argc/argv\na bit like parseopt does) and also the ones that allow the separate\nmode, and the one that do both.\n\n The force-stick-mode is a regression for them.\n\n> > - if it is given as \"-s\", and there is a next word, and if the option\n> > has long format counterpart as well, then see if the next word is\n> > plausible as its argument. Get it and signal the caller you\n> > consumed it, if it is. Ignore it and signal the caller you didn't,\n> > if it isn't.\n> \n> Similarly, what is the goal here?\n> \n> 1. Have we ever supported \"-s foo\"? Not for -B/-M/-C, nor for\n> shortlog's -w.\n\n Yes for git tag -n for example, and there are some other\nexamples, look at maint for commands that have been migrated to\nparse_options, some behave like that, and more than one for sure.\n\n> 3. It's longer to type.\n\n It's way more readable, but YMMV.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63476","messageId":"7vhcih6oj8.fsf@gitster.siamese.dyndns.org","threadId":"11270","inReplyTo":"20071217203143.GA2105@coredump.intra.peff.net","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-17T20:42:51Z","receivedAt":"2007-12-17T20:42:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Dec 17, 2007 at 11:52:29AM -0800, Junio C Hamano wrote:\n>\n>> So in short, for an option that takes optional option-argument:\n>\n> I agree with everything you said, except...\n>\n>> - if it is given as \"--long-name\", and there is a next word, see if\n>> that is plausible as its argument. Get it and signal the caller\n>> you consumed it, if it is. Ignore it and signal the caller you\n>> didn't, if it isn't.\n>\n> This \"plausible\" makes me a little nervous, and I wonder why we want to\n> support this at all. Is it\n>\n> 1. We have traditionally supported \"--abbrev 10\"? I don't think this\n> is the case.\n> 2. Consistency with \"--non-optional-arg foo\"? Do we have any such\n> non-optional long arguments? I didn't see any; I think we stick\n> with --non-optional-arg=foo everywhere.\n> 3. More convenience to the user? I don't see how \" \" is easier than\n> \"=\".\n\nYou forgot one case.\n\n 4. Everybody who does _not_ know that we traditionally did not\n support the form would expect \"--abbrev 10\" and \"-n 4\" to work.\n"},{"id":"63478","messageId":"20071217205355.GC2105@coredump.intra.peff.net","threadId":"11270","inReplyTo":"7vhcih6oj8.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-17T20:53:55Z","receivedAt":"2007-12-17T20:53:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 17, 2007 at 12:42:51PM -0800, Junio C Hamano wrote:\n\n> You forgot one case.\n> \n> 4. Everybody who does _not_ know that we traditionally did not\n> support the form would expect \"--abbrev 10\" and \"-n 4\" to work.\n\nI would expect \"-n 4\" to work, but not \"--abbrev 10\". But perhaps that\nis just me. If that is the expectation, I think the behavior you\noutlined is sensible.\n\n-Peff\n"},{"id":"63480","messageId":"20071217210116.GB13004@artemis.madism.org","threadId":"11270","inReplyTo":"20071217204204.GA13004@artemis.madism.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T21:01:16Z","receivedAt":"2007-12-17T21:01:16Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 08:42:04PM +0000, Pierre Habouzit wrote:\n> On Mon, Dec 17, 2007 at 08:31:43PM +0000, Jeff King wrote:\n> > On Mon, Dec 17, 2007 at 11:52:29AM -0800, Junio C Hamano wrote:\n> > \n> > > So in short, for an option that takes optional option-argument:\n> > \n> > I agree with everything you said, except...\n> > \n> > > - if it is given as \"--long-name\", and there is a next word, see if\n> > > that is plausible as its argument. Get it and signal the caller\n> > > you consumed it, if it is. Ignore it and signal the caller you\n> > > didn't, if it isn't.\n> > \n> > This \"plausible\" makes me a little nervous, and I wonder why we want to\n> > support this at all. Is it\n> > \n> > 1. We have traditionally supported \"--abbrev 10\"? I don't think this\n> > is the case.\n> \n> Yes, that's why the restriction bugs me a bit too.\n\n Err I misread your point, _yes_ we do, see builtin-show-ref.c, or see\n--start-number in builtin-log.c. There is a precedent.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63482","messageId":"20071217211106.GC13004@artemis.madism.org","threadId":"11270","inReplyTo":"7vy7bt6qv6.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T21:11:06Z","receivedAt":"2007-12-17T21:11:06Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 07:52:29PM +0000, Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n> > After having written this mail 4 time already, I came up with an idea I\n> > kind of like: like find, we could make {} be a placeholder for the\n> > \"default\" argument. For example:\n> >\n> > $ git foo --abbrev {} 10\n> > $ git log -M {} 1\n> > ...\n> >\n> > {} would have the same semantics as your --long-opt-default. It tells the\n> > option parser that \"no there isn't anything to grok for that command thank you\n> > very much\". Of course if for some reason you really want to pass \"{}\" to the\n> > command, the stuck form holds:\n> >\n> > $ git foo --long-opt={}\n> > $ git foo -o{}\n> >\n> > What do you think ?\n> \n> 1. {} means a completely different thing to find (\"place the real value\n> here\"); there is no similarity. I would strongly oppose to it. If\n\n okay we could make it be '_', but …\n\n> you want to invoke opt with default but still want to pass \"{}\" as an\n> argument unrelated to that opt, you would do \"--opt={} {}\". That's\n> double ugly.\n\nActually that would be --opt {} {} (or --opt _ _) and indeed it's not\nvery nice (euphemism for it sucks hard).\n\nI like the *-default idea a lot, but it's not really useful if we cannot\nfix the single letter switches at the same time. I had the idea to use !\n(or any other less shell-magical char) like this: --abbrev! but it\ndoesn't fly a lot better for single letter switches.\n\nWell maybe it's not worth fighting any longer, we should stick to the\nstuck form then :)\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63487","messageId":"20071217212433.GD13004@artemis.madism.org","threadId":"11270","inReplyTo":"20071217205355.GC2105@coredump.intra.peff.net","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T21:24:33Z","receivedAt":"2007-12-17T21:24:33Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 08:53:55PM +0000, Jeff King wrote:\n> On Mon, Dec 17, 2007 at 12:42:51PM -0800, Junio C Hamano wrote:\n> \n> > You forgot one case.\n> > \n> > 4. Everybody who does _not_ know that we traditionally did not\n> > support the form would expect \"--abbrev 10\" and \"-n 4\" to work.\n> \n> I would expect \"-n 4\" to work, but not \"--abbrev 10\". But perhaps that\n> is just me. If that is the expectation, I think the behavior you\n> outlined is sensible.\n\nFWIW that's exactly the opposite for me. -n4 is easy to type, and I\nalways do that. Though on a keyboard, ' ' is under the thumb, '=' is\nharder to catch, so I tend to prefer when CLIs don't force me to use =.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63513","messageId":"20071217230729.GE2105@coredump.intra.peff.net","threadId":"11270","inReplyTo":"20071217210116.GB13004@artemis.madism.org","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-17T23:07:29Z","receivedAt":"2007-12-17T23:07:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 17, 2007 at 10:01:16PM +0100, Pierre Habouzit wrote:\n\n> Err I misread your point, _yes_ we do, see builtin-show-ref.c, or see\n> --start-number in builtin-log.c. There is a precedent.\n\nUgh. Well, in that case, it seems we are stuck with it, and I think\nthe behavior Junio laid out is the right course of action.\n\n-Peff\n"},{"id":"63519","messageId":"20071217231459.GA4422@artemis.madism.org","threadId":"11270","inReplyTo":"20071217230729.GE2105@coredump.intra.peff.net","subject":"Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T23:14:59Z","receivedAt":"2007-12-17T23:14:59Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 11:07:29PM +0000, Jeff King wrote:\n> On Mon, Dec 17, 2007 at 10:01:16PM +0100, Pierre Habouzit wrote:\n> \n> > Err I misread your point, _yes_ we do, see builtin-show-ref.c, or see\n> > --start-number in builtin-log.c. There is a precedent.\n> \n> Ugh. Well, in that case, it seems we are stuck with it, and I think\n> the behavior Junio laid out is the right course of action.\n\nWell I agree, I was mostly trying to show what the code could look like\nif we tried to be more clever. I'm fine for enforcing the sticked usage\nfor optional flags, I was the one advocating it in the first place\nanyways. I just wanted to be sure we didn't missed something obvious.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"}]}