Sergey Organov <sorganov@xxxxxxxxx> 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".
That explains what the patch does, but it does not tell us why it is
useful [*].
> NOTE: originally proposed as '-d', and renamed to '--dd' due to Junio
> request to keep "short-and-sweet" '-d' reserved for other uses.
The note is not grammatical, and more importantly, readers of "git
log" 6 months down the road would not care. I'd rather not see it
in the proposed log message. It is suitable material to place after
the three-dash line, or in the cover letter for the iteration.
> diff --git a/diff-merges.c b/diff-merges.c
> index ec97616db1df..45507588a279 100644
> --- a/diff-merges.c
> +++ b/diff-merges.c
> @@ -131,6 +131,9 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
> } else if (!strcmp(arg, "--cc")) {
> set_dense_combined(revs);
> revs->merges_imply_patch = 1;
> + } else if (!strcmp(arg, "--dd")) {
> + set_first_parent(revs);
> + revs->merges_imply_patch = 1;
Quite straight-forward as expected. I do not think "--dd" clicks
for many people as "first parent diffs all over", though.
> } else if (!strcmp(arg, "--remerge-diff")) {
> set_remerge_diff(revs);
> revs->merges_imply_patch = 1;
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index 5de1d190759f..4b474808311e 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 --dd matches --diff-merges=1 -p' '
> + git log --diff-merges=1 -p master >result &&
> + process_diffs result >expected &&
> + git log --dd 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
Looking good.
Thanks.
[Footnote]
* As I said elsewhere, I do not think it is a good idea to encourage
users' to adopt a screwed-up worldview in which first parent is
special but not special, and does the wrong thing for reverse
merges. If the option were short-hand for "--first-parent -p",
at least I would be more sympathetic.
