{"thread":{"id":"38997","subject":"[PATCH v2 5/5] bash-completion: add support for git-log --merges= and log.merges","startedAt":"2015-04-04T01:21:57Z","lastAt":"2015-04-13T04:56:26Z","messageCount":14,"participants":["Koosha Khajehmoogahi","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"258970","messageId":"1428110521-31028-1-git-send-email-koosha@posteo.de","threadId":"38997","inReplyTo":"266077","subject":"[PATCH v2 1/5] revision: add --merges={show|only|hide} option","fromName":"Koosha Khajehmoogahi","fromEmail":"koosha@posteo.de","sentAt":"2015-04-04T01:21:57Z","receivedAt":"2015-04-04T01:21:57Z","isPatch":true,"sender":{"key":"koosha@posteo.de","avatar":null},"body":"From: Junio C Hamano <gitster@pobox.com>\n\nrevision: add a new option 'merges=' with\npossible values of 'only', 'show' and 'hide'.\nThe option is used when showing the list of commits.\nThe value 'only' lists only merges. The value 'show'\nis the default behavior which shows the commits as well\nas merges and the value 'hide' makes it just list commit\nitems.\n\n[kk: chose names for options; wrote commit message]\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Koosha Khajehmoogahi <koosha@posteo.de>\n---\n revision.c | 20 ++++++++++++++++++++\n revision.h |  1 +\n 2 files changed, 21 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex 6399a04..c3c3dcc 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1678,6 +1678,23 @@ static void add_message_grep(struct rev_info *revs, const char *pattern)\n \tadd_grep(revs, pattern, GREP_PATTERN_BODY);\n }\n \n+int parse_merges_opt(struct rev_info *revs, const char *param)\n+{\n+\tif (!strcmp(param, \"show\")) {\n+\t\trevs->min_parents = 0;\n+\t\trevs->max_parents = -1;\n+\t} else if (!strcmp(param, \"only\")) {\n+\t\trevs->min_parents = 2;\n+\t\trevs->max_parents = -1;\n+\t} else if (!strcmp(param, \"hide\")) {\n+\t\trevs->min_parents = 0;\n+\t\trevs->max_parents = 1;\n+\t} else {\n+\t\treturn -1;\n+\t}\n+\treturn 0;\n+}\n+\n static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n \t\t\t       int *unkc, const char **unkv)\n {\n@@ -1800,6 +1817,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->show_all = 1;\n \t} else if (!strcmp(arg, \"--remove-empty\")) {\n \t\trevs->remove_empty_trees = 1;\n+\t} else if (starts_with(arg, \"--merges=\")) {\n+\t\tif (parse_merges_opt(revs, arg + 9))\n+\t\t\tdie(\"unknown option: %s\", arg);\n \t} else if (!strcmp(arg, \"--merges\")) {\n \t\trevs->min_parents = 2;\n \t} else if (!strcmp(arg, \"--no-merges\")) {\ndiff --git a/revision.h b/revision.h\nindex 0ea8b4e..f9df58c 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -240,6 +240,7 @@ extern int setup_revisions(int argc, const char **argv, struct rev_info *revs,\n extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n \t\t\t       const struct option *options,\n \t\t\t       const char * const usagestr[]);\n+extern int parse_merges_opt(struct rev_info *, const char *);\n #define REVARG_CANNOT_BE_FILENAME 01\n #define REVARG_COMMITTISH 02\n extern int handle_revision_arg(const char *arg, struct rev_info *revs,\n-- \n2.3.3.263.g095251d.dirty\n"},{"id":"258969","messageId":"1428110521-31028-2-git-send-email-koosha@posteo.de","threadId":"38997","inReplyTo":"1428110521-31028-1-git-send-email-koosha@posteo.de","subject":"[PATCH v2 2/5] log: honor log.merges= option","fromName":"Koosha Khajehmoogahi","fromEmail":"koosha@posteo.de","sentAt":"2015-04-04T01:21:58Z","receivedAt":"2015-04-04T01:21:58Z","isPatch":true,"sender":{"key":"koosha@posteo.de","avatar":null},"body":"From: Junio C Hamano <gitster@pobox.com>\n\n[kk: wrote commit message]\n\nHelped-by: Eris Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Koosha Khajehmoogahi <koosha@posteo.de>\n---\n builtin/log.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex dd8f3fc..c7a7aad 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -36,6 +36,7 @@ static int decoration_given;\n static int use_mailmap_config;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n static const char *fmt_pretty;\n+static const char *log_merges;\n \n static const char * const builtin_log_usage[] = {\n \tN_(\"git log [<options>] [<revision range>] [[--] <path>...]\"),\n@@ -386,6 +387,9 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\t\tdecoration_style = 0; /* maybe warn? */\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"log.merges\")) {\n+\t\treturn git_config_string(&log_merges, var, value);\n+\t}\n \tif (!strcmp(var, \"log.showroot\")) {\n \t\tdefault_show_root = git_config_bool(var, value);\n \t\treturn 0;\n@@ -628,6 +632,8 @@ int cmd_log(int argc, const char **argv, const char *prefix)\n \n \tinit_revisions(&rev, prefix);\n \trev.always_show_header = 1;\n+\tif (log_merges && parse_merges_opt(&rev, log_merges))\n+\t\tdie(\"unknown config value for log.merges: %s\", log_merges);\n \tmemset(&opt, 0, sizeof(opt));\n \topt.def = \"HEAD\";\n \topt.revarg_opt = REVARG_COMMITTISH;\n-- \n2.3.3.263.g095251d.dirty\n"},{"id":"258971","messageId":"1428110521-31028-3-git-send-email-koosha@posteo.de","threadId":"38997","inReplyTo":"1428110521-31028-1-git-send-email-koosha@posteo.de","subject":"[PATCH v2 3/5] Documentation: add git-log --merges= option and log.merges config. var","fromName":"Koosha Khajehmoogahi","fromEmail":"koosha@posteo.de","sentAt":"2015-04-04T01:21:59Z","receivedAt":"2015-04-04T01:21:59Z","isPatch":true,"sender":{"key":"koosha@posteo.de","avatar":null},"body":"Helped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Koosha Khajehmoogahi <koosha@posteo.de>\n---\n Documentation/git-log.txt          |  3 +++\n Documentation/rev-list-options.txt | 18 +++++++++++++++---\n 2 files changed, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex 18bc716..e16f0f8 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -190,6 +190,9 @@ log.showRoot::\n \t`git log -p` output would be shown without a diff attached.\n \tThe default is `true`.\n \n+log.merges::\n+    Default for `--merges=` option. Defaults to `show`.\n+\n mailmap.*::\n \tSee linkgit:git-shortlog[1].\n \ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex f620ee4..0bb2390 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -96,12 +96,24 @@ if it is part of the log message.\n --remove-empty::\n \tStop when a given path disappears from the tree.\n \n+--merges={show|hide|only}::\n++\n+--\n+`show`: show both merge and non-merge commits\n+\n+`hide`: only show non-merge commits; same as `--max-parents=1`\n+\n+`only`: only show merge commits; same as `--min-parents=2`\n+\n+If `--merges=` is not specified, default value is `show`.\n+--\n++\n+\n --merges::\n-\tPrint only merge commits. This is exactly the same as `--min-parents=2`.\n+\tShorthand for `--merges=only`.\n \n --no-merges::\n-\tDo not print commits with more than one parent. This is\n-\texactly the same as `--max-parents=1`.\n+\tShorthand for `--merges=hide`.\n \n --min-parents=<number>::\n --max-parents=<number>::\n-- \n2.3.3.263.g095251d.dirty\n"},{"id":"258972","messageId":"1428110521-31028-4-git-send-email-koosha@posteo.de","threadId":"38997","inReplyTo":"1428110521-31028-1-git-send-email-koosha@posteo.de","subject":"[PATCH v2 4/5] t4202-log: add tests for --merges=","fromName":"Koosha Khajehmoogahi","fromEmail":"koosha@posteo.de","sentAt":"2015-04-04T01:22:00Z","receivedAt":"2015-04-04T01:22:00Z","isPatch":true,"sender":{"key":"koosha@posteo.de","avatar":null},"body":"Helped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Koosha Khajehmoogahi <koosha@posteo.de>\n---\n t/t4202-log.sh | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 84 insertions(+)\n\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 1b2e981..ceaaf4e 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -270,6 +270,90 @@ cat > expect <<\\EOF\n * initial\n EOF\n \n+test_expect_success 'setup --merges=' '\n+\tgit log --min-parents=2 --pretty=tformat:%s >expect_only &&\n+\tgit log --max-parents=1 --pretty=tformat:%s >expect_hide &&\n+\tgit log --min-parents=-1 --pretty=tformat:%s >expect_show\n+'\n+\n+test_expect_success 'log with config log.merges=show' '\n+\ttest_config log.merges show &&\n+\tgit log --pretty=tformat:%s >actual &&\n+\ttest_cmp expect_show actual\n+'\n+\n+test_expect_success 'log with config log.merges=only' '\n+\ttest_config log.merges only &&\n+\tgit log --pretty=tformat:%s >actual &&\n+\ttest_cmp expect_only actual\n+'\n+\n+test_expect_success 'log with config log.merges=hide' '\n+\ttest_config log.merges hide &&\n+\tgit log --pretty=tformat:%s >actual &&\n+\ttest_cmp expect_hide actual\n+'\n+\n+test_expect_success 'log with config log.merges=show with git log --no-merges' '\n+\ttest_config log.merges show &&\n+\tgit log --no-merges --pretty=tformat:%s >actual &&\n+\ttest_cmp expect_hide actual\n+'\n+\n+test_expect_success 'log with config log.merges=hide with git log ---merges' '\n+\ttest_config log.merges hide &&\n+\tgit log --merges --pretty=tformat:%s >actual &&\n+\ttest ! -s actual\n+'\n+\n+test_expect_success 'log --merges=show' '\n+\ttest_unconfig log.merges &&\n+\tgit log --merges=show --pretty=tformat:%s >actual &&\n+\ttest_cmp expect_show actual\n+'\n+\n+test_expect_success 'log --merges=only' '\n+\ttest_unconfig log.merges &&\n+\tgit log --merges=only --pretty=tformat:%s >actual &&\n+\ttest_cmp expect_only actual\n+'\n+\n+test_expect_success 'log --merges=hide' '\n+\ttest_unconfig log.merges &&\n+\tgit log --merges=hide --pretty=tformat:%s >actual &&\n+\ttest_cmp expect_hide actual\n+'\n+\n+test_log_merges() {\n+\ttest_config log.merges $2 &&\n+\tgit log --merges=$1 --pretty=tformat:%s >actual &&\n+\ttest_cmp $3 actual\n+}\n+\n+test_expect_success 'log --merges=show with config log.merges=hide' '\n+\ttest_log_merges show hide expect_show\n+'\n+\n+test_expect_success 'log --merges=show with config log.merges=only' '\n+\ttest_log_merges show only expect_show\n+'\n+\n+test_expect_success 'log --merges=only with config log.merges=show' '\n+\ttest_log_merges only show expect_only\n+'\n+\n+test_expect_success 'log --merges=only with config log.merges=hide' '\n+\ttest_log_merges only hide expect_only\n+'\n+\n+test_expect_success 'log --merges=hide with config log.merges=show' '\n+\ttest_log_merges hide show expect_hide\n+'\n+\n+test_expect_success 'log --merges=hide with config log.merges=only' '\n+\ttest_log_merges hide only expect_hide\n+'\n+\n test_expect_success 'log --graph with merge' '\n \tgit log --graph --date-order --pretty=tformat:%s |\n \t\tsed \"s/ *\\$//\" >actual &&\n-- \n2.3.3.263.g095251d.dirty\n"},{"id":"258968","messageId":"1428110521-31028-5-git-send-email-koosha@posteo.de","threadId":"38997","inReplyTo":"1428110521-31028-1-git-send-email-koosha@posteo.de","subject":"[PATCH v2 5/5] bash-completion: add support for git-log --merges= and log.merges","fromName":"Koosha Khajehmoogahi","fromEmail":"koosha@posteo.de","sentAt":"2015-04-04T01:22:01Z","receivedAt":"2015-04-04T01:22:01Z","isPatch":true,"sender":{"key":"koosha@posteo.de","avatar":null},"body":"Helped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Koosha Khajehmoogahi <koosha@posteo.de>\n---\n contrib/completion/git-completion.bash | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex fbe5972..a75d7f5 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1406,7 +1406,7 @@ _git_ls_tree ()\n __git_log_common_options=\"\n \t--not --all\n \t--branches --tags --remotes\n-\t--first-parent --merges --no-merges\n+\t--first-parent --merges --merges= --no-merges\n \t--max-count=\n \t--max-age= --since= --after=\n \t--min-age= --until= --before=\n@@ -1451,7 +1451,11 @@ _git_log ()\n \t\t__gitcomp \"long short\" \"\" \"${cur##--decorate=}\"\n \t\treturn\n \t\t;;\n-\t--*)\n+\t--merges=*)\n+\t\t__gitcomp \"show hide only\" \"\" \"${cur##--merges=}\"\n+\t\treturn\n+\t\t;;\n+\t\t--*)\n \t\t__gitcomp \"\n \t\t\t$__git_log_common_options\n \t\t\t$__git_log_shortlog_options\n@@ -1861,6 +1865,10 @@ _git_config ()\n \t\t__gitcomp \"$__git_log_date_formats\"\n \t\treturn\n \t\t;;\n+\tlog.merges)\n+\t\t__gitcomp \"show hide only\"\n+\t\treturn\n+\t\t;;\n \tsendemail.aliasesfiletype)\n \t\t__gitcomp \"mutt mailrc pine elm gnus\"\n \t\treturn\n@@ -2150,6 +2158,7 @@ _git_config ()\n \t\tinteractive.singlekey\n \t\tlog.date\n \t\tlog.decorate\n+\t\tlog.merges\n \t\tlog.showroot\n \t\tmailmap.file\n \t\tman.\n-- \n2.3.3.263.g095251d.dirty\n"},{"id":"258999","messageId":"xmqqy4m7ek9q.fsf@gitster.dls.corp.google.com","threadId":"38997","inReplyTo":"1428110521-31028-2-git-send-email-koosha@posteo.de","subject":"Re: [PATCH v2 2/5] log: honor log.merges= option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-04T20:00:33Z","receivedAt":"2015-04-04T20:00:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Koosha Khajehmoogahi <koosha@posteo.de> writes:\n\n> From: Junio C Hamano <gitster@pobox.com>\n>\n> [kk: wrote commit message]\n\nEhh, what exactly did you write ;-)?\n\nI think the most important thing that needs to be explained by the\nlog message for this change is that the variable is honored only by\nlog and it needs to explain why other Porcelain commands in the same\n\"log\" family, like \"whatchanged\", should ignore the variable.\n\nI think that we must not to allow format-patch and show to be\naffected by this variable, because it is silly if log.merges=only\nbroke format-patch output or made \"git show\" silent.  But I didn't\nthink about others.  Whoever is doing this change needs to explain\nin the log message the reason why it was decided that only \"git log\"\nshould pay attention to it.\n\n> Helped-by: Eris Sunshine <sunshine@sunshineco.com>\n> Signed-off-by: Koosha Khajehmoogahi <koosha@posteo.de>\n> ---\n\n\n>  builtin/log.c | 6 ++++++\n>  1 file changed, 6 insertions(+)\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index dd8f3fc..c7a7aad 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -36,6 +36,7 @@ static int decoration_given;\n>  static int use_mailmap_config;\n>  static const char *fmt_patch_subject_prefix = \"PATCH\";\n>  static const char *fmt_pretty;\n> +static const char *log_merges;\n\nThe variable name may want to be updated to mimic other variables\nused in a similar way, e.g. default_show_root is used to store the\nvalue from log.showroot.\n\nPerhaps \"default_show_merge\" or something?\n\nThanks.\n\n>  static const char * const builtin_log_usage[] = {\n>  \tN_(\"git log [<options>] [<revision range>] [[--] <path>...]\"),\n> @@ -386,6 +387,9 @@ static int git_log_config(const char *var, const char *value, void *cb)\n>  \t\t\tdecoration_style = 0; /* maybe warn? */\n>  \t\treturn 0;\n>  \t}\n> +\tif (!strcmp(var, \"log.merges\")) {\n> +\t\treturn git_config_string(&log_merges, var, value);\n> +\t}\n>  \tif (!strcmp(var, \"log.showroot\")) {\n>  \t\tdefault_show_root = git_config_bool(var, value);\n>  \t\treturn 0;\n> @@ -628,6 +632,8 @@ int cmd_log(int argc, const char **argv, const char *prefix)\n>  \n>  \tinit_revisions(&rev, prefix);\n>  \trev.always_show_header = 1;\n> +\tif (log_merges && parse_merges_opt(&rev, log_merges))\n> +\t\tdie(\"unknown config value for log.merges: %s\", log_merges);\n>  \tmemset(&opt, 0, sizeof(opt));\n>  \topt.def = \"HEAD\";\n>  \topt.revarg_opt = REVARG_COMMITTISH;\n"},{"id":"259036","messageId":"xmqq8ue6cky4.fsf@gitster.dls.corp.google.com","threadId":"38997","inReplyTo":"1428110521-31028-3-git-send-email-koosha@posteo.de","subject":"Re: [PATCH v2 3/5] Documentation: add git-log --merges= option and log.merges config. var","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-05T21:41:07Z","receivedAt":"2015-04-05T21:41:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Koosha Khajehmoogahi <koosha@posteo.de> writes:\n\n> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\n> index f620ee4..0bb2390 100644\n> --- a/Documentation/rev-list-options.txt\n> +++ b/Documentation/rev-list-options.txt\n> @@ -96,12 +96,24 @@ if it is part of the log message.\n>  --remove-empty::\n>  \tStop when a given path disappears from the tree.\n>  \n> +--merges={show|hide|only}::\n> ++\n> +--\n> +`show`: show both merge and non-merge commits\n> +\n> +`hide`: only show non-merge commits; same as `--max-parents=1`\n> +\n> +`only`: only show merge commits; same as `--min-parents=2`\n> +\n> +If `--merges=` is not specified, default value is `show`.\n> +--\n> ++\n> +\n\nI am not sure if the \"default value is `show`\" is something we would\neven want to mention like this.  It does not tell the whole story\nand may even confuse the users, who did\n\n\tgit log --merge\n\tgit log --max-parent=...\n\nbut did not say any \"--merges=<something>\".\n\nI think the importat point we want to teach users is that this is an\noption to use when you want to limit what is output (and by default,\nwe show all but nothing else in the manpage says we hide things,\nso...).  And it would be beneficial to highlight that 'show' is only\nthere to defeat an unusual log.merges setting in users' config.\n\nAlso the formatting of this part looks rather unusual.  I would have\nexpected that these three items to be listed as a true AsciiDoc\nenumeration, not three hand-crafted enumration-looking separate\nparagraphs.\n\nTaking both points together, we may want to do something more like\nthis, perhaps?\n\n--merges={show|hide|only}::\n\n\tLimit the output by type of commits.\n\n\t`hide`;;\n\t\tHide merge commits from the output.\n\n\t`only`;;\n\t\tHide non-merge commits from the output (i.e showing\n\t\tonly merge commits).\n\n\t`show`;;\n\t\tDo not hide either merge or non-merge commits.  This\n\t\tis primarily useful when the user has non-standard\n\t\tsetting of `log.merges` configuration variable that\n\t\tneeds to be overriden from the command line.\n\n\nThanks.\n"},{"id":"259113","messageId":"CAPig+cQ434roQFA0E5wvGAgp5UdGu5oKCq-uNvohMhsdG3S1og@mail.gmail.com","threadId":"38997","inReplyTo":"1428110521-31028-1-git-send-email-koosha@posteo.de","subject":"Re: [PATCH v2 1/5] revision: add --merges={show|only|hide} option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-04-07T05:16:47Z","receivedAt":"2015-04-07T05:16:47Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Apr 3, 2015 at 9:21 PM, Koosha Khajehmoogahi <koosha@posteo.de> wrote:\n> From: Junio C Hamano <gitster@pobox.com>\n>\n> revision: add a new option 'merges=' with\n> possible values of 'only', 'show' and 'hide'.\n> The option is used when showing the list of commits.\n> The value 'only' lists only merges. The value 'show'\n> is the default behavior which shows the commits as well\n> as merges and the value 'hide' makes it just list commit\n> items.\n\nA couple very minor comments if you to re-roll this series, otherwise\nkeep them in mind for future submissions:\n\nSince \"revision:\" already prefixes the first line of the commit\nmessage (in the Subject:), its reiteration at the start of this\nparagraph is unnecessary. It's sufficient to start the paragraph with\n\"Add new --merges= option...\".\n\nThe excessively jagged right edge of the paragraph makes it slightly\ndifficult to read. Try to wrap paragraphs to about 70-72 characters.\n\n> [kk: chose names for options; wrote commit message]\n>\n> Helped-by: Eric Sunshine <sunshine@sunshineco.com>\n> Signed-off-by: Koosha Khajehmoogahi <koosha@posteo.de>\n> ---\n>  revision.c | 20 ++++++++++++++++++++\n>  revision.h |  1 +\n>  2 files changed, 21 insertions(+)\n>\n> diff --git a/revision.c b/revision.c\n> index 6399a04..c3c3dcc 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -1678,6 +1678,23 @@ static void add_message_grep(struct rev_info *revs, const char *pattern)\n>         add_grep(revs, pattern, GREP_PATTERN_BODY);\n>  }\n>\n> +int parse_merges_opt(struct rev_info *revs, const char *param)\n> +{\n> +       if (!strcmp(param, \"show\")) {\n> +               revs->min_parents = 0;\n> +               revs->max_parents = -1;\n> +       } else if (!strcmp(param, \"only\")) {\n> +               revs->min_parents = 2;\n> +               revs->max_parents = -1;\n> +       } else if (!strcmp(param, \"hide\")) {\n> +               revs->min_parents = 0;\n> +               revs->max_parents = 1;\n> +       } else {\n> +               return -1;\n> +       }\n> +       return 0;\n> +}\n> +\n>  static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n>                                int *unkc, const char **unkv)\n>  {\n> @@ -1800,6 +1817,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n>                 revs->show_all = 1;\n>         } else if (!strcmp(arg, \"--remove-empty\")) {\n>                 revs->remove_empty_trees = 1;\n> +       } else if (starts_with(arg, \"--merges=\")) {\n> +               if (parse_merges_opt(revs, arg + 9))\n> +                       die(\"unknown option: %s\", arg);\n>         } else if (!strcmp(arg, \"--merges\")) {\n>                 revs->min_parents = 2;\n>         } else if (!strcmp(arg, \"--no-merges\")) {\n> diff --git a/revision.h b/revision.h\n> index 0ea8b4e..f9df58c 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -240,6 +240,7 @@ extern int setup_revisions(int argc, const char **argv, struct rev_info *revs,\n>  extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n>                                const struct option *options,\n>                                const char * const usagestr[]);\n> +extern int parse_merges_opt(struct rev_info *, const char *);\n>  #define REVARG_CANNOT_BE_FILENAME 01\n>  #define REVARG_COMMITTISH 02\n>  extern int handle_revision_arg(const char *arg, struct rev_info *revs,\n> --\n> 2.3.3.263.g095251d.dirty\n"},{"id":"259114","messageId":"CAPig+cStJ9WC1HaJXGEpPqZUi+S2aE2-1vV-D4B1ZBekKpLcLQ@mail.gmail.com","threadId":"38997","inReplyTo":"1428110521-31028-2-git-send-email-koosha@posteo.de","subject":"Re: [PATCH v2 2/5] log: honor log.merges= option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-04-07T05:18:24Z","receivedAt":"2015-04-07T05:18:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Apr 3, 2015 at 9:21 PM, Koosha Khajehmoogahi <koosha@posteo.de> wrote:\n> From: Junio C Hamano <gitster@pobox.com>\n>\n> [kk: wrote commit message]\n>\n> Helped-by: Eris Sunshine <sunshine@sunshineco.com>\n\ns/Eris/Eric/\n\n> Signed-off-by: Koosha Khajehmoogahi <koosha@posteo.de>\n> ---\n>  builtin/log.c | 6 ++++++\n>  1 file changed, 6 insertions(+)\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index dd8f3fc..c7a7aad 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -36,6 +36,7 @@ static int decoration_given;\n>  static int use_mailmap_config;\n>  static const char *fmt_patch_subject_prefix = \"PATCH\";\n>  static const char *fmt_pretty;\n> +static const char *log_merges;\n>\n>  static const char * const builtin_log_usage[] = {\n>         N_(\"git log [<options>] [<revision range>] [[--] <path>...]\"),\n> @@ -386,6 +387,9 @@ static int git_log_config(const char *var, const char *value, void *cb)\n>                         decoration_style = 0; /* maybe warn? */\n>                 return 0;\n>         }\n> +       if (!strcmp(var, \"log.merges\")) {\n> +               return git_config_string(&log_merges, var, value);\n> +       }\n>         if (!strcmp(var, \"log.showroot\")) {\n>                 default_show_root = git_config_bool(var, value);\n>                 return 0;\n> @@ -628,6 +632,8 @@ int cmd_log(int argc, const char **argv, const char *prefix)\n>\n>         init_revisions(&rev, prefix);\n>         rev.always_show_header = 1;\n> +       if (log_merges && parse_merges_opt(&rev, log_merges))\n> +               die(\"unknown config value for log.merges: %s\", log_merges);\n>         memset(&opt, 0, sizeof(opt));\n>         opt.def = \"HEAD\";\n>         opt.revarg_opt = REVARG_COMMITTISH;\n> --\n> 2.3.3.263.g095251d.dirty\n"},{"id":"259117","messageId":"20150407073257.GA45051@flurp.local","threadId":"38997","inReplyTo":"1428110521-31028-4-git-send-email-koosha@posteo.de","subject":"Re: [PATCH v2 4/5] t4202-log: add tests for --merges=","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-04-07T07:32:57Z","receivedAt":"2015-04-07T07:32:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 04, 2015 at 03:22:00AM +0200, Koosha Khajehmoogahi wrote:\n> Helped-by: Eric Sunshine <sunshine@sunshineco.com>\n> Signed-off-by: Koosha Khajehmoogahi <koosha@posteo.de>\n> ---\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 1b2e981..ceaaf4e 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -270,6 +270,90 @@ cat > expect <<\\EOF\n>  * initial\n>  EOF\n>  \n> +test_expect_success 'setup --merges=' '\n> +\tgit log --min-parents=2 --pretty=tformat:%s >expect_only &&\n> +\tgit log --max-parents=1 --pretty=tformat:%s >expect_hide &&\n> +\tgit log --min-parents=-1 --pretty=tformat:%s >expect_show\n> +'\n> +\n> +test_expect_success 'log with config log.merges=show' '\n> +\ttest_config log.merges show &&\n> +\tgit log --pretty=tformat:%s >actual &&\n> +\ttest_cmp expect_show actual\n> +'\n> +\n[...much repetitious code omitted...]\n> +test_expect_success 'log with config log.merges=show with git log --no-merges' '\n> +\ttest_config log.merges show &&\n> +\tgit log --no-merges --pretty=tformat:%s >actual &&\n> +\ttest_cmp expect_hide actual\n> +'\n> +\n[...much repetitious code omitted...]\n> +test_log_merges() {\n> +\ttest_config log.merges $2 &&\n> +\tgit log --merges=$1 --pretty=tformat:%s >actual &&\n> +\ttest_cmp $3 actual\n> +}\n> +\n> +test_expect_success 'log --merges=show with config log.merges=hide' '\n> +\ttest_log_merges show hide expect_show\n> +'\n[...much repetitious code omitted...]\n\nIn my previous review[1], I mentioned that the significant repetition\nin the implementation of the new tests increased potential for errors\nwhen composing them, and made it more difficult to review. To resolve\nthe issue, I suggested factoring out the repeated code into a helper\nfunction, and gave an idea of its interface and how it could be\napplied to eliminate all the repetition.\n\nIn this version of the patch, you've made some progress at\neliminating repetition, but much still remains. I had hoped to see\nmuch more aggressive repetition elimination, collapsing the code to\nthe bare minimum while still testing all combinations of states.\nHere's what I had in mind:\n\n---- 8< ----\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 1b2e981..be6d34c 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -270,6 +270,35 @@ cat > expect <<\\EOF\n * initial\n EOF\n \n+test_expect_success 'setup merges' '\n+\tgit log --min-parents=2 --pretty=tformat:%s >expect_only &&\n+\tgit log --max-parents=1 --pretty=tformat:%s >expect_hide &&\n+\tgit log --min-parents=-1 --pretty=tformat:%s >expect_show\n+'\n+\n+test_log_merges() {\n+\texpect=expect_$1\n+\tconfig=${2:+-c log.merges=$2}\n+\toption=${3:+--merges=$3}\n+\toption=${4:-$option}\n+\ttest_expect_success \"merges${config:+ $config}${option:+ $option}\" \"\n+\t\tgit $config log $option --pretty=tformat:%s >actual &&\n+\t\ttest_cmp $expect actual\n+\t\"\n+}\n+\n+states=\"show only hide\"\n+for c in '' $states\n+do\n+\tfor o in '' $states\n+\tdo\n+\t\ttest_log_merges ${o:-${c:-show}} \"$c\" \"$o\"\n+\tdone\n+done\n+\n+test_log_merges hide show '' --no-merges\n+test_log_merges only hide '' '--merges --max-parents=2'\n+\n test_expect_success 'log --graph with merge' '\n \tgit log --graph --date-order --pretty=tformat:%s |\n \t\tsed \"s/ *\\$//\" >actual &&\n---- 8< ----\n\nThe output of the above test code looks like this:\n\n---- 8< ----\nok 29 - setup merges\nok 30 - merges\nok 31 - merges --merges=show\nok 32 - merges --merges=only\nok 33 - merges --merges=hide\nok 34 - merges -c log.merges=show\nok 35 - merges -c log.merges=show --merges=show\nok 36 - merges -c log.merges=show --merges=only\nok 37 - merges -c log.merges=show --merges=hide\nok 38 - merges -c log.merges=only\nok 39 - merges -c log.merges=only --merges=show\nok 40 - merges -c log.merges=only --merges=only\nok 41 - merges -c log.merges=only --merges=hide\nok 42 - merges -c log.merges=hide\nok 43 - merges -c log.merges=hide --merges=show\nok 44 - merges -c log.merges=hide --merges=only\nok 45 - merges -c log.merges=hide --merges=hide\nok 46 - merges -c log.merges=show --no-merges\nok 47 - merges -c log.merges=hide --merges --max-parents=2\n---- 8< ----\n\nHmm, it seems that I've rewritten pretty much the entire patch, so\nperhaps the From: header ought to mention my name instead. Oops.\n\n[1]: http://git.661346.n2.nabble.com/PATCH-1-5-Add-a-new-option-merges-to-revision-c-td7627662.html#a7627683\n"},{"id":"259157","messageId":"5524571C.90007@posteo.de","threadId":"38997","inReplyTo":"xmqqy4m7ek9q.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 2/5] log: honor log.merges= option","fromName":"Koosha Khajehmoogahi","fromEmail":"koosha@posteo.de","sentAt":"2015-04-07T22:15:56Z","receivedAt":"2015-04-07T22:15:56Z","isPatch":true,"sender":{"key":"koosha@posteo.de","avatar":null},"body":"\n\nOn 04/04/2015 10:00 PM, Junio C Hamano wrote:\n> Koosha Khajehmoogahi <koosha@posteo.de> writes:\n> \n>> From: Junio C Hamano <gitster@pobox.com>\n>>\n>> [kk: wrote commit message]\n> \n> Ehh, what exactly did you write ;-)?\n> \n> I think the most important thing that needs to be explained by the\n> log message for this change is that the variable is honored only by\n> log and it needs to explain why other Porcelain commands in the same\n> \"log\" family, like \"whatchanged\", should ignore the variable.\n>\nSo, what would be the reason? \n \n> I think that we must not to allow format-patch and show to be\n> affected by this variable, because it is silly if log.merges=only\n> broke format-patch output or made \"git show\" silent.  But I didn't\n> think about others.  Whoever is doing this change needs to explain\n> in the log message the reason why it was decided that only \"git log\"\n> should pay attention to it.\n> \n\n\n"},{"id":"259160","messageId":"xmqq4morcq0l.fsf@gitster.dls.corp.google.com","threadId":"38997","inReplyTo":"5524571C.90007@posteo.de","subject":"Re: [PATCH v2 2/5] log: honor log.merges= option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-08T02:28:26Z","receivedAt":"2015-04-08T02:28:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Koosha Khajehmoogahi <koosha@posteo.de> writes:\n\n> On 04/04/2015 10:00 PM, Junio C Hamano wrote:\n>> Koosha Khajehmoogahi <koosha@posteo.de> writes:\n>> \n>>> From: Junio C Hamano <gitster@pobox.com>\n>>>\n>>> [kk: wrote commit message]\n>> \n>> Ehh, what exactly did you write ;-)?\n>> \n>> I think the most important thing that needs to be explained by the\n>> log message for this change is that the variable is honored only by\n>> log and it needs to explain why other Porcelain commands in the same\n>> \"log\" family, like \"whatchanged\", should ignore the variable.\n>>\n> So, what would be the reason? \n\nIt is strange that you have to ask me to give you the reason why you\nchose it that way, isn't it?\n\n>> I think that we must not to allow format-patch and show to be\n>> affected by this variable, because it is silly if log.merges=only\n>> broke format-patch output or made \"git show\" silent.  But I didn't\n>> think about others.  Whoever is doing this change needs to explain\n>> in the log message the reason why it was decided that only \"git log\"\n>> should pay attention to it.\n>> \n"},{"id":"259165","messageId":"5525062A.2010905@posteo.de","threadId":"38997","inReplyTo":"xmqq4morcq0l.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 2/5] log: honor log.merges= option","fromName":"Koosha Khajehmoogahi","fromEmail":"koosha@posteo.de","sentAt":"2015-04-08T10:42:50Z","receivedAt":"2015-04-08T10:42:50Z","isPatch":true,"sender":{"key":"koosha@posteo.de","avatar":null},"body":"\n\nOn 04/08/2015 04:28 AM, Junio C Hamano wrote:\n> Koosha Khajehmoogahi <koosha@posteo.de> writes:\n> \n>> On 04/04/2015 10:00 PM, Junio C Hamano wrote:\n>>> Koosha Khajehmoogahi <koosha@posteo.de> writes:\n>>>\n>>>> From: Junio C Hamano <gitster@pobox.com>\n>>>>\n>>>> [kk: wrote commit message]\n>>>\n>>> Ehh, what exactly did you write ;-)?\n>>>\n>>> I think the most important thing that needs to be explained by the\n>>> log message for this change is that the variable is honored only by\n>>> log and it needs to explain why other Porcelain commands in the same\n>>> \"log\" family, like \"whatchanged\", should ignore the variable.\n>>>\n>> So, what would be the reason? \n> \n> It is strange that you have to ask me to give you the reason why you\n> chose it that way, isn't it?\n\nAFAIK, the only other command that supports --merges and --no-merges options is\nrev-list. This new feature aims to make a default behavior for the commands\nthat have these options. The command-line option is supported by the two commands.\nHowever, the config var is only used by git-log and rev-list ignores it. I didn't\nexclude rev-list for any particular reason. If we need, I could also handle it in\nrev-list.\n"},{"id":"259273","messageId":"xmqqoams4oed.fsf@gitster.dls.corp.google.com","threadId":"38997","inReplyTo":"5525062A.2010905@posteo.de","subject":"Re: [PATCH v2 2/5] log: honor log.merges= option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-13T04:56:26Z","receivedAt":"2015-04-13T04:56:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Koosha Khajehmoogahi <koosha@posteo.de> writes:\n\n> On 04/08/2015 04:28 AM, Junio C Hamano wrote:\n>\n>> It is strange that you have to ask me to give you the reason why you\n>> chose it that way, isn't it?\n>\n> AFAIK, the only other command that supports --merges and --no-merges options is\n> rev-list. This new feature aims to make a default behavior for the commands\n> that have these options. The command-line option is supported by the two commands.\n> However, the config var is only used by git-log and rev-list ignores it. I didn't\n> exclude rev-list for any particular reason. If we need, I could also handle it in\n> rev-list.\n\nAs rev-list is plumbing, it shouldn't be affected by the UI level\ncustomization knobs like this one; otherwise you will break people's\nscripts when end users choose to use the new knobs.\n\nHistorically, \"whatchanged\" has been the way to ask for \"log\", with\ndifferent default output format (but not different commit selection\nlogic).  I would think people who are used to \"whatchanged\" would\nexpect that the command would pay attention to what \"log\" would.\n\nAs I already mentioned with the reason why, I do not think \"show\"\nand \"format-patch\" should pay attention to it.\n\nThere may be other commands from the \"log\" family (i.e. what is\ndefined in builtin/log.c and/or uses get_revision() API to walk the\ncommit graph), for which similar reasoning should be done to decide\nif each of them should or should not pay attention to it.\n\nThanks.\n"}]}