Re: [PATCH 2/2] diff-merges: introduce '-d' option

[Junio C Hamano <gitster@pobox.com>]

Sergey Organov <sorganov@gmail.com> writes:

> This option provides a shortcut to request diff with respect to first
> parent for any kind of commit, universally. It's implemented as pure
> synonym for "--diff-merges=first-parent --patch".
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---

Sounds very straight-forward.

Given that "--first-parent" in "git log --first-parent -p" already
defeats "-m" and shows the diff against the first parent only,
people may find it confusing if "git log -d" does not act as a
shorthand for that.  From the above and also from the documentation
update, it is hard to tell if that is what you implemented, or it
only affects the "diff-merges" part.

Other than that, the patch looks quite small and to the point.

Thanks.

>  Documentation/diff-options.txt | 4 ++++
>  Documentation/git-log.txt      | 2 +-
>  diff-merges.c                  | 3 +++
>  t/t4013-diff-various.sh        | 8 ++++++++
>  4 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index f93aa3e46a52..d773dafcb10a 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -51,6 +51,10 @@ ifdef::git-log[]
>  Note: This option not implying `-p` is legacy feature that is
>  preserved for the sake of backward compatibility.
>  
> +-d::
> +	Produce diff with respect to first parent.
> +	Shortcut for '--diff-merges=first-parent -p'.
> +
>  -c::
>  	Produce combined diff output for merge commits.
>  	Shortcut for '--diff-merges=combined -p'.
> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> index 9b7ec96e767a..59bd74a1a596 100644
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -120,7 +120,7 @@ By default, `git log` does not generate any diff output. The options
>  below can be used to show the changes made by each commit.
>  
>  Note that unless one of `--diff-merges` variants (including short
> -`-m`, `-c`, and `--cc` options) is explicitly given, merge commits
> +`-d`, `-m`, `-c`, and `--cc` options) is explicitly given, merge commits
>  will not show a diff, even if a diff format like `--patch` is
>  selected, nor will they match search options like `-S`. The exception
>  is when `--first-parent` is in use, in which case `first-parent` is
> diff --git a/diff-merges.c b/diff-merges.c
> index ec97616db1df..6eb72e6fc28a 100644
> --- a/diff-merges.c
> +++ b/diff-merges.c
> @@ -125,6 +125,9 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
>  	if (!suppress_m_parsing && !strcmp(arg, "-m")) {
>  		set_to_default(revs);
>  		revs->merges_need_diff = 0;
> +	} else if (!strcmp(arg, "-d")) {
> +		set_first_parent(revs);
> +		revs->merges_imply_patch = 1;
>  	} else if (!strcmp(arg, "-c")) {
>  		set_combined(revs);
>  		revs->merges_imply_patch = 1;
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index 5de1d190759f..a07d6eb6dd97 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -473,6 +473,14 @@ test_expect_success 'log --diff-merges=on matches --diff-merges=separate' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'log -d matches --diff-merges=1 -p' '
> +	git log --diff-merges=1 -p master >result &&
> +	process_diffs result >expected &&
> +	git log -d master >result &&
> +	process_diffs result >actual &&
> +	test_cmp expected actual
> +'
> +
>  test_expect_success 'deny wrong log.diffMerges config' '
>  	test_config log.diffMerges wrong-value &&
>  	test_expect_code 128 git log