{"thread":{"id":"24542","subject":"[PATCH 2/4] Allow detached form for git diff --stat-name-width and --stat-width.","startedAt":"2010-07-27T21:21:55Z","lastAt":"2010-07-29T16:54:25Z","messageCount":24,"participants":["Matthieu Moy","Jonathan Nieder","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"146541","messageId":"1280265719-30968-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24542","inReplyTo":null,"subject":"[PATCH 0/4] Allow detached forms (--option arg) for git log and friends","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-27T21:21:55Z","receivedAt":"2010-07-27T21:21:55Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"After discssion on the early RFC, I decided that a migration to\nparse-option would be too much work given my git time budget and the\nexpected benefits. I'm accomplishing the same goal with very simple\nmacros, and one can now do e.g.\n\n git log -S foo\n git log --grep bar\n\nOptions with optional arguments do not accept this form.\n\nTo ease review, I'm splitting the serie into 4 batches of applications\nof the same pattern.\n\nMatthieu Moy (4):\n Allow detached form (e.g. \"-S foo\" instead of \"-Sfoo\") for diff\n options\n Allow detached form for git diff --stat-name-width and --stat-width.\n Allow detached form (e.g. \"git log --grep foo\") in log options.\n Allow detached form for --glob, --branches, --tags and --remote.\n\n diff.c | 75 +++++++++++++++++++++------\n diff.h | 15 +++++\n revision.c | 117 +++++++++++++++++++++++++++---------------\n t/t4013-diff-various.sh | 1 +\n t/t4013/diff.log_-S_F_master | 7 +++\n t/t4202-log.sh | 12 ++++\n t/t6018-rev-list-glob.sh | 6 ++\n 7 files changed, 176 insertions(+), 57 deletions(-)\n create mode 100644 t/t4013/diff.log_-S_F_master\n\n-- \n1.7.2.25.g9ebe3\n"},{"id":"146538","messageId":"1280265719-30968-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"24542","inReplyTo":"1280265719-30968-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 1/4] Allow detached form (e.g. \"-S foo\" instead of \"-Sfoo\") for diff options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-27T21:21:56Z","receivedAt":"2010-07-27T21:21:56Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"This patch does not handle --stat-name-width and --stat-width, which are\nspecial-cases where IF_LONG_OPT do not apply. They are handled in a\nseparate patch to ease review.\n\nSigned-off-by: Matthieu Moy \n---\n diff.c | 61 +++++++++++++++++++++++++++++++-----------\n diff.h | 15 ++++++++++\n t/t4013-diff-various.sh | 1 +\n t/t4013/diff.log_-S_F_master | 7 +++++\n t/t4202-log.sh | 5 +++\n 5 files changed, 73 insertions(+), 16 deletions(-)\n create mode 100644 t/t4013/diff.log_-S_F_master\n\ndiff --git a/diff.c b/diff.c\nindex 17873f3..41b39be 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2990,9 +2990,23 @@ 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-int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n+int diff_opt_parse(struct diff_options *options, const char **argv, int ac)\n {\n-\tconst char *arg = av[0];\n+\tconst char *arg = argv[0];\n+\tconst char *optarg;\n+\tint argcount;\n+\n+#define IF_SHORT_OPT(optname)\t\t\t\\\n+\t((!strcmp(arg, \"-\" #optname)\t\t\\\n+\t && (argv[1] || (die(\"Option `\" #optname \"' requires a value\"), 1),\t\\\n+\t optarg = argv[1],\t \t\t\\\n+\t argcount = 2,\t\t\t\\\n+\t 1)) ||\t\t\t\t\\\n+\t (!prefixcmp(arg, \"-\" #optname)\t\t\\\n+\t && (optarg = arg + strlen(\"-\" #optname),\t\\\n+\t argcount = 1,\t\t\t\\\n+\t 1)))\n+\n \n \t/* Output format options */\n \tif (!strcmp(arg, \"-p\") || !strcmp(arg, \"-u\") || !strcmp(arg, \"--patch\"))\n@@ -3149,10 +3163,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 (IF_LONG_OPT(word-diff-regex)) {\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 +3195,28 @@ 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 (!strcmp(arg, \"-l\")) {\n+\t\toptions->rename_limit = strtoul(optarg, NULL, 10);\n+\t\treturn 2;\n+\t}\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 (IF_SHORT_OPT(S)) {\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 (IF_SHORT_OPT(O)) {\n+\t\toptions->orderfile = optarg;\n+\t\treturn argcount;\n+\t}\n+\telse if (IF_LONG_OPT(diff-filter)) {\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 +3226,24 @@ 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 (IF_LONG_OPT(src-prefix)) {\n+\t\toptions->a_prefix = optarg;\n+\t\treturn argcount;\n+\t}\n+\telse if (IF_LONG_OPT(dst-prefix)) {\n+\t\toptions->b_prefix = optarg;\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 (IF_LONG_OPT(output)) {\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..ca1a693 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -214,6 +214,21 @@ 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, only meant to be used in\n+ * handle_revision_opt and diff_opt_parse.\n+ */\n+#define IF_LONG_OPT(optname)\t\t\t\t\t\\\n+\t((!strcmp(arg, \"--\" #optname)\t\t\t\t\\\n+\t && (argv[1] || (die(\"Option `\" #optname \"' requires a value\"), 1), \\\n+\t optarg = argv[1],\t \t\t\t\\\n+\t argcount = 2,\t\t\t\t\t\\\n+\t 1)) ||\t\t\t\t\t\t\\\n+\t (!prefixcmp(arg, \"--\" #optname \"=\")\t\t\t\\\n+\t && (optarg = arg + strlen(\"--\" #optname \"=\"),\t\t\\\n+\t argcount = 1,\t\t\t\t\t\\\n+\t 1)))\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..8036e00 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\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 \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..3352935 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -101,11 +101,16 @@ test_expect_success 'oneline' '\n test_expect_success 'diff-filter=A' '\n \n \tactual=$(git log --pretty=\"format:%s\" --diff-filter=A HEAD) &&\n+\tactual_detached=$(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+\ttest \"$actual\" = \"$actual_detached\" || {\n+\t\techo Oops. Detached form broken\n+\t\techo \"Actual_detached: $actual_detached\"\n \t}\n \n '\n-- \n1.7.2.25.g9ebe3\n"},{"id":"146536","messageId":"1280265719-30968-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"24542","inReplyTo":"1280265719-30968-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 2/4] Allow detached form for git diff --stat-name-width and --stat-width.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-27T21:21:57Z","receivedAt":"2010-07-27T21:21:57Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy \n---\n diff.c | 14 ++++++++++++++\n 1 files changed, 14 insertions(+), 0 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 41b39be..c8ee603 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3044,6 +3044,7 @@ int diff_opt_parse(struct diff_options *options, const char **argv, int ac)\n \telse if (!strcmp(arg, \"-s\"))\n \t\toptions->output_format |= DIFF_FORMAT_NO_OUTPUT;\n \telse if (!prefixcmp(arg, \"--stat\")) {\n+\t\targcount = 1;\n \t\tchar *end;\n \t\tint width = options->stat_width;\n \t\tint name_width = options->stat_name_width;\n@@ -3052,8 +3053,20 @@ int diff_opt_parse(struct diff_options *options, const char **argv, int ac)\n \n \t\tswitch (*arg) {\n \t\tcase '-':\n+\t\t\tif (!strcmp(arg, \"-width\")) {\n+\t\t\t\tif (!argv[1])\n+\t\t\t\t\tdie(\"Option `--stat-width' requires a value\");\n+\t\t\t\twidth = strtoul(argv[1], &end, 10);\n+\t\t\t\targcount = 2;\n+\t\t\t}\n \t\t\tif (!prefixcmp(arg, \"-width=\"))\n \t\t\t\twidth = strtoul(arg + 7, &end, 10);\n+\t\t\telse if (!strcmp(arg, \"-name-width\")) {\n+\t\t\t\tif (!argv[1])\n+\t\t\t\t\tdie(\"Option `--stat-name-width' requires a value\");\n+\t\t\t\tname_width = strtoul(argv[1], &end, 10);\n+\t\t\t\targcount = 2;\n+\t\t\t}\t\t\t\t\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@@ -3069,6 +3082,7 @@ int diff_opt_parse(struct diff_options *options, const char **argv, int ac)\n \t\toptions->output_format |= DIFF_FORMAT_DIFFSTAT;\n \t\toptions->stat_name_width = name_width;\n \t\toptions->stat_width = width;\n+\t\treturn argcount;\n \t}\n \n \t/* renames options */\n-- \n1.7.2.25.g9ebe3\n"},{"id":"146539","messageId":"1280265719-30968-4-git-send-email-Matthieu.Moy@imag.fr","threadId":"24542","inReplyTo":"1280265719-30968-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 3/4] Allow detached form (e.g. \"git log --grep foo\") in log options.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-27T21:21:58Z","receivedAt":"2010-07-27T21:21:58Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy \n---\n revision.c | 86 ++++++++++++++++++++++++++++++++++---------------------\n t/t4202-log.sh | 7 ++++\n 2 files changed, 60 insertions(+), 33 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 7e82efd..5d62340 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 (IF_LONG_OPT(max-count)) {\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 (IF_LONG_OPT(skip)) {\n+\t\trevs->skip_count = atoi(optarg);\n+\t\treturn argcount;\n \t} else if ((*arg == '-') && isdigit(arg[1])) {\n \t/* accept -, 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 (IF_LONG_OPT(max-age)) {\n+\t\trevs->max_age = atoi(optarg);\n+\t\treturn argcount;\n+\t} else if (IF_LONG_OPT(since)) {\n+\t\trevs->max_age = approxidate(optarg);\n+\t\treturn argcount;\n+\t} else if (IF_LONG_OPT(after)) {\n+\t\trevs->max_age = approxidate(optarg);\n+\t\treturn argcount;\n+\t} else if (IF_LONG_OPT(min-age)) {\n+\t\trevs->min_age = atoi(optarg);\n+\t\treturn argcount;\n+\t} else if (IF_LONG_OPT(before)) {\n+\t\trevs->min_age = approxidate(optarg);\n+\t\treturn argcount;\n+\t} else if (IF_LONG_OPT(until)) {\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,27 +1305,32 @@ 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 \t} else if (!strcmp(arg, \"--show-notes\")) {\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n-\t} else if (!prefixcmp(arg, \"--show-notes=\")) {\n+\t} else if (IF_LONG_OPT(show-notes)) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n \t\tif (!revs->notes_opt.extra_notes_refs)\n \t\t\trevs->notes_opt.extra_notes_refs = xcalloc(1, sizeof(struct string_list));\n-\t\tif (!prefixcmp(arg+13, \"refs/\"))\n+\t\tif (!prefixcmp(optarg, \"refs/\"))\n \t\t\t/* happy */;\n-\t\telse if (!prefixcmp(arg+13, \"notes/\"))\n+\t\telse if (!prefixcmp(optarg, \"notes/\"))\n \t\t\tstrbuf_addstr(&buf, \"refs/\");\n \t\telse\n \t\t\tstrbuf_addstr(&buf, \"refs/notes/\");\n-\t\tstrbuf_addstr(&buf, arg+13);\n+\t\tstrbuf_addstr(&buf, optarg);\n \t\tstring_list_append(revs->notes_opt.extra_notes_refs,\n \t\t\t\t strbuf_detach(&buf, NULL));\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--no-notes\")) {\n \t\trevs->show_notes = 0;\n \t\trevs->show_notes_given = 1;\n@@ -1343,12 +1358,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->abbrev = 0;\n \t} else if (!strcmp(arg, \"--abbrev\")) {\n \t\trevs->abbrev = DEFAULT_ABBREV;\n-\t} else if (!prefixcmp(arg, \"--abbrev=\")) {\n-\t\trevs->abbrev = strtoul(arg + 9, NULL, 10);\n+\t} else if (IF_LONG_OPT(abbrev)) {\n+\t\trevs->abbrev = strtoul(optarg, NULL, 10);\n \t\tif (revs->abbrev < MINIMUM_ABBREV)\n \t\t\trevs->abbrev = MINIMUM_ABBREV;\n \t\telse if (revs->abbrev > 40)\n \t\t\trevs->abbrev = 40;\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--abbrev-commit\")) {\n \t\trevs->abbrev_commit = 1;\n \t} else if (!strcmp(arg, \"--full-diff\")) {\n@@ -1359,21 +1375,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 (IF_LONG_OPT(date)) {\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 (IF_LONG_OPT(author)) {\n+\t\tadd_header_grep(revs, GREP_HEADER_AUTHOR, optarg);\n+\t\treturn argcount;\n+\t} else if (IF_LONG_OPT(committer)) {\n+\t\tadd_header_grep(revs, GREP_HEADER_COMMITTER, optarg);\n+\t\treturn argcount;\n+\t} else if (IF_LONG_OPT(grep)) {\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 +1402,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 (IF_LONG_OPT(encoding)) {\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 3352935..f912589 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -208,6 +208,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.25.g9ebe3\n"},{"id":"146537","messageId":"1280265719-30968-5-git-send-email-Matthieu.Moy@imag.fr","threadId":"24542","inReplyTo":"1280265719-30968-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 4/4] Allow detached form for --glob, --branches, --tags and --remote.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-27T21:21:59Z","receivedAt":"2010-07-27T21:21:59Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy \n---\n revision.c | 31 +++++++++++++++++++++++--------\n t/t6018-rev-list-glob.sh | 6 ++++++\n 2 files changed, 29 insertions(+), 8 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 5d62340..93e6233 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1486,6 +1486,7 @@ 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 \n \t/* First, search for \"--\" */\n \tseen_dashdash = 0;\n@@ -1501,6 +1502,20 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\tbreak;\n \t}\n \n+\t/*\n+\t * Variant of IF_LONG_OPT, for setup_revisions (use argv[i+1]\n+\t * instead of argv[1], increment i directly)\n+\t */\n+#define IF_LONG_OPT_SR(optname)\t\t\t\t\t\\\n+\t((!strcmp(arg, \"--\" #optname)\t\t\t\t\\\n+\t && (argv[i+1] || (die(\"Option `\" #optname \"' requires a value\"), 1), \\\n+\t optarg = argv[i+1],\t \t\t\t\\\n+\t i++,\t\t\t\t\t\t\\\n+\t 1)) ||\t\t\t\t\t\t\\\n+\t (!prefixcmp(arg, \"--\" #optname \"=\")\t\t\t\\\n+\t && (optarg = arg + strlen(\"--\" #optname \"=\"),\t\t\\\n+\t 1)))\n+\n \t/* Second, deal with arguments and options */\n \tflags = 0;\n \tread_from_stdin = 0;\n@@ -1532,28 +1547,28 @@ 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 (IF_LONG_OPT_SR(glob)) {\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=\")) {\n+\t\t\tif (IF_LONG_OPT_SR(branches)) {\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_in(handle_one_ref, arg + 11, \"refs/heads/\", &cb);\n+\t\t\t\tfor_each_glob_ref_in(handle_one_ref, optarg, \"refs/heads/\", &cb);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--tags=\")) {\n+\t\t\tif (IF_LONG_OPT_SR(tags)) {\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_in(handle_one_ref, arg + 7, \"refs/tags/\", &cb);\n+\t\t\t\tfor_each_glob_ref_in(handle_one_ref, optarg, \"refs/tags/\", &cb);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--remotes=\")) {\n+\t\t\tif (IF_LONG_OPT_SR(remotes)) {\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_in(handle_one_ref, arg + 10, \"refs/remotes/\", &cb);\n+\t\t\t\tfor_each_glob_ref_in(handle_one_ref, optarg, \"refs/remotes/\", &cb);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--reflog\")) {\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.25.g9ebe3\n"},{"id":"146543","messageId":"20100727213750.GB11768@burratino","threadId":"24542","inReplyTo":"1280265719-30968-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 1/4] Allow detached form (e.g. \"-S foo\" instead of \"-Sfoo\") for diff options","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-27T21:37:50Z","receivedAt":"2010-07-27T21:37:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Matthieu Moy wrote:\n\n> +++ b/diff.c\n> @@ -2990,9 +2990,23 @@ static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *va\n[...]\n> +#define IF_SHORT_OPT(optname)\t\t\t\\\n> +\t((!strcmp(arg, \"-\" #optname)\t\t\\\n> +\t && (argv[1] || (die(\"Option `\" #optname \"' requires a value\"), 1),\t\\\n> +\t optarg = argv[1],\t \t\t\\\n> +\t argcount = 2,\t\t\t\\\n> +\t 1)) ||\t\t\t\t\\\n> +\t (!prefixcmp(arg, \"-\" #optname)\t\t\\\n> +\t && (optarg = arg + strlen(\"-\" #optname),\t\\\n> +\t argcount = 1,\t\t\t\\\n> +\t 1)))\n> +\n\nWhy not something like this?\n\n\tstatic inline int short_opt(char opt, const char *arg,\n\t\t\t\t\tconst char *argv, const char **optarg)\n\t{\n\t\tif (arg[0] != '-' || arg[1] != opt)\n\t\t\treturn 0;\n\t\tif (arg[2] != '\\0') {\n\t\t\t*optarg = arg + strlen(\"-c\");\n\t\t\treturn 1;\n\t\t}\n\t\tif (!argv[1])\n\t\t\tdie(\"Option '%c' requires a value\", opt);\n\t\t*optarg = argv[1];\n\t\treturn 2;\n\t}\n\nand then:\n\n\tif ((argcount = short_opt('S', arg, argv, &optarg))) {\n\t\t...\n\n> +++ b/diff.h\n> @@ -214,6 +214,21 @@ 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, only meant to be used in\n> + * handle_revision_opt and diff_opt_parse.\n> + */\n> +#define IF_LONG_OPT(optname)\t\t\t\t\t\\\n> +\t((!strcmp(arg, \"--\" #optname)\t\t\t\t\\\n> +\t && (argv[1] || (die(\"Option `\" #optname \"' requires a value\"), 1), \\\n> +\t optarg = argv[1],\t \t\t\t\\\n> +\t argcount = 2,\t\t\t\t\t\\\n> +\t 1)) ||\t\t\t\t\t\t\\\n> +\t (!prefixcmp(arg, \"--\" #optname \"=\")\t\t\t\\\n> +\t && (optarg = arg + strlen(\"--\" #optname \"=\"),\t\t\\\n> +\t argcount = 1,\t\t\t\t\t\\\n> +\t 1)))\n\n\n static int long_opt(const char *opt, const char *arg,\n\t\t\t\tconst char **argv, const char **optarg)\n {\n\tif (arg[0] != '-' || arg[1] != '-')\n\t\treturn 0;\n\tif (prefixcmp(arg, opt))\n\t\treturn 0;\n\targ += strlen(\"--\") + strlen(opt);\n\tif (*arg == '=') {\n\t\t*optarg = arg + 1;\n\t\treturn 1;\n\t}\n\tif (*arg != '\\0')\n\t\treturn 0;\n\tif (!argv[1])\n\t\tdie(\"Option '--%s' requires a value\", opt);\n\t*optarg = argv[1];\n\treturn 2;\n }\n\nJonathan\n"},{"id":"146587","messageId":"vpqr5ioukca.fsf@bauges.imag.fr","threadId":"24542","inReplyTo":"20100727213750.GB11768@burratino","subject":"Re: [PATCH 1/4] Allow detached form (e.g. \"-S foo\" instead of \"-Sfoo\") for diff options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-07-28T07:38:45Z","receivedAt":"2010-07-28T07:38:45Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jonathan Nieder writes:\n\n> Matthieu Moy wrote:\n>\n>> +++ b/diff.c\n>> @@ -2990,9 +2990,23 @@ static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *va\n> [...]\n>> +#define IF_SHORT_OPT(optname)\t\t\t\\\n>> +\t((!strcmp(arg, \"-\" #optname)\t\t\\\n>> +\t && (argv[1] || (die(\"Option `\" #optname \"' requires a value\"), 1),\t\\\n>> +\t optarg = argv[1],\t \t\t\\\n>> +\t argcount = 2,\t\t\t\\\n>> +\t 1)) ||\t\t\t\t\\\n>> +\t (!prefixcmp(arg, \"-\" #optname)\t\t\\\n>> +\t && (optarg = arg + strlen(\"-\" #optname),\t\\\n>> +\t argcount = 1,\t\t\t\\\n>> +\t 1)))\n>> +\n>\n> Why not something like this?\n>\n> \tstatic inline int short_opt(char opt, const char *arg,\n> \t\t\t\t\tconst char *argv, const char **optarg)\n\nSounds nice too. I'll do that in the next round.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146599","messageId":"1280310062-16793-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24542","inReplyTo":"vpqr5ioukca.fsf@bauges.imag.fr","subject":"[PATCH 1/4 v2] Allow detached form (e.g. \"-S foo\" instead of \"-Sfoo\") for diff options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-28T09:40:59Z","receivedAt":"2010-07-28T09:40:59Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"This 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\nSigned-off-by: Matthieu Moy \n---\n diff.c | 86 ++++++++++++++++++++++++++++++++++--------\n diff.h | 7 +++\n t/t4013-diff-various.sh | 1 +\n t/t4013/diff.log_-S_F_master | 7 +++\n t/t4202-log.sh | 5 ++\n 5 files changed, 90 insertions(+), 16 deletions(-)\n create mode 100644 t/t4013/diff.log_-S_F_master\n\ndiff --git a/diff.c b/diff.c\nindex 17873f3..1868b3e 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,24 @@ 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}\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..8036e00 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\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 \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..3352935 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -101,11 +101,16 @@ test_expect_success 'oneline' '\n test_expect_success 'diff-filter=A' '\n \n \tactual=$(git log --pretty=\"format:%s\" --diff-filter=A HEAD) &&\n+\tactual_detached=$(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+\ttest \"$actual\" = \"$actual_detached\" || {\n+\t\techo Oops. Detached form broken\n+\t\techo \"Actual_detached: $actual_detached\"\n \t}\n \n '\n-- \n1.7.2.25.g9ebe3\n"},{"id":"146598","messageId":"1280310062-16793-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"24542","inReplyTo":"vpqr5ioukca.fsf@bauges.imag.fr","subject":"[PATCH 2/4 v2] Allow detached form for git diff --stat-name-width and --stat-width.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-28T09:41:00Z","receivedAt":"2010-07-28T09:41:00Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy \n---\n diff.c | 14 ++++++++++++++\n 1 files changed, 14 insertions(+), 0 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 1868b3e..04e97ef 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3071,6 +3071,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \telse if (!strcmp(arg, \"-s\"))\n \t\toptions->output_format |= DIFF_FORMAT_NO_OUTPUT;\n \telse if (!prefixcmp(arg, \"--stat\")) {\n+\t\targcount = 1;\n \t\tchar *end;\n \t\tint width = options->stat_width;\n \t\tint name_width = options->stat_name_width;\n@@ -3079,8 +3080,20 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \n \t\tswitch (*arg) {\n \t\tcase '-':\n+\t\t\tif (!strcmp(arg, \"-width\")) {\n+\t\t\t\tif (!av[1])\n+\t\t\t\t\tdie(\"Option `--stat-width' requires a value\");\n+\t\t\t\twidth = strtoul(av[1], &end, 10);\n+\t\t\t\targcount = 2;\n+\t\t\t}\n \t\t\tif (!prefixcmp(arg, \"-width=\"))\n \t\t\t\twidth = strtoul(arg + 7, &end, 10);\n+\t\t\telse if (!strcmp(arg, \"-name-width\")) {\n+\t\t\t\tif (!av[1])\n+\t\t\t\t\tdie(\"Option `--stat-name-width' requires a value\");\n+\t\t\t\tname_width = strtoul(av[1], &end, 10);\n+\t\t\t\targcount = 2;\n+\t\t\t}\t\t\t\t\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@@ -3096,6 +3109,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\toptions->output_format |= DIFF_FORMAT_DIFFSTAT;\n \t\toptions->stat_name_width = name_width;\n \t\toptions->stat_width = width;\n+\t\treturn argcount;\n \t}\n \n \t/* renames options */\n-- \n1.7.2.25.g9ebe3\n"},{"id":"146601","messageId":"1280310062-16793-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"24542","inReplyTo":"vpqr5ioukca.fsf@bauges.imag.fr","subject":"[PATCH 3/4 v2] Allow detached form (e.g. \"git log --grep foo\") in log options.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-28T09:41:01Z","receivedAt":"2010-07-28T09:41:01Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy \n---\n revision.c | 86 ++++++++++++++++++++++++++++++++++---------------------\n t/t4202-log.sh | 7 ++++\n 2 files changed, 60 insertions(+), 33 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 7e82efd..92035a3 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 -, 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,27 +1305,32 @@ 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 \t} else if (!strcmp(arg, \"--show-notes\")) {\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n-\t} else if (!prefixcmp(arg, \"--show-notes=\")) {\n+\t} else if ((argcount = diff_long_opt(\"show-notes\", argv, &optarg))) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n \t\tif (!revs->notes_opt.extra_notes_refs)\n \t\t\trevs->notes_opt.extra_notes_refs = xcalloc(1, sizeof(struct string_list));\n-\t\tif (!prefixcmp(arg+13, \"refs/\"))\n+\t\tif (!prefixcmp(optarg, \"refs/\"))\n \t\t\t/* happy */;\n-\t\telse if (!prefixcmp(arg+13, \"notes/\"))\n+\t\telse if (!prefixcmp(optarg, \"notes/\"))\n \t\t\tstrbuf_addstr(&buf, \"refs/\");\n \t\telse\n \t\t\tstrbuf_addstr(&buf, \"refs/notes/\");\n-\t\tstrbuf_addstr(&buf, arg+13);\n+\t\tstrbuf_addstr(&buf, optarg);\n \t\tstring_list_append(revs->notes_opt.extra_notes_refs,\n \t\t\t\t strbuf_detach(&buf, NULL));\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--no-notes\")) {\n \t\trevs->show_notes = 0;\n \t\trevs->show_notes_given = 1;\n@@ -1343,12 +1358,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->abbrev = 0;\n \t} else if (!strcmp(arg, \"--abbrev\")) {\n \t\trevs->abbrev = DEFAULT_ABBREV;\n-\t} else if (!prefixcmp(arg, \"--abbrev=\")) {\n-\t\trevs->abbrev = strtoul(arg + 9, NULL, 10);\n+\t} else if ((argcount = diff_long_opt(\"abbrev\", argv, &optarg))) {\n+\t\trevs->abbrev = strtoul(optarg, NULL, 10);\n \t\tif (revs->abbrev < MINIMUM_ABBREV)\n \t\t\trevs->abbrev = MINIMUM_ABBREV;\n \t\telse if (revs->abbrev > 40)\n \t\t\trevs->abbrev = 40;\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--abbrev-commit\")) {\n \t\trevs->abbrev_commit = 1;\n \t} else if (!strcmp(arg, \"--full-diff\")) {\n@@ -1359,21 +1375,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 +1402,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 3352935..f912589 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -208,6 +208,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.25.g9ebe3\n"},{"id":"146603","messageId":"1280310062-16793-4-git-send-email-Matthieu.Moy@imag.fr","threadId":"24542","inReplyTo":"vpqr5ioukca.fsf@bauges.imag.fr","subject":"[PATCH 4/4 v2] Allow detached form for --glob, --branches, --tags and --remote.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-28T09:41:02Z","receivedAt":"2010-07-28T09:41:02Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy \n---\n revision.c | 22 ++++++++++++++--------\n t/t6018-rev-list-glob.sh | 6 ++++++\n 2 files changed, 20 insertions(+), 8 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 92035a3..33fa0b5 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1486,6 +1486,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@@ -1532,28 +1534,32 @@ 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=\")) {\n+\t\t\tif ((argcount = diff_long_opt(\"branches\", 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_in(handle_one_ref, arg + 11, \"refs/heads/\", &cb);\n+\t\t\t\tfor_each_glob_ref_in(handle_one_ref, optarg, \"refs/heads/\", &cb);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--tags=\")) {\n+\t\t\tif ((argcount = diff_long_opt(\"tags\", 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_in(handle_one_ref, arg + 7, \"refs/tags/\", &cb);\n+\t\t\t\tfor_each_glob_ref_in(handle_one_ref, optarg, \"refs/tags/\", &cb);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--remotes=\")) {\n+\t\t\tif ((argcount = diff_long_opt(\"remotes\", 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_in(handle_one_ref, arg + 10, \"refs/remotes/\", &cb);\n+\t\t\t\tfor_each_glob_ref_in(handle_one_ref, optarg, \"refs/remotes/\", &cb);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--reflog\")) {\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.25.g9ebe3\n"},{"id":"146605","messageId":"AANLkTikzL-sgysKD+0CZ100xHWZro=-hDgDgUL2Pb3yw@mail.gmail.com","threadId":"24542","inReplyTo":"1280310062-16793-3-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 3/4 v2] Allow detached form (e.g. \"git log --grep foo\") in log options.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-28T10:11:20Z","receivedAt":"2010-07-28T10:11:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Jul 28, 2010 at 09:41, Matthieu Moy wrote:\n\nHere's a better commit message, the subject was >50 chars> (see\nDocumentation/SubmittingPatches):\n\n Subject: git-log: Parse detached options like --grep foo\n\n Change the option parsing logic in revision.c to accept detached forms\n like `--grep foo' in addition to `--grep=foo'. The rest of git already\n accepted this form, but revision.c still used its own option parsing.\n\n> +test_expect_success 'log --grep option parsing' '\n> +       echo second >expect &&\n> +       git log -1 --pretty=\"tformat:%s\" --grep sec >actual &&\n> +       test_cmp expect actual &&\n> +       test_must_fail git log -1 --pretty=\"tformat:%s\" --grep\n> +'\n\nThere's a lot of behavior change in this series, but only two small\ntests that I can see. It would be easy to change the parsing code back\nwithout triggering a regression test.\n"},{"id":"146614","messageId":"vpq8w4vhmkj.fsf@bauges.imag.fr","threadId":"24542","inReplyTo":"AANLkTikzL-sgysKD+0CZ100xHWZro=-hDgDgUL2Pb3yw@mail.gmail.com","subject":"Re: [PATCH 3/4 v2] Allow detached form (e.g. \"git log --grep foo\") in log options.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-07-28T11:29:00Z","receivedAt":"2010-07-28T11:29:00Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Wed, Jul 28, 2010 at 09:41, Matthieu Moy wrote:\n>\n> Here's a better commit message, the subject was >50 chars> (see\n> Documentation/SubmittingPatches):\n\nI don't see a mention of 50 chars there. Conventions usually limit\nlines to <80 chars (sometimes 72), and my subject line is 64\nchars-wide if you don't count [PATCH], which won't end up in git.git.\n\n>> +test_expect_success 'log --grep option parsing' '\n>> +       echo second >expect &&\n>> +       git log -1 --pretty=\"tformat:%s\" --grep sec >actual &&\n>> +       test_cmp expect actual &&\n>> +       test_must_fail git log -1 --pretty=\"tformat:%s\" --grep\n>> +'\n>\n> There's a lot of behavior change in this series, but only two small\n> tests that I can see.\n\n4, not 2.\n\n> It would be easy to change the parsing code back without triggering\n> a regression test.\n\nI've introduced a test for each pattern, but deliberately did not do\nexhaustive testing: I'm testing the patterns, not the way they are\napplied. In the same way, I don't think there's a testcase for each\nuse of parse-option in the testsuite.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146616","messageId":"AANLkTi=TRzn-QWduEYH7qO-4=a-nGqCGSMkZCGn2iB=W@mail.gmail.com","threadId":"24542","inReplyTo":"vpq8w4vhmkj.fsf@bauges.imag.fr","subject":"Re: [PATCH 3/4 v2] Allow detached form (e.g. \"git log --grep foo\") in log options.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-28T12:56:49Z","receivedAt":"2010-07-28T12:56:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Jul 28, 2010 at 11:29, Matthieu Moy\n wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> On Wed, Jul 28, 2010 at 09:41, Matthieu Moy wrote:\n>>\n>> Here's a better commit message, the subject was >50 chars> (see\n>> Documentation/SubmittingPatches):\n>\n> I don't see a mention of 50 chars there. Conventions usually limit\n> lines to <80 chars (sometimes 72), and my subject line is 64\n> chars-wide if you don't count [PATCH], which won't end up in git.git.\n\nHrm, I thought it was explicitly mentioned there, but it says:\n\n\t- the first line of the commit message should be a short\n\t description and should skip the full stop\n\nSince your patch subject had a full stop I thought I'd mention it.\n\nDISCUSSION in git-commit(1) and gittutorial(7) mention 50 characters\nexplicitly though, and a lot of tools that present commits in short\nform use that as a soft limit, e.g. gitweb and github.\n\nI'll submit a patch to SubmittingPatches citing the 50 char soft\nlimit.\n\n>>> +test_expect_success 'log --grep option parsing' '\n>>> +       echo second >expect &&\n>>> +       git log -1 --pretty=\"tformat:%s\" --grep sec >actual &&\n>>> +       test_cmp expect actual &&\n>>> +       test_must_fail git log -1 --pretty=\"tformat:%s\" --grep\n>>> +'\n>>\n>> There's a lot of behavior change in this series, but only two small\n>> tests that I can see.\n>\n> 4, not 2.\n\nI only saw two test_expect_success additions in your v2 series, maybe\nI missed one.\n\n>> It would be easy to change the parsing code back without triggering\n>> a regression test.\n>\n> I've introduced a test for each pattern, but deliberately did not do\n> exhaustive testing: I'm testing the patterns, not the way they are\n> applied. In the same way, I don't think there's a testcase for each\n> use of parse-option in the testsuite.\n\nNot for everything it seems, but the coverage is pretty good:\n\n http://v.nix.is/~avar/cover_db_html/parse-options-c.html\n http://v.nix.is/~avar/cover_db_html/test-parse-options-c.html\n\nAnyway. Regression tests can be added later, and I think this series\nis good enough for inclusion as-is. But option parsing has been a\nmoving target in Git for a while now, and it's quite likely that this\ncode will get rewritten down the line to use a new library that can\nparse all of Git's option syntax.\n\nMore tests coverage would be valuable for catching regressions in such\nfuture rewrites. In general that's the main value, not making sure\n*your* code works, but that someone else doesn't break it.\n\nAnyway, here's how you can easily get almost complete coverage at\nalmost no cost for this series, first make a t/lib-log.sh like this:\n\n #!/bin/sh\n\n log_expect_success() {\n desc=$1\n text=$2\n test_expect_success \"Phony test with attached options: $1\" \"$2\"\n mocked=$(echo \"$2\" | sed -r 's/([A-Za-z-]+)=/\\1 /')\n test_expect_success \"Phony test with detached options: $1\" \"$mocked\"\n }\n\nThen just change the t4*log*.sh tests that aren't doing setup work\n(like git init) to use it:\n\n diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\n index cb9f2bd..7ccc4bb 100755\n --- a/t/t4205-log-pretty-formats.sh\n +++ b/t/t4205-log-pretty-formats.sh\n @@ -20 +20 @@ test_expect_success 'set up basic repos' '\n -test_expect_success 'alias builtin format' '\n +log_expect_success 'alias builtin format' '\n\nWe'll be running tests twice, but it'll catch these at least:\n\n $ ack -h -i '(--[a-z-]+=)' --output '$1' t4*log*.sh | sort | uniq\n --abbrev=\n --author=\n --color=\n --decorate=\n --diff-filter=\n --format=\n --grep=\n --pretty=\n\nI can submit something like that later if I get around to it, or you\ncan include it in your series if you want.\n"},{"id":"146623","messageId":"vpq39v3670o.fsf@bauges.imag.fr","threadId":"24542","inReplyTo":"AANLkTi=TRzn-QWduEYH7qO-4=a-nGqCGSMkZCGn2iB=W@mail.gmail.com","subject":"Re: [PATCH 3/4 v2] Allow detached form (e.g. \"git log --grep foo\") in log options.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-07-28T14:00:23Z","receivedAt":"2010-07-28T14:00:23Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n\n> DISCUSSION in git-commit(1) and gittutorial(7) mention 50 characters\n> explicitly though, and a lot of tools that present commits in short\n> form use that as a soft limit, e.g. gitweb and github.\n\nWhere does github have this limitation?\n\nOn http://github.com/git/git/commits/master, I can see messages like\nthis:\n\ngit-read-tree.txt: acknowledge the directory matching bug in sparse checkout\n\n76 chars-wide, untruncated.\n\n> I'll submit a patch to SubmittingPatches citing the 50 char soft\n> limit.\n\ngit$ git log --format='%s' | wc -l\n22700\ngit$ git log --format='%s' | grep '..................................................' | wc -l \n9392\n\nAlmost half of the commits already there do not comply with this\nsupposed rule.\n\nI'm fine with rewriting the subject, but the claim that commit\nmessages should be <50 chars is just not reasonable and has never been\napplied to git.git.\n\n> I only saw two test_expect_success additions in your v2 series, maybe\n> I missed one.\n\nI'm putting the diff for the t/ directory below to sum up the tests\nadded.\n\n> Not for everything it seems, but the coverage is pretty good:\n>\n> http://v.nix.is/~avar/cover_db_html/parse-options-c.html\n> http://v.nix.is/~avar/cover_db_html/test-parse-options-c.html\n\nYou're precisely illustrating my point: these coverage results are\nabout parse-option, not about the places where it is used. For\nexample:\n\ngit/t$ git grep -e '--message'\ngit/t$\n\n=> git commit --message is not tested, at all (but git commit -m\nobviously is).\n\nIn my case, I didn't gcov it, but I think I've got 100% coverage on\nthe helper functions (short_opt and diff_long_opt), just not 100% on\nthe places where they're used.\n\n> Anyway, here's how you can easily get almost complete coverage at\n> almost no cost for this series, first make a t/lib-log.sh like this:\n>\n> #!/bin/sh\n>\n> log_expect_success() {\n> desc=$1\n> text=$2\n> test_expect_success \"Phony test with attached options: $1\" \"$2\"\n> mocked=$(echo \"$2\" | sed -r 's/([A-Za-z-]+)=/\\1 /')\n> test_expect_success \"Phony test with detached options: $1\" \"$mocked\"\n> }\n\nYou should check whether \"$mocked\" == \"$2\" to avoid useless re-run.\n\n> I can submit something like that later if I get around to it,\n\nGo ahead, I've exhausted my git time budget ;-).\n\n\n\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex dae6358..8036e00 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\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 \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..f912589 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -101,11 +101,16 @@ test_expect_success 'oneline' '\n test_expect_success 'diff-filter=A' '\n \n \tactual=$(git log --pretty=\"format:%s\" --diff-filter=A HEAD) &&\n+\tactual_detached=$(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+\ttest \"$actual\" = \"$actual_detached\" || {\n+\t\techo Oops. Detached form broken\n+\t\techo \"Actual_detached: $actual_detached\"\n \t}\n \n '\n@@ -203,6 +208,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 &&\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\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146631","messageId":"AANLkTikdy9W=mQ0RqGFRPXktTeqvFNrhipqgbVEbJzBC@mail.gmail.com","threadId":"24542","inReplyTo":"vpq39v3670o.fsf@bauges.imag.fr","subject":"Re: [PATCH 3/4 v2] Allow detached form (e.g. \"git log --grep foo\") in log options.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-28T15:03:11Z","receivedAt":"2010-07-28T15:03:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Jul 28, 2010 at 14:00, Matthieu Moy\n wrote:\n> Ęvar Arnfjörš Bjarmason writes:\n>\n>> DISCUSSION in git-commit(1) and gittutorial(7) mention 50 characters\n>> explicitly though, and a lot of tools that present commits in short\n>> form use that as a soft limit, e.g. gitweb and github.\n>\n> Where does github have this limitation?\n>\n> On http://github.com/git/git/commits/master, I can see messages like\n> this:\n>\n> git-read-tree.txt: acknowledge the directory matching bug in sparse checkout\n>\n> 76 chars-wide, untruncated.\n\nA lot of applications do it in their public timeline, e.g. at GitHub:\n\n http://github.com/mirrors\n\nAlthough their limit seems to be closer to 65 characters now, but I'm\nfairly sure that it was 50 before. It's also overflowing my screen now\nas a result.\n\n>> I'll submit a patch to SubmittingPatches citing the 50 char soft\n>> limit.\n>\n> git$ git log --format='%s' | wc -l\n> 22700\n> git$ git log --format='%s' | grep '..................................................' | wc -l\n> 9392\n>\n> Almost half of the commits already there do not comply with this\n> supposed rule.\n\nIt's not a rule, just a soft limit. Anyway, I just mentioned it\nbecause the subject ended with a full stop. Which indicated that you\nhadn't spotted that bit in SubmittingPatches, and might want to know\nfor future submissions.\n\nThere's also a quickly decreasing long tail after 50 characters.\n\n $ for i in {50..70}; do git log --format='%s' | egrep \".{$i}\" | wc\n-l | tr '\\n' ' '; done && echo\n 9391 8847 8341 7838 7338 6840 6379 5915 5456 5018 4585 4184 3779\n3418 3045 2751 2434 2153 1871 1655 1438\n\n(Use Extended Regular Expressions, your \".\" key will thank you >:)\n\n> I'm fine with rewriting the subject, but the claim that commit\n> messages should be <50 chars is just not reasonable and has never been\n> applied to git.git.\n>\n>> I only saw two test_expect_success additions in your v2 series, maybe\n>> I missed one.\n>\n> I'm putting the diff for the t/ directory below to sum up the tests\n> added.\n\nThanks. I missed 2/4 because I only grep-ed for the test harness\nfunctions.\n\n>> Not for everything it seems, but the coverage is pretty good:\n>>\n>>     http://v.nix.is/~avar/cover_db_html/parse-options-c.html\n>>     http://v.nix.is/~avar/cover_db_html/test-parse-options-c.html\n>\n> You're precisely illustrating my point: these coverage results are\n> about parse-option, not about the places where it is used. For\n> example:\n>\n> git/t$ git grep -e '--message'\n> git/t$\n\n*added to my list of stuff to test*\n\n> => git commit --message is not tested, at all (but git commit -m\n> obviously is).\n>\n> In my case, I didn't gcov it, but I think I've got 100% coverage on\n> the helper functions (short_opt and diff_long_opt), just not 100% on\n> the places where they're used.\n\nI just mentioned the coverage as an aside (and probably shouldn't\nhave. What I mainly meant is that it would be easy to regress on\noption parsing in revision.c in the future because e.g. nothing tests\nboth \"--abbrev foo' and \"--abbrev=foo\".\n\n>> Anyway, here's how you can easily get almost complete coverage at\n>> almost no cost for this series, first make a t/lib-log.sh like this:\n>>\n>>     #!/bin/sh\n>>\n>>     log_expect_success() {\n>>         desc=$1\n>>         text=$2\n>>         test_expect_success \"Phony test with attached options: $1\" \"$2\"\n>>         mocked=$(echo \"$2\" | sed -r 's/([A-Za-z-]+)=/\\1 /')\n>>         test_expect_success \"Phony test with detached options: $1\" \"$mocked\"\n>>     }\n>\n> You should check whether \"$mocked\" == \"$2\" to avoid useless re-run.\n\nYeah, it's just pseudocode. Assigning to desc and text and not using\nthem is also pretty useless. I didn't even run it, but I'm delighted\nto find that it at least compiles and executes.\n\n>> I can submit something like that later if I get around to it,\n>\n> Go ahead, I've exhausted my git time budget ;-).\n\nMaybe I will. Anyway, like I said this patch looks just fine. I mainly\njust wanted to note that it was under-tested (but that it should go in\nanyway, IMO), if for no other reason than as a future note to\nmyself. Or anyone else wanting to improve our test coverage.\n"},{"id":"146681","messageId":"20100729020031.GI29156@dert.cs.uchicago.edu","threadId":"24542","inReplyTo":"1280310062-16793-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 1/4 v2] Allow detached form (e.g. \"-S foo\" instead of \"-Sfoo\") for diff options","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-29T02:00:31Z","receivedAt":"2010-07-29T02:00:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Matthieu Moy wrote:\n\n> 5 files changed, 90 insertions(+), 16 deletions(-)\n> create mode 100644 t/t4013/diff.log_-S_F_master\n\nWell, I like it, though I’m obviously biased.\n\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\nNote to self: a nice mini-series might be to blur these out.\n\n> +++ b/t/t4202-log.sh\n> @@ -101,11 +101,16 @@ test_expect_success 'oneline' '\n> test_expect_success 'diff-filter=A' '\n> \n> \tactual=$(git log --pretty=\"format:%s\" --diff-filter=A HEAD) &&\n> +\tactual_detached=$(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> +\ttest \"$actual\" = \"$actual_detached\" || {\n> +\t\techo Oops. Detached form broken\n> +\t\techo \"Actual_detached: $actual_detached\"\n> \t}\n\nYou left out the crucial \"false\"! :)\n\nBelow is an add-on patch to use a more readable style. With\nor without that change:\n\nReviewed-by: Jonathan Nieder \n\nThanks.\n\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 3352935..36870c5 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -100,18 +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-\tactual_detached=$(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-\ttest \"$actual\" = \"$actual_detached\" || {\n-\t\techo Oops. Detached form broken\n-\t\techo \"Actual_detached: $actual_detached\"\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-- \n"},{"id":"146685","messageId":"20100729023627.GJ29156@dert.cs.uchicago.edu","threadId":"24542","inReplyTo":"1280310062-16793-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 2/4 v2] Allow detached form for git diff --stat-name-width and --stat-width.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-29T02:36:28Z","receivedAt":"2010-07-29T02:36:28Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Matthieu Moy wrote:\n\n> diff.c | 14 ++++++++++++++\n> 1 files changed, 14 insertions(+), 0 deletions(-)\n\nJust some nitpicks:\n \n> +++ b/diff.c\n> @@ -3071,6 +3071,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n> \telse if (!strcmp(arg, \"-s\"))\n> \t\toptions->output_format |= DIFF_FORMAT_NO_OUTPUT;\n> \telse if (!prefixcmp(arg, \"--stat\")) {\n> +\t\targcount = 1;\n> \t\tchar *end;\n\ndeclaration after statement.\n\n> @@ -3079,8 +3080,20 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n> \n> \t\tswitch (*arg) {\n> \t\tcase '-':\n> +\t\t\tif (!strcmp(arg, \"-width\")) {\n> +\t\t\t\tif (!av[1])\n> +\t\t\t\t\tdie(\"Option `--stat-width' requires a value\");\n> +\t\t\t\twidth = strtoul(av[1], &end, 10);\n> +\t\t\t\targcount = 2;\n> +\t\t\t}\n> \t\t\tif (!prefixcmp(arg, \"-width=\"))\n\nCould save an extra string comparison with an \"else\" here.\n\n[...]\n> +\t\t\t\tname_width = strtoul(av[1], &end, 10);\n> +\t\t\t\targcount = 2;\n> +\t\t\t}\t\t\t\t\n\nWhitespace damage: trailing tabs.\n\nHow about something like the following?\n\nJonathan Nieder (1):\n diff: split off a function for --stat-* option parsing\n\nMatthieu Moy (1):\n diff: allow --stat-width n, --stat-name-width n\n\n diff.c | 80 ++++++++++++++++++++++++++++++++++++++++++---------------------\n 1 files changed, 53 insertions(+), 27 deletions(-)\n"},{"id":"146686","messageId":"20100729023750.GK29156@dert.cs.uchicago.edu","threadId":"24542","inReplyTo":"20100729023627.GJ29156@dert.cs.uchicago.edu","subject":"[PATCH 1/2] diff: split off a function for --stat-* option parsing","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-29T02:37:50Z","receivedAt":"2010-07-29T02:37:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"As 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 \n---\n diff.c | 62 +++++++++++++++++++++++++++++++++++---------------------------\n 1 files changed, 35 insertions(+), 27 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 1868b3e..641a326 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.6.5\n"},{"id":"146687","messageId":"20100729023835.GL29156@dert.cs.uchicago.edu","threadId":"24542","inReplyTo":"20100729023627.GJ29156@dert.cs.uchicago.edu","subject":"[PATCH 2/2] diff: allow --stat-width n, --stat-name-width n","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-29T02:38:35Z","receivedAt":"2010-07-29T02:38:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Matthieu Moy \n\nPart of a campaign for unstuck forms of options.\n\n[jn: with some refactoring]\n\nSigned-off-by: Matthieu Moy \nSigned-off-by: Jonathan Nieder \n---\n diff.c | 28 +++++++++++++++++++++++-----\n 1 files changed, 23 insertions(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 641a326..55edc3e 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.6.5\n"},{"id":"146688","messageId":"20100729024638.GM29156@dert.cs.uchicago.edu","threadId":"24542","inReplyTo":"1280310062-16793-3-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 3/4 v2] Allow detached form (e.g. \"git log --grep foo\") in log options.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-29T02:46:38Z","receivedAt":"2010-07-29T02:46:38Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Matthieu Moy wrote:\n\n> +++ b/revision.c\n[...]\n> @@ -1295,27 +1305,32 @@ 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> \t} else if (!strcmp(arg, \"--show-notes\")) {\n> \t\trevs->show_notes = 1;\n> \t\trevs->show_notes_given = 1;\n> -\t} else if (!prefixcmp(arg, \"--show-notes=\")) {\n> +\t} else if ((argcount = diff_long_opt(\"show-notes\", argv, &optarg))) {\n\nWhy is this not like the --pretty case?\n\n[...]\n> @@ -1343,12 +1358,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> \t\trevs->abbrev = 0;\n> \t} else if (!strcmp(arg, \"--abbrev\")) {\n> \t\trevs->abbrev = DEFAULT_ABBREV;\n> -\t} else if (!prefixcmp(arg, \"--abbrev=\")) {\n> -\t\trevs->abbrev = strtoul(arg + 9, NULL, 10);\n> +\t} else if ((argcount = diff_long_opt(\"abbrev\", argv, &optarg))) {\n> +\t\trevs->abbrev = strtoul(optarg, NULL, 10);\n\nLikewise.\n\n> +\ttest_must_fail git log -1 --pretty=\"tformat:%s\" --grep\n\nThanks for checking an edge case.\n\nWith or without the following change,\nReviewed-by: Jonathan Nieder \n\ndiff --git a/revision.c b/revision.c\nindex 92035a3..7006340 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1315,22 +1315,21 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--show-notes\")) {\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n-\t} else if ((argcount = diff_long_opt(\"show-notes\", argv, &optarg))) {\n+\t} else if (!prefixcmp(arg, \"--show-notes=\")) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n \t\tif (!revs->notes_opt.extra_notes_refs)\n \t\t\trevs->notes_opt.extra_notes_refs = xcalloc(1, sizeof(struct string_list));\n-\t\tif (!prefixcmp(optarg, \"refs/\"))\n+\t\tif (!prefixcmp(arg+13, \"refs/\"))\n \t\t\t/* happy */;\n-\t\telse if (!prefixcmp(optarg, \"notes/\"))\n+\t\telse if (!prefixcmp(arg+13, \"notes/\"))\n \t\t\tstrbuf_addstr(&buf, \"refs/\");\n \t\telse\n \t\t\tstrbuf_addstr(&buf, \"refs/notes/\");\n-\t\tstrbuf_addstr(&buf, optarg);\n+\t\tstrbuf_addstr(&buf, arg+13);\n \t\tstring_list_append(revs->notes_opt.extra_notes_refs,\n \t\t\t\t strbuf_detach(&buf, NULL));\n-\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--no-notes\")) {\n \t\trevs->show_notes = 0;\n \t\trevs->show_notes_given = 1;\n@@ -1358,13 +1357,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->abbrev = 0;\n \t} else if (!strcmp(arg, \"--abbrev\")) {\n \t\trevs->abbrev = DEFAULT_ABBREV;\n-\t} else if ((argcount = diff_long_opt(\"abbrev\", argv, &optarg))) {\n-\t\trevs->abbrev = strtoul(optarg, NULL, 10);\n+\t} else if (!prefixcmp(arg, \"--abbrev=\")) {\n+\t\trevs->abbrev = strtoul(arg + 9, NULL, 10);\n \t\tif (revs->abbrev < MINIMUM_ABBREV)\n \t\t\trevs->abbrev = MINIMUM_ABBREV;\n \t\telse if (revs->abbrev > 40)\n \t\t\trevs->abbrev = 40;\n-\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--abbrev-commit\")) {\n \t\trevs->abbrev_commit = 1;\n \t} else if (!strcmp(arg, \"--full-diff\")) {\n-- \n"},{"id":"146689","messageId":"20100729024834.GN29156@dert.cs.uchicago.edu","threadId":"24542","inReplyTo":"1280310062-16793-4-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 4/4 v2] Allow detached form for --glob, --branches, --tags and --remote.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-29T02:48:34Z","receivedAt":"2010-07-29T02:48:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Matthieu Moy wrote:\n\n> -\t\t\tif (!prefixcmp(arg, \"--branches=\")) {\n> +\t\t\tif ((argcount = diff_long_opt(\"branches\", argv + i, &optarg))) {\n> +\t\t\t\ti += argcount - 1;\n> \t\t\t\tstruct all_refs_cb cb;\n\nAs before: is this the right change where the argument is\noptional?\n\nThanks for a pleasant read.\nJonathan\n"},{"id":"146698","messageId":"vpqeiemoivf.fsf@bauges.imag.fr","threadId":"24542","inReplyTo":"20100729020031.GI29156@dert.cs.uchicago.edu","subject":"Re: [PATCH 1/4 v2] Allow detached form (e.g. \"-S foo\" instead of \"-Sfoo\") for diff options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-07-29T07:19:16Z","receivedAt":"2010-07-29T07:19:16Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jonathan Nieder writes:\n\n>> +\t} &&\n>> +\ttest \"$actual\" = \"$actual_detached\" || {\n>> +\t\techo Oops. Detached form broken\n>> +\t\techo \"Actual_detached: $actual_detached\"\n>> \t}\n>\n> You left out the crucial \"false\"! :)\n\nOups, right.\n\n> Below is an add-on patch to use a more readable style.\n\nI was mimicking the style right above, but that makes sense to\nclean-up while we're there, yes. I'll squash your change in the next\nserie.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146723","messageId":"7vvd7yteim.fsf@alter.siamese.dyndns.org","threadId":"24542","inReplyTo":"vpqeiemoivf.fsf@bauges.imag.fr","subject":"Re: [PATCH 1/4 v2] Allow detached form (e.g. \"-S foo\" instead of \"-Sfoo\") for diff options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-29T16:54:25Z","receivedAt":"2010-07-29T16:54:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Jonathan Nieder writes:\n>>\n>> You left out the crucial \"false\"! :)\n>\n> Oups, right.\n>\n>> Below is an add-on patch to use a more readable style.\n>\n> I was mimicking the style right above, but that makes sense to\n> clean-up while we're there, yes. I'll squash your change in the next\n> serie.\n\nThanks both; I've also seen Jonathan's --stat-* rewrite and agree with\nhim. Looking forward to see a re-roll.\n"}]}