From 4320815eb9a002b4ee64f70dda9b1c1e019f4894 Mon Sep 17 00:00:00 2001 From: Sergey Organov Date: Tue, 13 Apr 2021 14:41:14 +0300 Subject: [PATCH 1/5] diff-merges: introduce --diff-merges=on Introduce the notion of default diff format for merges, and the option "on" to select it. The default format is "separate" and can't yet be changed, so effectively "on" is just a synonym for "separate" for now. Add corresponding test to t4013. This is in preparation for introducing log.diffMerges configuration option that will let --diff-merges=on to be configured to any supported format. Signed-off-by: Sergey Organov Signed-off-by: Junio C Hamano --- diff-merges.c | 7 +++++++ t/t4013-diff-various.sh | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/diff-merges.c b/diff-merges.c index 146bb50316..ff227368bd 100644 --- a/diff-merges.c +++ b/diff-merges.c @@ -2,6 +2,11 @@ #include "revision.h" +typedef void (*diff_merges_setup_func_t)(struct rev_info *); +static void set_separate(struct rev_info *revs); + +static diff_merges_setup_func_t set_to_default = set_separate; + static void suppress(struct rev_info *revs) { revs->separate_merges = 0; @@ -66,6 +71,8 @@ static void set_diff_merges(struct rev_info *revs, const char *optarg) set_combined(revs); else if (!strcmp(optarg, "cc") || !strcmp(optarg, "dense-combined")) set_dense_combined(revs); + else if (!strcmp(optarg, "on")) + set_to_default(revs); else die(_("unknown value for --diff-merges: %s"), optarg); diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 6cca8b84a6..26a7b4d19d 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -452,6 +452,14 @@ diff-tree --stat --compact-summary initial mode diff-tree -R --stat --compact-summary initial mode EOF +test_expect_success 'log --diff-merges=on matches --diff-merges=separate' ' + git log -p --diff-merges=separate master >result && + process_diffs result >expected && + git log -p --diff-merges=on master >result && + process_diffs result >actual && + test_cmp expected actual +' + test_expect_success 'log -S requires an argument' ' test_must_fail git log -S ' From 26a0f58da84a7da11f9175144c9a926e7b376349 Mon Sep 17 00:00:00 2001 From: Sergey Organov Date: Tue, 13 Apr 2021 14:41:15 +0300 Subject: [PATCH 2/5] diff-merges: refactor set_diff_merges() Split set_diff_merges() into separate parsing and execution functions, the former to be reused for parsing of configuration values later in the patch series. Signed-off-by: Sergey Organov Signed-off-by: Junio C Hamano --- diff-merges.c | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/diff-merges.c b/diff-merges.c index ff227368bd..66c8ba0cc6 100644 --- a/diff-merges.c +++ b/diff-merges.c @@ -55,29 +55,35 @@ static void set_dense_combined(struct rev_info *revs) revs->dense_combined_merges = 1; } +static diff_merges_setup_func_t func_by_opt(const char *optarg) +{ + if (!strcmp(optarg, "off") || !strcmp(optarg, "none")) + return suppress; + if (!strcmp(optarg, "1") || !strcmp(optarg, "first-parent")) + return set_first_parent; + else if (!strcmp(optarg, "m") || !strcmp(optarg, "separate")) + return set_separate; + else if (!strcmp(optarg, "c") || !strcmp(optarg, "combined")) + return set_combined; + else if (!strcmp(optarg, "cc") || !strcmp(optarg, "dense-combined")) + return set_dense_combined; + else if (!strcmp(optarg, "on")) + return set_to_default; + return NULL; +} + static void set_diff_merges(struct rev_info *revs, const char *optarg) { - if (!strcmp(optarg, "off") || !strcmp(optarg, "none")) { - suppress(revs); - /* Return early to leave revs->merges_need_diff unset */ - return; - } + diff_merges_setup_func_t func = func_by_opt(optarg); - if (!strcmp(optarg, "1") || !strcmp(optarg, "first-parent")) - set_first_parent(revs); - else if (!strcmp(optarg, "m") || !strcmp(optarg, "separate")) - set_separate(revs); - else if (!strcmp(optarg, "c") || !strcmp(optarg, "combined")) - set_combined(revs); - else if (!strcmp(optarg, "cc") || !strcmp(optarg, "dense-combined")) - set_dense_combined(revs); - else if (!strcmp(optarg, "on")) - set_to_default(revs); - else + if (!func) die(_("unknown value for --diff-merges: %s"), optarg); - /* The flag is cleared by set_xxx() functions, so don't move this up */ - revs->merges_need_diff = 1; + func(revs); + + /* NOTE: the merges_need_diff flag is cleared by func() call */ + if (func != suppress) + revs->merges_need_diff = 1; } /* From 38fc4dbbc2f110192752a3b2c99abb745f0494bf Mon Sep 17 00:00:00 2001 From: Sergey Organov Date: Tue, 13 Apr 2021 14:41:16 +0300 Subject: [PATCH 3/5] diff-merges: adapt -m to enable default diff format Let -m option (and --diff-merges=m) enable the default format instead of "separate", to be able to tune it with log.diffMerges option. Signed-off-by: Sergey Organov Signed-off-by: Junio C Hamano --- diff-merges.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/diff-merges.c b/diff-merges.c index 66c8ba0cc6..9d19225b3e 100644 --- a/diff-merges.c +++ b/diff-merges.c @@ -34,10 +34,10 @@ static void set_m(struct rev_info *revs) { /* * To "diff-index", "-m" means "match missing", and to the "log" - * family of commands, it means "show full diff for merges". Set + * family of commands, it means "show default diff for merges". Set * both fields appropriately. */ - set_separate(revs); + set_to_default(revs); revs->match_missing = 1; } @@ -61,13 +61,13 @@ static diff_merges_setup_func_t func_by_opt(const char *optarg) return suppress; if (!strcmp(optarg, "1") || !strcmp(optarg, "first-parent")) return set_first_parent; - else if (!strcmp(optarg, "m") || !strcmp(optarg, "separate")) + else if (!strcmp(optarg, "separate")) return set_separate; else if (!strcmp(optarg, "c") || !strcmp(optarg, "combined")) return set_combined; else if (!strcmp(optarg, "cc") || !strcmp(optarg, "dense-combined")) return set_dense_combined; - else if (!strcmp(optarg, "on")) + else if (!strcmp(optarg, "m") || !strcmp(optarg, "on")) return set_to_default; return NULL; } From 17c13e60fd799336b28d320dcba06fe1ef06e01d Mon Sep 17 00:00:00 2001 From: Sergey Organov Date: Tue, 13 Apr 2021 14:41:17 +0300 Subject: [PATCH 4/5] diff-merges: introduce log.diffMerges config variable New log.diffMerges configuration variable sets the format that --diff-merges=on will be using. The default is "separate". t4013: add the following tests for log.diffMerges config: * Test that wrong values are denied. * Test that the value of log.diffMerges properly affects both --diff-merges=on and -m. t9902: fix completion tests for log.d* to match log.diffMerges. Added documentation for log.diffMerges. Signed-off-by: Sergey Organov Signed-off-by: Junio C Hamano --- Documentation/config/log.txt | 5 +++++ builtin/log.c | 2 ++ diff-merges.c | 11 +++++++++++ diff-merges.h | 2 ++ t/t4013-diff-various.sh | 23 +++++++++++++++++++++++ t/t9902-completion.sh | 3 +++ 6 files changed, 46 insertions(+) diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt index 208d5fdcaa..456eb07800 100644 --- a/Documentation/config/log.txt +++ b/Documentation/config/log.txt @@ -24,6 +24,11 @@ log.excludeDecoration:: the config option can be overridden by the `--decorate-refs` option. +log.diffMerges:: + Set default diff format to be used for merge commits. See + `--diff-merges` in linkgit:git-log[1] for details. + Defaults to `separate`. + log.follow:: If `true`, `git log` will act as if the `--follow` option was used when a single is given. This has the same limitations as `--follow`, diff --git a/builtin/log.c b/builtin/log.c index f67b67d80e..e52127a9a5 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -481,6 +481,8 @@ static int git_log_config(const char *var, const char *value, void *cb) decoration_style = 0; /* maybe warn? */ return 0; } + if (!strcmp(var, "log.diffmerges")) + return diff_merges_config(value); if (!strcmp(var, "log.showroot")) { default_show_root = git_config_bool(var, value); return 0; diff --git a/diff-merges.c b/diff-merges.c index 9d19225b3e..f3a9daed7e 100644 --- a/diff-merges.c +++ b/diff-merges.c @@ -90,6 +90,17 @@ static void set_diff_merges(struct rev_info *revs, const char *optarg) * Public functions. They are in the order they are called. */ +int diff_merges_config(const char *value) +{ + diff_merges_setup_func_t func = func_by_opt(value); + + if (!func) + return -1; + + set_to_default = func; + return 0; +} + int diff_merges_parse_opts(struct rev_info *revs, const char **argv) { int argcount = 1; diff --git a/diff-merges.h b/diff-merges.h index 659467c99a..09d9a6c9a4 100644 --- a/diff-merges.h +++ b/diff-merges.h @@ -9,6 +9,8 @@ struct rev_info; +int diff_merges_config(const char *value); + int diff_merges_parse_opts(struct rev_info *revs, const char **argv); void diff_merges_suppress(struct rev_info *revs); diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 26a7b4d19d..87def81699 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -460,6 +460,29 @@ test_expect_success 'log --diff-merges=on matches --diff-merges=separate' ' test_cmp expected actual ' +test_expect_success 'deny wrong log.diffMerges config' ' + test_config log.diffMerges wrong-value && + test_expect_code 128 git log +' + +test_expect_success 'git config log.diffMerges first-parent' ' + git log -p --diff-merges=first-parent master >result && + process_diffs result >expected && + test_config log.diffMerges first-parent && + git log -p --diff-merges=on master >result && + process_diffs result >actual && + test_cmp expected actual +' + +test_expect_success 'git config log.diffMerges first-parent vs -m' ' + git log -p --diff-merges=first-parent master >result && + process_diffs result >expected && + test_config log.diffMerges first-parent && + git log -p -m master >result && + process_diffs result >actual && + test_cmp expected actual +' + test_expect_success 'log -S requires an argument' ' test_must_fail git log -S ' diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 04ce884ef5..4d732d6d4f 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -2306,6 +2306,7 @@ test_expect_success 'git config - variable name' ' test_completion "git config log.d" <<-\EOF log.date Z log.decorate Z + log.diffMerges Z EOF ' @@ -2327,6 +2328,7 @@ test_expect_success 'git -c - variable name' ' test_completion "git -c log.d" <<-\EOF log.date=Z log.decorate=Z + log.diffMerges=Z EOF ' @@ -2348,6 +2350,7 @@ test_expect_success 'git clone --config= - variable name' ' test_completion "git clone --config=log.d" <<-\EOF log.date=Z log.decorate=Z + log.diffMerges=Z EOF ' From 364bc11fe5711fe7fbb98abf376168a207e7a448 Mon Sep 17 00:00:00 2001 From: Sergey Organov Date: Tue, 13 Apr 2021 14:41:18 +0300 Subject: [PATCH 5/5] doc/diff-options: document new --diff-merges features Document changes in -m and --diff-merges=m semantics, as well as new --diff-merges=on option. Signed-off-by: Sergey Organov Signed-off-by: Junio C Hamano --- Documentation/diff-options.txt | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index aa2b5c11f2..6d968b9012 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -34,7 +34,7 @@ endif::git-diff[] endif::git-format-patch[] ifdef::git-log[] ---diff-merges=(off|none|first-parent|1|separate|m|combined|c|dense-combined|cc):: +--diff-merges=(off|none|on|first-parent|1|separate|m|combined|c|dense-combined|cc):: --no-diff-merges:: Specify diff format to be used for merge commits. Default is {diff-merges-default} unless `--first-parent` is in use, in which case @@ -45,17 +45,24 @@ ifdef::git-log[] Disable output of diffs for merge commits. Useful to override implied value. + +--diff-merges=on::: +--diff-merges=m::: +-m::: + This option makes diff output for merge commits to be shown in + the default format. `-m` will produce the output only if `-p` + is given as well. The default format could be changed using + `log.diffMerges` configuration parameter, which default value + is `separate`. ++ --diff-merges=first-parent::: --diff-merges=1::: This option makes merge commits show the full diff with respect to the first parent only. + --diff-merges=separate::: ---diff-merges=m::: --m::: This makes merge commits show the full diff with respect to each of the parents. Separate log entry and diff is generated - for each parent. `-m` doesn't produce any output without `-p`. + for each parent. + --diff-merges=combined::: --diff-merges=c:::