Sergey Organov <sorganov@xxxxxxxxx> writes:
> @@ -49,10 +49,11 @@ ifdef::git-log[]
> --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
> + 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 --diff-merges=hide'
> +
I found this description difficult to parse, since
a) it wasn't clear that multiple "--diff-merges" would all be respected
b) I had to read the --diff-merges=hide documentation to understand what
this means
Keeping the plain english description would help, something like:
`-m` only produces the output if `-p` is also given, i.e. it is a
shortcut for '--diff-merges=on --diff-merges=hide'.
> diff --git a/builtin/log.c b/builtin/log.c
> index 56e2d95e869d..e031021e53b2 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -581,6 +581,8 @@ static int git_log_config(const char *var, const char *value, void *cb)
> }
> if (!strcmp(var, "log.diffmerges"))
> return diff_merges_config(value);
> + if (!strcmp(var, "log.diffmergeshide"))
> + return diff_merges_hide_config(git_config_bool(var, value));
> if (!strcmp(var, "log.showroot")) {
> default_show_root = git_config_bool(var, value);
> return 0;
Since we have log.diffmergeshide that is different from log.diffmerges,
it seems like it would be more consistent to have '--diff-merges-hide'
as a separate flag.
> @@ -69,6 +87,10 @@ static diff_merges_setup_func_t func_by_opt(const char *optarg)
> {
> if (!strcmp(optarg, "off") || !strcmp(optarg, "none"))
> return set_none;
> + if (!strcmp(optarg, "hide"))
> + return set_hide;
> + if (!strcmp(optarg, "no-hide"))
> + return set_no_hide;
> if (!strcmp(optarg, "1") || !strcmp(optarg, "first-parent"))
> return set_first_parent;
> if (!strcmp(optarg, "separate"))
> @@ -105,7 +127,19 @@ int diff_merges_config(const char *value)
> if (!func)
> return -1;
>
> - set_to_default = func;
> + if (func == set_hide)
> + hide = 1;
> + else if (func == set_no_hide)
> + hide = 0;
> + else
> + set_to_default = func;
> +
> + return 0;
> +}
The code is also simpler if we took a separate CLI flag, e.g. we could
get rid of this special casing of "(func == X)".
