These patch series is yet another attempt to improve '-m' behavior by first improving --diff-merges to be able to provide current '-m' semantics, and then introducing a configuration variable to tweak '-m' behavior. Below is some historic context of the issue, followed by more thorough description of the patch series. The last attempt to fix '-m' option for "git log" to imply '-p', to make it consistent with similar options (-c/--cc), was called by the request on the mailing list, here: https://lore.kernel.org/git/CAMMLpeR-W35Qq6a343ifrxJ=mwBc_VcXZtVrBYDpJTySNBroFw@xxxxxxxxxxxxxx/ However, the suggested (and accepted at first) patch series: https://lore.kernel.org/git/20210520214703.27323-11-sorganov@xxxxxxxxx/#t appeared to have two problems: * --diff-merges options are incomplete and have no way to provide exact existing '-m' semantics. * implying '-p' by '-m' by default broke some legacy uses of "git log --firt-parent -m". Due to this, the last patch of those patch series has been later reverted: https://lore.kernel.org/git/YQyUM2uZdFBX8G0r@xxxxxxxxxx/ effectively leaving the issue unresolved. These patch series in turn consists of the following patches: 1. --diff-merges=[no]-hide The set of diff-merges options happened to be incomplete, and failed to implement exact semantics of -m option that hides output of diffs for merge commits unless -p option is active as well. The new "hide" option fixes this issue, so that now --diff-merges=on --diff-merges=hide combo is the exact synonym for -m. The log.diffMerges configuration also accepts "hide" and "no-hide" values, and governs the default value for the hide bit. The configuration behaves as if "--diff-merges=[no-]hide" is inserted first in the command-line. A new log.diffMergesHide configuration is introduced as well, to aid in avoiding of multiple log.diffMerges configuration entries when both format and hide flag are to be configured. 2. log.diffMerges-m-imply-p Historically, '-m' doesn't imply '-p' whereas similar '-c' and '--cc' options do. Simply fixing this inconsistency by unconditional modification of '-m' semantics appeared to be a bad idea, as it broke some legacy scripts/aliases. This patch rather provides configuration variable to tweak '-m' behavior accordingly. 3. Support list of values for --diff-merges This allows for shorter --diff-merges=on,hide forms. 4. Issue warning for lone '-m'. Lone '-m' is in use by scripts/aliases that aim at enabling diff output for merge commits, but only if '-p' is then specified as well. As '-m' may now be configured to imply '-p' (using 'log.diffMerges-m-imply-p'), issue warning if lone '-m' is specified, and suggest to instead use '--diff-merges=on,hide' that does not depend on user configuration. This is expected to give a provision for enabling log.diffMerges-m-imply-p by default in the future. 5. Improve documentation for --diff-merges. Updates in v1: * Added background context to the patch series foreword. * Documentation improvements added in a separate commit. * Remove commit unrelated to '-m' behavior. * Fix style: (p != NULL) -> (p). * Fix documentation placement of 'hide' option. * Fix log.diffMergesForce tests to avoid dependency on current defaults. Sergey Organov (5): diff-merges: implement [no-]hide option and log.diffMergesHide config diff-merges: implement log.diffMerges-m-imply-p config diff-merges: support list of values for --diff-merges diff-merges: issue warning on lone '-m' option diff-merges: improve --diff-merges documentation Documentation/config/log.txt | 9 ++ Documentation/diff-options.txt | 104 +++++++++++------- builtin/log.c | 4 + diff-merges.c | 76 +++++++++++-- diff-merges.h | 4 + t/t4013-diff-various.sh | 71 +++++++++++- ...ges=first-parent_--diff-merges=hide_master | 34 ++++++ t/t9902-completion.sh | 6 + 8 files changed, 257 insertions(+), 51 deletions(-) create mode 100644 t/t4013/diff.log_--diff-merges=first-parent_--diff-merges=hide_master Interdiff against v0: diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt index 7452c7fad638..265a57312e58 100644 --- a/Documentation/config/log.txt +++ b/Documentation/config/log.txt @@ -43,17 +43,6 @@ log.diffMergesHide:: log.diffMerges-m-imply-p:: `true` enables implication of `-p` by `-m`. -log.diffMergesForce:: - Use specified log format for -c, --cc, and --remerge-diff - options instead of their respective formats when the option - appears on the command line one time. See `--diff-merges` in - linkgit:git-log[1] for allowed values. Using 'off' or 'none' - disables the override (default). -+ -The override is useful when external tool hard-codes one of the above -options. Using any of these options two (or more) times will get back -the original meaning of the options. - log.follow:: If `true`, `git log` will act as if the `--follow` option was used when a single <path> is given. This has the same limitations as `--follow`, diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 46c98c87e24f..a3fbdb85a8b8 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -34,76 +34,86 @@ endif::git-diff[] endif::git-format-patch[] ifdef::git-log[] ---diff-merges=(off|none|on|first-parent|1|separate|m|combined|c|dense-combined|cc|remerge|r|[no-]hide):: +-m:: + Show diffs for merge commits in the default format. + Shortcut for '--diff-merges=on,hide' unless + `log.diffMerges-m-imply-p` configuration is active, in which + case it's a shortcut for '--diff-merges=on -p'. + +-c:: + Shortcut for '--diff-merges=combined -p'. + +--cc:: + Shortcut for '--diff-merges=dense-combined -p'. + +--remerge-diff:: + Shortcut for '--diff-merges=remerge -p'. + --no-diff-merges:: - Specify diff format to be used for merge commits. Default is + Synonym for '--diff-merges=off'. + +--diff-merges=(<format>|<flag>)[,(<format>|<flag>),...]:: + Specify diff format and flags to be used for merge commits. Default is {diff-merges-default} unless `--first-parent` is in use, in which case - `first-parent` is the default. Comma-separated list of - supported values is accepted as well. + `first-parent` is the default. + ---diff-merges=(off|none)::: ---no-diff-merges::: +The last format specified has precedence, whereas flags are +cumulative. Comma-separated list is handy to provide flag(s) along +with format, e.g.: `--diff-merges=first-parent,hide` is handy form of +`--diff-merges=first-parent --diff-merges=hide`. ++ +The following formats are supported: ++ +-- +off, none:: Disable output of diffs for merge commits. Useful to override implied value. + ---diff-merges=on::: ---diff-merges=m::: --m::: - These options make diff output for merge commits to be shown in - the default format. The default format could be changed using +on, m:: + Make diff output for merge commits to be shown in the default + format. The default format could be changed using `log.diffMerges` configuration parameter, which default value is `separate`. + - `-m` is a shortcut for '--diff-merges=on,hide'. - In addition it implies `-p` when `log.diffMerges-m-imply-p` is - active. -+ ---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::: - 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. -+ ---diff-merges=remerge::: ---diff-merges=r::: ---remerge-diff::: - With this option, two-parent merge commits are remerged to - create a temporary tree object -- potentially containing files - with conflict markers and such. A diff is then shown between - that temporary tree and the actual merge commit. -+ ---diff-merges=hide::: ---diff-merges=no-hide::: - Hide (do not hide) the diff for merge commits unless `-p` options is given - as well. The default `no-hide` could be changed using `log.diffMerges` - configuration parameter. +first-parent, 1:: + Show full diff with respect to first parent. This is the same + format as `--patch` produces for non-merge commits. ++ +separate:: + Show full diff with respect to each of parents. + Separate log entry and diff is generated for each parent. ++ +remerge, r:: + Remerge two-parent merge commits to create a temporary tree + object--potentially containing files with conflict markers + and such. A diff is then shown between that temporary tree + and the actual merge commit. + The output emitted when this option is used is subject to change, and so is its interaction with other options (unless explicitly documented). + ---diff-merges=combined::: ---diff-merges=c::: --c::: - With this option, diff output for a merge commit shows the - differences from each of the parents to the merge result - simultaneously instead of showing pairwise diff between a - parent and the result one at a time. Furthermore, it lists - only files which were modified from all parents. `-c` implies - `-p`. +combined, c:: + Show differences from each of the parents to the merge + result simultaneously instead of showing pairwise diff between + a parent and the result one at a time. Furthermore, it lists + only files which were modified from all parents. + ---diff-merges=dense-combined::: ---diff-merges=cc::: ---cc::: - With this option the output produced by - `--diff-merges=combined` is further compressed by omitting - uninteresting hunks whose contents in the parents have only - two variants and the merge result picks one of them without - modification. `--cc` implies `-p`. +dense-combined, cc:: + Further compress output produced by `--diff-merges=combined` + by omitting uninteresting hunks whose contents in the parents + have only two variants and the merge result picks one of them + without modification. +-- ++ +The following flags are supported: ++ +-- +[no-]hide:: + Hide diff for merge commits unless `-p` options is given as + well. The default `no-hide` could be changed using + `log.diffMerges` configuration parameter. +-- --combined-all-paths:: This flag causes combined diffs (used for merge commits) to diff --git a/builtin/log.c b/builtin/log.c index 1e8d0a2545a9..332b5e478cc5 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -585,8 +585,6 @@ static int git_log_config(const char *var, const char *value, void *cb) return diff_merges_hide_config(git_config_bool(var, value)); if (!strcmp(var, "log.diffmerges-m-imply-p")) return diff_merges_m_imply_p_config(git_config_bool(var, value)); - if (!strcmp(var, "log.diffmergesforce")) - return diff_merges_force_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 b3b3c9e44ba8..1f4e43e16940 100644 --- a/diff-merges.c +++ b/diff-merges.c @@ -7,7 +7,6 @@ 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 diff_merges_setup_func_t force_func = NULL; static int suppress_m_parsing; static int hide = 0; static int m_imply_p = 0; @@ -166,21 +165,6 @@ int diff_merges_m_imply_p_config(int on) return 0; } -int diff_merges_force_config(const char *value) -{ - diff_merges_setup_func_t func = func_by_opt(value); - - if (!func) - return -1; - - if (func == set_none) - force_func = NULL; - else if (func != set_hide && func != set_no_hide) - force_func = func; - - return 0; -} - void diff_merges_suppress_m_parsing(void) { suppress_m_parsing = 1; @@ -191,7 +175,6 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv) int argcount = 1; const char *optarg; const char *arg = argv[0]; - diff_merges_setup_func_t set_func = NULL; if (!suppress_m_parsing && !strcmp(arg, "-m")) { set_to_default(revs); @@ -199,11 +182,14 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv) revs->merges_imply_patch = m_imply_p; got_m = 1; } else if (!strcmp(arg, "-c")) { - set_func = set_combined; + set_combined(revs); + revs->merges_imply_patch = 1; } else if (!strcmp(arg, "--cc")) { - set_func = set_dense_combined; + set_dense_combined(revs); + revs->merges_imply_patch = 1; } else if (!strcmp(arg, "--remerge-diff")) { - set_func = set_remerge_diff; + set_remerge_diff(revs); + revs->merges_imply_patch = 1; } else if (!strcmp(arg, "--no-diff-merges")) { set_none(revs); } else if (!strcmp(arg, "--combined-all-paths")) { @@ -213,12 +199,6 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv) } else return 0; - if (set_func != NULL) { - (force_func ? force_func : set_func)(revs); - force_func = NULL; - revs->merges_imply_patch = 1; - } - revs->explicit_diff_merges = 1; return argcount; } diff --git a/diff-merges.h b/diff-merges.h index 6ef0cc87bb2a..9f0b3901fe82 100644 --- a/diff-merges.h +++ b/diff-merges.h @@ -15,8 +15,6 @@ int diff_merges_hide_config(int hide); int diff_merges_m_imply_p_config(int on); -int diff_merges_force_config(const char *value); - void diff_merges_suppress_m_parsing(void); int diff_merges_parse_opts(struct rev_info *revs, const char **argv); diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 2c68d06074ed..a07513e67fd6 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -565,24 +565,6 @@ test_expect_success 'git config log.diffMerges-m-imply-p has proper effect' ' test_cmp expected actual ' -test_expect_success 'git config log.diffMergesForce has proper effect' ' - git log -m -p master >result && - process_diffs result >expected && - test_config log.diffMergesForce on && - git log --cc master >result && - process_diffs result >actual && - test_cmp expected actual -' - -test_expect_success 'git config log.diffMergesForce override by duplicate' ' - git log --cc master >result && - process_diffs result >expected && - test_config log.diffMergesForce on && - git log --cc --cc master >result && - process_diffs result >actual && - test_cmp expected actual -' - # -m in "git diff-index" means "match missing", that differs # from its meaning in "git diff". Let's check it in diff-index. # The line in the output for removed file should disappear when diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 411e08b2fa1b..26a7e4ff877c 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -2498,7 +2498,6 @@ test_expect_success 'git config - variable name' ' log.decorate Z log.diffMerges Z log.diffMergesHide Z - log.diffMergesForce Z log.diffMerges-m-imply-p Z EOF ' @@ -2529,7 +2528,6 @@ test_expect_success 'git -c - variable name' ' log.decorate=Z log.diffMerges=Z log.diffMergesHide=Z - log.diffMergesForce=Z log.diffMerges-m-imply-p=Z EOF ' @@ -2554,7 +2552,6 @@ test_expect_success 'git clone --config= - variable name' ' log.decorate=Z log.diffMerges=Z log.diffMergesHide=Z - log.diffMergesForce=Z log.diffMerges-m-imply-p=Z EOF ' -- 2.25.1
