{"thread":{"id":"10310","subject":"[parse-options] proposal for merge, take 1","startedAt":"2007-10-16T08:16:31Z","lastAt":"2007-10-17T19:06:46Z","messageCount":24,"participants":["Pierre Habouzit","Michael Witten","Johannes Schindelin","René Scharfe","Nicolas Pitre","Alex Riesen","Shawn O. Pearce"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"55983","messageId":"1192522616-16274-1-git-send-email-madcoder@debian.org","threadId":"10310","inReplyTo":null,"subject":"[parse-options] proposal for merge, take 1","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-16T08:16:31Z","receivedAt":"2007-10-16T08:16:31Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":" Hi, thanks to the tremendous help I received from Dscho to make its\nAPI and internals even better, I believe that my parseopt branch is\nready for inclusion.\n\n There is only _one_ small backward incompatibility (if other ones,\nthen it's not done on purpose and should be fixed) in git-show-ref: it\nhad a -h short option, synonym of --head. I made this -H because -h is\nautomatically diverted to \"show usage\" now, and it's counter-intuitive\nanyways. I rebased -i my series so that this patch comes last if people\nthink it's a bad idea, so that the rest can be still be merged.\n\n Note that patch 7/25 has a small discussion in it.\n\n The series comes in 2 parts: the parse-options construction, split\ninto small pieces to introduce each feature. And the migration of a few\nbuiltins. Diffstats and shortlogs follow.\n\n The branch is available from git://git.madism.org/git.git in the topic\nbranch ph/parseopt. I just rebased it onto the last \"next\" spearce\nproduced.\n\n Thanks to Alex Riesen, Johannes Schindelin, Jonas Fonseca and Kristian\nHøgsberg for their help, patches and advices.\n\n==============================================================================\nPart 1\n------------------------------------------------------------------------------\n\n Alex Riesen (1):\n Rework make_usage to print the usage message immediately\n\n Johannes Schindelin (2):\n Add tests for parse-options.c\n parse-options: Allow abbreviated options when unambiguous\n\n Pierre Habouzit (5):\n Add a simple option parser.\n parse-options: be able to generate usages automatically\n Add shortcuts for very often used options.\n parse-options: make some arguments optional, add callbacks.\n parse-options: allow callbacks to take no arguments at all.\n\n .gitignore | 1 +\n Makefile | 6 +-\n parse-options.c | 328 ++++++++++++++++++++++++++++++++++++++++++++++\n parse-options.h | 70 ++++++++++\n t/t0040-parse-options.sh | 92 +++++++++++++\n test-parse-options.c | 35 +++++\n 6 files changed, 529 insertions(+), 3 deletions(-)\n\n\n==============================================================================\nPart 2\n------------------------------------------------------------------------------\n\n Jonas Fonseca (1):\n Update manpages to reflect new short and long option aliases\n\n Kristian Høgsberg (1):\n Port builtin-add.c to use the new option parser.\n\n Pierre Habouzit (16):\n Make builtin-rm.c use parse_options.\n Make builtin-mv.c use parse-options\n Make builtin-branch.c use parse_options.\n Make builtin-describe.c use parse_options\n Make builtin-fetch.c use parse_options.\n Make builtin-revert.c use parse_options.\n Make builtin-update-ref.c use parse_options\n Make builtin-symbolic-ref.c use parse_options.\n Make builtin-http-fetch.c use parse_options.\n Make builtin-for-each-ref.c use parse-opts.\n Make builtin-fsck.c use parse_options.\n Make builtin-count-objects.c use parse_options.\n Make builtin-name-rev.c use parse_options.\n Make builtin-pack-refs.c use parse_options.\n Make builtin-show-ref.c use parse_options [small backward incompatibility].\n Make builtin-pack-objects.c use parse_options.\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-show-ref.txt | 4 +-\n Documentation/git-symbolic-ref.txt | 2 +-\n builtin-add.c | 70 +++------\n builtin-branch.c | 147 +++++++------------\n builtin-count-objects.c | 32 ++--\n builtin-describe.c | 70 ++++-----\n builtin-fetch.c | 146 ++++++------------\n builtin-for-each-ref.c | 138 ++++++++----------\n builtin-fsck.c | 80 +++-------\n builtin-http-fetch.c | 65 ++++-----\n builtin-mv.c | 84 +++++------\n builtin-name-rev.c | 64 +++-----\n builtin-pack-objects.c | 294 +++++++++++++++++-------------------\n builtin-pack-refs.c | 47 +++----\n builtin-revert.c | 67 ++++-----\n builtin-rm.c | 54 +++----\n builtin-show-ref.c | 127 ++++++----------\n builtin-symbolic-ref.c | 52 +++----\n builtin-update-ref.c | 65 +++-----\n 23 files changed, 654 insertions(+), 966 deletions(-)\n"},{"id":"55984","messageId":"20071016082010.GD6919@artemis.corp","threadId":"10310","inReplyTo":"1192522616-16274-1-git-send-email-madcoder@debian.org","subject":"Re: [parse-options] proposal for merge, take 1","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-16T08:20:10Z","receivedAt":"2007-10-16T08:20:10Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On mar, oct 16, 2007 at 08:16:31 +0000, Pierre Habouzit wrote:\n> The branch is available from git://git.madism.org/git.git in the topic\n> branch ph/parseopt. I just rebased it onto the last \"next\" spearce\n> produced.\n\n> Make builtin-pack-objects.c use parse_options.\n\n ERRR DON'T TAKE 25/25, it's WIP and is broken atm, format-patch again\nhasn't DWIM. It is not pushed to git://git.madism.org/git.git btw.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"55988","messageId":"20071016084335.GH6919@artemis.corp","threadId":"10310","inReplyTo":"20071016082010.GD6919@artemis.corp","subject":"Re: [parse-options] proposal for merge, take 1","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-16T08:43:35Z","receivedAt":"2007-10-16T08:43:35Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On mar, oct 16, 2007 at 08:20:10 +0000, Pierre Habouzit wrote:\n> On mar, oct 16, 2007 at 08:16:31 +0000, Pierre Habouzit wrote:\n> > The branch is available from git://git.madism.org/git.git in the topic\n> > branch ph/parseopt. I just rebased it onto the last \"next\" spearce\n> > produced.\n> \n> > Make builtin-pack-objects.c use parse_options.\n> \n> ERRR DON'T TAKE 25/25, it's WIP and is broken atm, format-patch again\n> hasn't DWIM. It is not pushed to git://git.madism.org/git.git btw.\n\nokay I'm wrong, sorry for the alarm, I just generated the --stat and shortlog\ntoo broadly, but 25/25 is definitely OK.\n\nProper stats for part2 are:\n\n Jonas Fonseca (1):\n Update manpages to reflect new short and long option aliases\n\n Kristian Høgsberg (1):\n Port builtin-add.c to use the new option parser.\n\n Pierre Habouzit (15):\n Make builtin-rm.c use parse_options.\n Make builtin-mv.c use parse-options\n Make builtin-branch.c use parse_options.\n Make builtin-describe.c use parse_options\n Make builtin-fetch.c use parse_options.\n Make builtin-revert.c use parse_options.\n Make builtin-update-ref.c use parse_options\n Make builtin-symbolic-ref.c use parse_options.\n Make builtin-http-fetch.c use parse_options.\n Make builtin-for-each-ref.c use parse-opts.\n Make builtin-fsck.c use parse_options.\n Make builtin-count-objects.c use parse_options.\n Make builtin-name-rev.c use parse_options.\n Make builtin-pack-refs.c use parse_options.\n Make builtin-show-ref.c use parse_options [small backward incompatibility].\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-show-ref.txt | 4 +-\n Documentation/git-symbolic-ref.txt | 2 +-\n builtin-add.c | 70 ++++++-----------\n builtin-branch.c | 147 +++++++++++++-----------------------\n builtin-count-objects.c | 32 ++++----\n builtin-describe.c | 70 +++++++----------\n builtin-fetch.c | 146 ++++++++++++------------------------\n builtin-for-each-ref.c | 138 +++++++++++++++-------------------\n builtin-fsck.c | 80 ++++++-------------\n builtin-http-fetch.c | 65 +++++++---------\n builtin-mv.c | 84 +++++++++------------\n builtin-name-rev.c | 64 ++++++----------\n builtin-pack-refs.c | 47 +++++-------\n builtin-revert.c | 67 +++++++---------\n builtin-rm.c | 54 ++++++--------\n builtin-show-ref.c | 127 +++++++++++--------------------\n builtin-symbolic-ref.c | 52 +++++--------\n builtin-update-ref.c | 65 ++++++----------\n 22 files changed, 515 insertions(+), 811 deletions(-)\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"55989","messageId":"20071016084510.GI6919@artemis.corp","threadId":"10310","inReplyTo":"1192523998-19474-7-git-send-email-madcoder@debian.org","subject":"Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-16T08:45:10Z","receivedAt":"2007-10-16T08:45:10Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"This bit is to allow to aggregate options with arguments together when\nthe argument is numeric.\n\n +#if 0\n +\t\t/* can be used to understand -A1B1 like -A1 -B1 */\n +\t\tif (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {\n +\t\t\t*(int *)opt->value = strtol(opt->opt, (char **)&opt->opt, 10);\n +\t\t\treturn 0;\n +\t\t}\n +#endif\n\nI'm not a huge fan, but people may like it. Feel free to keep the\nchunk, drop it, or enable it to your liking.\n\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"55995","messageId":"2209D123-A245-43C4-8DD9-A83386852556@mit.edu","threadId":"10310","inReplyTo":"1192523998-19474-9-git-send-email-madcoder@debian.org","subject":"Re: [PATCH 09/25] Port builtin-add.c to use the new option parser.","fromName":"Michael Witten","fromEmail":"mfwitten@mit.edu","sentAt":"2007-10-16T08:55:41Z","receivedAt":"2007-10-16T08:55:41Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"\nOn 16 Oct 2007, at 4:39:42 AM, Pierre Habouzit wrote:\n\n> +\tOPT_BOOLEAN('u', NULL, &take_worktree_changes, \"update only files \n> that git already knows about\"),\n\n\"update only files in the current directory that git already knows \nabout\"\n"},{"id":"55998","messageId":"362B5F9A-315F-46A4-A4CF-85EE9E26BA3B@mit.edu","threadId":"10310","inReplyTo":"2209D123-A245-43C4-8DD9-A83386852556@mit.edu","subject":"Re: [PATCH 09/25] Port builtin-add.c to use the new option parser.","fromName":"Michael Witten","fromEmail":"mfwitten@mit.edu","sentAt":"2007-10-16T09:36:04Z","receivedAt":"2007-10-16T09:36:04Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"\nOn 16 Oct 2007, at 4:55:41 AM, Michael Witten wrote:\n\n> On 16 Oct 2007, at 4:39:42 AM, Pierre Habouzit wrote:\n>\n>> +\tOPT_BOOLEAN('u', NULL, &take_worktree_changes, \"update only \n>> files that git already knows about\"),\n>\n> \"update only files in the current directory that git already knows \n> about\"\n\nBetter grammar:\n\n\"update only files that git already knows about in the current \ndirectory\"\n"},{"id":"56036","messageId":"Pine.LNX.4.64.0710161417150.25221@racer.site","threadId":"10310","inReplyTo":"2209D123-A245-43C4-8DD9-A83386852556@mit.edu","subject":"Re: [PATCH 09/25] Port builtin-add.c to use the new option parser.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-16T13:17:29Z","receivedAt":"2007-10-16T13:17:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 16 Oct 2007, Michael Witten wrote:\n\n> On 16 Oct 2007, at 4:39:42 AM, Pierre Habouzit wrote:\n> \n> > +\tOPT_BOOLEAN('u', NULL, &take_worktree_changes, \"update only files\n> > that git already knows about\"),\n> \n> \"update only files in the current directory that git already knows about\"\n\n\"update tracked files\"\n\nCiao,\nDscho\n"},{"id":"56037","messageId":"Pine.LNX.4.64.0710161417440.25221@racer.site","threadId":"10310","inReplyTo":"20071016084510.GI6919@artemis.corp","subject":"Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-16T13:18:15Z","receivedAt":"2007-10-16T13:18:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 16 Oct 2007, Pierre Habouzit wrote:\n\n> This bit is to allow to aggregate options with arguments together when\n> the argument is numeric.\n> \n> +#if 0\n> +\t\t/* can be used to understand -A1B1 like -A1 -B1 */\n> +\t\tif (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {\n> +\t\t\t*(int *)opt->value = strtol(opt->opt, (char **)&opt->opt, 10);\n> +\t\t\treturn 0;\n> +\t\t}\n> +#endif\n> \n> I'm not a huge fan, but people may like it. Feel free to keep the\n> chunk, drop it, or enable it to your liking.\n\nFWIW I like it. It allows me to aggregate options such as -M30 with other \nshort options.\n\nCiao,\nDscho\n"},{"id":"56063","messageId":"646BC634-0D2C-4207-903B-D78883365188@MIT.EDU","threadId":"10310","inReplyTo":"Pine.LNX.4.64.0710161417150.25221@racer.site","subject":"Re: [PATCH 09/25] Port builtin-add.c to use the new option parser.","fromName":"Michael Witten","fromEmail":"mfwitten@mit.edu","sentAt":"2007-10-16T15:36:51Z","receivedAt":"2007-10-16T15:36:51Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"\nOn 16 Oct 2007, at 9:17:29 AM, Johannes Schindelin wrote:\n\n> Hi,\n>\n> On Tue, 16 Oct 2007, Michael Witten wrote:\n>\n>> On 16 Oct 2007, at 4:39:42 AM, Pierre Habouzit wrote:\n>>\n>>> +\tOPT_BOOLEAN('u', NULL, &take_worktree_changes, \"update only files\n>>> that git already knows about\"),\n>>\n>> \"update only files that git already knows about in the current \n>> directory\"\n>\n> \"update tracked files\"\n\n\"update tracked files in .\"\n\n;-)\n\n\nConsider the description for git-commit's -a:\n\n> Tell the command to automatically stage files that have been\n> modified and deleted, but new files you have not told git about\n> are not affected.\n\nHowever, that's not what -u does.\n"},{"id":"56074","messageId":"4714E90C.80305@lsrfire.ath.cx","threadId":"10310","inReplyTo":"20071016084510.GI6919@artemis.corp","subject":"Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2007-10-16T16:38:36Z","receivedAt":"2007-10-16T16:38:36Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Pierre Habouzit schrieb:\n> This bit is to allow to aggregate options with arguments together when\n> the argument is numeric.\n> \n> +#if 0\n> +\t\t/* can be used to understand -A1B1 like -A1 -B1 */\n> +\t\tif (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {\n> +\t\t\t*(int *)opt->value = strtol(opt->opt, (char **)&opt->opt, 10);\n> +\t\t\treturn 0;\n> +\t\t}\n> +#endif\n\nI don't like it, it complicates number options with unit suffixes (e.g.\n--windows-memory of git-pack-objects).\n\nRené\n"},{"id":"56075","messageId":"Pine.LNX.4.64.0710161742040.25221@racer.site","threadId":"10310","inReplyTo":"4714E90C.80305@lsrfire.ath.cx","subject":"Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-16T16:44:44Z","receivedAt":"2007-10-16T16:44:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 16 Oct 2007, Ren? Scharfe wrote:\n\n> Pierre Habouzit schrieb:\n> > This bit is to allow to aggregate options with arguments together when\n> > the argument is numeric.\n> > \n> > +#if 0\n> > +\t\t/* can be used to understand -A1B1 like -A1 -B1 */\n> > +\t\tif (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {\n> > +\t\t\t*(int *)opt->value = strtol(opt->opt, (char **)&opt->opt, 10);\n> > +\t\t\treturn 0;\n> > +\t\t}\n> > +#endif\n> \n> I don't like it, it complicates number options with unit suffixes (e.g.\n> --windows-memory of git-pack-objects).\n\nWhy? It only means that you cannot say -W10mxabc instead of -W10m xabc. \n\nRemember: this is a special case for OPT_INTEGER. Nothing to do with \nOPT_SIZE, which you'd probably implement as a callback.\n\nCiao,\nDscho\n"},{"id":"56076","messageId":"alpine.LFD.0.9999.0710161244190.19446@xanadu.home","threadId":"10310","inReplyTo":"4714E90C.80305@lsrfire.ath.cx","subject":"Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-10-16T16:44:53Z","receivedAt":"2007-10-16T16:44:53Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 16 Oct 2007, René Scharfe wrote:\n\n> Pierre Habouzit schrieb:\n> > This bit is to allow to aggregate options with arguments together when\n> > the argument is numeric.\n> > \n> > +#if 0\n> > +\t\t/* can be used to understand -A1B1 like -A1 -B1 */\n> > +\t\tif (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {\n> > +\t\t\t*(int *)opt->value = strtol(opt->opt, (char **)&opt->opt, 10);\n> > +\t\t\treturn 0;\n> > +\t\t}\n> > +#endif\n> \n> I don't like it, it complicates number options with unit suffixes (e.g.\n> --windows-memory of git-pack-objects).\n\nThis is my opinion too.\n\n\nNicolas\n"},{"id":"56078","messageId":"20071016165045.GB13946@artemis.corp","threadId":"10310","inReplyTo":"4714E90C.80305@lsrfire.ath.cx","subject":"Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-16T16:50:45Z","receivedAt":"2007-10-16T16:50:45Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Tue, Oct 16, 2007 at 04:38:36PM +0000, René Scharfe wrote:\n> Pierre Habouzit schrieb:\n> > This bit is to allow to aggregate options with arguments together when\n> > the argument is numeric.\n> > \n> > +#if 0\n> > +\t\t/* can be used to understand -A1B1 like -A1 -B1 */\n> > +\t\tif (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {\n> > +\t\t\t*(int *)opt->value = strtol(opt->opt, (char **)&opt->opt, 10);\n> > +\t\t\treturn 0;\n> > +\t\t}\n> > +#endif\n> \n> I don't like it, it complicates number options with unit suffixes (e.g.\n> --windows-memory of git-pack-objects).\n\n Oh yeah, you're right, well you example is not an issue, but indeed\nyou pointed out a real probable issue:\n\n With that chunk, if an option that takes now an integer, becomes an\noption with a suffix, we would then break backward compatibility. I'm\nnot sure I'm clear, but if you had a -S option (for size) that for now\ngets sizes in kilooctet, git foo -S1000. Then if we decide that it's\nworth understanting M/G/.. suffixes for this option, we would make the\ntype of the option be a CALLBACK using git_parse_ulong. This would mean\nthat with such a change:\n\nbefore:\n git foo -S1000a would mean git foo -S1000 -a\n\nAfter:\n git foo -S1000a would be rejected because 'a' isn't a valid size\n suffix.\n\n This of course become worse if we take git foo -S1000k where the\nbreakage would be silent.\n\n\n This is a very strong argument _against_ this chunk IMO.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"56079","messageId":"20071016165319.GC13946@artemis.corp","threadId":"10310","inReplyTo":"Pine.LNX.4.64.0710161742040.25221@racer.site","subject":"Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-16T16:53:19Z","receivedAt":"2007-10-16T16:53:19Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Tue, Oct 16, 2007 at 04:44:44PM +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Tue, 16 Oct 2007, Ren? Scharfe wrote:\n> \n> > Pierre Habouzit schrieb:\n> > > This bit is to allow to aggregate options with arguments together when\n> > > the argument is numeric.\n> > > \n> > > +#if 0\n> > > +\t\t/* can be used to understand -A1B1 like -A1 -B1 */\n> > > +\t\tif (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {\n> > > +\t\t\t*(int *)opt->value = strtol(opt->opt, (char **)&opt->opt, 10);\n> > > +\t\t\treturn 0;\n> > > +\t\t}\n> > > +#endif\n> > \n> > I don't like it, it complicates number options with unit suffixes (e.g.\n> > --windows-memory of git-pack-objects).\n> \n> Why? It only means that you cannot say -W10mxabc instead of -W10m xabc. \n> \n> Remember: this is a special case for OPT_INTEGER. Nothing to do with \n> OPT_SIZE, which you'd probably implement as a callback.\n\n Yeah but the point is that you can't migrate an option currently being\nan integer to an OPT_SIZE because of that (see my other mail). Meaning\nthat once an argument is of type OPT_INTEGER you can't change it's type\nin the future _AT ALL_ without breaking backward compatibility badly.\nI'd say it's a rather sucky design.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"56081","messageId":"20071016165558.GD13946@artemis.corp","threadId":"10310","inReplyTo":"2209D123-A245-43C4-8DD9-A83386852556@mit.edu","subject":"Re: [PATCH 09/25] Port builtin-add.c to use the new option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-16T16:55:58Z","receivedAt":"2007-10-16T16:55:58Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On mar, oct 16, 2007 at 08:55:41 +0000, Michael Witten wrote:\n> \n> On 16 Oct 2007, at 4:39:42 AM, Pierre Habouzit wrote:\n> \n> >+\tOPT_BOOLEAN('u', NULL, &take_worktree_changes, \"update only files that \n> >git already knows about\"),\n> \n> \"update only files in the current directory that git already knows about\"\n\n As a general rule, as I'm not a native speaker, I'd be glad if someone\nwent through the diffs and made it better english. I'm not sure what the\nmore efficient way to merge those corrections is, as I'm not able to see\nif it's any better or not. So the someone mergint it (Shawn ?) may have\nto go through every OPTION_* and check the helps strings, and --amend\nfixes on top of the patches if needed. It looks like the best way to me.\n\n Alternatively, a native speaker could propose a 26th patch that fixes\nall of the bad strings on top of this series. But again, I won't do\nthat, not because of lazyness, just because I can't decide :)\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"56084","messageId":"4714EF2F.70506@lsrfire.ath.cx","threadId":"10310","inReplyTo":"Pine.LNX.4.64.0710161742040.25221@racer.site","subject":"Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2007-10-16T17:04:47Z","receivedAt":"2007-10-16T17:04:47Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Johannes Schindelin schrieb:\n> Hi,\n> \n> On Tue, 16 Oct 2007, Ren? Scharfe wrote:\n> \n>> Pierre Habouzit schrieb:\n>>> This bit is to allow to aggregate options with arguments together when\n>>> the argument is numeric.\n>>>\n>>> +#if 0\n>>> +\t\t/* can be used to understand -A1B1 like -A1 -B1 */\n>>> +\t\tif (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {\n>>> +\t\t\t*(int *)opt->value = strtol(opt->opt, (char **)&opt->opt, 10);\n>>> +\t\t\treturn 0;\n>>> +\t\t}\n>>> +#endif\n>> I don't like it, it complicates number options with unit suffixes (e.g.\n>> --windows-memory of git-pack-objects).\n> \n> Why? It only means that you cannot say -W10mxabc instead of -W10m xabc. \n> \n> Remember: this is a special case for OPT_INTEGER. Nothing to do with \n> OPT_SIZE, which you'd probably implement as a callback.\n\nYou mean I need to take a look at the actual patch to get a bit more\ncontext? ;-) Now that I did, I retract my comment.\n\nPierre, FYI: I didn't see your patches coming through the NNTP gateway\nof gmane.org, which is my way of reading this list. Its web interface\ndoesn't show them, either, so it's probably not caused by my news reader.\n\nRené\n"},{"id":"56096","messageId":"alpine.LFD.0.9999.0710161320400.19446@xanadu.home","threadId":"10310","inReplyTo":"20071016165319.GC13946@artemis.corp","subject":"Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-10-16T17:21:55Z","receivedAt":"2007-10-16T17:21:55Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 16 Oct 2007, Pierre Habouzit wrote:\n\n> On Tue, Oct 16, 2007 at 04:44:44PM +0000, Johannes Schindelin wrote:\n> > Hi,\n> > \n> > On Tue, 16 Oct 2007, Ren? Scharfe wrote:\n> > \n> > > Pierre Habouzit schrieb:\n> > > > This bit is to allow to aggregate options with arguments together when\n> > > > the argument is numeric.\n> > > > \n> > > > +#if 0\n> > > > +\t\t/* can be used to understand -A1B1 like -A1 -B1 */\n> > > > +\t\tif (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {\n> > > > +\t\t\t*(int *)opt->value = strtol(opt->opt, (char **)&opt->opt, 10);\n> > > > +\t\t\treturn 0;\n> > > > +\t\t}\n> > > > +#endif\n> > > \n> > > I don't like it, it complicates number options with unit suffixes (e.g.\n> > > --windows-memory of git-pack-objects).\n> > \n> > Why? It only means that you cannot say -W10mxabc instead of -W10m xabc. \n> > \n> > Remember: this is a special case for OPT_INTEGER. Nothing to do with \n> > OPT_SIZE, which you'd probably implement as a callback.\n> \n> Yeah but the point is that you can't migrate an option currently being\n> an integer to an OPT_SIZE because of that (see my other mail). Meaning\n> that once an argument is of type OPT_INTEGER you can't change it's type\n> in the future _AT ALL_ without breaking backward compatibility badly.\n> I'd say it's a rather sucky design.\n\nAnd what's the point of supporting so criptic arguments?\nIt doesn't have to go that far.\n\n\nNicolas\n"},{"id":"56102","messageId":"20071016175650.GA9992@steel.home","threadId":"10310","inReplyTo":"1192523998-19474-4-git-send-email-madcoder@debian.org","subject":"Re: [PATCH 04/25] Rework make_usage to print the usage message immediately","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-10-16T17:56:50Z","receivedAt":"2007-10-16T17:56:50Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Pierre Habouzit, Tue, Oct 16, 2007 10:39:37 +0200:\n> From: Alex Riesen \n> \n> Signed-off-by: Alex Riesen \n> ---\n> parse-options.c | 48 ++++++++++++++++++++++--------------------------\n> 1 files changed, 22 insertions(+), 26 deletions(-)\n\nGot it three times: you put git@kernel.org into To: and Cc:\n\nWhy stderr, BTW? For instance, the output from \"git help\" is on\nstdout. To be fair, I don't know why it is stdout there either.\n"},{"id":"56136","messageId":"20071016221555.GB31695@olympe.madism.org","threadId":"10310","inReplyTo":"20071016175650.GA9992@steel.home","subject":"Re: [PATCH 04/25] Rework make_usage to print the usage message immediately","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-16T22:15:55Z","receivedAt":"2007-10-16T22:15:55Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Tue, Oct 16, 2007 at 05:56:50PM +0000, Alex Riesen wrote:\n> Pierre Habouzit, Tue, Oct 16, 2007 10:39:37 +0200:\n> > From: Alex Riesen \n> > \n> > Signed-off-by: Alex Riesen \n> > ---\n> > parse-options.c | 48 ++++++++++++++++++++++--------------------------\n> > 1 files changed, 22 insertions(+), 26 deletions(-)\n> \n> Got it three times: you put git@kernel.org into To: and Cc:\n\n sorry, but it's not the reason, I sent it three time, there was a\nproblem with Shawn O. Pierce, it wasn't escaped properly and VGER ate\nthe mails because of that.\n\n> Why stderr, BTW? For instance, the output from \"git help\" is on\n> stdout. To be fair, I don't know why it is stdout there either.\n because it's what usage() does already ?\n> \n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"56199","messageId":"20071017044405.GV13801@spearce.org","threadId":"10310","inReplyTo":"20071016165045.GB13946@artemis.corp","subject":"Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-17T04:44:05Z","receivedAt":"2007-10-17T04:44:05Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Pierre Habouzit wrote:\n> On Tue, Oct 16, 2007 at 04:38:36PM +0000, René Scharfe wrote:\n> > Pierre Habouzit schrieb:\n> > > This bit is to allow to aggregate options with arguments together when\n> > > the argument is numeric.\n> > > \n> > > +#if 0\n> > > +\t\t/* can be used to understand -A1B1 like -A1 -B1 */\n> > > +\t\tif (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {\n> > > +\t\t\t*(int *)opt->value = strtol(opt->opt, (char **)&opt->opt, 10);\n> > > +\t\t\treturn 0;\n> > > +\t\t}\n> > > +#endif\n> > \n> > I don't like it, it complicates number options with unit suffixes (e.g.\n> > --windows-memory of git-pack-objects).\n...\n> This is a very strong argument _against_ this chunk IMO.\n\nSince everyone (including myself) is apparently strongly against this\nhunk I removed it when I cherry-picked this series from Pierre into\nmy tree. The series will be in my pu tonight, but minus this hunk.\n\n-- \nShawn.\n"},{"id":"56214","messageId":"20071017072419.GZ13801@spearce.org","threadId":"10310","inReplyTo":"1192523721-18985-1-git-send-email-madcoder@debian.org","subject":"Re: [PATCH 01/25] Add a simple option parser.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-17T07:24:19Z","receivedAt":"2007-10-17T07:24:19Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Pierre Habouzit wrote:\n> The option parser takes argc, argv, an array of struct option\n> and a usage string. ...\n\nOK, I've chewed down some version of this series. ;-)\n\nTo be more specific I fetched ph/parseopt (11b83dc4da) from your\ntree at git://git.madism.org/git.git and split it apart somewhat.\nAll of the patches were rebased onto my most recent master but I\nalso yanked the two that impacted the builtin-fetch series out and\nlayered them over a merge of db/fetch-pack and my version of your\nph/parseopt series.\n\nWhy? Well I want to keep our options open about which series\ngraduates to master first. Although builtin-fetch has been cooking\nfor a while there's been a number of issues with that code.\nThere exists a (perhaps small) chance that ph/parseopt will be\nready before db/fetch-pack.\n\nCurrently ph/parseopt is in pu. Tomorrow I'll look at the usage\nstrings in more depth and see if any improvements can be easily\nmade. I already made one suggested by Dscho in builtin-add.\n\n-- \nShawn.\n"},{"id":"56218","messageId":"20071017075206.GA17911@artemis.corp","threadId":"10310","inReplyTo":"20071017072419.GZ13801@spearce.org","subject":"Re: [PATCH 01/25] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-17T07:52:06Z","receivedAt":"2007-10-17T07:52:06Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Oct 17, 2007 at 07:24:19AM +0000, Shawn O. Pearce wrote:\n> Pierre Habouzit wrote:\n> > The option parser takes argc, argv, an array of struct option\n> > and a usage string. ...\n> \n> OK, I've chewed down some version of this series. ;-)\n> \n> To be more specific I fetched ph/parseopt (11b83dc4da) from your\n> tree at git://git.madism.org/git.git and split it apart somewhat.\n> All of the patches were rebased onto my most recent master but I\n> also yanked the two that impacted the builtin-fetch series out and\n> layered them over a merge of db/fetch-pack and my version of your\n> ph/parseopt series.\n> \n> Why? Well I want to keep our options open about which series\n> graduates to master first. Although builtin-fetch has been cooking\n> for a while there's been a number of issues with that code.\n> There exists a (perhaps small) chance that ph/parseopt will be\n> ready before db/fetch-pack.\n\n Yes, this makes sense, otoh the migration of the fetch commands are\nreally independant so you can put those appart, even if I end up needing\nto rewrite them it's not an issue.\n\n> Currently ph/parseopt is in pu. Tomorrow I'll look at the usage\n> strings in more depth and see if any improvements can be easily\n> made. I already made one suggested by Dscho in builtin-add.\n\n wonderful, thanks.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"56270","messageId":"Pine.LNX.4.64.0710171900060.25221@racer.site","threadId":"10310","inReplyTo":"20071017044405.GV13801@spearce.org","subject":"Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-17T18:00:35Z","receivedAt":"2007-10-17T18:00:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 17 Oct 2007, Shawn O. Pearce wrote:\n\n> Pierre Habouzit wrote:\n> > On Tue, Oct 16, 2007 at 04:38:36PM +0000, René Scharfe wrote:\n> > > Pierre Habouzit schrieb:\n> > > > This bit is to allow to aggregate options with arguments together when\n> > > > the argument is numeric.\n> > > > \n> > > > +#if 0\n> > > > +\t\t/* can be used to understand -A1B1 like -A1 -B1 */\n> > > > +\t\tif (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {\n> > > > +\t\t\t*(int *)opt->value = strtol(opt->opt, (char **)&opt->opt, 10);\n> > > > +\t\t\treturn 0;\n> > > > +\t\t}\n> > > > +#endif\n> > > \n> > > I don't like it, it complicates number options with unit suffixes (e.g.\n> > > --windows-memory of git-pack-objects).\n> ...\n> > This is a very strong argument _against_ this chunk IMO.\n> \n> Since everyone (including myself)\n\n... except for me ...\n\n> is apparently strongly against this hunk I removed it when I \n> cherry-picked this series from Pierre into my tree. The series will be \n> in my pu tonight, but minus this hunk.\n\nI can live without this hunk.\n\nCiao,\nDscho\n"},{"id":"56278","messageId":"20071017190646.GA3107@steel.home","threadId":"10310","inReplyTo":"20071016221555.GB31695@olympe.madism.org","subject":"Re: [PATCH 04/25] Rework make_usage to print the usage message immediately","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-10-17T19:06:46Z","receivedAt":"2007-10-17T19:06:46Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Pierre Habouzit, Wed, Oct 17, 2007 00:15:55 +0200:\n> > Why stderr, BTW? For instance, the output from \"git help\" is on\n> > stdout. To be fair, I don't know why it is stdout there either.\n> because it's what usage() does already ?\n\nfair enough.\n"}]}