{"thread":{"id":"24570","subject":"[PATCH 1/5] diff: parse detached options like -S foo","startedAt":"2010-07-29T08:20:24Z","lastAt":"2010-08-04T02:41:03Z","messageCount":25,"participants":["Matthieu Moy","Jonathan Nieder","Junio C Hamano","Miles Bader"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"146700","messageId":"1280391629-30017-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24570","inReplyTo":null,"subject":"[PATCH 0/5 v3] log and diff: accept detached forms (--option value)","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-29T08:20:24Z","receivedAt":"2010-07-29T08:20:24Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Since last version :\n\n* I had missed several optional arguments, for which detached form\n  should not be allowed (otherwise, --option --other-option is\n  ambiguous). In most cases, my changes were harmless since the code\n  had already checked for the parameterless form before reaching mine,\n  but that was definitely bad anyway.\n\n* One missing \"return optarg;\" (that even Jonathan had missed ;-) )\n\n* One more test for \"git log -S\" failure (no argument to -S)\n\n* Refactoring by Jonathan Nieder for --stat-*\n\n* Rewording of commit messages.\n\nJonathan Nieder (1):\n  diff: split off a function for --stat-* option parsing\n\nMatthieu Moy (4):\n  diff: parse detached options like -S foo\n  diff: parse detached options --stat-width n, --stat-name-width n\n  log: parse detached options like git log --grep foo\n  log: parse detached option for --glob\n\n diff.c                       |  167 +++++++++++++++++++++++++++++++-----------\n diff.h                       |    7 ++\n revision.c                   |   79 +++++++++++++-------\n t/t4013-diff-various.sh      |    5 +\n t/t4013/diff.log_-S_F_master |    7 ++\n t/t4202-log.sh               |   19 +++--\n t/t6018-rev-list-glob.sh     |    6 ++\n 7 files changed, 211 insertions(+), 79 deletions(-)\n create mode 100644 t/t4013/diff.log_-S_F_master\n\n-- \n1.7.2.21.ge9796\n"},{"id":"146699","messageId":"1280391629-30017-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"24570","inReplyTo":"1280391629-30017-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 1/5] diff: parse detached options like -S foo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-29T08:20:25Z","receivedAt":"2010-07-29T08:20:25Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Change the option parsing logic in revision.c to accept detached forms\nlike `-S foo' in addition to `-Sfoo'. The rest of git already accepted\nthis form, but revision.c still used its own option parsing.\n\nThis patch does not handle --stat-name-width and --stat-width, which are\nspecial-cases where diff_long_opt do not apply. They are handled in a\nseparate patch to ease review.\n\nOriginal patch by Matthieu Moy, plus refactoring by Jonathan Nieder.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n diff.c                       |   87 ++++++++++++++++++++++++++++++++++--------\n diff.h                       |    7 +++\n t/t4013-diff-various.sh      |    5 ++\n t/t4013/diff.log_-S_F_master |    7 +++\n t/t4202-log.sh               |   12 ++---\n 5 files changed, 95 insertions(+), 23 deletions(-)\n create mode 100644 t/t4013/diff.log_-S_F_master\n\ndiff --git a/diff.c b/diff.c\nindex 17873f3..d89ea20 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2990,9 +2990,50 @@ static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *va\n \n static int diff_scoreopt_parse(const char *opt);\n \n+static inline int short_opt(char opt, const char **argv,\n+\t\t\t    const char **optarg)\n+{\n+\tconst char *arg = argv[0];\n+\tif (arg[0] != '-' || arg[1] != opt)\n+\t\treturn 0;\n+\tif (arg[2] != '\\0') {\n+\t\t*optarg = arg + strlen(\"-c\");\n+\t\treturn 1;\n+\t}\n+\tif (!argv[1])\n+\t\tdie(\"Option '%c' requires a value\", opt);\n+\t*optarg = argv[1];\n+\treturn 2;\n+}\n+\n+int diff_long_opt(const char *opt, const char **argv,\n+\t\t  const char **optarg)\n+{\n+\tconst char *arg = argv[0];\n+\tif (arg[0] != '-' || arg[1] != '-')\n+\t\treturn 0;\n+\targ += strlen(\"--\");\n+\tif (prefixcmp(arg, opt))\n+\t\treturn 0;\n+\targ += strlen(opt);\n+\tif (*arg == '=') { /* Sticky form: --option=value */\n+\t\t*optarg = arg + 1;\n+\t\treturn 1;\n+\t}\n+\tif (*arg != '\\0')\n+\t\treturn 0;\n+\t/* detached form: --option value */\n+\tif (!argv[1])\n+\t\tdie(\"Option '--%s' requires a value\", opt);\n+\t*optarg = argv[1];\n+\treturn 2;\n+}\n+\n int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n {\n \tconst char *arg = av[0];\n+\tconst char *optarg;\n+\tint argcount;\n \n \t/* Output format options */\n \tif (!strcmp(arg, \"-p\") || !strcmp(arg, \"-u\") || !strcmp(arg, \"--patch\"))\n@@ -3149,10 +3190,11 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\telse\n \t\t\tdie(\"bad --word-diff argument: %s\", type);\n \t}\n-\telse if (!prefixcmp(arg, \"--word-diff-regex=\")) {\n+\telse if ((argcount = diff_long_opt(\"word-diff-regex\", av, &optarg))) {\n \t\tif (options->word_diff == DIFF_WORDS_NONE)\n \t\t\toptions->word_diff = DIFF_WORDS_PLAIN;\n-\t\toptions->word_regex = arg + 18;\n+\t\toptions->word_regex = optarg;\n+\t\treturn argcount;\n \t}\n \telse if (!strcmp(arg, \"--exit-code\"))\n \t\tDIFF_OPT_SET(options, EXIT_WITH_STATUS);\n@@ -3180,18 +3222,26 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t/* misc options */\n \telse if (!strcmp(arg, \"-z\"))\n \t\toptions->line_termination = 0;\n-\telse if (!prefixcmp(arg, \"-l\"))\n-\t\toptions->rename_limit = strtoul(arg+2, NULL, 10);\n-\telse if (!prefixcmp(arg, \"-S\"))\n-\t\toptions->pickaxe = arg + 2;\n+\telse if ((argcount = short_opt('l', av, &optarg))) {\n+\t\toptions->rename_limit = strtoul(optarg, NULL, 10);\n+\t\treturn argcount;\n+\t}\n+\telse if ((argcount = short_opt('S', av, &optarg))) {\n+\t\toptions->pickaxe = optarg;\n+\t\treturn argcount;\n+\t}\n \telse if (!strcmp(arg, \"--pickaxe-all\"))\n \t\toptions->pickaxe_opts = DIFF_PICKAXE_ALL;\n \telse if (!strcmp(arg, \"--pickaxe-regex\"))\n \t\toptions->pickaxe_opts = DIFF_PICKAXE_REGEX;\n-\telse if (!prefixcmp(arg, \"-O\"))\n-\t\toptions->orderfile = arg + 2;\n-\telse if (!prefixcmp(arg, \"--diff-filter=\"))\n-\t\toptions->filter = arg + 14;\n+\telse if ((argcount = short_opt('O', av, &optarg))) {\n+\t\toptions->orderfile = optarg;\n+\t\treturn argcount;\n+\t}\n+\telse if ((argcount = diff_long_opt(\"diff-filter\", av, &optarg))) {\n+\t\toptions->filter = optarg;\n+\t\treturn argcount;\n+\t}\n \telse if (!strcmp(arg, \"--abbrev\"))\n \t\toptions->abbrev = DEFAULT_ABBREV;\n \telse if (!prefixcmp(arg, \"--abbrev=\")) {\n@@ -3201,20 +3251,25 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\telse if (40 < options->abbrev)\n \t\t\toptions->abbrev = 40;\n \t}\n-\telse if (!prefixcmp(arg, \"--src-prefix=\"))\n-\t\toptions->a_prefix = arg + 13;\n-\telse if (!prefixcmp(arg, \"--dst-prefix=\"))\n-\t\toptions->b_prefix = arg + 13;\n+\telse if ((argcount = diff_long_opt(\"src-prefix\", av, &optarg))) {\n+\t\toptions->a_prefix = optarg;\n+\t\treturn argcount;\n+\t}\n+\telse if ((argcount = diff_long_opt(\"dst-prefix\", av, &optarg))) {\n+\t\toptions->b_prefix = optarg;\n+\t\treturn argcount;\n+\t}\n \telse if (!strcmp(arg, \"--no-prefix\"))\n \t\toptions->a_prefix = options->b_prefix = \"\";\n \telse if (opt_arg(arg, '\\0', \"inter-hunk-context\",\n \t\t\t &options->interhunkcontext))\n \t\t;\n-\telse if (!prefixcmp(arg, \"--output=\")) {\n-\t\toptions->file = fopen(arg + strlen(\"--output=\"), \"w\");\n+\telse if ((argcount = diff_long_opt(\"output\", av, &optarg))) {\n+\t\toptions->file = fopen(optarg, \"w\");\n \t\tif (!options->file)\n \t\t\tdie_errno(\"Could not open '%s'\", arg + strlen(\"--output=\"));\n \t\toptions->close_file = 1;\n+\t\treturn argcount;\n \t} else\n \t\treturn 0;\n \treturn 1;\ndiff --git a/diff.h b/diff.h\nindex 063d10a..3b11732 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -214,6 +214,13 @@ extern void diff_unmerge(struct diff_options *,\n #define DIFF_SETUP_USE_CACHE\t\t2\n #define DIFF_SETUP_USE_SIZE_CACHE\t4\n \n+/*\n+ * Poor man's alternative to parse-option, to allow both sticky form\n+ * (--option=value) and detached form (--option value).\n+ */\n+extern int diff_long_opt(const char *opt, const char **argv,\n+\t\t\t const char **optarg);\n+\n extern int git_diff_basic_config(const char *var, const char *value, void *cb);\n extern int git_diff_ui_config(const char *var, const char *value, void *cb);\n extern int diff_use_color_default;\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex dae6358..19857f4 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -208,6 +208,7 @@ log -p --first-parent master\n log -m -p --first-parent master\n log -m -p master\n log -SF master\n+log -S F master\n log -SF -p master\n log --decorate --all\n log --decorate=full --all\n@@ -282,4 +283,8 @@ diff master master^ side\n diff --dirstat master~1 master~2\n EOF\n \n+test_expect_success 'log -S requires an argument' '\n+\ttest_must_fail git log -S\n+'\n+\n test_done\ndiff --git a/t/t4013/diff.log_-S_F_master b/t/t4013/diff.log_-S_F_master\nnew file mode 100644\nindex 0000000..978d2b4\n--- /dev/null\n+++ b/t/t4013/diff.log_-S_F_master\n@@ -0,0 +1,7 @@\n+$ git log -S F master\n+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0\n+Author: A U Thor <author@example.com>\n+Date:   Mon Jun 26 00:02:00 2006 +0000\n+\n+    Third\n+$\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 2230e60..36870c5 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -100,13 +100,11 @@ test_expect_success 'oneline' '\n \n test_expect_success 'diff-filter=A' '\n \n-\tactual=$(git log --pretty=\"format:%s\" --diff-filter=A HEAD) &&\n-\texpect=$(echo fifth ; echo fourth ; echo third ; echo initial) &&\n-\ttest \"$actual\" = \"$expect\" || {\n-\t\techo Oops\n-\t\techo \"Actual: $actual\"\n-\t\tfalse\n-\t}\n+\tgit log --pretty=\"format:%s\" --diff-filter=A HEAD > actual &&\n+\tgit log --pretty=\"format:%s\" --diff-filter A HEAD > actual-detached &&\n+\tprintf \"fifth\\nfourth\\nthird\\ninitial\" > expect &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect actual-detached\n \n '\n \n-- \n1.7.2.21.ge9796\n"},{"id":"146704","messageId":"1280391629-30017-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"24570","inReplyTo":"1280391629-30017-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 2/5] diff: split off a function for --stat-* option parsing","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-29T08:20:26Z","receivedAt":"2010-07-29T08:20:26Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"From: Jonathan Nieder <jrnieder@gmail.com>\n\nAs an optimization, the diff_opt_parse() switchboard has\na single case for all the --stat-* options.  Split it\noff into a separate function so we can enhance it\nwithout bringing code dangerously close to the right\nmargin.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n diff.c |   62 +++++++++++++++++++++++++++++++++++---------------------------\n 1 files changed, 35 insertions(+), 27 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d89ea20..d5f5d4b 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3029,6 +3029,38 @@ int diff_long_opt(const char *opt, const char **argv,\n \treturn 2;\n }\n \n+static int stat_opt(struct diff_options *options, const char **av)\n+{\n+\tconst char *arg = av[0];\n+\tchar *end;\n+\tint width = options->stat_width;\n+\tint name_width = options->stat_name_width;\n+\n+\targ += strlen(\"--stat\");\n+\tend = (char *)arg;\n+\n+\tswitch (*arg) {\n+\tcase '-':\n+\t\tif (!prefixcmp(arg, \"-width=\"))\n+\t\t\twidth = strtoul(arg + 7, &end, 10);\n+\t\telse if (!prefixcmp(arg, \"-name-width=\"))\n+\t\t\tname_width = strtoul(arg + 12, &end, 10);\n+\t\tbreak;\n+\tcase '=':\n+\t\twidth = strtoul(arg+1, &end, 10);\n+\t\tif (*end == ',')\n+\t\t\tname_width = strtoul(end+1, &end, 10);\n+\t}\n+\n+\t/* Important! This checks all the error cases! */\n+\tif (*end)\n+\t\treturn 0;\n+\toptions->output_format |= DIFF_FORMAT_DIFFSTAT;\n+\toptions->stat_name_width = name_width;\n+\toptions->stat_width = width;\n+\treturn 1;\n+}\n+\n int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n {\n \tconst char *arg = av[0];\n@@ -3070,33 +3102,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\toptions->output_format |= DIFF_FORMAT_NAME_STATUS;\n \telse if (!strcmp(arg, \"-s\"))\n \t\toptions->output_format |= DIFF_FORMAT_NO_OUTPUT;\n-\telse if (!prefixcmp(arg, \"--stat\")) {\n-\t\tchar *end;\n-\t\tint width = options->stat_width;\n-\t\tint name_width = options->stat_name_width;\n-\t\targ += 6;\n-\t\tend = (char *)arg;\n-\n-\t\tswitch (*arg) {\n-\t\tcase '-':\n-\t\t\tif (!prefixcmp(arg, \"-width=\"))\n-\t\t\t\twidth = strtoul(arg + 7, &end, 10);\n-\t\t\telse if (!prefixcmp(arg, \"-name-width=\"))\n-\t\t\t\tname_width = strtoul(arg + 12, &end, 10);\n-\t\t\tbreak;\n-\t\tcase '=':\n-\t\t\twidth = strtoul(arg+1, &end, 10);\n-\t\t\tif (*end == ',')\n-\t\t\t\tname_width = strtoul(end+1, &end, 10);\n-\t\t}\n-\n-\t\t/* Important! This checks all the error cases! */\n-\t\tif (*end)\n-\t\t\treturn 0;\n-\t\toptions->output_format |= DIFF_FORMAT_DIFFSTAT;\n-\t\toptions->stat_name_width = name_width;\n-\t\toptions->stat_width = width;\n-\t}\n+\telse if (!prefixcmp(arg, \"--stat\"))\n+\t\t/* --stat, --stat-width, or --stat-name-width */\n+\t\treturn stat_opt(options, av);\n \n \t/* renames options */\n \telse if (!prefixcmp(arg, \"-B\")) {\n-- \n1.7.2.21.ge9796\n"},{"id":"146702","messageId":"1280391629-30017-4-git-send-email-Matthieu.Moy@imag.fr","threadId":"24570","inReplyTo":"1280391629-30017-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 3/5] diff: parse detached options --stat-width n, --stat-name-width n","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-29T08:20:27Z","receivedAt":"2010-07-29T08:20:27Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Part of a campaign for unstuck forms of options.\n\n[jn: with some refactoring]\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n diff.c |   28 +++++++++++++++++++++++-----\n 1 files changed, 23 insertions(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d5f5d4b..90b07e1 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3035,16 +3035,34 @@ static int stat_opt(struct diff_options *options, const char **av)\n \tchar *end;\n \tint width = options->stat_width;\n \tint name_width = options->stat_name_width;\n+\tint argcount = 1;\n \n \targ += strlen(\"--stat\");\n \tend = (char *)arg;\n \n \tswitch (*arg) {\n \tcase '-':\n-\t\tif (!prefixcmp(arg, \"-width=\"))\n-\t\t\twidth = strtoul(arg + 7, &end, 10);\n-\t\telse if (!prefixcmp(arg, \"-name-width=\"))\n-\t\t\tname_width = strtoul(arg + 12, &end, 10);\n+\t\tif (!prefixcmp(arg, \"-width\")) {\n+\t\t\targ += strlen(\"-width\");\n+\t\t\tif (*arg == '=')\n+\t\t\t\twidth = strtoul(arg + 1, &end, 10);\n+\t\t\telse if (!*arg && !av[1])\n+\t\t\t\tdie(\"Option '--stat-width' requires a value\");\n+\t\t\telse if (!*arg) {\n+\t\t\t\twidth = strtoul(av[1], &end, 10);\n+\t\t\t\targcount = 2;\n+\t\t\t}\n+\t\t} else if (!prefixcmp(arg, \"-name-width\")) {\n+\t\t\targ += strlen(\"-name-width\");\n+\t\t\tif (*arg == '=')\n+\t\t\t\tname_width = strtoul(arg + 1, &end, 10);\n+\t\t\telse if (!*arg && !av[1])\n+\t\t\t\tdie(\"Option '--stat-name-width' requires a value\");\n+\t\t\telse if (!*arg) {\n+\t\t\t\tname_width = strtoul(av[1], &end, 10);\n+\t\t\t\targcount = 2;\n+\t\t\t}\n+\t\t}\n \t\tbreak;\n \tcase '=':\n \t\twidth = strtoul(arg+1, &end, 10);\n@@ -3058,7 +3076,7 @@ static int stat_opt(struct diff_options *options, const char **av)\n \toptions->output_format |= DIFF_FORMAT_DIFFSTAT;\n \toptions->stat_name_width = name_width;\n \toptions->stat_width = width;\n-\treturn 1;\n+\treturn argcount;\n }\n \n int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n-- \n1.7.2.21.ge9796\n"},{"id":"146703","messageId":"1280391629-30017-5-git-send-email-Matthieu.Moy@imag.fr","threadId":"24570","inReplyTo":"1280391629-30017-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 4/5] log: parse detached options like git log --grep foo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-29T08:20:28Z","receivedAt":"2010-07-29T08:20:28Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n revision.c     |   72 +++++++++++++++++++++++++++++++++++---------------------\n t/t4202-log.sh |    7 +++++\n 2 files changed, 52 insertions(+), 27 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 7e82efd..7006340 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1148,6 +1148,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\t\t       int *unkc, const char **unkv)\n {\n \tconst char *arg = argv[0];\n+\tconst char *optarg;\n+\tint argcount;\n \n \t/* pseudo revision arguments */\n \tif (!strcmp(arg, \"--all\") || !strcmp(arg, \"--branches\") ||\n@@ -1160,11 +1162,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\treturn 1;\n \t}\n \n-\tif (!prefixcmp(arg, \"--max-count=\")) {\n-\t\trevs->max_count = atoi(arg + 12);\n+\tif ((argcount = diff_long_opt(\"max-count\", argv, &optarg))) {\n+\t\trevs->max_count = atoi(optarg);\n \t\trevs->no_walk = 0;\n-\t} else if (!prefixcmp(arg, \"--skip=\")) {\n-\t\trevs->skip_count = atoi(arg + 7);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"skip\", argv, &optarg))) {\n+\t\trevs->skip_count = atoi(optarg);\n+\t\treturn argcount;\n \t} else if ((*arg == '-') && isdigit(arg[1])) {\n \t/* accept -<digit>, like traditional \"head\" */\n \t\trevs->max_count = atoi(arg + 1);\n@@ -1178,18 +1182,24 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!prefixcmp(arg, \"-n\")) {\n \t\trevs->max_count = atoi(arg + 2);\n \t\trevs->no_walk = 0;\n-\t} else if (!prefixcmp(arg, \"--max-age=\")) {\n-\t\trevs->max_age = atoi(arg + 10);\n-\t} else if (!prefixcmp(arg, \"--since=\")) {\n-\t\trevs->max_age = approxidate(arg + 8);\n-\t} else if (!prefixcmp(arg, \"--after=\")) {\n-\t\trevs->max_age = approxidate(arg + 8);\n-\t} else if (!prefixcmp(arg, \"--min-age=\")) {\n-\t\trevs->min_age = atoi(arg + 10);\n-\t} else if (!prefixcmp(arg, \"--before=\")) {\n-\t\trevs->min_age = approxidate(arg + 9);\n-\t} else if (!prefixcmp(arg, \"--until=\")) {\n-\t\trevs->min_age = approxidate(arg + 8);\n+\t} else if ((argcount = diff_long_opt(\"max-age\", argv, &optarg))) {\n+\t\trevs->max_age = atoi(optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"since\", argv, &optarg))) {\n+\t\trevs->max_age = approxidate(optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"after\", argv, &optarg))) {\n+\t\trevs->max_age = approxidate(optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"min-age\", argv, &optarg))) {\n+\t\trevs->min_age = atoi(optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"before\", argv, &optarg))) {\n+\t\trevs->min_age = approxidate(optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"until\", argv, &optarg))) {\n+\t\trevs->min_age = approxidate(optarg);\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--first-parent\")) {\n \t\trevs->first_parent_only = 1;\n \t} else if (!strcmp(arg, \"--ancestry-path\")) {\n@@ -1295,6 +1305,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->pretty_given = 1;\n \t\tget_commit_format(arg+8, revs);\n \t} else if (!prefixcmp(arg, \"--pretty=\") || !prefixcmp(arg, \"--format=\")) {\n+\t\t/*\n+\t\t * Detached form (\"--pretty X\" as opposed to \"--pretty=X\")\n+\t\t * not allowed, since the argument is optional.\n+\t\t */\n \t\trevs->verbose_header = 1;\n \t\trevs->pretty_given = 1;\n \t\tget_commit_format(arg+9, revs);\n@@ -1359,21 +1373,25 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--relative-date\")) {\n \t\trevs->date_mode = DATE_RELATIVE;\n \t\trevs->date_mode_explicit = 1;\n-\t} else if (!strncmp(arg, \"--date=\", 7)) {\n-\t\trevs->date_mode = parse_date_format(arg + 7);\n+\t} else if ((argcount = diff_long_opt(\"date\", argv, &optarg))) {\n+\t\trevs->date_mode = parse_date_format(optarg);\n \t\trevs->date_mode_explicit = 1;\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--log-size\")) {\n \t\trevs->show_log_size = 1;\n \t}\n \t/*\n \t * Grepping the commit log\n \t */\n-\telse if (!prefixcmp(arg, \"--author=\")) {\n-\t\tadd_header_grep(revs, GREP_HEADER_AUTHOR, arg+9);\n-\t} else if (!prefixcmp(arg, \"--committer=\")) {\n-\t\tadd_header_grep(revs, GREP_HEADER_COMMITTER, arg+12);\n-\t} else if (!prefixcmp(arg, \"--grep=\")) {\n-\t\tadd_message_grep(revs, arg+7);\n+\telse if ((argcount = diff_long_opt(\"author\", argv, &optarg))) {\n+\t\tadd_header_grep(revs, GREP_HEADER_AUTHOR, optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"committer\", argv, &optarg))) {\n+\t\tadd_header_grep(revs, GREP_HEADER_COMMITTER, optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"grep\", argv, &optarg))) {\n+\t\tadd_message_grep(revs, optarg);\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--extended-regexp\") || !strcmp(arg, \"-E\")) {\n \t\trevs->grep_filter.regflags |= REG_EXTENDED;\n \t} else if (!strcmp(arg, \"--regexp-ignore-case\") || !strcmp(arg, \"-i\")) {\n@@ -1382,12 +1400,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->grep_filter.fixed = 1;\n \t} else if (!strcmp(arg, \"--all-match\")) {\n \t\trevs->grep_filter.all_match = 1;\n-\t} else if (!prefixcmp(arg, \"--encoding=\")) {\n-\t\targ += 11;\n+\t} else if ((argcount = diff_long_opt(\"encoding\", argv, &optarg))) {\n \t\tif (strcmp(arg, \"none\"))\n-\t\t\tgit_log_output_encoding = xstrdup(arg);\n+\t\t\tgit_log_output_encoding = xstrdup(optarg);\n \t\telse\n \t\t\tgit_log_output_encoding = \"\";\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--reverse\")) {\n \t\trevs->reverse ^= 1;\n \t} else if (!strcmp(arg, \"--children\")) {\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 36870c5..dfbc173 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -201,6 +201,13 @@ test_expect_success 'log --grep' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'log --grep option parsing' '\n+\techo second >expect &&\n+\tgit log -1 --pretty=\"tformat:%s\" --grep sec >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_must_fail git log -1 --pretty=\"tformat:%s\" --grep\n+'\n+\n test_expect_success 'log -i --grep' '\n \techo Second >expect &&\n \tgit log -1 --pretty=\"tformat:%s\" -i --grep=sec >actual &&\n-- \n1.7.2.21.ge9796\n"},{"id":"146701","messageId":"1280391629-30017-6-git-send-email-Matthieu.Moy@imag.fr","threadId":"24570","inReplyTo":"1280391629-30017-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 5/5] log: parse detached option for --glob","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-29T08:20:29Z","receivedAt":"2010-07-29T08:20:29Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n revision.c               |    7 +++++--\n t/t6018-rev-list-glob.sh |    6 ++++++\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 7006340..188fa21 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1484,6 +1484,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n {\n \tint i, flags, left, seen_dashdash, read_from_stdin, got_rev_arg = 0;\n \tconst char **prune_data = NULL;\n+\tconst char *optarg;\n+\tint argcount;\n \n \t/* First, search for \"--\" */\n \tseen_dashdash = 0;\n@@ -1530,10 +1532,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\t\thandle_refs(revs, flags, for_each_remote_ref);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--glob=\")) {\n+\t\t\tif ((argcount = diff_long_opt(\"glob\", argv + i, &optarg))) {\n+\t\t\t\ti += argcount - 1;\n \t\t\t\tstruct all_refs_cb cb;\n \t\t\t\tinit_all_refs_cb(&cb, revs, flags);\n-\t\t\t\tfor_each_glob_ref(handle_one_ref, arg + 7, &cb);\n+\t\t\t\tfor_each_glob_ref(handle_one_ref, optarg, &cb);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!prefixcmp(arg, \"--branches=\")) {\ndiff --git a/t/t6018-rev-list-glob.sh b/t/t6018-rev-list-glob.sh\nindex 58428d9..fb8291c 100755\n--- a/t/t6018-rev-list-glob.sh\n+++ b/t/t6018-rev-list-glob.sh\n@@ -123,6 +123,12 @@ test_expect_success 'rev-list --glob=refs/heads/subspace/*' '\n \n '\n \n+test_expect_success 'rev-list --glob refs/heads/subspace/*' '\n+\n+\tcompare rev-list \"subspace/one subspace/two\" \"--glob refs/heads/subspace/*\"\n+\n+'\n+\n test_expect_success 'rev-list --glob=heads/subspace/*' '\n \n \tcompare rev-list \"subspace/one subspace/two\" \"--glob=heads/subspace/*\"\n-- \n1.7.2.21.ge9796\n"},{"id":"146743","messageId":"20100729222436.GA1824@burratino","threadId":"24570","inReplyTo":"1280391629-30017-5-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 4/5] log: parse detached options like git log --grep foo","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-29T22:25:16Z","receivedAt":"2010-07-29T22:25:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nGlance over this again:\n\n> +++ b/revision.c\n> @@ -1382,12 +1400,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n>  \t\trevs->grep_filter.fixed = 1;\n>  \t} else if (!strcmp(arg, \"--all-match\")) {\n>  \t\trevs->grep_filter.all_match = 1;\n> -\t} else if (!prefixcmp(arg, \"--encoding=\")) {\n> -\t\targ += 11;\n> +\t} else if ((argcount = diff_long_opt(\"encoding\", argv, &optarg))) {\n>  \t\tif (strcmp(arg, \"none\"))\n> -\t\t\tgit_log_output_encoding = xstrdup(arg);\n> +\t\t\tgit_log_output_encoding = xstrdup(optarg);\n\nI think arg should be optarg in the string comparison, too.  Sorry I\ndidn’t notice last time.\n\nI wonder why t3900 and t8005 do not catch this.\n\n>  \t\telse\n>  \t\t\tgit_log_output_encoding = \"\";\n\nCurious,\nJonathan\n"},{"id":"146761","messageId":"vpq7hkdml22.fsf@bauges.imag.fr","threadId":"24570","inReplyTo":"20100729222436.GA1824@burratino","subject":"Re: [PATCH 4/5] log: parse detached options like git log --grep foo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-07-30T08:27:17Z","receivedAt":"2010-07-30T08:27:17Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> I think arg should be optarg in the string comparison, too.\n\nDefinitely, yes.\n\n> Sorry I didn’t notice last time.\n\nAhem, sorry for making the bug in the first place ;-).\n\n> I wonder why t3900 and t8005 do not catch this.\n\nGood question. I don't understand exactly what's going on with commit\nmessages encoding, so I can't say.\n\nResend follows.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146766","messageId":"1280478669-22973-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24570","inReplyTo":"vpq7hkdml22.fsf@bauges.imag.fr","subject":"[PATCH 1/5 v4] diff: parse detached options like -S foo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-30T08:31:05Z","receivedAt":"2010-07-30T08:31:05Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Change the option parsing logic in revision.c to accept detached forms\nlike `-S foo' in addition to `-Sfoo'. The rest of git already accepted\nthis form, but revision.c still used its own option parsing.\n\nThis patch does not handle --stat-name-width and --stat-width, which are\nspecial-cases where diff_long_opt do not apply. They are handled in a\nseparate patch to ease review.\n\nOriginal patch by Matthieu Moy, plus refactoring by Jonathan Nieder.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n diff.c                       |   87 ++++++++++++++++++++++++++++++++++--------\n diff.h                       |    7 +++\n t/t4013-diff-various.sh      |    5 ++\n t/t4013/diff.log_-S_F_master |    7 +++\n t/t4202-log.sh               |   12 ++---\n 5 files changed, 95 insertions(+), 23 deletions(-)\n create mode 100644 t/t4013/diff.log_-S_F_master\n\ndiff --git a/diff.c b/diff.c\nindex 17873f3..d89ea20 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2990,9 +2990,50 @@ static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *va\n \n static int diff_scoreopt_parse(const char *opt);\n \n+static inline int short_opt(char opt, const char **argv,\n+\t\t\t    const char **optarg)\n+{\n+\tconst char *arg = argv[0];\n+\tif (arg[0] != '-' || arg[1] != opt)\n+\t\treturn 0;\n+\tif (arg[2] != '\\0') {\n+\t\t*optarg = arg + strlen(\"-c\");\n+\t\treturn 1;\n+\t}\n+\tif (!argv[1])\n+\t\tdie(\"Option '%c' requires a value\", opt);\n+\t*optarg = argv[1];\n+\treturn 2;\n+}\n+\n+int diff_long_opt(const char *opt, const char **argv,\n+\t\t  const char **optarg)\n+{\n+\tconst char *arg = argv[0];\n+\tif (arg[0] != '-' || arg[1] != '-')\n+\t\treturn 0;\n+\targ += strlen(\"--\");\n+\tif (prefixcmp(arg, opt))\n+\t\treturn 0;\n+\targ += strlen(opt);\n+\tif (*arg == '=') { /* Sticky form: --option=value */\n+\t\t*optarg = arg + 1;\n+\t\treturn 1;\n+\t}\n+\tif (*arg != '\\0')\n+\t\treturn 0;\n+\t/* detached form: --option value */\n+\tif (!argv[1])\n+\t\tdie(\"Option '--%s' requires a value\", opt);\n+\t*optarg = argv[1];\n+\treturn 2;\n+}\n+\n int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n {\n \tconst char *arg = av[0];\n+\tconst char *optarg;\n+\tint argcount;\n \n \t/* Output format options */\n \tif (!strcmp(arg, \"-p\") || !strcmp(arg, \"-u\") || !strcmp(arg, \"--patch\"))\n@@ -3149,10 +3190,11 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\telse\n \t\t\tdie(\"bad --word-diff argument: %s\", type);\n \t}\n-\telse if (!prefixcmp(arg, \"--word-diff-regex=\")) {\n+\telse if ((argcount = diff_long_opt(\"word-diff-regex\", av, &optarg))) {\n \t\tif (options->word_diff == DIFF_WORDS_NONE)\n \t\t\toptions->word_diff = DIFF_WORDS_PLAIN;\n-\t\toptions->word_regex = arg + 18;\n+\t\toptions->word_regex = optarg;\n+\t\treturn argcount;\n \t}\n \telse if (!strcmp(arg, \"--exit-code\"))\n \t\tDIFF_OPT_SET(options, EXIT_WITH_STATUS);\n@@ -3180,18 +3222,26 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t/* misc options */\n \telse if (!strcmp(arg, \"-z\"))\n \t\toptions->line_termination = 0;\n-\telse if (!prefixcmp(arg, \"-l\"))\n-\t\toptions->rename_limit = strtoul(arg+2, NULL, 10);\n-\telse if (!prefixcmp(arg, \"-S\"))\n-\t\toptions->pickaxe = arg + 2;\n+\telse if ((argcount = short_opt('l', av, &optarg))) {\n+\t\toptions->rename_limit = strtoul(optarg, NULL, 10);\n+\t\treturn argcount;\n+\t}\n+\telse if ((argcount = short_opt('S', av, &optarg))) {\n+\t\toptions->pickaxe = optarg;\n+\t\treturn argcount;\n+\t}\n \telse if (!strcmp(arg, \"--pickaxe-all\"))\n \t\toptions->pickaxe_opts = DIFF_PICKAXE_ALL;\n \telse if (!strcmp(arg, \"--pickaxe-regex\"))\n \t\toptions->pickaxe_opts = DIFF_PICKAXE_REGEX;\n-\telse if (!prefixcmp(arg, \"-O\"))\n-\t\toptions->orderfile = arg + 2;\n-\telse if (!prefixcmp(arg, \"--diff-filter=\"))\n-\t\toptions->filter = arg + 14;\n+\telse if ((argcount = short_opt('O', av, &optarg))) {\n+\t\toptions->orderfile = optarg;\n+\t\treturn argcount;\n+\t}\n+\telse if ((argcount = diff_long_opt(\"diff-filter\", av, &optarg))) {\n+\t\toptions->filter = optarg;\n+\t\treturn argcount;\n+\t}\n \telse if (!strcmp(arg, \"--abbrev\"))\n \t\toptions->abbrev = DEFAULT_ABBREV;\n \telse if (!prefixcmp(arg, \"--abbrev=\")) {\n@@ -3201,20 +3251,25 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\telse if (40 < options->abbrev)\n \t\t\toptions->abbrev = 40;\n \t}\n-\telse if (!prefixcmp(arg, \"--src-prefix=\"))\n-\t\toptions->a_prefix = arg + 13;\n-\telse if (!prefixcmp(arg, \"--dst-prefix=\"))\n-\t\toptions->b_prefix = arg + 13;\n+\telse if ((argcount = diff_long_opt(\"src-prefix\", av, &optarg))) {\n+\t\toptions->a_prefix = optarg;\n+\t\treturn argcount;\n+\t}\n+\telse if ((argcount = diff_long_opt(\"dst-prefix\", av, &optarg))) {\n+\t\toptions->b_prefix = optarg;\n+\t\treturn argcount;\n+\t}\n \telse if (!strcmp(arg, \"--no-prefix\"))\n \t\toptions->a_prefix = options->b_prefix = \"\";\n \telse if (opt_arg(arg, '\\0', \"inter-hunk-context\",\n \t\t\t &options->interhunkcontext))\n \t\t;\n-\telse if (!prefixcmp(arg, \"--output=\")) {\n-\t\toptions->file = fopen(arg + strlen(\"--output=\"), \"w\");\n+\telse if ((argcount = diff_long_opt(\"output\", av, &optarg))) {\n+\t\toptions->file = fopen(optarg, \"w\");\n \t\tif (!options->file)\n \t\t\tdie_errno(\"Could not open '%s'\", arg + strlen(\"--output=\"));\n \t\toptions->close_file = 1;\n+\t\treturn argcount;\n \t} else\n \t\treturn 0;\n \treturn 1;\ndiff --git a/diff.h b/diff.h\nindex 063d10a..3b11732 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -214,6 +214,13 @@ extern void diff_unmerge(struct diff_options *,\n #define DIFF_SETUP_USE_CACHE\t\t2\n #define DIFF_SETUP_USE_SIZE_CACHE\t4\n \n+/*\n+ * Poor man's alternative to parse-option, to allow both sticky form\n+ * (--option=value) and detached form (--option value).\n+ */\n+extern int diff_long_opt(const char *opt, const char **argv,\n+\t\t\t const char **optarg);\n+\n extern int git_diff_basic_config(const char *var, const char *value, void *cb);\n extern int git_diff_ui_config(const char *var, const char *value, void *cb);\n extern int diff_use_color_default;\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex dae6358..19857f4 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -208,6 +208,7 @@ log -p --first-parent master\n log -m -p --first-parent master\n log -m -p master\n log -SF master\n+log -S F master\n log -SF -p master\n log --decorate --all\n log --decorate=full --all\n@@ -282,4 +283,8 @@ diff master master^ side\n diff --dirstat master~1 master~2\n EOF\n \n+test_expect_success 'log -S requires an argument' '\n+\ttest_must_fail git log -S\n+'\n+\n test_done\ndiff --git a/t/t4013/diff.log_-S_F_master b/t/t4013/diff.log_-S_F_master\nnew file mode 100644\nindex 0000000..978d2b4\n--- /dev/null\n+++ b/t/t4013/diff.log_-S_F_master\n@@ -0,0 +1,7 @@\n+$ git log -S F master\n+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0\n+Author: A U Thor <author@example.com>\n+Date:   Mon Jun 26 00:02:00 2006 +0000\n+\n+    Third\n+$\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 2230e60..36870c5 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -100,13 +100,11 @@ test_expect_success 'oneline' '\n \n test_expect_success 'diff-filter=A' '\n \n-\tactual=$(git log --pretty=\"format:%s\" --diff-filter=A HEAD) &&\n-\texpect=$(echo fifth ; echo fourth ; echo third ; echo initial) &&\n-\ttest \"$actual\" = \"$expect\" || {\n-\t\techo Oops\n-\t\techo \"Actual: $actual\"\n-\t\tfalse\n-\t}\n+\tgit log --pretty=\"format:%s\" --diff-filter=A HEAD > actual &&\n+\tgit log --pretty=\"format:%s\" --diff-filter A HEAD > actual-detached &&\n+\tprintf \"fifth\\nfourth\\nthird\\ninitial\" > expect &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect actual-detached\n \n '\n \n-- \n1.7.2.252.gb848\n"},{"id":"146763","messageId":"1280478669-22973-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"24570","inReplyTo":"vpq7hkdml22.fsf@bauges.imag.fr","subject":"[PATCH 2/5 v4] diff: split off a function for --stat-* option parsing","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-30T08:31:06Z","receivedAt":"2010-07-30T08:31:06Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"From: Jonathan Nieder <jrnieder@gmail.com>\n\nAs an optimization, the diff_opt_parse() switchboard has\na single case for all the --stat-* options.  Split it\noff into a separate function so we can enhance it\nwithout bringing code dangerously close to the right\nmargin.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n diff.c |   62 +++++++++++++++++++++++++++++++++++---------------------------\n 1 files changed, 35 insertions(+), 27 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d89ea20..d5f5d4b 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3029,6 +3029,38 @@ int diff_long_opt(const char *opt, const char **argv,\n \treturn 2;\n }\n \n+static int stat_opt(struct diff_options *options, const char **av)\n+{\n+\tconst char *arg = av[0];\n+\tchar *end;\n+\tint width = options->stat_width;\n+\tint name_width = options->stat_name_width;\n+\n+\targ += strlen(\"--stat\");\n+\tend = (char *)arg;\n+\n+\tswitch (*arg) {\n+\tcase '-':\n+\t\tif (!prefixcmp(arg, \"-width=\"))\n+\t\t\twidth = strtoul(arg + 7, &end, 10);\n+\t\telse if (!prefixcmp(arg, \"-name-width=\"))\n+\t\t\tname_width = strtoul(arg + 12, &end, 10);\n+\t\tbreak;\n+\tcase '=':\n+\t\twidth = strtoul(arg+1, &end, 10);\n+\t\tif (*end == ',')\n+\t\t\tname_width = strtoul(end+1, &end, 10);\n+\t}\n+\n+\t/* Important! This checks all the error cases! */\n+\tif (*end)\n+\t\treturn 0;\n+\toptions->output_format |= DIFF_FORMAT_DIFFSTAT;\n+\toptions->stat_name_width = name_width;\n+\toptions->stat_width = width;\n+\treturn 1;\n+}\n+\n int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n {\n \tconst char *arg = av[0];\n@@ -3070,33 +3102,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\toptions->output_format |= DIFF_FORMAT_NAME_STATUS;\n \telse if (!strcmp(arg, \"-s\"))\n \t\toptions->output_format |= DIFF_FORMAT_NO_OUTPUT;\n-\telse if (!prefixcmp(arg, \"--stat\")) {\n-\t\tchar *end;\n-\t\tint width = options->stat_width;\n-\t\tint name_width = options->stat_name_width;\n-\t\targ += 6;\n-\t\tend = (char *)arg;\n-\n-\t\tswitch (*arg) {\n-\t\tcase '-':\n-\t\t\tif (!prefixcmp(arg, \"-width=\"))\n-\t\t\t\twidth = strtoul(arg + 7, &end, 10);\n-\t\t\telse if (!prefixcmp(arg, \"-name-width=\"))\n-\t\t\t\tname_width = strtoul(arg + 12, &end, 10);\n-\t\t\tbreak;\n-\t\tcase '=':\n-\t\t\twidth = strtoul(arg+1, &end, 10);\n-\t\t\tif (*end == ',')\n-\t\t\t\tname_width = strtoul(end+1, &end, 10);\n-\t\t}\n-\n-\t\t/* Important! This checks all the error cases! */\n-\t\tif (*end)\n-\t\t\treturn 0;\n-\t\toptions->output_format |= DIFF_FORMAT_DIFFSTAT;\n-\t\toptions->stat_name_width = name_width;\n-\t\toptions->stat_width = width;\n-\t}\n+\telse if (!prefixcmp(arg, \"--stat\"))\n+\t\t/* --stat, --stat-width, or --stat-name-width */\n+\t\treturn stat_opt(options, av);\n \n \t/* renames options */\n \telse if (!prefixcmp(arg, \"-B\")) {\n-- \n1.7.2.252.gb848\n"},{"id":"146764","messageId":"1280478669-22973-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"24570","inReplyTo":"vpq7hkdml22.fsf@bauges.imag.fr","subject":"[PATCH 3/5 v4] diff: parse detached options --stat-width n, --stat-name-width n","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-30T08:31:07Z","receivedAt":"2010-07-30T08:31:07Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Part of a campaign for unstuck forms of options.\n\n[jn: with some refactoring]\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n diff.c |   28 +++++++++++++++++++++++-----\n 1 files changed, 23 insertions(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d5f5d4b..90b07e1 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3035,16 +3035,34 @@ static int stat_opt(struct diff_options *options, const char **av)\n \tchar *end;\n \tint width = options->stat_width;\n \tint name_width = options->stat_name_width;\n+\tint argcount = 1;\n \n \targ += strlen(\"--stat\");\n \tend = (char *)arg;\n \n \tswitch (*arg) {\n \tcase '-':\n-\t\tif (!prefixcmp(arg, \"-width=\"))\n-\t\t\twidth = strtoul(arg + 7, &end, 10);\n-\t\telse if (!prefixcmp(arg, \"-name-width=\"))\n-\t\t\tname_width = strtoul(arg + 12, &end, 10);\n+\t\tif (!prefixcmp(arg, \"-width\")) {\n+\t\t\targ += strlen(\"-width\");\n+\t\t\tif (*arg == '=')\n+\t\t\t\twidth = strtoul(arg + 1, &end, 10);\n+\t\t\telse if (!*arg && !av[1])\n+\t\t\t\tdie(\"Option '--stat-width' requires a value\");\n+\t\t\telse if (!*arg) {\n+\t\t\t\twidth = strtoul(av[1], &end, 10);\n+\t\t\t\targcount = 2;\n+\t\t\t}\n+\t\t} else if (!prefixcmp(arg, \"-name-width\")) {\n+\t\t\targ += strlen(\"-name-width\");\n+\t\t\tif (*arg == '=')\n+\t\t\t\tname_width = strtoul(arg + 1, &end, 10);\n+\t\t\telse if (!*arg && !av[1])\n+\t\t\t\tdie(\"Option '--stat-name-width' requires a value\");\n+\t\t\telse if (!*arg) {\n+\t\t\t\tname_width = strtoul(av[1], &end, 10);\n+\t\t\t\targcount = 2;\n+\t\t\t}\n+\t\t}\n \t\tbreak;\n \tcase '=':\n \t\twidth = strtoul(arg+1, &end, 10);\n@@ -3058,7 +3076,7 @@ static int stat_opt(struct diff_options *options, const char **av)\n \toptions->output_format |= DIFF_FORMAT_DIFFSTAT;\n \toptions->stat_name_width = name_width;\n \toptions->stat_width = width;\n-\treturn 1;\n+\treturn argcount;\n }\n \n int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n-- \n1.7.2.252.gb848\n"},{"id":"146765","messageId":"1280478669-22973-4-git-send-email-Matthieu.Moy@imag.fr","threadId":"24570","inReplyTo":"vpq7hkdml22.fsf@bauges.imag.fr","subject":"[PATCH 4/5 v4] log: parse detached options like git log --grep foo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-30T08:31:08Z","receivedAt":"2010-07-30T08:31:08Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n revision.c     |   74 ++++++++++++++++++++++++++++++++++---------------------\n t/t4202-log.sh |    7 +++++\n 2 files changed, 53 insertions(+), 28 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 7e82efd..359b1a1 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1148,6 +1148,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\t\t       int *unkc, const char **unkv)\n {\n \tconst char *arg = argv[0];\n+\tconst char *optarg;\n+\tint argcount;\n \n \t/* pseudo revision arguments */\n \tif (!strcmp(arg, \"--all\") || !strcmp(arg, \"--branches\") ||\n@@ -1160,11 +1162,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\treturn 1;\n \t}\n \n-\tif (!prefixcmp(arg, \"--max-count=\")) {\n-\t\trevs->max_count = atoi(arg + 12);\n+\tif ((argcount = diff_long_opt(\"max-count\", argv, &optarg))) {\n+\t\trevs->max_count = atoi(optarg);\n \t\trevs->no_walk = 0;\n-\t} else if (!prefixcmp(arg, \"--skip=\")) {\n-\t\trevs->skip_count = atoi(arg + 7);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"skip\", argv, &optarg))) {\n+\t\trevs->skip_count = atoi(optarg);\n+\t\treturn argcount;\n \t} else if ((*arg == '-') && isdigit(arg[1])) {\n \t/* accept -<digit>, like traditional \"head\" */\n \t\trevs->max_count = atoi(arg + 1);\n@@ -1178,18 +1182,24 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!prefixcmp(arg, \"-n\")) {\n \t\trevs->max_count = atoi(arg + 2);\n \t\trevs->no_walk = 0;\n-\t} else if (!prefixcmp(arg, \"--max-age=\")) {\n-\t\trevs->max_age = atoi(arg + 10);\n-\t} else if (!prefixcmp(arg, \"--since=\")) {\n-\t\trevs->max_age = approxidate(arg + 8);\n-\t} else if (!prefixcmp(arg, \"--after=\")) {\n-\t\trevs->max_age = approxidate(arg + 8);\n-\t} else if (!prefixcmp(arg, \"--min-age=\")) {\n-\t\trevs->min_age = atoi(arg + 10);\n-\t} else if (!prefixcmp(arg, \"--before=\")) {\n-\t\trevs->min_age = approxidate(arg + 9);\n-\t} else if (!prefixcmp(arg, \"--until=\")) {\n-\t\trevs->min_age = approxidate(arg + 8);\n+\t} else if ((argcount = diff_long_opt(\"max-age\", argv, &optarg))) {\n+\t\trevs->max_age = atoi(optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"since\", argv, &optarg))) {\n+\t\trevs->max_age = approxidate(optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"after\", argv, &optarg))) {\n+\t\trevs->max_age = approxidate(optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"min-age\", argv, &optarg))) {\n+\t\trevs->min_age = atoi(optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"before\", argv, &optarg))) {\n+\t\trevs->min_age = approxidate(optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"until\", argv, &optarg))) {\n+\t\trevs->min_age = approxidate(optarg);\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--first-parent\")) {\n \t\trevs->first_parent_only = 1;\n \t} else if (!strcmp(arg, \"--ancestry-path\")) {\n@@ -1295,6 +1305,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->pretty_given = 1;\n \t\tget_commit_format(arg+8, revs);\n \t} else if (!prefixcmp(arg, \"--pretty=\") || !prefixcmp(arg, \"--format=\")) {\n+\t\t/*\n+\t\t * Detached form (\"--pretty X\" as opposed to \"--pretty=X\")\n+\t\t * not allowed, since the argument is optional.\n+\t\t */\n \t\trevs->verbose_header = 1;\n \t\trevs->pretty_given = 1;\n \t\tget_commit_format(arg+9, revs);\n@@ -1359,21 +1373,25 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--relative-date\")) {\n \t\trevs->date_mode = DATE_RELATIVE;\n \t\trevs->date_mode_explicit = 1;\n-\t} else if (!strncmp(arg, \"--date=\", 7)) {\n-\t\trevs->date_mode = parse_date_format(arg + 7);\n+\t} else if ((argcount = diff_long_opt(\"date\", argv, &optarg))) {\n+\t\trevs->date_mode = parse_date_format(optarg);\n \t\trevs->date_mode_explicit = 1;\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--log-size\")) {\n \t\trevs->show_log_size = 1;\n \t}\n \t/*\n \t * Grepping the commit log\n \t */\n-\telse if (!prefixcmp(arg, \"--author=\")) {\n-\t\tadd_header_grep(revs, GREP_HEADER_AUTHOR, arg+9);\n-\t} else if (!prefixcmp(arg, \"--committer=\")) {\n-\t\tadd_header_grep(revs, GREP_HEADER_COMMITTER, arg+12);\n-\t} else if (!prefixcmp(arg, \"--grep=\")) {\n-\t\tadd_message_grep(revs, arg+7);\n+\telse if ((argcount = diff_long_opt(\"author\", argv, &optarg))) {\n+\t\tadd_header_grep(revs, GREP_HEADER_AUTHOR, optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"committer\", argv, &optarg))) {\n+\t\tadd_header_grep(revs, GREP_HEADER_COMMITTER, optarg);\n+\t\treturn argcount;\n+\t} else if ((argcount = diff_long_opt(\"grep\", argv, &optarg))) {\n+\t\tadd_message_grep(revs, optarg);\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--extended-regexp\") || !strcmp(arg, \"-E\")) {\n \t\trevs->grep_filter.regflags |= REG_EXTENDED;\n \t} else if (!strcmp(arg, \"--regexp-ignore-case\") || !strcmp(arg, \"-i\")) {\n@@ -1382,12 +1400,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->grep_filter.fixed = 1;\n \t} else if (!strcmp(arg, \"--all-match\")) {\n \t\trevs->grep_filter.all_match = 1;\n-\t} else if (!prefixcmp(arg, \"--encoding=\")) {\n-\t\targ += 11;\n-\t\tif (strcmp(arg, \"none\"))\n-\t\t\tgit_log_output_encoding = xstrdup(arg);\n+\t} else if ((argcount = diff_long_opt(\"encoding\", argv, &optarg))) {\n+\t\tif (strcmp(optarg, \"none\"))\n+\t\t\tgit_log_output_encoding = xstrdup(optarg);\n \t\telse\n \t\t\tgit_log_output_encoding = \"\";\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--reverse\")) {\n \t\trevs->reverse ^= 1;\n \t} else if (!strcmp(arg, \"--children\")) {\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 36870c5..dfbc173 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -201,6 +201,13 @@ test_expect_success 'log --grep' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'log --grep option parsing' '\n+\techo second >expect &&\n+\tgit log -1 --pretty=\"tformat:%s\" --grep sec >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_must_fail git log -1 --pretty=\"tformat:%s\" --grep\n+'\n+\n test_expect_success 'log -i --grep' '\n \techo Second >expect &&\n \tgit log -1 --pretty=\"tformat:%s\" -i --grep=sec >actual &&\n-- \n1.7.2.252.gb848\n"},{"id":"146762","messageId":"1280478669-22973-5-git-send-email-Matthieu.Moy@imag.fr","threadId":"24570","inReplyTo":"vpq7hkdml22.fsf@bauges.imag.fr","subject":"[PATCH 5/5 v4] log: parse detached option for --glob","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-30T08:31:09Z","receivedAt":"2010-07-30T08:31:09Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n revision.c               |    7 +++++--\n t/t6018-rev-list-glob.sh |    6 ++++++\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 359b1a1..e7a367a 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1484,6 +1484,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n {\n \tint i, flags, left, seen_dashdash, read_from_stdin, got_rev_arg = 0;\n \tconst char **prune_data = NULL;\n+\tconst char *optarg;\n+\tint argcount;\n \n \t/* First, search for \"--\" */\n \tseen_dashdash = 0;\n@@ -1530,10 +1532,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\t\thandle_refs(revs, flags, for_each_remote_ref);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--glob=\")) {\n+\t\t\tif ((argcount = diff_long_opt(\"glob\", argv + i, &optarg))) {\n \t\t\t\tstruct all_refs_cb cb;\n+\t\t\t\ti += argcount - 1;\n \t\t\t\tinit_all_refs_cb(&cb, revs, flags);\n-\t\t\t\tfor_each_glob_ref(handle_one_ref, arg + 7, &cb);\n+\t\t\t\tfor_each_glob_ref(handle_one_ref, optarg, &cb);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!prefixcmp(arg, \"--branches=\")) {\ndiff --git a/t/t6018-rev-list-glob.sh b/t/t6018-rev-list-glob.sh\nindex 58428d9..fb8291c 100755\n--- a/t/t6018-rev-list-glob.sh\n+++ b/t/t6018-rev-list-glob.sh\n@@ -123,6 +123,12 @@ test_expect_success 'rev-list --glob=refs/heads/subspace/*' '\n \n '\n \n+test_expect_success 'rev-list --glob refs/heads/subspace/*' '\n+\n+\tcompare rev-list \"subspace/one subspace/two\" \"--glob refs/heads/subspace/*\"\n+\n+'\n+\n test_expect_success 'rev-list --glob=heads/subspace/*' '\n \n \tcompare rev-list \"subspace/one subspace/two\" \"--glob=heads/subspace/*\"\n-- \n1.7.2.252.gb848\n"},{"id":"146956","messageId":"7v62ztnesc.fsf@alter.siamese.dyndns.org","threadId":"24570","inReplyTo":"1280478669-22973-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 1/5 v4] diff: parse detached options like -S foo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-02T16:46:27Z","receivedAt":"2010-08-02T16:46:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> Change the option parsing logic in revision.c to accept detached forms\n> like `-S foo' in addition to `-Sfoo'. The rest of git already accepted\n> this form, but revision.c still used its own option parsing.\n>\n> This patch does not handle --stat-name-width and --stat-width, which are\n> special-cases where diff_long_opt do not apply. They are handled in a\n> separate patch to ease review.\n>\n> Original patch by Matthieu Moy, plus refactoring by Jonathan Nieder.\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n>  diff.c                       |   87 ++++++++++++++++++++++++++++++++++--------\n>  diff.h                       |    7 +++\n>  t/t4013-diff-various.sh      |    5 ++\n>  t/t4013/diff.log_-S_F_master |    7 +++\n>  t/t4202-log.sh               |   12 ++---\n>  5 files changed, 95 insertions(+), 23 deletions(-)\n>  create mode 100644 t/t4013/diff.log_-S_F_master\n>\n> diff --git a/diff.c b/diff.c\n> index 17873f3..d89ea20 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2990,9 +2990,50 @@ static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *va\n>  \n>  static int diff_scoreopt_parse(const char *opt);\n>  \n> +static inline int short_opt(char opt, const char **argv,\n> +\t\t\t    const char **optarg)\n> +{\n> +\tconst char *arg = argv[0];\n> +\tif (arg[0] != '-' || arg[1] != opt)\n> +\t\treturn 0;\n> +\tif (arg[2] != '\\0') {\n> +\t\t*optarg = arg + strlen(\"-c\");\n\nJust a style thing, but I think \"arg + 2\" is much easier to read in this\nparticular case, as it won't risk tempting readers to go \"Huh?  What does\nthat 'c' mean\"?  I know the code wants to skip over an option that is a\nsingle dash followed by a single byte and 'c' is just a placeholder for\nthat single unknown byte, but when the reader reaches that realization,\nthe reader already knows that the code wants to skip exactly two bytes,\nno?  strlen(\"--\") in diff_long_opt() is much easier to justify, though.  I\nlike the reduction of \"arg + N\"s that follow prefixcmp(arg, \"--whatever\")s\nin diff_opt_parse() this patch gives us in general.  I just think the\nabove \"-c\" is overdoing it.\n\nDo we have an option that can take a zero-length string as its value and\ndo something meaningful?  I don't think of any offhand (\"log -S'' -p\" is\nnot it---it may be meaningful but it is not useful), but this code would\nstart giving \"-p\" instead of \"\" to the option in such a case.\n\nA longer option always required '=' before the value, so it won't share\nthe above issue (if it is an issue, that is).\n\nOther than that minor style issue and a nagging -X<empty> worry, the\nresulting code looks a lot nicer than the original.\n\nThanks.\n"},{"id":"146957","messageId":"7v1vahnebk.fsf@alter.siamese.dyndns.org","threadId":"24570","inReplyTo":"1280478669-22973-3-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 3/5 v4] diff: parse detached options --stat-width n, --stat-name-width n","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-02T16:56:31Z","receivedAt":"2010-08-02T16:56:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> Part of a campaign for unstuck forms of options.\n>\n>  \tswitch (*arg) {\n>  \tcase '-':\n> -\t\tif (!prefixcmp(arg, \"-width=\"))\n> -\t\t\twidth = strtoul(arg + 7, &end, 10);\n> -\t\telse if (!prefixcmp(arg, \"-name-width=\"))\n> -\t\t\tname_width = strtoul(arg + 12, &end, 10);\n> +\t\tif (!prefixcmp(arg, \"-width\")) {\n> +\t\t\targ += strlen(\"-width\");\n> +\t\t\tif (*arg == '=')\n> +\t\t\t\twidth = strtoul(arg + 1, &end, 10);\n> +\t\t\telse if (!*arg && !av[1])\n> +\t\t\t\tdie(\"Option '--stat-width' requires a value\");\n> +\t\t\telse if (!*arg) {\n> +\t\t\t\twidth = strtoul(av[1], &end, 10);\n> +\t\t\t\targcount = 2;\n> +\t\t\t}\n> +\t\t} else if (!prefixcmp(arg, \"-name-width\")) {\n> +\t\t\targ += strlen(\"-name-width\");\n> +\t\t\tif (*arg == '=')\n> +\t\t\t\tname_width = strtoul(arg + 1, &end, 10);\n> +\t\t\telse if (!*arg && !av[1])\n> +\t\t\t\tdie(\"Option '--stat-name-width' requires a value\");\n> +\t\t\telse if (!*arg) {\n> +\t\t\t\tname_width = strtoul(av[1], &end, 10);\n> +\t\t\t\targcount = 2;\n> +\t\t\t}\n> +\t\t}\n>  \t\tbreak;\n>  \tcase '=':\n>  \t\twidth = strtoul(arg+1, &end, 10);\n\nThis will accept \"--stat-width=40\", \"--stat-width 40\", and \"--stat=40,20\"\nbut not \"--stat 40,20\" --- am I reading the patch correctly?\n\nNot a complaint but trying to double-check.  I think it is Ok not to try\nguessing wrong (the user may be interested in a path 40,20, for example).\n\nThanks.\n"},{"id":"146958","messageId":"7vvd7tlzfu.fsf@alter.siamese.dyndns.org","threadId":"24570","inReplyTo":"1280478669-22973-4-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 4/5 v4] log: parse detached options like git log --grep foo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-02T17:03:17Z","receivedAt":"2010-08-02T17:03:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n>  revision.c     |   74 ++++++++++++++++++++++++++++++++++---------------------\n>  t/t4202-log.sh |    7 +++++\n>  2 files changed, 53 insertions(+), 28 deletions(-)\n>\n> diff --git a/revision.c b/revision.c\n> index 7e82efd..359b1a1 100644\n> --- a/revision.c\n> +++ b/revision.c\n> ...\n> @@ -1295,6 +1305,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n>  \t\trevs->pretty_given = 1;\n>  \t\tget_commit_format(arg+8, revs);\n>  \t} else if (!prefixcmp(arg, \"--pretty=\") || !prefixcmp(arg, \"--format=\")) {\n> +\t\t/*\n> +\t\t * Detached form (\"--pretty X\" as opposed to \"--pretty=X\")\n> +\t\t * not allowed, since the argument is optional.\n> +\t\t */\n>  \t\trevs->verbose_header = 1;\n>  \t\trevs->pretty_given = 1;\n>  \t\tget_commit_format(arg+9, revs);\n\nThe patch overall looks good, and this comments illustrates the issue\nrather well.  When the user wants to use \"--longopt val\" syntax, s/he\nneeds to know that \"--longopt\" will always take a value.  Arguably\nmajority of options that can take value will, but like \"--stat X,Y\" this\nleaves things inconsistent.  Without \"--longopt value\" patch there won't\nbe such an inconsistency, but I think this patch series is lessor of two\nevils.\n\nDon't you by the way regret the naming of the parsing function by now?\nThere is nothing \"diff\" about it anymore.\n"},{"id":"146969","messageId":"vpqzkx4df88.fsf@bauges.imag.fr","threadId":"24570","inReplyTo":"7v1vahnebk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/5 v4] diff: parse detached options --stat-width n, --stat-name-width n","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-02T18:47:03Z","receivedAt":"2010-08-02T18:47:03Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> This will accept \"--stat-width=40\", \"--stat-width 40\", and \"--stat=40,20\"\n> but not \"--stat 40,20\" --- am I reading the patch correctly?\n\nYes.\n\n> Not a complaint but trying to double-check.  I think it is Ok not to try\n> guessing wrong (the user may be interested in a path 40,20, for example).\n\nRight. Same goes for all optional arguments, the sticky form is\nrequired.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146971","messageId":"vpqtyncdeuc.fsf@bauges.imag.fr","threadId":"24570","inReplyTo":"7vvd7tlzfu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/5 v4] log: parse detached options like git log --grep foo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-02T18:55:23Z","receivedAt":"2010-08-02T18:55:23Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> The patch overall looks good, and this comments illustrates the issue\n> rather well.  When the user wants to use \"--longopt val\" syntax, s/he\n> needs to know that \"--longopt\" will always take a value.  Arguably\n> majority of options that can take value will, but like \"--stat X,Y\" this\n> leaves things inconsistent.  Without \"--longopt value\" patch there won't\n> be such an inconsistency, but I think this patch series is lessor of two\n> evils.\n\n... especially when parse-option already does this:\n\ngit commit --message foo   => works\ngit gc --prune 'last week' => doesn't\n\nJust like most GNU tools:\n\ngrep --regexp foo => works\ngrep --color auto => doesn't\n\n> Don't you by the way regret the naming of the parsing function by now?\n> There is nothing \"diff\" about it anymore.\n\nRight. I'll rename them to \"parse_long_opt\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146976","messageId":"vpqocdkdcms.fsf@bauges.imag.fr","threadId":"24570","inReplyTo":"7v62ztnesc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/5 v4] diff: parse detached options like -S foo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-02T19:43:07Z","receivedAt":"2010-08-02T19:43:07Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Just a style thing, but I think \"arg + 2\" is much easier to read in this\n> particular case, as it won't risk tempting readers to go \"Huh?  What does\n> that 'c' mean\"?\n\nI'll take this \"+ 2\" in the next round.\n\n> Do we have an option that can take a zero-length string as its value and\n> do something meaningful?  I don't think of any offhand (\"log -S'' -p\" is\n> not it---it may be meaningful but it is not useful), but this code would\n> start giving \"-p\" instead of \"\" to the option in such a case.\n\nI make it explicit in the commit message: only -S, -l and -O are\ntouched by my change among the one-letter options => no problem here.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146977","messageId":"20100802194724.GC2180@burratino","threadId":"24570","inReplyTo":"7v62ztnesc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/5 v4] diff: parse detached options like -S foo","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-02T19:47:24Z","receivedAt":"2010-08-02T19:47:24Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n>> +\tif (arg[2] != '\\0') {\n>> +\t\t*optarg = arg + strlen(\"-c\");\n>\n> Just a style thing, but I think \"arg + 2\" is much easier to read in this\n> particular case, as it won't risk tempting readers to go \"Huh?  What does\n> that 'c' mean\"?\n\nYeah, especially after the \"if (arg[2] != ...)\".  Thanks for a sanity\ncheck.\n\n> Do we have an option that can take a zero-length string as its value and\n> do something meaningful?  I don't think of any offhand (\"log -S'' -p\" is\n> not it---it may be meaningful but it is not useful), but this code would\n> start giving \"-p\" instead of \"\" to the option in such a case.\n\n -l<num>:\t\"\" is not a number.\n\n -S<artifact>:\n\t\tCould \"\" come up if a user tries to find a commit\n\t\t\"Adding/removing string:\" without typing a string to look\n\t\tfor in gitk?  No, gitk protects against that.\n\t\tHow about creating a view with an empty \"Search string:\"?\n\t\tNo, that is protected against, too.\n\n\t\tCurrently \"git log -S\" does not seem to do anything\n\t\tmeaningful, anyway.\n\n -O<file>:\t\"\" is not a file name.\n"},{"id":"146997","messageId":"7vsk2wk8lc.fsf@alter.siamese.dyndns.org","threadId":"24570","inReplyTo":"vpqtyncdeuc.fsf@bauges.imag.fr","subject":"Re: [PATCH 4/5 v4] log: parse detached options like git log --grep foo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-02T21:28:31Z","receivedAt":"2010-08-02T21:28:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> The patch overall looks good, and this comments illustrates the issue\n>> rather well.  When the user wants to use \"--longopt val\" syntax, s/he\n>> needs to know that \"--longopt\" will always take a value.  Arguably\n>> majority of options that can take value will, but like \"--stat X,Y\" this\n>> leaves things inconsistent.  Without \"--longopt value\" patch there won't\n>> be such an inconsistency, but I think this patch series is lessor of two\n>> evils.\n>\n> ... especially when parse-option already does this:\n>\n> git commit --message foo   => works\n> git gc --prune 'last week' => doesn't\n>\n> Just like most GNU tools:\n>\n> grep --regexp foo => works\n> grep --color auto => doesn't\n\nHmm.\n\nAre you hinting that we should keep \"you can say '-Ofoo' and '-O foo'\"\nbits but we should drop \"you can also say '--opt=foo' and '--opt foo' as\nlong as --opt always takes an argument\"?\n\nI actually think that may make sense.\n\n>> Don't you by the way regret the naming of the parsing function by now?\n>> There is nothing \"diff\" about it anymore.\n>\n> Right. I'll rename them to \"parse_long_opt\".\n\nThanks.\n"},{"id":"147020","messageId":"vpq8w4ob3yz.fsf@bauges.imag.fr","threadId":"24570","inReplyTo":"7vsk2wk8lc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/5 v4] log: parse detached options like git log --grep foo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-03T06:33:08Z","receivedAt":"2010-08-03T06:33:08Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> The patch overall looks good, and this comments illustrates the issue\n>>> rather well.  When the user wants to use \"--longopt val\" syntax, s/he\n>>> needs to know that \"--longopt\" will always take a value.  Arguably\n>>> majority of options that can take value will, but like \"--stat X,Y\" this\n>>> leaves things inconsistent.  Without \"--longopt value\" patch there won't\n>>> be such an inconsistency, but I think this patch series is lessor of two\n>>> evils.\n>>\n>> ... especially when parse-option already does this:\n>>\n>> git commit --message foo   => works\n>> git gc --prune 'last week' => doesn't\n>>\n>> Just like most GNU tools:\n>>\n>> grep --regexp foo => works\n>> grep --color auto => doesn't\n>\n> Hmm.\n>\n> Are you hinting that we should keep \"you can say '-Ofoo' and '-O foo'\"\n> bits but we should drop \"you can also say '--opt=foo' and '--opt foo' as\n> long as --opt always takes an argument\"?\n>\n> I actually think that may make sense.\n\nI'm not sure I parsed your sentence correctly, but if I did, then no,\nI don't think we should drop separate forms when --opt always takes an\nargument, just the opposite.\n\n-O<string> => always accepted\n-O <string> => OK if <string> is mandatory and always non-empty\n--opt=<sting> => always accepted\n--opt <string> => OK if <string> is mandatory (possibly empty)\n\nMy remark was that there's nothing very original here, what my patch\ndoes is what other places of Git did, and that other common tools did\nyears before.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"147060","messageId":"7vlj8nipl9.fsf@alter.siamese.dyndns.org","threadId":"24570","inReplyTo":"vpq8w4ob3yz.fsf@bauges.imag.fr","subject":"Re: [PATCH 4/5 v4] log: parse detached options like git log --grep foo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-03T17:16:34Z","receivedAt":"2010-08-03T17:16:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n>>> ... especially when parse-option already does this:\n>>>\n>>> git commit --message foo   => works\n>>> git gc --prune 'last week' => doesn't\n>>>\n>>> Just like most GNU tools:\n>>>\n>>> grep --regexp foo => works\n>>> grep --color auto => doesn't\n>>\n>> Hmm.\n>>\n>> Are you hinting that we should keep \"you can say '-Ofoo' and '-O foo'\"\n>> bits but we should drop \"you can also say '--opt=foo' and '--opt foo' as\n>> long as --opt always takes an argument\"?\n>>\n>> I actually think that may make sense.\n>\n> I'm not sure I parsed your sentence correctly, but if I did, then no,\n> I don't think we should drop separate forms when --opt always takes an\n> argument, just the opposite.\n\nWhat I meant was somewhat different.  I agree that we _could_ take\nseparate \"--opt <string>\" form when --opt is known to always take an\nargument without ambiguity (the same goes for \"-O <string>\").\n\nBut I thought you were suggesting to accept:\n\n    -O<string>\n    -O <string>\n    --opt=<string>\n\nbut never take:\n\n    --opt <string> (even when --opt always takes an argument)\n\nbecause that would further reduce one source of potential confusion\n(i.e. the user needs to remember if --opt always takes an argument or\nnot).  I thought you mentioned that neither parse-options nor GNU tools\ntake \"--prune last\", \"--color auto\" as a good supporting argument for\nthis---the users won't miss the \"--opt <string>\" feature because that is\nnot a common practice.\n\nAnd I was agreeing to that.\n"},{"id":"147068","messageId":"vpq39uvzd7e.fsf@bauges.imag.fr","threadId":"24570","inReplyTo":"7vlj8nipl9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/5 v4] log: parse detached options like git log --grep foo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-03T19:52:05Z","receivedAt":"2010-08-03T19:52:05Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> But I thought you were suggesting to [...] never take:\n>\n>     --opt <string> (even when --opt always takes an argument)\n>\n> because that would further reduce one source of potential confusion\n> (i.e. the user needs to remember if --opt always takes an argument or\n> not).  I thought you mentioned that neither parse-options nor GNU tools\n> take \"--prune last\",\n\nNo, I said the opposite: both do.\n\n> \"--color auto\" as a good supporting argument for this\n\n\"grep --color auto\" is refused because the \"auto\" part is optional.\nBut both \"grep --regex foo\" and \"grep --regexp=foo\" are accepted. Same\nfor \"git commit --message=foo\" and \"git commit --message foo\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"147086","messageId":"87sk2v14n4.fsf@catnip.gol.com","threadId":"24570","inReplyTo":"7vlj8nipl9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/5 v4] log: parse detached options like git log --grep foo","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2010-08-04T02:41:03Z","receivedAt":"2010-08-04T02:41:03Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n> this---the users won't miss the \"--opt <string>\" feature because that is\n> not a common practice.\n>\n> And I was agreeing to that.\n\nNo, that would be not good -- GNU getopt_long supports that form, and\nAFAICT, both the \"--foo=bar\" and \"--foo bar\" forms are widely used (I\ncertainly use both all the time), for roughly the same reasons both\n\"-Oarg\" and \"-O arg\" are both common[*].\n\nIn the case of \"optional-argument options\", the \"--foo bar\" (and \"-O\nbar\") form doesn't work, which is an unfortunate (but apparently\nunavoidable) inconsistency, but optional-argument options are rare\nenough that this doesn't really effect the usage patterns of more\nconventional options.\n\n[*] both forms are useful -- sometimes it's handy to bundle up the\noption-argument with the option, e.g., when writing a throw-away script,\nto avoid whitespace issues, but sometimes it's useful to keep it\nseparate, e.g. so that shell filename completion works on the\noption-argument.\n\n-Miles\n\n-- \nJustice, n. A commodity which in a more or less adulterated condition the\nState sells to the citizen as a reward for his allegiance, taxes and personal\nservice.\n"}]}