{"thread":{"id":"10264","subject":"[RFC] CLI option parsing and usage generation for porcelains","startedAt":"2007-10-13T13:29:02Z","lastAt":"2007-10-16T13:05:26Z","messageCount":42,"participants":["Pierre Habouzit","Johannes Schindelin","Wincent Colaiuta","Alex Riesen","Eric Wong","Jonas Fonseca","Benoit SIGOURE","Andreas Ericsson","David Kastrup","Karl Hasselström","Chris Shoemaker","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"55602","messageId":"1192282153-26684-1-git-send-email-madcoder@debian.org","threadId":"10264","inReplyTo":null,"subject":"[RFC] CLI option parsing and usage generation for porcelains","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-13T13:29:02Z","receivedAt":"2007-10-13T13:29:02Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":" Following Kristian momentum, I've reworked his parse_option module\nquite a lot, and now have some quite interesting features. The series is\navailable from git://git.madism.org/git.git (branch ph/strbuf).\n\n The following series is open for comments, it's not 100% ready for\ninclusion IMHO, as some details may need to be sorted out first, and\nthat I've not re-read the patches thoroughly yet. Though I uses the tip\nof that branch as my everyday git for 2 weeks or so without any\nnoticeable issues.\n\n And as examples are always easier to grok:\n\n$ git fetch -h\nusage: git-fetch [options] [ ...]\n\n -q, --quiet be quiet\n -v, --verbose be verbose\n\n -a, --append append in .git/FETCH_HEAD\n -f, --force force non fast-forwards updates\n --no-tags don't follow tags at all\n -t, --tags fetch all tags\n --depth deepen history of a shallow clone\n\nAdvanced Options\n -k, --keep keep downloaded pack\n -u, --update-head-ok allow to update the head in the current branch\n --upload-pack path to git-upload-pack on the remote\n\n$ git rm -rf xdiff # yeah -rf now works !\nrm 'xdiff/xdiff.h'\nrm 'xdiff/xdiffi.c'\nrm 'xdiff/xdiffi.h'\nrm 'xdiff/xemit.c'\nrm 'xdiff/xemit.h'\nrm 'xdiff/xinclude.h'\nrm 'xdiff/xmacros.h'\nrm 'xdiff/xmerge.c'\nrm 'xdiff/xprepare.c'\nrm 'xdiff/xprepare.h'\nrm 'xdiff/xtypes.h'\nrm 'xdiff/xutils.c'\nrm 'xdiff/xutils.h'\n"},{"id":"55604","messageId":"Pine.LNX.4.64.0710131519510.25221@racer.site","threadId":"10264","inReplyTo":"1192282153-26684-2-git-send-email-madcoder@debian.org","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-13T14:39:10Z","receivedAt":"2007-10-13T14:39:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 13 Oct 2007, Pierre Habouzit wrote:\n\n> Aggregation of single switches is allowed:\n> -rC0 is the same as -r -C 0 (supposing that -C wants an arg).\n\nI'd be more interested in \"-rC 0\" working... Is that supported, too?\n\n> diff --git a/parse-options.c b/parse-options.c\n> new file mode 100644\n> index 0000000..07abb50\n> --- /dev/null\n> +++ b/parse-options.c\n> @@ -0,0 +1,227 @@\n> +#include \"git-compat-util.h\"\n> +#include \"parse-options.h\"\n> +#include \"strbuf.h\"\n> +\n> +#define OPT_SHORT 1\n> +#define OPT_UNSET 2\n> +\n> +struct optparse_t {\n> +\tconst char **argv;\n> +\tint argc;\n> +\tconst char *opt;\n> +};\n> +\n> +static inline const char *get_arg(struct optparse_t *p)\n> +{\n> +\tif (p->opt) {\n> +\t\tconst char *res = p->opt;\n> +\t\tp->opt = NULL;\n> +\t\treturn res;\n> +\t}\n> +\tp->argc--;\n> +\treturn *++p->argv;\n> +}\n\nThis is only used once; I wonder if it is really that more readable having \nthis as a function in its own right.\n\n> +static inline const char *skippfx(const char *str, const char *prefix)\n\nPersonally, I do not like abbreviations like that. They do not save that \nmuch screen estate (skip_prefix is only 4 characters longer, but much more \nreadable). Same goes for \"cnt\" later.\n\n> +static int get_value(struct optparse_t *p, struct option *opt, int flags)\n> +{\n> +\tif (p->opt && (flags & OPT_UNSET))\n> +\t\treturn opterror(opt, \"takes no value\", flags);\n> +\n> +\tswitch (opt->type) {\n> +\tcase OPTION_BOOLEAN:\n> +\t\tif (!(flags & OPT_SHORT) && p->opt)\n> +\t\t\treturn opterror(opt, \"takes no value\", flags);\n> +\t\tif (flags & OPT_UNSET) {\n> +\t\t\t*(int *)opt->value = 0;\n> +\t\t} else {\n> +\t\t\t(*(int *)opt->value)++;\n> +\t\t}\n> +\t\treturn 0;\n> +\n> +\tcase OPTION_STRING:\n> +\t\tif (flags & OPT_UNSET) {\n> +\t\t\t*(const char **)opt->value = (const char *)NULL;\n> +\t\t} else {\n> +\t\t\tif (!p->opt && p->argc < 1)\n> +\t\t\t\treturn opterror(opt, \"requires a value\", flags);\n> +\t\t\t*(const char **)opt->value = get_arg(p);\n> +\t\t}\n> +\t\treturn 0;\n> +\n> +\tcase OPTION_INTEGER:\n> +\t\tif (flags & OPT_UNSET) {\n> +\t\t\t*(int *)opt->value = 0;\n> +\t\t} else {\n> +\t\t\tconst char *s;\n> +\t\t\tif (!p->opt && p->argc < 1)\n> +\t\t\t\treturn opterror(opt, \"requires a value\", flags);\n> +\t\t\t*(int *)opt->value = strtol(*p->argv, (char **)&s, 10);\n> +\t\t\tif (*s)\n> +\t\t\t\treturn opterror(opt, \"expects a numerical value\", flags);\n> +\t\t}\n> +\t\treturn 0;\n> +\n> +\tdefault:\n> +\t\tdie(\"should not happen, someone must be hit on the forehead\");\n\n:-P\n\n> +static int parse_long_opt(struct optparse_t *p, const char *arg,\n> + struct option *options, int count)\n> +{\n> +\tint i;\n> +\n> +\tfor (i = 0; i < count; i++) {\n> +\t\tconst char *rest;\n> +\t\tint flags = 0;\n> +\t\t\n> +\t\tif (!options[i].long_name)\n> +\t\t\tcontinue;\n> +\n> +\t\trest = skippfx(arg, options[i].long_name);\n> +\t\tif (!rest && !strncmp(arg, \"no-\", 3)) {\n> +\t\t\trest = skippfx(arg + 3, options[i].long_name);\n> +\t\t\tflags |= OPT_SHORT;\n> +\t\t}\n\nWould this not be more intuitive as\n\n\t\tif (!prefixcmp(arg, \"no-\")) {\n\t\t\targ += 3;\n\t\t\tflags |= OPT_UNSET;\n\t\t}\n\t\trest = skip_prefix(arg, options[i].long_name);\n\nHm? (Note that I say UNSET, not SHORT... ;-)\n\n> +\t\tif (!rest)\n> +\t\t\tcontinue;\n> +\t\tif (*rest) {\n> +\t\t\tif (*rest != '=')\n> +\t\t\t\tcontinue;\n\nIs this really no error? For example, \"git log \n--decorate-walls-and-roofs\" would not fail...\n\n> +int parse_options(int argc, const char **argv,\n> + struct option *options, int count,\n> +\t\t\t\t const char * const usagestr[], int flags)\n\nPlease indent by the same amount.\n\n> +\t\tif (arg[1] != '-') {\n> +\t\t\toptp.opt = arg + 1;\n> +\t\t\tdo {\n> +\t\t\t\tif (*optp.opt == 'h')\n> +\t\t\t\t\tmake_usage(usagestr, options, count);\n\nHow about calling this \"usage_with_options()\"? With that name I expected \nmake_usage() to return a strbuf.\n\n> +\t\tif (!arg[2]) { /* \"--\" */\n> +\t\t\tif (!(flags & OPT_COPY_DASHDASH))\n> +\t\t\t\toptp.argc--, optp.argv++;\n\nI would prefer this as \n\n\t\t\tif (!(flags & OPT_COPY_DASHDASH)) {\n\t\t\t\toptp.argc--;\n\t\t\t\toptp.argv++;\n\t\t\t}\n\nWhile I'm at it: could you use \"args\" instead of \"optp\"? It is misleading \nboth in that it not only contains options (but other arguments, too) as in \nthat it is not a pointer (the trailing \"p\" is used as an indicator of that \nvery often, including git's source code).\n\nIn the same vein, OPT_COPY_DASHDASH should be named \nPARSE_OPT_KEEP_DASHDASH.\n\n> +\t\tif (opts->short_name) {\n> +\t\t\tstrbuf_addf(&sb, \"-%c\", opts->short_name);\n> +\t\t}\n> +\t\tif (opts->long_name) {\n> +\t\t\tstrbuf_addf(&sb, opts->short_name ? \", --%s\" : \"--%s\",\n> +\t\t\t\t\t\topts->long_name);\n> +\t\t}\n\nPlease lose the curly brackets.\n\n> +\t\tif (sb.len - pos <= USAGE_OPTS_WIDTH) {\n> +\t\t\tint pad = USAGE_OPTS_WIDTH - (sb.len - pos) + USAGE_GAP;\n> +\t\t\tstrbuf_addf(&sb, \"%*s%s\\n\", pad, \"\", opts->help);\n> +\t\t} else {\n> +\t\t\tstrbuf_addf(&sb, \"\\n%*s%s\\n\", USAGE_OPTS_WIDTH + USAGE_GAP, \"\",\n> +\t\t\t\t\t\topts->help);\n> +\t\t}\n\nSame here. (And I'd also make sure that the lines are not that long.)\n\n> diff --git a/parse-options.h b/parse-options.h\n> new file mode 100644\n> index 0000000..4b33d17\n> --- /dev/null\n> +++ b/parse-options.h\n> @@ -0,0 +1,37 @@\n> +#ifndef PARSE_OPTIONS_H\n> +#define PARSE_OPTIONS_H\n> +\n> +enum option_type {\n> +\tOPTION_BOOLEAN,\n\nI know that I proposed \"BOOLEAN\", but actually, you use it more like an \n\"INCREMENTAL\", right?\n\nOther than that: I like it very much.\n\nCiao,\nDscho\n"},{"id":"55607","messageId":"Pine.LNX.4.64.0710131544030.25221@racer.site","threadId":"10264","inReplyTo":"1192282153-26684-3-git-send-email-madcoder@debian.org","subject":"Re: [PATCH] Port builtin-add.c to use the new option parser.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-13T14:47:20Z","receivedAt":"2007-10-13T14:47:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 13 Oct 2007, Pierre Habouzit wrote:\n\n> +static struct option builtin_add_options[] = {\n> +\tOPT_BOOLEAN('i', \"interactive\", &add_interactive, \"interactive picking\"),\n> +\tOPT_BOOLEAN('n', NULL, &show_only, \"dry-run\"),\n> +\tOPT_BOOLEAN('f', NULL, &ignored_too, \"allow adding otherwise ignored files\"),\n> +\tOPT_BOOLEAN('v', NULL, &verbose, \"be verbose\"),\n> +\tOPT_BOOLEAN('u', NULL, &take_worktree_changes, \"update only files that git already knows about\"),\n> +\tOPT_BOOLEAN( 0 , \"refresh\", &refresh_only, \"don't add, only refresh stat() informations in the index\"),\n> +};\n\nI see you terminate the list by a \",\". How does this play with the option \nparser?\n\nThinking about this more, I am reverting my stance on the ARRAY_SIZE() \nissue. I think if you introduce a \"OPTION_NONE = 0\" in the enum, then \nthis single last comma should be enough.\n\nIn the same vein, you would not need the NULL in builtin_add_usage[], \nright?\n\nCiao,\nDscho\n"},{"id":"55609","messageId":"14AB1C41-E8D6-41E7-AE09-A85589A3FB92@wincent.com","threadId":"10264","inReplyTo":"1192282153-26684-1-git-send-email-madcoder@debian.org","subject":"Re: [RFC] CLI option parsing and usage generation for porcelains","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-10-13T14:53:10Z","receivedAt":"2007-10-13T14:53:10Z","isPatch":false,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 13/10/2007, a las 15:29, Pierre Habouzit escribió:\n\n> The following series is open for comments, it's not 100% ready for\n> inclusion IMHO, as some details may need to be sorted out first, and\n> that I've not re-read the patches thoroughly yet. Though I uses the \n> tip\n> of that branch as my everyday git for 2 weeks or so without any\n> noticeable issues.\n\nGreat to see two things:\n\n- the simplification in the commands switched over to use the options \nparser\n\n- the improved readability and usefulness of the options help\n\nGreat work, Pierre! I'll take a closer look at this and trial it in \nmy local Git install for a while to see if any issues come up.\n\nWincent\n"},{"id":"55610","messageId":"20071013145809.GG7110@artemis.corp","threadId":"10264","inReplyTo":"Pine.LNX.4.64.0710131519510.25221@racer.site","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-13T14:58:09Z","receivedAt":"2007-10-13T14:58:09Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sat, Oct 13, 2007 at 02:39:10PM +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sat, 13 Oct 2007, Pierre Habouzit wrote:\n> \n> > Aggregation of single switches is allowed:\n> > -rC0 is the same as -r -C 0 (supposing that -C wants an arg).\n> \n> I'd be more interested in \"-rC 0\" working... Is that supported, too?\n\n yes it is.\n\n> > +static inline const char *get_arg(struct optparse_t *p)\n> > +{\n> > +\tif (p->opt) {\n> > +\t\tconst char *res = p->opt;\n> > +\t\tp->opt = NULL;\n> > +\t\treturn res;\n> > +\t}\n> > +\tp->argc--;\n> > +\treturn *++p->argv;\n> > +}\n> \n> This is only used once; I wonder if it is really that more readable having \n> this as a function in its own right.\n\n it's used twice, and it makes the code more readable I believe.\n\n> > +static inline const char *skippfx(const char *str, const char *prefix)\n> \n> Personally, I do not like abbreviations like that. They do not save that \n> much screen estate (skip_prefix is only 4 characters longer, but much more \n> readable). Same goes for \"cnt\" later.\n\n Ack I'll fix that.\n\n> > +static int parse_long_opt(struct optparse_t *p, const char *arg,\n> > + struct option *options, int count)\n> > +{\n> > +\tint i;\n> > +\n> > +\tfor (i = 0; i < count; i++) {\n> > +\t\tconst char *rest;\n> > +\t\tint flags = 0;\n> > +\t\t\n> > +\t\tif (!options[i].long_name)\n> > +\t\t\tcontinue;\n> > +\n> > +\t\trest = skippfx(arg, options[i].long_name);\n> > +\t\tif (!rest && !strncmp(arg, \"no-\", 3)) {\n> > +\t\t\trest = skippfx(arg + 3, options[i].long_name);\n> > +\t\t\tflags |= OPT_SHORT;\n> > +\t\t}\n> \n> Would this not be more intuitive as\n> \n> \t\tif (!prefixcmp(arg, \"no-\")) {\n> \t\t\targ += 3;\n> \t\t\tflags |= OPT_UNSET;\n> \t\t}\n> \t\trest = skip_prefix(arg, options[i].long_name);\n\n Yes, that can be done indeed, but the point is, we have sometimes\noption whose long-name is \"no-foo\" (because it's what makes sense) but I\ncan rework that.\n\n> Hm? (Note that I say UNSET, not SHORT... ;-)\n\n fsck, good catch.\n\n> > +\t\tif (!rest)\n> > +\t\t\tcontinue;\n> > +\t\tif (*rest) {\n> > +\t\t\tif (*rest != '=')\n> > +\t\t\t\tcontinue;\n> \n> Is this really no error? For example, \"git log \n> --decorate-walls-and-roofs\" would not fail...\n\n it would be an error, it will yield a \"option not found\".\n\n> > +int parse_options(int argc, const char **argv,\n> > + struct option *options, int count,\n> > +\t\t\t\t const char * const usagestr[], int flags)\n> \n> Please indent by the same amount.\n\n oops, stupid space vs. tab thing.\n\n> > +\t\tif (arg[1] != '-') {\n> > +\t\t\toptp.opt = arg + 1;\n> > +\t\t\tdo {\n> > +\t\t\t\tif (*optp.opt == 'h')\n> > +\t\t\t\t\tmake_usage(usagestr, options, count);\n> \n> How about calling this \"usage_with_options()\"? With that name I expected \n> make_usage() to return a strbuf.\n\n will do.\n\n> > +\t\tif (!arg[2]) { /* \"--\" */\n> > +\t\t\tif (!(flags & OPT_COPY_DASHDASH))\n> > +\t\t\t\toptp.argc--, optp.argv++;\n> \n> I would prefer this as \n> \n> \t\t\tif (!(flags & OPT_COPY_DASHDASH)) {\n> \t\t\t\toptp.argc--;\n> \t\t\t\toptp.argv++;\n> \t\t\t}\n> \n> While I'm at it: could you use \"args\" instead of \"optp\"? It is misleading \n> both in that it not only contains options (but other arguments, too) as in \n> that it is not a pointer (the trailing \"p\" is used as an indicator of that \n> very often, including git's source code).\n\n okay.\n\n> In the same vein, OPT_COPY_DASHDASH should be named \n> PARSE_OPT_KEEP_DASHDASH.\n\n okay.\n\n> \n> > +\t\tif (opts->short_name) {\n> > +\t\t\tstrbuf_addf(&sb, \"-%c\", opts->short_name);\n> > +\t\t}\n> > +\t\tif (opts->long_name) {\n> > +\t\t\tstrbuf_addf(&sb, opts->short_name ? \", --%s\" : \"--%s\",\n> > +\t\t\t\t\t\topts->long_name);\n> > +\t\t}\n> \n> Please lose the curly brackets.\n> \n> > +\t\tif (sb.len - pos <= USAGE_OPTS_WIDTH) {\n> > +\t\t\tint pad = USAGE_OPTS_WIDTH - (sb.len - pos) + USAGE_GAP;\n> > +\t\t\tstrbuf_addf(&sb, \"%*s%s\\n\", pad, \"\", opts->help);\n> > +\t\t} else {\n> > +\t\t\tstrbuf_addf(&sb, \"\\n%*s%s\\n\", USAGE_OPTS_WIDTH + USAGE_GAP, \"\",\n> > +\t\t\t\t\t\topts->help);\n> > +\t\t}\n> \n> Same here. (And I'd also make sure that the lines are not that long.)\n\n okay.\n\n> \n> > diff --git a/parse-options.h b/parse-options.h\n> > new file mode 100644\n> > index 0000000..4b33d17\n> > --- /dev/null\n> > +++ b/parse-options.h\n> > @@ -0,0 +1,37 @@\n> > +#ifndef PARSE_OPTIONS_H\n> > +#define PARSE_OPTIONS_H\n> > +\n> > +enum option_type {\n> > +\tOPTION_BOOLEAN,\n> \n> I know that I proposed \"BOOLEAN\", but actually, you use it more like an \n> \"INCREMENTAL\", right?\n\n yes, I don't like _BOOLEAN either, I would have prefered _FLAG or sth\nlike that. INCREMENTAL is just too long.\n\n> Other than that: I like it very much.\n\n:P\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"55611","messageId":"20071013150306.GH7110@artemis.corp","threadId":"10264","inReplyTo":"Pine.LNX.4.64.0710131544030.25221@racer.site","subject":"Re: [PATCH] Port builtin-add.c to use the new option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-13T15:03:06Z","receivedAt":"2007-10-13T15:03:06Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sat, Oct 13, 2007 at 02:47:20PM +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sat, 13 Oct 2007, Pierre Habouzit wrote:\n> \n> > +static struct option builtin_add_options[] = {\n> > +\tOPT_BOOLEAN('i', \"interactive\", &add_interactive, \"interactive picking\"),\n> > +\tOPT_BOOLEAN('n', NULL, &show_only, \"dry-run\"),\n> > +\tOPT_BOOLEAN('f', NULL, &ignored_too, \"allow adding otherwise ignored files\"),\n> > +\tOPT_BOOLEAN('v', NULL, &verbose, \"be verbose\"),\n> > +\tOPT_BOOLEAN('u', NULL, &take_worktree_changes, \"update only files that git already knows about\"),\n> > +\tOPT_BOOLEAN( 0 , \"refresh\", &refresh_only, \"don't add, only refresh stat() informations in the index\"),\n> > +};\n> \n> I see you terminate the list by a \",\". How does this play with the option \n> parser?\n> \n> Thinking about this more, I am reverting my stance on the ARRAY_SIZE() \n> issue. I think if you introduce a \"OPTION_NONE = 0\" in the enum, then \n> this single last comma should be enough.\n\n adding a trailing comma does not add a NULL after that, it's ignored,\nyou're confused.\n\n ┌─(17:00)────\n └[artemis] cat a.c\n #include \n\n int main(void) {\n\tconst char * const arr[] = { \"1\", \"2\", };\n\tprintf(\"%d\\n\", sizeof(arr) / sizeof(arr[0]));\n\treturn 0;\n };\n ┌─(17:00)────\n └[artemis] ./a\n 2\n\n Very few compilers do not grok trailing commas, I always put them\nbecause it avoids spurious diffs for nothing, and that you can reorder\nlines easily too.\n\n Note that I don't really like using ARRAY_SIZE either, I kept it that\nway, but my taste would rather be to have an \"empty\" option, and\nexplicitely mark the end of the array.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"55622","messageId":"20071013191655.GA2875@steel.home","threadId":"10264","inReplyTo":"1192282153-26684-2-git-send-email-madcoder@debian.org","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-10-13T19:16:55Z","receivedAt":"2007-10-13T19:16:55Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Pierre Habouzit, Sat, Oct 13, 2007 15:29:03 +0200:\n> +static int opterror(struct option *opt, const char *reason, int flags)\n\n\"const struct option *opt\"? You never modify the struct option itself,\nonly the values under the pointers it contains. Using const here will\nallow the compiler to reuse string constants (not that there will be\nmuch of the opportunity, but anyway) in the option arrays.\n\n> +static int get_value(struct optparse_t *p, struct option *opt, int flags)\n\n\"const struct option *opt\"?\n\n> +static int parse_short_opt(struct optparse_t *p, struct option *options, int count)\n\n\"const struct option *options\"?\n\n> +int parse_options(int argc, const char **argv,\n> + struct option *options, int count,\n> +\t\t\t\t const char * const usagestr[], int flags)\n\n\"const struct option *options\"?\n\n> +void make_usage(const char * const usagestr[], struct option *opts, int cnt)\n\n\"const struct option *opts\"?\n\nWhy not \"const char *const *usagestr\"? Especially if you change\n\"usagestr\" (the pointer itself) later. \"[]\" is sometimes a hint that\nthe pointer itself should not be changed, being an array.\n\nAnd you want make opts const.\n\nBTW, it does not \"make\" usage. It calls the usage() or prints a usage\ndescription. \"make\" implies it creates the \"usage\", which according to\nthe prototype is later nowhere to be found.\n\n> +{\n> +\tstruct strbuf sb;\n> +\n> +\tstrbuf_init(&sb, 4096);\n> +\tdo {\n> +\t\tstrbuf_addstr(&sb, *usagestr++);\n> +\t\tstrbuf_addch(&sb, '\\n');\n> +\t} while (*usagestr);\n\nThis will crash for empty usagestr, like \"{ NULL }\". Was it\ndeliberately? (I'd make it deliberately, if I were you. I'd even used\ncnt of opts, to force people to document all options).\n\n> + strbuf_addf(&sb, \"\\n%*s%s\\n\", USAGE_OPTS_WIDTH + USAGE_GAP, \"\",\n> +\t\t opts->help);\n...\n> +\tusage(sb.buf);\n\nBTW, if you just printed the usage message out (it is about usage of a\nprogram, isn't it?) and called exit() everyone would be just as happy.\nAnd you wouldn't have to include strbuf (it is the only use of it),\nless code, too. It'd make simplier to stea^Wcopy your implementation,\nwhich I like :)\n"},{"id":"55623","messageId":"20071013192213.GB2875@steel.home","threadId":"10264","inReplyTo":"20071013150306.GH7110@artemis.corp","subject":"Re: [PATCH] Port builtin-add.c to use the new option parser.","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-10-13T19:22:13Z","receivedAt":"2007-10-13T19:22:13Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Pierre Habouzit, Sat, Oct 13, 2007 17:03:06 +0200:\n> On Sat, Oct 13, 2007 at 02:47:20PM +0000, Johannes Schindelin wrote:\n> > Thinking about this more, I am reverting my stance on the ARRAY_SIZE() \n> > issue. I think if you introduce a \"OPTION_NONE = 0\" in the enum, then \n> > this single last comma should be enough.\n> \n> adding a trailing comma does not add a NULL after that, it's ignored,\n> you're confused.\n\nYep\n\n> Note that I don't really like using ARRAY_SIZE either, I kept it that\n> way, but my taste would rather be to have an \"empty\" option, and\n> explicitely mark the end of the array.\n\nYou can have both. Just stop at NULL-entry or when the 'size' elements\npassed, whatever happens first.\n"},{"id":"55643","messageId":"20071013202706.GJ7110@artemis.corp","threadId":"10264","inReplyTo":"20071013192213.GB2875@steel.home","subject":"Re: [PATCH] Port builtin-add.c to use the new option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-13T20:27:06Z","receivedAt":"2007-10-13T20:27:06Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sat, Oct 13, 2007 at 07:22:13PM +0000, Alex Riesen wrote:\n> Pierre Habouzit, Sat, Oct 13, 2007 17:03:06 +0200:\n> > On Sat, Oct 13, 2007 at 02:47:20PM +0000, Johannes Schindelin wrote:\n> > > Thinking about this more, I am reverting my stance on the ARRAY_SIZE() \n> > > issue. I think if you introduce a \"OPTION_NONE = 0\" in the enum, then \n> > > this single last comma should be enough.\n> > \n> > adding a trailing comma does not add a NULL after that, it's ignored,\n> > you're confused.\n> \n> Yep\n> \n> > Note that I don't really like using ARRAY_SIZE either, I kept it that\n> > way, but my taste would rather be to have an \"empty\" option, and\n> > explicitely mark the end of the array.\n> \n> You can have both. Just stop at NULL-entry or when the 'size' elements\n> passed, whatever happens first.\n\n Well I dislike the \"count\" thing, and Dscho agreed that it somehow\nsucked too. If you go see the current state of the ph/parseopt series\nyou'll see it's not here anymore.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"55647","messageId":"20071013205404.GK7110@artemis.corp","threadId":"10264","inReplyTo":"20071013191655.GA2875@steel.home","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-13T20:54:04Z","receivedAt":"2007-10-13T20:54:04Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sat, Oct 13, 2007 at 07:16:55PM +0000, Alex Riesen wrote:\n> Pierre Habouzit, Sat, Oct 13, 2007 15:29:03 +0200:\n> > [...]\n> \n> \"const struct option *opts\"?\n> \n> Why not \"const char *const *usagestr\"? Especially if you change\n> \"usagestr\" (the pointer itself) later. \"[]\" is sometimes a hint that\n> the pointer itself should not be changed, being an array.\n> \n> And you want make opts const.\n\n Ok.\n\n> BTW, it does not \"make\" usage. It calls the usage() or prints a usage\n> description. \"make\" implies it creates the \"usage\", which according to\n> the prototype is later nowhere to be found.\n\n Yes this has been spotted and fixed already.\n\n> \n> > +{\n> > +\tstruct strbuf sb;\n> > +\n> > +\tstrbuf_init(&sb, 4096);\n> > +\tdo {\n> > +\t\tstrbuf_addstr(&sb, *usagestr++);\n> > +\t\tstrbuf_addch(&sb, '\\n');\n> > +\t} while (*usagestr);\n> \n> This will crash for empty usagestr, like \"{ NULL }\". Was it\n> deliberately? (I'd make it deliberately, if I were you. I'd even used\n> cnt of opts, to force people to document all options).\n\n Yes this is intentional, there should be at least on string in the\nusagestr array.\n\n\n> > + strbuf_addf(&sb, \"\\n%*s%s\\n\", USAGE_OPTS_WIDTH + USAGE_GAP, \"\",\n> > +\t\t opts->help);\n> ....\n> > +\tusage(sb.buf);\n> \n> BTW, if you just printed the usage message out (it is about usage of a\n> program, isn't it?) and called exit() everyone would be just as happy.\n> And you wouldn't have to include strbuf (it is the only use of it),\n> less code, too. It'd make simplier to stea^Wcopy your implementation,\n> which I like :)\n\n the reason is that usage() is a wrapper around a callback, and I\nsuppose it's used by some GUI's or anything like that.\n\n FWIW you can rework the .c like this:\n\n pos = 0; /* and not pos = sb.len */\n\n replace the strbuf_add* by the equivalents:\n pos += printf(\"....\");\n\n and tada, you're done.\n\n\n Note that in the most recent version, I also deal with a\nOPTION_CALLBACK that passes the value to a callback.\n\nCheers,\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"55650","messageId":"20071013221450.GC2875@steel.home","threadId":"10264","inReplyTo":"20071013205404.GK7110@artemis.corp","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-10-13T22:14:50Z","receivedAt":"2007-10-13T22:14:50Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Pierre Habouzit, Sat, Oct 13, 2007 22:54:04 +0200:\n> On Sat, Oct 13, 2007 at 07:16:55PM +0000, Alex Riesen wrote:\n> > Pierre Habouzit, Sat, Oct 13, 2007 15:29:03 +0200:\n> > BTW, if you just printed the usage message out (it is about usage of a\n> > program, isn't it?) and called exit() everyone would be just as happy.\n> > And you wouldn't have to include strbuf (it is the only use of it),\n> > less code, too. It'd make simplier to stea^Wcopy your implementation,\n> > which I like :)\n> \n> the reason is that usage() is a wrapper around a callback, and I\n> suppose it's used by some GUI's or anything like that.\n\nIt is not. Not yet. What could they use a usage text for?\nBesides, you could just export the callback (call_usage_callback or\nsomething) from usage.c and call it.\n\n> FWIW you can rework the .c like this:\n\non top of yours:\n\nFrom: Alex Riesen \nDate: Sun, 14 Oct 2007 00:10:51 +0200\nSubject: [PATCH] Rework make_usage to print the usage message immediately\n\nSigned-off-by: Alex Riesen \n---\n parse-options.c | 60 ++++++++++++++++++++++++------------------------------\n 1 files changed, 27 insertions(+), 33 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 07abb50..1e3940f 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -1,6 +1,5 @@\n #include \"git-compat-util.h\"\n #include \"parse-options.h\"\n-#include \"strbuf.h\"\n \n #define OPT_SHORT 1\n #define OPT_UNSET 2\n@@ -171,57 +170,52 @@ int parse_options(int argc, const char **argv,\n \n void make_usage(const char * const usagestr[], struct option *opts, int cnt)\n {\n-\tstruct strbuf sb;\n-\n-\tstrbuf_init(&sb, 4096);\n-\tdo {\n-\t\tstrbuf_addstr(&sb, *usagestr++);\n-\t\tstrbuf_addch(&sb, '\\n');\n-\t} while (*usagestr);\n+\tfprintf(stderr, \"usage: \");\n+\twhile (*usagestr)\n+\t\tfprintf(stderr, \"%s\\n\", *usagestr++);\n \n \tif (cnt && opts->type != OPTION_GROUP)\n-\t\tstrbuf_addch(&sb, '\\n');\n+\t\tfputc('\\n', stderr);\n \n \tfor (; cnt-- > 0; opts++) {\n \t\tsize_t pos;\n \n \t\tif (opts->type == OPTION_GROUP) {\n-\t\t\tstrbuf_addch(&sb, '\\n');\n+\t\t\tfputc('\\n', stderr);\n \t\t\tif (*opts->help)\n-\t\t\t\tstrbuf_addf(&sb, \"%s\\n\", opts->help);\n+\t\t\t\tfprintf(stderr, \"%s\\n\", opts->help);\n \t\t\tcontinue;\n \t\t}\n \n-\t\tpos = sb.len;\n-\t\tstrbuf_addstr(&sb, \" \");\n-\t\tif (opts->short_name) {\n-\t\t\tstrbuf_addf(&sb, \"-%c\", opts->short_name);\n-\t\t}\n-\t\tif (opts->long_name) {\n-\t\t\tstrbuf_addf(&sb, opts->short_name ? \", --%s\" : \"--%s\",\n-\t\t\t\t\t\topts->long_name);\n-\t\t}\n+\t\tpos = fprintf(stderr, \" \");\n+\t\tif (opts->short_name)\n+\t\t\tpos += fprintf(stderr, \"-%c\", opts->short_name);\n+\t\tif (opts->long_name)\n+\t\t\tpos += fprintf(stderr,\n+\t\t\t\t opts->short_name ? \", --%s\" : \"--%s\",\n+\t\t\t\t opts->long_name);\n \t\tswitch (opts->type) {\n \t\tcase OPTION_INTEGER:\n-\t\t\tstrbuf_addstr(&sb, \" \");\n+\t\t\tfputs(\" \", stderr);\n+\t\t\tpos += 4;\n \t\t\tbreak;\n \t\tcase OPTION_STRING:\n-\t\t\tif (opts->argh) {\n-\t\t\t\tstrbuf_addf(&sb, \" <%s>\", opts->argh);\n-\t\t\t} else {\n-\t\t\t\tstrbuf_addstr(&sb, \" ...\");\n+\t\t\tif (opts->argh)\n+\t\t\t\tpos += fprintf(stderr, \" <%s>\", opts->argh);\n+\t\t\telse {\n+\t\t\t\tfputs(\" ...\", stderr);\n+\t\t\t\tpos += 4;\n \t\t\t}\n \t\t\tbreak;\n \t\tdefault:\n \t\t\tbreak;\n \t\t}\n-\t\tif (sb.len - pos <= USAGE_OPTS_WIDTH) {\n-\t\t\tint pad = USAGE_OPTS_WIDTH - (sb.len - pos) + USAGE_GAP;\n-\t\t\tstrbuf_addf(&sb, \"%*s%s\\n\", pad, \"\", opts->help);\n-\t\t} else {\n-\t\t\tstrbuf_addf(&sb, \"\\n%*s%s\\n\", USAGE_OPTS_WIDTH + USAGE_GAP, \"\",\n-\t\t\t\t\t\topts->help);\n-\t\t}\n+\t\tif (pos <= USAGE_OPTS_WIDTH) {\n+\t\t\tint pad = USAGE_OPTS_WIDTH - pos + USAGE_GAP;\n+\t\t\tfprintf(stderr, \"%*s%s\\n\", pad, \"\", opts->help);\n+\t\t} else\n+\t\t\tfprintf(stderr, \"\\n%*s%s\\n\",\n+\t\t\t\tUSAGE_OPTS_WIDTH + USAGE_GAP, \"\", opts->help);\n \t}\n-\tusage(sb.buf);\n+\texit(129);\n }\n-- \n1.5.3.4.232.ga843\n"},{"id":"55668","messageId":"20071014070219.GA1198@artemis.corp","threadId":"10264","inReplyTo":"20071013221450.GC2875@steel.home","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-14T07:02:19Z","receivedAt":"2007-10-14T07:02:19Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sat, Oct 13, 2007 at 10:14:50PM +0000, Alex Riesen wrote:\n> Pierre Habouzit, Sat, Oct 13, 2007 22:54:04 +0200:\n> > On Sat, Oct 13, 2007 at 07:16:55PM +0000, Alex Riesen wrote:\n> > > Pierre Habouzit, Sat, Oct 13, 2007 15:29:03 +0200:\n> > > BTW, if you just printed the usage message out (it is about usage of a\n> > > program, isn't it?) and called exit() everyone would be just as happy.\n> > > And you wouldn't have to include strbuf (it is the only use of it),\n> > > less code, too. It'd make simplier to stea^Wcopy your implementation,\n> > > which I like :)\n> > \n> > the reason is that usage() is a wrapper around a callback, and I\n> > suppose it's used by some GUI's or anything like that.\n> \n> It is not. Not yet. What could they use a usage text for?\n> Besides, you could just export the callback (call_usage_callback or\n> something) from usage.c and call it.\n\n Okay makes sense.\n\n> > FWIW you can rework the .c like this:\n> \n> on top of yours:\n\n Added (reworked a bit for the current state of parse_options), and pushed.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"55687","messageId":"20071014091855.GA17397@soma","threadId":"10264","inReplyTo":"1192282153-26684-1-git-send-email-madcoder@debian.org","subject":"Re: [RFC] CLI option parsing and usage generation for porcelains","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-10-14T09:18:55Z","receivedAt":"2007-10-14T09:18:55Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Pierre Habouzit wrote:\n> Following Kristian momentum, I've reworked his parse_option module\n> quite a lot, and now have some quite interesting features. The series is\n> available from git://git.madism.org/git.git (branch ph/strbuf).\n> \n> The following series is open for comments, it's not 100% ready for\n> inclusion IMHO, as some details may need to be sorted out first, and\n> that I've not re-read the patches thoroughly yet. Though I uses the tip\n> of that branch as my everyday git for 2 weeks or so without any\n> noticeable issues.\n> \n> And as examples are always easier to grok:\n> \n> $ git fetch -h\n> usage: git-fetch [options] [ ...]\n> \n> -q, --quiet be quiet\n> -v, --verbose be verbose\n> \n> -a, --append append in .git/FETCH_HEAD\n> -f, --force force non fast-forwards updates\n> --no-tags don't follow tags at all\n> -t, --tags fetch all tags\n> --depth deepen history of a shallow clone\n> \n> Advanced Options\n> -k, --keep keep downloaded pack\n> -u, --update-head-ok allow to update the head in the current branch\n> --upload-pack path to git-upload-pack on the remote\n> \n> $ git rm -rf xdiff # yeah -rf now works !\n\nVery nice. I worked on gitopt around summer of 2006 but never had the\ntime to test it thoroughly. It was a _lot_ more intrusive than yours\ncurrently is (it touched the diff + revision family of commands).\n\nOne feature I really like is automatically handling of long option\nabbreviations. gitopt supported this at the expense of complexity\nand the aforementioned intrusivenes. This allows automatic handling\nof the abbreviation style seen commonly in git shell scripts:\n\n --a|--am|--ame|--amen|--amend) (from git-commit.sh)\n\n-- \nEric Wong\n"},{"id":"55692","messageId":"20071014095755.GF1198@artemis.corp","threadId":"10264","inReplyTo":"20071014091855.GA17397@soma","subject":"Re: [RFC] CLI option parsing and usage generation for porcelains","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-14T09:57:55Z","receivedAt":"2007-10-14T09:57:55Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Oct 14, 2007 at 09:18:55AM +0000, Eric Wong wrote:\n> Pierre Habouzit wrote:\n> > Following Kristian momentum, I've reworked his parse_option module\n> > quite a lot, and now have some quite interesting features. The series is\n> > available from git://git.madism.org/git.git (branch ph/strbuf).\n> > \n> > The following series is open for comments, it's not 100% ready for\n> > inclusion IMHO, as some details may need to be sorted out first, and\n> > that I've not re-read the patches thoroughly yet. Though I uses the tip\n> > of that branch as my everyday git for 2 weeks or so without any\n> > noticeable issues.\n> > \n> > And as examples are always easier to grok:\n> > \n> > $ git fetch -h\n> > usage: git-fetch [options] [ ...]\n> > \n> > -q, --quiet be quiet\n> > -v, --verbose be verbose\n> > \n> > -a, --append append in .git/FETCH_HEAD\n> > -f, --force force non fast-forwards updates\n> > --no-tags don't follow tags at all\n> > -t, --tags fetch all tags\n> > --depth deepen history of a shallow clone\n> > \n> > Advanced Options\n> > -k, --keep keep downloaded pack\n> > -u, --update-head-ok allow to update the head in the current branch\n> > --upload-pack path to git-upload-pack on the remote\n> > \n> > $ git rm -rf xdiff # yeah -rf now works !\n> \n> Very nice. I worked on gitopt around summer of 2006 but never had the\n> time to test it thoroughly. It was a _lot_ more intrusive than yours\n> currently is (it touched the diff + revision family of commands).\n> \n> One feature I really like is automatically handling of long option\n> abbreviations. gitopt supported this at the expense of complexity\n> and the aforementioned intrusivenes. This allows automatic handling\n> of the abbreviation style seen commonly in git shell scripts:\n> \n> --a|--am|--ame|--amen|--amend) (from git-commit.sh)\n\n Yes, but if you do that, you can't order options in the order you\nwant (because of first match issues), making the help dumps hopelessly\nrandom. I prefer exact match, especially since your shell can help you\nautocomplete the proper command.\n\n I intend to have some magic in the parse_options module to dump the\noptions in a machine parseable way, so that zsh/bash completion for the\nparseopt aware commands is almost trivial. (this was requested from one\nof the zsh upstream developpers, and it definitely make sense).\n\n Note that I didn't migrated all the commands yet especially not\ndiff.c, We'll need a new construct for that: embedding a struct options\narray into another to inherit its flags, though I'm not sure it's\nenough, as a struct options right now embeds pointers to the variables\nit fills, which doesn't work with the \"pure\" `diff_opt_parse` approach\nright now. But I'm sure I'll come up with something :)\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"55718","messageId":"20071014140116.GA20970@diku.dk","threadId":"10264","inReplyTo":"1192282153-26684-10-git-send-email-madcoder@debian.org","subject":"[PATCH] Simplify usage string printing","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2007-10-14T14:01:16Z","receivedAt":"2007-10-14T14:01:16Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"Signed-off-by: Jonas Fonseca \n---\n builtin-branch.c | 1 -\n builtin-update-ref.c | 1 -\n parse-options.c | 2 +-\n 3 files changed, 1 insertions(+), 3 deletions(-)\n\n Pierre Habouzit wrote Sat, Oct 13, 2007:\n > Signed-off-by: Pierre Habouzit \n > ---\n > builtin-update-ref.c | 71 +++++++++++++++++++++-----------------------------\n > 1 files changed, 30 insertions(+), 41 deletions(-)\n > \n > diff --git a/builtin-update-ref.c b/builtin-update-ref.c\n > index fe1f74c..eafb642 100644\n > --- a/builtin-update-ref.c\n > +++ b/builtin-update-ref.c\n > @@ -1,59 +1,48 @@\n > #include \"cache.h\"\n > #include \"refs.h\"\n > #include \"builtin.h\"\n > +#include \"parse-options.h\"\n > \n > -static const char git_update_ref_usage[] =\n > -\"git-update-ref [-m ] (-d | [--no-deref] [])\";\n > +static const char * const git_update_ref_usage[] = {\n > +\t\"\",\n > +\t\"git-update-ref [options] -d \",\n > +\t\"git-update-ref [options] []\",\n > +\tNULL\n > +};\n\n How about something like this to get rid of these empty strings\n that look strange?\n\n\t> ./git update-ref -h\n\tusage: git-update-ref [options] -d \n\t or: git-update-ref [options] []\n\n\t -m reason of the update\n\t -d deletes the reference\n\t --no-deref update not the one it points to\n\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex fbf983e..d7c4657 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -14,7 +14,6 @@\n #include \"parse-options.h\"\n \n static const char * const builtin_branch_usage[] = {\n-\t\"\",\n \t\"git-branch [options] [-r | -a]\",\n \t\"git-branch [options] [-l] [-f] []\",\n \t\"git-branch [options] [-r] (-d | -D) \",\ndiff --git a/builtin-update-ref.c b/builtin-update-ref.c\nindex d66d9b5..0cd7817 100644\n--- a/builtin-update-ref.c\n+++ b/builtin-update-ref.c\n@@ -4,7 +4,6 @@\n #include \"parse-options.h\"\n \n static const char * const git_update_ref_usage[] = {\n-\t\"\",\n \t\"git-update-ref [options] -d \",\n \t\"git-update-ref [options] []\",\n \tNULL\ndiff --git a/parse-options.c b/parse-options.c\nindex c45bb9b..b1d9608 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -181,7 +181,7 @@ void usage_with_options(const char * const *usagestr,\n {\n \tfprintf(stderr, \"usage: %s\\n\", *usagestr);\n \twhile (*++usagestr)\n-\t\tfprintf(stderr, \" %s\\n\", *usagestr);\n+\t\tfprintf(stderr, \" or: %s\\n\", *usagestr);\n \n \tif (opts->type != OPTION_GROUP)\n \t\tfputc('\\n', stderr);\n-- \n1.5.3.4.1166.gf076\n\n-- \nJonas Fonseca\n"},{"id":"55719","messageId":"20071014141042.GA21197@diku.dk","threadId":"10264","inReplyTo":"1192282153-26684-2-git-send-email-madcoder@debian.org","subject":"[PATCH] Update manpages to reflect new short and long option aliases","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2007-10-14T14:10:42Z","receivedAt":"2007-10-14T14:10:42Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"Signed-off-by: Jonas Fonseca \n---\n Documentation/git-add.txt | 4 ++--\n Documentation/git-branch.txt | 2 +-\n Documentation/git-mv.txt | 2 +-\n Documentation/git-rm.txt | 4 ++--\n Documentation/git-symbolic-ref.txt | 2 +-\n 5 files changed, 7 insertions(+), 7 deletions(-)\n\n Maybe this should wait, but it is just to document that this series (as\n of the version in your git tree) also adds new option aliases.\n \n BTW, I didn't bother to change the synopsis lines but maybe I should.\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 2fe7355..963e1ab 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -50,10 +50,10 @@ OPTIONS\n \tand `dir/file2`) can be given to add all files in the\n \tdirectory, recursively.\n \n--n::\n+-n, \\--dry-run::\n Don't actually add the file(s), just show if they exist.\n \n--v::\n+-v, \\--verbose::\n Be verbose.\n \n -f::\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex b7285bc..5e81aa4 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -85,7 +85,7 @@ OPTIONS\n -a::\n \tList both remote-tracking branches and local branches.\n \n--v::\n+-v, --verbose::\n \tShow sha1 and commit subject line for each head.\n \n --abbrev=::\ndiff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt\nindex 2c9cf74..3b8ca76 100644\n--- a/Documentation/git-mv.txt\n+++ b/Documentation/git-mv.txt\n@@ -34,7 +34,7 @@ OPTIONS\n \tcondition. An error happens when a source is neither existing nor\n controlled by GIT, or when it would overwrite an existing\n file unless '-f' is given.\n--n::\n+-n, \\--dry-run::\n \tDo nothing; only show what would happen\n \n \ndiff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt\nindex be61a82..48c1d97 100644\n--- a/Documentation/git-rm.txt\n+++ b/Documentation/git-rm.txt\n@@ -30,7 +30,7 @@ OPTIONS\n -f::\n \tOverride the up-to-date check.\n \n--n::\n+-n, \\--dry-run::\n Don't actually remove the file(s), just show if they exist in\n the index.\n \n@@ -51,7 +51,7 @@ OPTIONS\n \\--ignore-unmatch::\n \tExit with a zero status even if no files matched.\n \n-\\--quiet::\n+-q, \\--quiet::\n \tgit-rm normally outputs one line (in the form of an \"rm\" command)\n \tfor each file removed. This option suppresses that output.\n \ndiff --git a/Documentation/git-symbolic-ref.txt b/Documentation/git-symbolic-ref.txt\nindex a88f722..694caba 100644\n--- a/Documentation/git-symbolic-ref.txt\n+++ b/Documentation/git-symbolic-ref.txt\n@@ -26,7 +26,7 @@ a regular file whose contents is `ref: refs/heads/master`.\n OPTIONS\n -------\n \n--q::\n+-q, --quiet::\n \tDo not issue an error message if the is not a\n \tsymbolic ref but a detached HEAD; instead exit with\n \tnon-zero status silently.\n-- \n1.5.3.4.1166.gf076\n\n-- \nJonas Fonseca\n"},{"id":"55732","messageId":"20071014162628.GG1198@artemis.corp","threadId":"10264","inReplyTo":"20071014140116.GA20970@diku.dk","subject":"Re: [PATCH] Simplify usage string printing","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-14T16:26:28Z","receivedAt":"2007-10-14T16:26:28Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Oct 14, 2007 at 02:01:16PM +0000, Jonas Fonseca wrote:\n> Signed-off-by: Jonas Fonseca \n> ---\n> builtin-branch.c | 1 -\n> builtin-update-ref.c | 1 -\n> parse-options.c | 2 +-\n> 3 files changed, 1 insertions(+), 3 deletions(-)\n> \n> Pierre Habouzit wrote Sat, Oct 13, 2007:\n> > Signed-off-by: Pierre Habouzit \n> > ---\n> > builtin-update-ref.c | 71 +++++++++++++++++++++-----------------------------\n> > 1 files changed, 30 insertions(+), 41 deletions(-)\n> > \n> > diff --git a/builtin-update-ref.c b/builtin-update-ref.c\n> > index fe1f74c..eafb642 100644\n> > --- a/builtin-update-ref.c\n> > +++ b/builtin-update-ref.c\n> > @@ -1,59 +1,48 @@\n> > #include \"cache.h\"\n> > #include \"refs.h\"\n> > #include \"builtin.h\"\n> > +#include \"parse-options.h\"\n> > \n> > -static const char git_update_ref_usage[] =\n> > -\"git-update-ref [-m ] (-d | [--no-deref] [])\";\n> > +static const char * const git_update_ref_usage[] = {\n> > +\t\"\",\n> > +\t\"git-update-ref [options] -d \",\n> > +\t\"git-update-ref [options] []\",\n> > +\tNULL\n> > +};\n> \n> How about something like this to get rid of these empty strings\n> that look strange?\n> \n> \t> ./git update-ref -h\n> \tusage: git-update-ref [options] -d \n> \t or: git-update-ref [options] []\n\n I like the idea, though we may want to have more text to explain some\nthings about the command, so I'll do something in between that uses or:\nuntil an empty line is met, and just prefix the result with four spaces\nelse, this way we can have:\n\nusage: git-foo ...\n or: git-foo ...\n\n Did you know that you can do bar with git-foo ?\n but beware that it cannot do quux.\n\n -m reason of the update\n -d deletes the reference\n --no-deref update not the one it points to\n\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"55733","messageId":"20071014162653.GH1198@artemis.corp","threadId":"10264","inReplyTo":"20071014141042.GA21197@diku.dk","subject":"Re: [PATCH] Update manpages to reflect new short and long option aliases","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-14T16:26:53Z","receivedAt":"2007-10-14T16:26:53Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":" added and pushed.\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"55738","messageId":"Pine.LNX.4.64.0710141751530.25221@racer.site","threadId":"10264","inReplyTo":"20071014095755.GF1198@artemis.corp","subject":"[PATCH] parse-options: Allow abbreviated options when unambiguous","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-14T16:54:06Z","receivedAt":"2007-10-14T16:54:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWhen there is an option \"--amend\", the option parser now recognizes\n\"--am\" for that option, provided that there is no other option beginning\nwith \"--am\".\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Sun, 14 Oct 2007, Pierre Habouzit wrote:\n\n\t> On Sun, Oct 14, 2007 at 09:18:55AM +0000, Eric Wong wrote:\n\t> > \n\t> > One feature I really like is automatically handling of long \n\t> > option abbreviations. gitopt supported this at the expense of \n\t> > complexity and the aforementioned intrusivenes. This allows \n\t> > automatic handling of the abbreviation style seen commonly in \n\t> > git shell scripts:\n\t> > \n\t> > --a|--am|--ame|--amen|--amend) (from git-commit.sh)\n\t> \n\t> Yes, but if you do that, you can't order options in the order \n\t> you want (because of first match issues), making the help dumps \n\t> hopelessly random.\n\nI think this patch proves that you do not need to order the options...\n\n;-)\n\n parse-options.c | 32 ++++++++++++++++++++++++++++++++\n t/t0040-parse-options.sh | 22 ++++++++++++++++++++++\n 2 files changed, 54 insertions(+), 0 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 72656a8..afc6c89 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -102,6 +102,13 @@ static int parse_short_opt(struct optparse_t *p, const struct option *options)\n static int parse_long_opt(struct optparse_t *p, const char *arg,\n const struct option *options)\n {\n+\tconst char *arg_end = strchr(arg, '=');\n+\tconst struct option *abbrev_option = NULL;\n+\tint abbrev_flags = 0;\n+\n+\tif (!arg_end)\n+\t\targ_end = arg + strlen(arg);\n+\n \tfor (; options->type != OPTION_END; options++) {\n \t\tconst char *rest;\n \t\tint flags = 0;\n@@ -111,10 +118,33 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,\n \n \t\trest = skip_prefix(arg, options->long_name);\n \t\tif (!rest) {\n+\t\t\t/* abbreviated? */\n+\t\t\tif (!strncmp(options->long_name, arg, arg_end - arg)) {\n+is_abbreviated:\n+\t\t\t\tif (abbrev_option)\n+\t\t\t\t\tdie (\"Ambiguous option: %s \"\n+\t\t\t\t\t\t\"(could be --%s%s or --%s%s)\",\n+\t\t\t\t\t\targ,\n+\t\t\t\t\t\t(flags & OPT_UNSET) ?\n+\t\t\t\t\t\t\t\"no-\" : \"\",\n+\t\t\t\t\t\toptions->long_name,\n+\t\t\t\t\t\t(abbrev_flags & OPT_UNSET) ?\n+\t\t\t\t\t\t\t\"no-\" : \"\",\n+\t\t\t\t\t\tabbrev_option->long_name);\n+\t\t\t\tif (!(flags & OPT_UNSET) && *arg_end)\n+\t\t\t\t\tp->opt = arg_end + 1;\n+\t\t\t\tabbrev_option = options;\n+\t\t\t\tabbrev_flags = flags;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\t/* negated? */\n \t\t\tif (strncmp(arg, \"no-\", 3))\n \t\t\t\tcontinue;\n \t\t\tflags |= OPT_UNSET;\n \t\t\trest = skip_prefix(arg + 3, options->long_name);\n+\t\t\t/* abbreviated and negated? */\n+\t\t\tif (!rest && !prefixcmp(options->long_name, arg + 3))\n+\t\t\t\tgoto is_abbreviated;\n \t\t\tif (!rest)\n \t\t\t\tcontinue;\n \t\t}\n@@ -125,6 +155,8 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,\n \t\t}\n \t\treturn get_value(p, options, flags);\n \t}\n+\tif (abbrev_option)\n+\t\treturn get_value(p, abbrev_option, abbrev_flags);\n \treturn error(\"unknown option `%s'\", arg);\n }\n \ndiff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh\nindex 09b3230..e4dd86f 100755\n--- a/t/t0040-parse-options.sh\n+++ b/t/t0040-parse-options.sh\n@@ -66,4 +66,26 @@ test_expect_success 'intermingled arguments' '\n \tgit diff expect output\n '\n \n+cat > expect << EOF\n+boolean: 0\n+integer: 2\n+string: (not set)\n+EOF\n+\n+test_expect_success 'unambiguously abbreviated option' '\n+\ttest-parse-options --int 2 --boolean --no-bo > output 2> output.err &&\n+\ttest ! -s output.err &&\n+\tgit diff expect output\n+'\n+\n+test_expect_success 'unambiguously abbreviated option with \"=\"' '\n+\ttest-parse-options --int=2 > output 2> output.err &&\n+\ttest ! -s output.err &&\n+\tgit diff expect output\n+'\n+\n+test_expect_failure 'ambiguously abbreviated option' '\n+\ttest-parse-options --strin 123\n+'\n+\n test_done\n-- \n1.5.3.4.1174.gcd0d6-dirty\n"},{"id":"55754","messageId":"Pine.LNX.4.64.0710141901450.25221@racer.site","threadId":"10264","inReplyTo":"Pine.LNX.4.64.0710141751530.25221@racer.site","subject":"Re: [PATCH] parse-options: Allow abbreviated options when unambiguous","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-14T18:02:33Z","receivedAt":"2007-10-14T18:02:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 14 Oct 2007, Johannes Schindelin wrote:\n\n> When there is an option \"--amend\", the option parser now recognizes \n> \"--am\" for that option, provided that there is no other option beginning \n> with \"--am\".\n\nAnd an amend for ultra-abbreviated options (as you noticed on IRC):\n\ndiff --git a/parse-options.c b/parse-options.c\nindex afc6c89..acabb98 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -137,6 +137,11 @@ is_abbreviated:\n \t\t\t\tabbrev_flags = flags;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\t/* negated and abbreviated very much? */\n+\t\t\tif (!prefixcmp(\"no-\", arg)) {\n+\t\t\t\tflags |= OPT_UNSET;\n+\t\t\t\tgoto is_abbreviated;\n+\t\t\t}\n \t\t\t/* negated? */\n \t\t\tif (strncmp(arg, \"no-\", 3))\n \t\t\t\tcontinue;\n"},{"id":"55756","messageId":"20071014180815.GK1198@artemis.corp","threadId":"10264","inReplyTo":"Pine.LNX.4.64.0710141901450.25221@racer.site","subject":"Re: [PATCH] parse-options: Allow abbreviated options when unambiguous","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-14T18:08:15Z","receivedAt":"2007-10-14T18:08:15Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Oct 14, 2007 at 06:02:33PM +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 14 Oct 2007, Johannes Schindelin wrote:\n> \n> > When there is an option \"--amend\", the option parser now recognizes \n> > \"--am\" for that option, provided that there is no other option beginning \n> > with \"--am\".\n> \n> And an amend for ultra-abbreviated options (as you noticed on IRC):\n> \n> diff --git a/parse-options.c b/parse-options.c\n> index afc6c89..acabb98 100644\n> --- a/parse-options.c\n> +++ b/parse-options.c\n> @@ -137,6 +137,11 @@ is_abbreviated:\n> \t\t\t\tabbrev_flags = flags;\n> \t\t\t\tcontinue;\n> \t\t\t}\n> +\t\t\t/* negated and abbreviated very much? */\n> +\t\t\tif (!prefixcmp(\"no-\", arg)) {\n> +\t\t\t\tflags |= OPT_UNSET;\n> +\t\t\t\tgoto is_abbreviated;\n> +\t\t\t}\n> \t\t\t/* negated? */\n> \t\t\tif (strncmp(arg, \"no-\", 3))\n> \t\t\t\tcontinue;\n\n squashed on top on the previous, and pushed to my ph/parseopt branch.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"55773","messageId":"20071014210130.GA17675@soma","threadId":"10264","inReplyTo":"20071014180815.GK1198@artemis.corp","subject":"Re: [PATCH] parse-options: Allow abbreviated options when unambiguous","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-10-14T21:01:30Z","receivedAt":"2007-10-14T21:01:30Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Pierre Habouzit wrote:\n> On Sun, Oct 14, 2007 at 06:02:33PM +0000, Johannes Schindelin wrote:\n> > Hi,\n> > \n> > On Sun, 14 Oct 2007, Johannes Schindelin wrote:\n> > \n> > > When there is an option \"--amend\", the option parser now recognizes \n> > > \"--am\" for that option, provided that there is no other option beginning \n> > > with \"--am\".\n> > \n> > And an amend for ultra-abbreviated options (as you noticed on IRC):\n> > \n> > diff --git a/parse-options.c b/parse-options.c\n> > index afc6c89..acabb98 100644\n> > --- a/parse-options.c\n> > +++ b/parse-options.c\n> > @@ -137,6 +137,11 @@ is_abbreviated:\n> > \t\t\t\tabbrev_flags = flags;\n> > \t\t\t\tcontinue;\n> > \t\t\t}\n> > +\t\t\t/* negated and abbreviated very much? */\n> > +\t\t\tif (!prefixcmp(\"no-\", arg)) {\n> > +\t\t\t\tflags |= OPT_UNSET;\n> > +\t\t\t\tgoto is_abbreviated;\n> > +\t\t\t}\n> > \t\t\t/* negated? */\n> > \t\t\tif (strncmp(arg, \"no-\", 3))\n> > \t\t\t\tcontinue;\n> \n> squashed on top on the previous, and pushed to my ph/parseopt branch.\n\nAwesome. Thanks to both of you.\n\n-- \nEric Wong\n"},{"id":"55784","messageId":"Pine.LNX.4.64.0710142309010.25221@racer.site","threadId":"10264","inReplyTo":"20071014210130.GA17675@soma","subject":"Re: [PATCH] parse-options: Allow abbreviated options when unambiguous","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-14T22:12:53Z","receivedAt":"2007-10-14T22:12:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 14 Oct 2007, Eric Wong wrote:\n\n> Pierre Habouzit wrote:\n> > On Sun, Oct 14, 2007 at 06:02:33PM +0000, Johannes Schindelin wrote:\n> > > Hi,\n> > > \n> > > On Sun, 14 Oct 2007, Johannes Schindelin wrote:\n> > > \n> > > > When there is an option \"--amend\", the option parser now recognizes \n> > > > \"--am\" for that option, provided that there is no other option beginning \n> > > > with \"--am\".\n> > > \n> > > And an amend for ultra-abbreviated options (as you noticed on IRC):\n> > > \n> > > diff --git a/parse-options.c b/parse-options.c\n> > > index afc6c89..acabb98 100644\n> > > --- a/parse-options.c\n> > > +++ b/parse-options.c\n> > > @@ -137,6 +137,11 @@ is_abbreviated:\n> > > \t\t\t\tabbrev_flags = flags;\n> > > \t\t\t\tcontinue;\n> > > \t\t\t}\n> > > +\t\t\t/* negated and abbreviated very much? */\n> > > +\t\t\tif (!prefixcmp(\"no-\", arg)) {\n> > > +\t\t\t\tflags |= OPT_UNSET;\n> > > +\t\t\t\tgoto is_abbreviated;\n> > > +\t\t\t}\n> > > \t\t\t/* negated? */\n> > > \t\t\tif (strncmp(arg, \"no-\", 3))\n> > > \t\t\t\tcontinue;\n> > \n> > squashed on top on the previous, and pushed to my ph/parseopt branch.\n> \n> Awesome. Thanks to both of you.\n\nHehe, you're welcome. Pierre even realised that my patch was not complete \n(it did not catch overly short abbreviations \"--n\" and \"--no\"), and that \nhas been fixed, too.\n\nWhile I have your attention: last weekend, I spoke to a guy from the \nffmpeg project, and he said that the only thing preventing them from \nswitching to git was the lack of svn:external support...\n\n(Of course I know that it is more difficult than that: ffmpeg itself is an \nsvn:external of MPlayer, but maybe we can get both of them to switch ;-)\n\nDo you have any idea when/if you're coming around to add that to git-svn?\n\nCiao,\nDscho\n"},{"id":"55793","messageId":"20071014224959.GA17828@untitled","threadId":"10264","inReplyTo":"Pine.LNX.4.64.0710142309010.25221@racer.site","subject":"Re: [PATCH] parse-options: Allow abbreviated options when unambiguous","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-10-14T22:49:59Z","receivedAt":"2007-10-14T22:49:59Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 14 Oct 2007, Eric Wong wrote:\n> \n> > Pierre Habouzit wrote:\n> > > On Sun, Oct 14, 2007 at 06:02:33PM +0000, Johannes Schindelin wrote:\n> > > > Hi,\n> > > > \n> > > > On Sun, 14 Oct 2007, Johannes Schindelin wrote:\n> > > > \n> > > > > When there is an option \"--amend\", the option parser now recognizes \n> > > > > \"--am\" for that option, provided that there is no other option beginning \n> > > > > with \"--am\".\n> > > > \n> > > > And an amend for ultra-abbreviated options (as you noticed on IRC):\n> > > > \n> > > > diff --git a/parse-options.c b/parse-options.c\n> > > > index afc6c89..acabb98 100644\n> > > > --- a/parse-options.c\n> > > > +++ b/parse-options.c\n> > > > @@ -137,6 +137,11 @@ is_abbreviated:\n> > > > \t\t\t\tabbrev_flags = flags;\n> > > > \t\t\t\tcontinue;\n> > > > \t\t\t}\n> > > > +\t\t\t/* negated and abbreviated very much? */\n> > > > +\t\t\tif (!prefixcmp(\"no-\", arg)) {\n> > > > +\t\t\t\tflags |= OPT_UNSET;\n> > > > +\t\t\t\tgoto is_abbreviated;\n> > > > +\t\t\t}\n> > > > \t\t\t/* negated? */\n> > > > \t\t\tif (strncmp(arg, \"no-\", 3))\n> > > > \t\t\t\tcontinue;\n> > > \n> > > squashed on top on the previous, and pushed to my ph/parseopt branch.\n> > \n> > Awesome. Thanks to both of you.\n> \n> Hehe, you're welcome. Pierre even realised that my patch was not complete \n> (it did not catch overly short abbreviations \"--n\" and \"--no\"), and that \n> has been fixed, too.\n \n> While I have your attention: last weekend, I spoke to a guy from the \n> ffmpeg project, and he said that the only thing preventing them from \n> switching to git was the lack of svn:external support...\n> \n> (Of course I know that it is more difficult than that: ffmpeg itself is an \n> svn:external of MPlayer, but maybe we can get both of them to switch ;-)\n> \n> Do you have any idea when/if you're coming around to add that to git-svn?\n\nSoonish, possibly within a next week, even. I have actually have\nstarted a project (using git) that wants to use SVN-hosted repositories\ndirectly submodules; so the fact that I'll actually need something like\nit bodes well for getting it implemented :)\n\n-- \nEric Wong\n"},{"id":"55796","messageId":"Pine.LNX.4.64.0710142359020.25221@racer.site","threadId":"10264","inReplyTo":"20071014224959.GA17828@untitled","subject":"git-svn and submodules, was Re: [PATCH] parse-options: Allow abbreviated options when unambiguous","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-14T22:59:42Z","receivedAt":"2007-10-14T22:59:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 14 Oct 2007, Eric Wong wrote:\n\n> Johannes Schindelin wrote:\n>\n> > While I have your attention: last weekend, I spoke to a guy from the \n> > ffmpeg project, and he said that the only thing preventing them from \n> > switching to git was the lack of svn:external support...\n> > \n> > (Of course I know that it is more difficult than that: ffmpeg itself \n> > is an svn:external of MPlayer, but maybe we can get both of them to \n> > switch ;-)\n> > \n> > Do you have any idea when/if you're coming around to add that to \n> > git-svn?\n> \n> Soonish, possibly within a next week, even. I have actually have \n> started a project (using git) that wants to use SVN-hosted repositories \n> directly submodules; so the fact that I'll actually need something like \n> it bodes well for getting it implemented :)\n\nHehe. Thanks!\n\nCiao,\nDscho\n"},{"id":"55823","messageId":"05CAB148-56ED-4FF1-8AAB-4BA2A0B70C2C@lrde.epita.fr","threadId":"10264","inReplyTo":"Pine.LNX.4.64.0710142359020.25221@racer.site","subject":"Re: git-svn and submodules","fromName":"Benoit SIGOURE","fromEmail":"tsuna@lrde.epita.fr","sentAt":"2007-10-15T07:07:21Z","receivedAt":"2007-10-15T07:07:21Z","isPatch":false,"sender":{"key":"tsunanet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/128281?v=4"},"body":"On Oct 15, 2007, at 12:59 AM, Johannes Schindelin wrote:\n\n> Hi,\n>\n> On Sun, 14 Oct 2007, Eric Wong wrote:\n>\n>> Johannes Schindelin wrote:\n>>\n>>> While I have your attention: last weekend, I spoke to a guy from the\n>>> ffmpeg project, and he said that the only thing preventing them from\n>>> switching to git was the lack of svn:external support...\n>>>\n>>> (Of course I know that it is more difficult than that: ffmpeg itself\n>>> is an svn:external of MPlayer, but maybe we can get both of them to\n>>> switch ;-)\n>>>\n>>> Do you have any idea when/if you're coming around to add that to\n>>> git-svn?\n>>\n>> Soonish, possibly within a next week, even. I have actually have\n>> started a project (using git) that wants to use SVN-hosted \n>> repositories\n>> directly submodules; so the fact that I'll actually need something \n>> like\n>> it bodes well for getting it implemented :)\n>\n> Hehe. Thanks!\n>\n\nThanks for making this another thread because I didn't read the \nanswers to that patch and I was going to try and implement this \n(svn:externals via submodules) sooner or later. Hadn't I seen this, \nwe'd probably end up duplicating effort. Maybe I can help with the \nimplementation?\n\nThis week I'm probably going to start to dive in git-svn by \nimplementing simpler things first:\n - git svn create-ignore (to create one .gitignore per directory \nfrom the svn:ignore properties. This has the disadvantage of \ncommitting the .gitignore during the next dcommit, but when you \nimport a repo with tons of ignores (>1000), using git svn show-ignore \nto build .git/info/exclude is *not* a good idea, because things like \ngit-status will end up doing >1000 fnmatch *per file* in the repo, \nwhich leads to git-status taking more than 4s on my Core2Duo 2Ghz 2G \nRAM)\n - git svn propget (to easily retrieve svn properties from withing \ngit-svn). git svn propset would be nice too, but I guess it's harder \nto implement.\n\nCheers,\n\n-- \nBenoit Sigoure aka Tsuna\nEPITA Research and Development Laboratory\n\n\n"},{"id":"55843","messageId":"47133A40.20303@op5.se","threadId":"10264","inReplyTo":"05CAB148-56ED-4FF1-8AAB-4BA2A0B70C2C@lrde.epita.fr","subject":"Re: git-svn and submodules","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-10-15T10:00:32Z","receivedAt":"2007-10-15T10:00:32Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Benoit SIGOURE wrote:\n> - git svn create-ignore (to create one .gitignore per directory from \n> the svn:ignore properties. This has the disadvantage of committing the \n> .gitignore during the next dcommit, but when you import a repo with tons \n> of ignores (>1000), using git svn show-ignore to build .git/info/exclude \n> is *not* a good idea, because things like git-status will end up doing \n> >1000 fnmatch *per file* in the repo, which leads to git-status taking \n> more than 4s on my Core2Duo 2Ghz 2G RAM)\n\nHow spoiled we are. I just ran cvs status on a checkout of a repo located\non a server in the local network. It took 6 seconds to complete :P\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"55846","messageId":"86lka4ofb8.fsf@lola.quinscape.zz","threadId":"10264","inReplyTo":"05CAB148-56ED-4FF1-8AAB-4BA2A0B70C2C@lrde.epita.fr","subject":"Re: git-svn and submodules","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-10-15T10:14:19Z","receivedAt":"2007-10-15T10:14:19Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Benoit SIGOURE writes:\n\n> This week I'm probably going to start to dive in git-svn by\n> implementing simpler things first:\n> - git svn create-ignore (to create one .gitignore per directory\n> from the svn:ignore properties. This has the disadvantage of\n> committing the .gitignore during the next dcommit, but when you\n> import a repo with tons of ignores (>1000), using git svn show-ignore\n> to build .git/info/exclude is *not* a good idea, because things like\n> git-status will end up doing >1000 fnmatch *per file* in the repo,\n> which leads to git-status taking more than 4s on my Core2Duo 2Ghz 2G\n> RAM)\n\nWell, then this should be fixed in git general, by sorting the ignores\n(wildcards in the first place where they can match), and then just\nmoving those patterns that can actually match according to sort order\nto the list of fnmatch candidates (and moving those files that can't\nmatch anymore die to the sort order out again).\n\nI don't think that the final \"solution\" for avoiding a lousy global\nO(n^2) algorithm is to replace it with lousy local O(n^2) algorithms\nand just hope for smaller values of n.\n\n-- \nDavid Kastrup\n"},{"id":"55848","messageId":"83CC7B21-DF40-4725-9550-A09AAFF88673@lrde.epita.fr","threadId":"10264","inReplyTo":"47133A40.20303@op5.se","subject":"Re: git-svn and submodules","fromName":"Benoit SIGOURE","fromEmail":"tsuna@lrde.epita.fr","sentAt":"2007-10-15T10:51:41Z","receivedAt":"2007-10-15T10:51:41Z","isPatch":false,"sender":{"key":"tsunanet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/128281?v=4"},"body":"On Oct 15, 2007, at 12:00 PM, Andreas Ericsson wrote:\n\n> Benoit SIGOURE wrote:\n>> - git svn create-ignore (to create one .gitignore per directory \n>> from the svn:ignore properties. This has the disadvantage of \n>> committing the .gitignore during the next dcommit, but when you \n>> import a repo with tons of ignores (>1000), using git svn show- \n>> ignore to build .git/info/exclude is *not* a good idea, because \n>> things like git-status will end up doing >1000 fnmatch *per file* \n>> in the repo, which leads to git-status taking more than 4s on my \n>> Core2Duo 2Ghz 2G RAM)\n>\n> How spoiled we are. I just ran cvs status on a checkout of a repo \n> located\n> on a server in the local network. It took 6 seconds to complete :P\n\nHehe, true, once you get used to the taste of Git, you'll never want \nto switch back to these disgusting SCMs.\n\n-- \nBenoit Sigoure aka Tsuna\nEPITA Research and Development Laboratory\n\n\n"},{"id":"55850","messageId":"AA453A15-BBF1-4EA6-B1AC-1C4E00E89FB2@lrde.epita.fr","threadId":"10264","inReplyTo":"86lka4ofb8.fsf@lola.quinscape.zz","subject":"Re: git-svn and submodules","fromName":"Benoit SIGOURE","fromEmail":"tsuna@lrde.epita.fr","sentAt":"2007-10-15T10:53:11Z","receivedAt":"2007-10-15T10:53:11Z","isPatch":false,"sender":{"key":"tsunanet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/128281?v=4"},"body":"On Oct 15, 2007, at 12:14 PM, David Kastrup wrote:\n\n> Benoit SIGOURE writes:\n>\n>> This week I'm probably going to start to dive in git-svn by\n>> implementing simpler things first:\n>> - git svn create-ignore (to create one .gitignore per directory\n>> from the svn:ignore properties. This has the disadvantage of\n>> committing the .gitignore during the next dcommit, but when you\n>> import a repo with tons of ignores (>1000), using git svn show-ignore\n>> to build .git/info/exclude is *not* a good idea, because things like\n>> git-status will end up doing >1000 fnmatch *per file* in the repo,\n>> which leads to git-status taking more than 4s on my Core2Duo 2Ghz 2G\n>> RAM)\n>\n> Well, then this should be fixed in git general, by sorting the ignores\n> (wildcards in the first place where they can match), and then just\n> moving those patterns that can actually match according to sort order\n> to the list of fnmatch candidates (and moving those files that can't\n> match anymore die to the sort order out again).\n>\n> I don't think that the final \"solution\" for avoiding a lousy global\n> O(n^2) algorithm is to replace it with lousy local O(n^2) algorithms\n> and just hope for smaller values of n.\n\nThat's entirely true, it's more of a workaround than a real \nsolution. Anyways, there could be other situations in which someone \nwould like to generate the .gitignore instead of using .git/info/ \nexclude, so this feature could be useful anyways.\n\nI can try to address this issue later, if I have enough free time in \nmy hands to do so.\n\n-- \nBenoit Sigoure aka Tsuna\nEPITA Research and Development Laboratory\n\n\n"},{"id":"55874","messageId":"20071015144513.GB7351@diana.vm.bytemark.co.uk","threadId":"10264","inReplyTo":"05CAB148-56ED-4FF1-8AAB-4BA2A0B70C2C@lrde.epita.fr","subject":"Re: git-svn and submodules","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-10-15T14:45:13Z","receivedAt":"2007-10-15T14:45:13Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-10-15 09:07:21 +0200, Benoit SIGOURE wrote:\n\n> - git svn create-ignore (to create one .gitignore per directory\n> from the svn:ignore properties. This has the disadvantage of\n> committing the .gitignore during the next dcommit,\n\nI built ignore support for git-svnignore a long time ago. It converts\nthe per-directory svn:ignore to per-directory .gitignore at commit\nimport time, which is very handy:\n\n-I ::\n Import the svn:ignore directory property to files with this\n name in each directory. (The Subversion and GIT ignore\n syntaxes are similar enough that using the Subversion patterns\n directly with \"-I .gitignore\" will almost always just work.)\n\nThe only downside with that is that svn ignore patterns are\nnon-recursive, while git ignore patterns are recursive. This could be\nsolved by prefixing them with a \"/\".\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"55876","messageId":"20071015151405.GA1655@pe.Belkin","threadId":"10264","inReplyTo":"20071015144513.GB7351@diana.vm.bytemark.co.uk","subject":".gitignore and svn:ignore [WAS: git-svn and submodules]","fromName":"Chris Shoemaker","fromEmail":"c.shoemaker@cox.net","sentAt":"2007-10-15T15:14:05Z","receivedAt":"2007-10-15T15:14:05Z","isPatch":false,"sender":{"key":"c.shoemaker@cox.net","avatar":null},"body":"On Mon, Oct 15, 2007 at 04:45:13PM +0200, Karl Hasselström wrote:\n> On 2007-10-15 09:07:21 +0200, Benoit SIGOURE wrote:\n> \n> > - git svn create-ignore (to create one .gitignore per directory\n> > from the svn:ignore properties. This has the disadvantage of\n> > committing the .gitignore during the next dcommit,\n> \n> I built ignore support for git-svnignore a long time ago. It converts\n> the per-directory svn:ignore to per-directory .gitignore at commit\n> import time, which is very handy:\n> \n> -I ::\n> Import the svn:ignore directory property to files with this\n> name in each directory. (The Subversion and GIT ignore\n> syntaxes are similar enough that using the Subversion patterns\n> directly with \"-I .gitignore\" will almost always just work.)\n> \n> The only downside with that is that svn ignore patterns are\n> non-recursive, while git ignore patterns are recursive. This could be\n> solved by prefixing them with a \"/\".\n\nHas anyone put any thought into mapping the other direction? \ni.e. .gitignore -> svn:ignore\n\n-chris\n"},{"id":"55878","messageId":"alpine.LFD.0.999.0710150848380.6887@woody.linux-foundation.org","threadId":"10264","inReplyTo":"05CAB148-56ED-4FF1-8AAB-4BA2A0B70C2C@lrde.epita.fr","subject":"Re: git-svn and submodules","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-15T15:53:13Z","receivedAt":"2007-10-15T15:53:13Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 15 Oct 2007, Benoit SIGOURE wrote:\n>\n> - git svn create-ignore (to create one .gitignore per directory from the\n> svn:ignore properties. This has the disadvantage of committing the .gitignore\n> during the next dcommit, but when you import a repo with tons of ignores\n> (>1000), using git svn show-ignore to build .git/info/exclude is *not* a good\n> idea, because things like git-status will end up doing >1000 fnmatch *per\n> file* in the repo, which leads to git-status taking more than 4s on my\n> Core2Duo 2Ghz 2G RAM)\n\nOuch.\n\nThat sounds largely unavoidable.. *But*.\n\nMaybe we have a bug here. In particular, we generally shouldn't care about \nthe exclude/.gitignore file for ay paths that we know about, which means \nthat during an import, we really shouldn't ever even care about \n.gitignore, since all the files are files we are expected to know about.\n\nSo yes, in general, \"git status\" is going to be slow in a tree that has \nbeen built (since things like object files etc will have to be checked \nagainst the exclude list! (*)), but if it's a clean import with no \ngenerated files and only files we already know about, that should not be \nthe case.\n\nSo maybe we have a totally unnecessary performance issue, and do all the \nfnmatch() on every path, whether we know about it or not?\n\n\t\tLinus\n\n(*) It might be that we could also re-order the exclude list so that \nentries that trigger are moved to the head of the list, because it's \nlikely that if you have tons of exclude entries, some of them trigger a \nlot more than others (ie \"*.o\"), and trying those first is likely a good \nidea.\n"},{"id":"55881","messageId":"C7EA8AD7-BACA-4116-9C6B-90BA23F0005C@lrde.epita.fr","threadId":"10264","inReplyTo":"alpine.LFD.0.999.0710150848380.6887@woody.linux-foundation.org","subject":"Performance issue with excludes (was: Re: git-svn and submodules)","fromName":"Benoit SIGOURE","fromEmail":"tsuna@lrde.epita.fr","sentAt":"2007-10-15T16:17:36Z","receivedAt":"2007-10-15T16:17:36Z","isPatch":false,"sender":{"key":"tsunanet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/128281?v=4"},"body":"On Oct 15, 2007, at 5:53 PM, Linus Torvalds wrote:\n\n> On Mon, 15 Oct 2007, Benoit SIGOURE wrote:\n>>\n>> - git svn create-ignore (to create one .gitignore per directory \n>> from the\n>> svn:ignore properties. This has the disadvantage of committing \n>> the .gitignore\n>> during the next dcommit, but when you import a repo with tons of \n>> ignores\n>> (>1000), using git svn show-ignore to build .git/info/exclude is \n>> *not* a good\n>> idea, because things like git-status will end up doing >1000 \n>> fnmatch *per\n>> file* in the repo, which leads to git-status taking more than 4s \n>> on my\n>> Core2Duo 2Ghz 2G RAM)\n>\n> Ouch.\n>\n> That sounds largely unavoidable.. *But*.\n>\n> Maybe we have a bug here. In particular, we generally shouldn't \n> care about\n> the exclude/.gitignore file for ay paths that we know about, which \n> means\n> that during an import, we really shouldn't ever even care about\n> .gitignore, since all the files are files we are expected to know \n> about.\n>\n> So yes, in general, \"git status\" is going to be slow in a tree that \n> has\n> been built (since things like object files etc will have to be checked\n> against the exclude list! (*)), but if it's a clean import with no\n> generated files and only files we already know about, that should \n> not be\n> the case.\n\nI re-used the test that was posted some time ago:\n\n------------------------------------------------------------------------ \n---\n#\n# first create a tree of roughly 100k files\n#\nmkdir bummer\ncd bummer\nfor ((i=0;i<100;i++)); do\nmkdir $i && pushd $i;\nfor ((j=0;j<1000;j++)); do\necho \"$j\" >$j; done; popd;\ndone\n\n#\n# init and add this to git\n#\ntime git init\ngit config user.email \"no@thx\"\ngit config user.name \"nothx\"\ntime git add .\ntime git commit -m 'buurrrrn' -a\n\nfor ((j=0;j<1000;j++)); do\n echo \"/pattern$j\" >.git/info/exclude\ndone\n\n#\n# git-status, tunes in at around ~8s for me\n#\ntime git-status\ntime git-status\ntime git-status\n------------------------------------------------------------------------ \n---\n\n[...]\ngit commit -m 'buurrrrn' -a 5.62s user 16.84s system 87% cpu 25.634 \ntotal\n# On branch master\nnothing to commit (working directory clean)\ngit-status 2.48s user 5.97s system 96% cpu 8.718 total\n# On branch master\nnothing to commit (working directory clean)\ngit-status 2.48s user 5.94s system 97% cpu 8.646 total\n# On branch master\nnothing to commit (working directory clean)\ngit-status 2.48s user 5.95s system 96% cpu 8.720 total\n\nMy machine is a Core2Duo 2Ghz 2G RAM.\n\n>\n> So maybe we have a totally unnecessary performance issue, and do \n> all the\n> fnmatch() on every path, whether we know about it or not?\n>\n> \t\tLinus\n>\n> (*) It might be that we could also re-order the exclude list so that\n> entries that trigger are moved to the head of the list, because it's\n> likely that if you have tons of exclude entries, some of them \n> trigger a\n> lot more than others (ie \"*.o\"), and trying those first is likely a \n> good\n> idea.\n\n-- \nBenoit Sigoure aka Tsuna\nEPITA Research and Development Laboratory\n\n\n"},{"id":"55882","messageId":"471394D5.3070509@op5.se","threadId":"10264","inReplyTo":"AA453A15-BBF1-4EA6-B1AC-1C4E00E89FB2@lrde.epita.fr","subject":"Re: git-svn and submodules","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-10-15T16:27:01Z","receivedAt":"2007-10-15T16:27:01Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Benoit SIGOURE wrote:\n> On Oct 15, 2007, at 12:14 PM, David Kastrup wrote:\n> \n>> Benoit SIGOURE writes:\n>>\n>>> This week I'm probably going to start to dive in git-svn by\n>>> implementing simpler things first:\n>>> - git svn create-ignore (to create one .gitignore per directory\n>>> from the svn:ignore properties. This has the disadvantage of\n>>> committing the .gitignore during the next dcommit, but when you\n>>> import a repo with tons of ignores (>1000), using git svn show-ignore\n>>> to build .git/info/exclude is *not* a good idea, because things like\n>>> git-status will end up doing >1000 fnmatch *per file* in the repo,\n>>> which leads to git-status taking more than 4s on my Core2Duo 2Ghz 2G\n>>> RAM)\n>>\n>> Well, then this should be fixed in git general, by sorting the ignores\n>> (wildcards in the first place where they can match), and then just\n>> moving those patterns that can actually match according to sort order\n>> to the list of fnmatch candidates (and moving those files that can't\n>> match anymore die to the sort order out again).\n>>\n>> I don't think that the final \"solution\" for avoiding a lousy global\n>> O(n^2) algorithm is to replace it with lousy local O(n^2) algorithms\n>> and just hope for smaller values of n.\n> \n> That's entirely true, it's more of a workaround than a real solution. \n> Anyways, there could be other situations in which someone would like to \n> generate the .gitignore instead of using .git/info/exclude, so this \n> feature could be useful anyways.\n> \n> I can try to address this issue later, if I have enough free time in my \n> hands to do so.\n> \n\nAh, finally found the thread. I sent a core.ignorefile patch to the list\n(Let users decide the name of the ignore file) a while ago, but didn't\nfind this mail to respond to. My apologies.\n\nIt's one way of solving it, which I'm currently using, although not so\nfitting for when you're importing svn repos permanently.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"55883","messageId":"alpine.LFD.0.999.0710150928450.6887@woody.linux-foundation.org","threadId":"10264","inReplyTo":"C7EA8AD7-BACA-4116-9C6B-90BA23F0005C@lrde.epita.fr","subject":"Re: Performance issue with excludes (was: Re: git-svn and submodules)","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-15T16:34:47Z","receivedAt":"2007-10-15T16:34:47Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 15 Oct 2007, Benoit SIGOURE wrote:\n> \n> I re-used the test that was posted some time ago:\n\nI think your test is scrogged. You should add the \".gitignore\" file \n*before* you do the \"git add .\". That's when it's going to hurt (since \nthat's when you have new files you don't yet know about).\n\nBut then it should hurt only for the \"git add .\" phase, not for anything \nelse (unless we have the performance bug of doing the ignore matching even \non files we know about). And more importantly, it should hurt only once \n(since afterwards, we'll know about the files and know not to ignore \nthem).\n\n\t\tLinus\n"},{"id":"55880","messageId":"45410184-8D7D-47ED-AB10-1A4E52D0ADB0@lrde.epita.fr","threadId":"10264","inReplyTo":"alpine.LFD.0.999.0710150928450.6887@woody.linux-foundation.org","subject":"Re: Performance issue with excludes (was: Re: git-svn and submodules)","fromName":"Benoit SIGOURE","fromEmail":"tsuna@lrde.epita.fr","sentAt":"2007-10-15T16:51:27Z","receivedAt":"2007-10-15T16:51:27Z","isPatch":false,"sender":{"key":"tsunanet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/128281?v=4"},"body":"On Oct 15, 2007, at 6:34 PM, Linus Torvalds wrote:\n\n> On Mon, 15 Oct 2007, Benoit SIGOURE wrote:\n>>\n>> I re-used the test that was posted some time ago:\n>\n> I think your test is scrogged. You should add the \".gitignore\" file\n> *before* you do the \"git add .\". That's when it's going to hurt (since\n> that's when you have new files you don't yet know about).\n>\n> But then it should hurt only for the \"git add .\" phase, not for \n> anything\n> else (unless we have the performance bug of doing the ignore \n> matching even\n> on files we know about). And more importantly, it should hurt only \n> once\n> (since afterwards, we'll know about the files and know not to ignore\n> them).\n\nThere is no .gitignore, only .git/info/exclude.\n\n-- \nBenoit Sigoure aka Tsuna\nEPITA Research and Development Laboratory\n\n\n"},{"id":"55885","messageId":"alpine.LFD.0.999.0710151009460.6887@woody.linux-foundation.org","threadId":"10264","inReplyTo":"45410184-8D7D-47ED-AB10-1A4E52D0ADB0@lrde.epita.fr","subject":"Re: Performance issue with excludes (was: Re: git-svn and submodules)","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-15T17:10:33Z","receivedAt":"2007-10-15T17:10:33Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 15 Oct 2007, Benoit SIGOURE wrote:\n> \n> There is no .gitignore, only .git/info/exclude.\n\nThey do exactly the same thing (apart from the nesting nature of \n.gitignore wrt subdirectories), so that doesn't change anything.\n\n\t\tLinus\n"},{"id":"55887","messageId":"DF6FA0BD-C227-4F62-82D8-F4873CC52B5A@lrde.epita.fr","threadId":"10264","inReplyTo":"alpine.LFD.0.999.0710151009460.6887@woody.linux-foundation.org","subject":"Re: Performance issue with excludes (was: Re: git-svn and submodules)","fromName":"Benoit SIGOURE","fromEmail":"tsuna@lrde.epita.fr","sentAt":"2007-10-15T17:38:12Z","receivedAt":"2007-10-15T17:38:12Z","isPatch":false,"sender":{"key":"tsunanet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/128281?v=4"},"body":"On Oct 15, 2007, at 7:10 PM, Linus Torvalds wrote:\n\n> On Mon, 15 Oct 2007, Benoit SIGOURE wrote:\n>>\n>> There is no .gitignore, only .git/info/exclude.\n>\n> They do exactly the same thing (apart from the nesting nature of\n> .gitignore wrt subdirectories), so that doesn't change anything.\n\nI fail to see how the mechanism work then. You said that I needed to \nadd the .gitignore before adding all the other bummer stuff, fair \nenough. AFAIK .git/info/exclude doesn't need to be added, it's just \nthere. But you can try to change the test, add the .git/info/exclude \n*first* and then make a commit and then add all the bummer stuff and \nthen commit, and finally, do a git-status, for me it still takes 9s.\n\n-- \nBenoit Sigoure aka Tsuna\nEPITA Research and Development Laboratory\n\n\n"},{"id":"55978","messageId":"20071016075827.GB32348@soma","threadId":"10264","inReplyTo":"20071015151405.GA1655@pe.Belkin","subject":"Re: .gitignore and svn:ignore [WAS: git-svn and submodules]","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-10-16T07:58:27Z","receivedAt":"2007-10-16T07:58:27Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Chris Shoemaker wrote:\n> On Mon, Oct 15, 2007 at 04:45:13PM +0200, Karl Hasselström wrote:\n> > On 2007-10-15 09:07:21 +0200, Benoit SIGOURE wrote:\n> > \n> > > - git svn create-ignore (to create one .gitignore per directory\n> > > from the svn:ignore properties. This has the disadvantage of\n> > > committing the .gitignore during the next dcommit,\n> > \n> > I built ignore support for git-svnignore a long time ago. It converts\n> > the per-directory svn:ignore to per-directory .gitignore at commit\n> > import time, which is very handy:\n> > \n> > -I ::\n> > Import the svn:ignore directory property to files with this\n> > name in each directory. (The Subversion and GIT ignore\n> > syntaxes are similar enough that using the Subversion patterns\n> > directly with \"-I .gitignore\" will almost always just work.)\n> > \n> > The only downside with that is that svn ignore patterns are\n> > non-recursive, while git ignore patterns are recursive. This could be\n> > solved by prefixing them with a \"/\".\n> \n> Has anyone put any thought into mapping the other direction? \n> i.e. .gitignore -> svn:ignore\n\nIf we support .gitignore <-> svn:ignore in git-svn; bidirectional,\ntransparent mapping is the only way I want to go.\n\n\nThis means that *all* .gitignore files will be translated to svn:ignore\nfiles and vice versa; and the .gitignore files will be NOT be committed\nto SVN itself, but present in the git-svn created mirrors. Recursive\n.gitignore definitions will be mapped to svn:ignore recursively on the\nclient side; and non-recursive ones will only map to one directory.\n\nSound good?\n\nI may be sleepy at the moment, but the thought of implementing this is\nsounding complicated now...\n\n\nOne goal of git-svn is that other users shouldn't be able to tell if a\nuser is using git-svn or plain svn; even.\n\n\nBut back to submodules, I plan on mapping svn:externals <=> .gitmodules\nfiles in a similar fashion. .gitmodule files will never be seen by SVN\nusers, period.\n\nThat being said, the first step to submodule/externals support in\ngit-svn will be to allow /any/ git repository to use a submodule that\npoints to SVN; and then git-submodule will invoke git-svn if it\nsees such a submodule.\n\nYes, I have a plan, sort of...\n\nSince externals/submodules don't operate recursively in either\nsystem like .gitignore; supporting svn:externals <=> submodules\nwill be much easier and done first[1] :)\n\n\n[1] - I've personally rarely bothered with putting svn:ignores in the\nrepository and have been very much spoiled by .git/info/exclude;\nwhereas externals support I have semi-immediate use for.\n\n-- \nEric Wong\n"},{"id":"55999","messageId":"20071016094330.GA5945@diana.vm.bytemark.co.uk","threadId":"10264","inReplyTo":"20071016075827.GB32348@soma","subject":"Re: .gitignore and svn:ignore [WAS: git-svn and submodules]","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-10-16T09:43:30Z","receivedAt":"2007-10-16T09:43:30Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-10-16 00:58:27 -0700, Eric Wong wrote:\n\n> If we support .gitignore <-> svn:ignore in git-svn; bidirectional,\n> transparent mapping is the only way I want to go.\n\nFair enough.\n\n> This means that *all* .gitignore files will be translated to\n> svn:ignore files and vice versa; and the .gitignore files will be\n> NOT be committed to SVN itself, but present in the git-svn created\n> mirrors.\n\nOK.\n\n> Recursive .gitignore definitions will be mapped to svn:ignore\n> recursively on the client side; and non-recursive ones will only map\n> to one directory.\n>\n> Sound good?\n>\n> I may be sleepy at the moment, but the thought of implementing this\n> is sounding complicated now...\n\nI think this is a mistake. If a user adds *.foo to the top-level\n.gitignore, this will add *.foo to svn:ignore of _every_ directory in\nthe whole tree. And coming up with semantics that are sane for e.g.\ngit -> svn -> git roundtrips seems difficult.\n\nIt would be better and far simpler to either\n\n 1. Move the contents of svn:ignore and .gitignore back and forth\n untouched, disregarding the slight semantic mismatch.\n git-svnignore does this (albeit only in one direction), and it\n works surprisingly well in my experience.\n\n 2. Do as in (1), but call the file .svnignore instead of .gitignore.\n And have a git-svn command that translates all the .svnignore\n files in the tree to corresponding .gitignore files.\n\n> One goal of git-svn is that other users shouldn't be able to tell if\n> a user is using git-svn or plain svn; even.\n\nAgreed.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"56030","messageId":"20071016130526.GA14263@pe.Belkin","threadId":"10264","inReplyTo":"20071016075827.GB32348@soma","subject":"Re: .gitignore and svn:ignore [WAS: git-svn and submodules]","fromName":"Chris Shoemaker","fromEmail":"c.shoemaker@cox.net","sentAt":"2007-10-16T13:05:26Z","receivedAt":"2007-10-16T13:05:26Z","isPatch":false,"sender":{"key":"c.shoemaker@cox.net","avatar":null},"body":"On Tue, Oct 16, 2007 at 12:58:27AM -0700, Eric Wong wrote:\n> Chris Shoemaker wrote:\n> > On Mon, Oct 15, 2007 at 04:45:13PM +0200, Karl Hasselström wrote:\n> > > On 2007-10-15 09:07:21 +0200, Benoit SIGOURE wrote:\n> > > \n> > > > - git svn create-ignore (to create one .gitignore per directory\n> > > > from the svn:ignore properties. This has the disadvantage of\n> > > > committing the .gitignore during the next dcommit,\n> > > \n> > > I built ignore support for git-svnignore a long time ago. It converts\n> > > the per-directory svn:ignore to per-directory .gitignore at commit\n> > > import time, which is very handy:\n> > > \n> > > -I ::\n> > > Import the svn:ignore directory property to files with this\n> > > name in each directory. (The Subversion and GIT ignore\n> > > syntaxes are similar enough that using the Subversion patterns\n> > > directly with \"-I .gitignore\" will almost always just work.)\n> > > \n> > > The only downside with that is that svn ignore patterns are\n> > > non-recursive, while git ignore patterns are recursive. This could be\n> > > solved by prefixing them with a \"/\".\n> > \n> > Has anyone put any thought into mapping the other direction? \n> > i.e. .gitignore -> svn:ignore\n> \n> If we support .gitignore <-> svn:ignore in git-svn; bidirectional,\n> transparent mapping is the only way I want to go.\n> \n> \n> This means that *all* .gitignore files will be translated to svn:ignore\n> files and vice versa; and the .gitignore files will be NOT be committed\n> to SVN itself, but present in the git-svn created mirrors. Recursive\n> .gitignore definitions will be mapped to svn:ignore recursively on the\n> client side; and non-recursive ones will only map to one directory.\n> \n> Sound good?\n> \n> I may be sleepy at the moment, but the thought of implementing this is\n> sounding complicated now...\n> \n\nOTOH, a general propset solution would probably be good enough that I\nwouldn't even miss any transparent .gitignore -> svn:ignore mapping.\n\nI would just accept that I'd have to explicitly specify the\nsvn:ignores.\n\n> Since externals/submodules don't operate recursively in either\n> system like .gitignore; supporting svn:externals <=> submodules\n> will be much easier and done first[1] :)\n> \n> [1] - I've personally rarely bothered with putting svn:ignores in the\n> repository and have been very much spoiled by .git/info/exclude;\n> whereas externals support I have semi-immediate use for.\n\nThat's great. I'm eager to see/test the svn:externals support. Thanks.\n\n-chris\n"}]}