Re: [PATCH 2/2] diff-merges: introduce '-d' option
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 11, 2023, 21:01 UTC
- Message-ID
- <xmqqtts0tof8.fsf@gitster.g>
- In-Reply-To
- <20230909125446.142715-3-sorganov@gmail.com>
Sergey Organov <sorganov@gmail.com> writes:
Show 6 quoted lines
> 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.
Show 67 quoted lines
> 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