* [PATCH 0/2] diff-merges: introduce '-d' option
@ 2023-09-09 12:54 Sergey Organov
2023-09-09 12:54 ` [PATCH 1/2] diff-merges: improve --diff-merges documentation Sergey Organov
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Sergey Organov @ 2023-09-09 12:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Sergey Organov
This new convenience option requests full diff with respect to first
parent, so that
git log -d
will output diff with respect to first parent for every commit,
universally, no matter how many parents the commit turns out to have.
It's implemented as pure synonym for
--diff-merges=first-parent --patch
The first commit in the series tweaks diff-merges documentation a bit,
and is valuable by itself. It's put here as '-d' implementation commit
depends on it in its documentation part.
Note: the need for this new convenience option mostly emerged from
denial by the community of patches that modify '-m' behavior to imply
'-p' as the rest of similar options (such as --cc) do.
Sergey Organov (2):
diff-merges: improve --diff-merges documentation
diff-merges: introduce '-d' option
Documentation/diff-options.txt | 101 +++++++++++++++++++--------------
Documentation/git-log.txt | 4 +-
diff-merges.c | 3 +
t/t4013-diff-various.sh | 8 +++
4 files changed, 71 insertions(+), 45 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] diff-merges: improve --diff-merges documentation
2023-09-09 12:54 [PATCH 0/2] diff-merges: introduce '-d' option Sergey Organov
@ 2023-09-09 12:54 ` Sergey Organov
2023-09-11 21:12 ` Junio C Hamano
2023-09-09 12:54 ` [PATCH 2/2] diff-merges: introduce '-d' option Sergey Organov
2023-09-20 15:02 ` [PATCH v2 0/2] " Sergey Organov
2 siblings, 1 reply; 18+ messages in thread
From: Sergey Organov @ 2023-09-09 12:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Sergey Organov
* Put descriptions of convenience shortcuts first, so they are the
first things reader observes, not lengthy stuff.
* Add explanation note on '-m' not implying '-p' unlike similar
options.
* Get rid of very long line containing all the --diff-merges formats
by replacing them with <format>, and putting each supported format
on its own line.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
Documentation/diff-options.txt | 97 +++++++++++++++++++---------------
Documentation/git-log.txt | 2 +-
2 files changed, 55 insertions(+), 44 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9f33f887711d..f93aa3e46a52 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -43,66 +43,77 @@ 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)::
+-m::
+ Show diffs for merge commits in the default format. This is
+ similar to '--diff-merges=on' (which see) except `-m` will
+ produce no output unless `-p` is given as well.
++
+Note: This option not implying `-p` is legacy feature that is
+preserved for the sake of backward compatibility.
+
+-c::
+ Produce combined diff output for merge commits.
+ Shortcut for '--diff-merges=combined -p'.
+
+--cc::
+ Produce dense combined diff output for merge commits.
+ Shortcut for '--diff-merges=dense-combined -p'.
+
+--remerge-diff::
+ Produce diff against re-merge.
+ Shortcut for '--diff-merges=remerge -p'.
+
--no-diff-merges::
+ Synonym for '--diff-merges=off'.
+
+--diff-merges=<format>::
Specify diff format 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.
+ {diff-merges-default} unless `--first-parent` is in use, in
+ which case `first-parent` is the default.
+
---diff-merges=(off|none):::
---no-diff-merges:::
+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:::
- 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
+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`.
+
---diff-merges=first-parent:::
---diff-merges=1:::
- This option makes merge commits show the full diff with
- respect to the first parent only.
+first-parent, 1::
+ Show full diff with respect to first parent. This is the same
+ format as `--patch` produces for non-merge commits.
+
---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.
+separate::
+ Show full diff with respect to each of 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.
+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.
+--
--combined-all-paths::
This flag causes combined diffs (used for merge commits) to
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 2a66cf888074..9b7ec96e767a 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -124,7 +124,7 @@ Note that unless one of `--diff-merges` variants (including short
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
-the default format.
+the default format for merge commits.
:git-log: 1
:diff-merges-default: `off`
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] diff-merges: introduce '-d' option
2023-09-09 12:54 [PATCH 0/2] diff-merges: introduce '-d' option Sergey Organov
2023-09-09 12:54 ` [PATCH 1/2] diff-merges: improve --diff-merges documentation Sergey Organov
@ 2023-09-09 12:54 ` Sergey Organov
2023-09-11 21:01 ` Junio C Hamano
2023-09-20 15:02 ` [PATCH v2 0/2] " Sergey Organov
2 siblings, 1 reply; 18+ messages in thread
From: Sergey Organov @ 2023-09-09 12:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Sergey Organov
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>
---
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
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] diff-merges: introduce '-d' option
2023-09-09 12:54 ` [PATCH 2/2] diff-merges: introduce '-d' option Sergey Organov
@ 2023-09-11 21:01 ` Junio C Hamano
2023-09-12 7:59 ` Sergey Organov
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2023-09-11 21:01 UTC (permalink / raw)
To: Sergey Organov; +Cc: git
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] diff-merges: improve --diff-merges documentation
2023-09-09 12:54 ` [PATCH 1/2] diff-merges: improve --diff-merges documentation Sergey Organov
@ 2023-09-11 21:12 ` Junio C Hamano
2023-09-12 7:37 ` Sergey Organov
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2023-09-11 21:12 UTC (permalink / raw)
To: Sergey Organov; +Cc: git
Sergey Organov <sorganov@gmail.com> writes:
> ifdef::git-log[]
> ---diff-merges=(off|none|on|first-parent|1|separate|m|combined|c|dense-combined|cc|remerge|r)::
> +-m::
> + Show diffs for merge commits in the default format. This is
> + similar to '--diff-merges=on' (which see) except `-m` will
> + produce no output unless `-p` is given as well.
> ++
> +Note: This option not implying `-p` is legacy feature that is
> +preserved for the sake of backward compatibility.
It is more like that `-p` does not imply `-m` (which used to mean
"consider showing the comparison between parent(s) and the child,
even for merge commits"), even though newer options like `-c`,
`--cc` and others do imply `-m` (simply because they do not make
much sense if they are not allowed to work on merges) that may make
new people confused. If `-p` implied `-m` (or if `-m` implied
`-p`), it would also have been utterly confusing and useless for
human consumption.
In either case, unless the reason why `-p` does not imply `-m`
unlike others is explained, I do not think the note adds that much
value. I'd suggest dropping it.
> --no-diff-merges::
> + Synonym for '--diff-merges=off'.
> +
> +--diff-merges=<format>::
> Specify diff format 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.
> + {diff-merges-default} unless `--first-parent` is in use, in
> + which case `first-parent` is the default.
> +
> +The following formats are supported:
> ++
> +--
> +off, none::
> Disable output of diffs for merge commits. Useful to override
> implied value.
> +
> +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`.
> +
> +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).
> +
> +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.
> +
> +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.
> +--
Looks reasonable, even though I didn't quite see much problem with
the original. If we were shuffling the sections like this patch, I
wonder if moving combined/dense-combined a bit higher (perhaps
before the "remerge") may make more sense, though (the ordering
would simply become "simpler to more involved").
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] diff-merges: improve --diff-merges documentation
2023-09-11 21:12 ` Junio C Hamano
@ 2023-09-12 7:37 ` Sergey Organov
2023-09-13 0:22 ` Junio C Hamano
0 siblings, 1 reply; 18+ messages in thread
From: Sergey Organov @ 2023-09-12 7:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com> writes:
>
>> ifdef::git-log[]
>> ---diff-merges=(off|none|on|first-parent|1|separate|m|combined|c|dense-combined|cc|remerge|r)::
>> +-m::
>> + Show diffs for merge commits in the default format. This is
>> + similar to '--diff-merges=on' (which see) except `-m` will
>> + produce no output unless `-p` is given as well.
>> ++
>> +Note: This option not implying `-p` is legacy feature that is
>> +preserved for the sake of backward compatibility.
>
> It is more like that `-p` does not imply `-m` (which used to mean
> "consider showing the comparison between parent(s) and the child,
> even for merge commits"), even though newer options like `-c`,
> `--cc` and others do imply `-m` (simply because they do not make
> much sense if they are not allowed to work on merges) that may make
> new people confused.
No, neither --cc nor -c imply -m.
-m is documented to produce very specific output that is neither -c nor
--cc, and it's indeed how it works.
-c and --cc imply -p, not -m, and it has been documented for ages
already, and it's indeed how it works, and that's what corresponding
commits that added the features claim.
Overall, --cc, -c, and --remerge-diff all imply -p, whereas -m does not.
This is simple fact.
So I feel we need to document why -m doesn't imply -p as other similar
options do.
> If `-p` implied `-m` (or if `-m` implied
> `-p`), it would also have been utterly confusing and useless for
> human consumption.
Fortunately, -p does not imply -m, but if -m implied -p, similar to --cc
and -c, it'd be rather very natural, and thus people keep asking why
it's not the case.
> In either case, unless the reason why `-p` does not imply `-m`
> unlike others is explained, I do not think the note adds that much
> value. I'd suggest dropping it.
-p does not imply others. It's others (--cc, etc.) that imply -p.
The problem being solved is that we periodically get (valid) questions
why -m does not behave similar to -c and --cc, and now --remerge-diff.
>
>> --no-diff-merges::
>> + Synonym for '--diff-merges=off'.
>> +
>> +--diff-merges=<format>::
>> Specify diff format 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.
>> + {diff-merges-default} unless `--first-parent` is in use, in
>> + which case `first-parent` is the default.
>> +
>> +The following formats are supported:
>> ++
>> +--
>> +off, none::
>> Disable output of diffs for merge commits. Useful to override
>> implied value.
>> +
>> +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`.
>> +
>> +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).
>> +
>> +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.
>> +
>> +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.
>> +--
>
> Looks reasonable, even though I didn't quite see much problem with
> the original.
The original --diff-merge=... line was so long it didn't fit, especially
after "remerge" has been added, and also was hard to grok.
> If we were shuffling the sections like this patch, I
> wonder if moving combined/dense-combined a bit higher (perhaps
> before the "remerge") may make more sense, though (the ordering
> would simply become "simpler to more involved").
I kept original order, but I agree combined/dense-combined fit better
above remerge.
I'll change the order in re-roll.
Thanks,
-- Sergey Organov
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] diff-merges: introduce '-d' option
2023-09-11 21:01 ` Junio C Hamano
@ 2023-09-12 7:59 ` Sergey Organov
2023-09-14 22:17 ` Junio C Hamano
0 siblings, 1 reply; 18+ messages in thread
From: Sergey Organov @ 2023-09-12 7:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> 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.
It doesn't, and I believe it's a good thing, as primary function of
--first-parent is to change history traversal rules, and if -d did that,
it would be extremely confusing.
Also, --first-parent is correctly documented as implying
--diff-merges=first-parent, not as defeating -m.
> 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.
If we read resulting documentation with a fresh eye, -d is similar to
--cc, and -c, just producing yet another kind of output, so I think all
this fits together quite nicely and shouldn't cause confusion.
>
> Other than that, the patch looks quite small and to the point.
Thanks,
-- Sergey Organov
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] diff-merges: improve --diff-merges documentation
2023-09-12 7:37 ` Sergey Organov
@ 2023-09-13 0:22 ` Junio C Hamano
2023-09-18 16:20 ` Sergey Organov
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2023-09-13 0:22 UTC (permalink / raw)
To: Sergey Organov; +Cc: git
Sergey Organov <sorganov@gmail.com> writes:
>> It is more like that `-p` does not imply `-m` (which used to mean
>> "consider showing the comparison between parent(s) and the child,
>> even for merge commits"), even though newer options like `-c`,
>> `--cc` and others do imply `-m` (simply because they do not make
>> much sense if they are not allowed to work on merges) that may make
>> new people confused.
>
> No, neither --cc nor -c imply -m.
I was only trying to help you polish the text you added to explain
what you called the "legacy feature" to reflect the reason behind
that legacy. As you obviously were not there back then when I made
"--cc" imply "-m" while keeping "-p" not to imply "-m".
Our "git log [--diff-output-options]" logic was (and still is) not
to show the comparison between parents and the child by default for
merge commits even with -p/--raw/--stat (these were what existed and
were common back then) and "git log --raw/--stat/-p" showed the
raw/diffstat/patch after the log message for one-parent commits but
only the log message for merges.
The reason behind that design choice is that Linus (and I and
others) did not find that the patches for merges are as useful as
patches for regular commits. We made "git log -p" to omit
patches for merges that tend to become large.
Side note: the first-parent patch is sort of readable, but
if you are not doing the "--first-parent" traversal (which
is a much later invention, so it wasn't even an option), you
are showing individual commits and their patches while
traversing the side branch, then the first-parent patch of a
merge amounts to the squash of individual changes on the
side branch that got merged. It was deemed redundant
presentation that is just wasteful and harder to grok than
reading individual commits. Worse, the patch against second
and later parent(s) have no real value (it shows how behind
the fork point of the side branch was relative to the tip of
the trunk, which is rarely useful).
But we also wanted to have a mode of "git log -p" that spews
everything to the output that could be used to reconstruct the
history, hence we added "-m" to tell "git log":
By default, you are designed not to show comparison between
parents and the child for merge commit. But when "-m" is
given, do show the comparison for merge commit in the format
that other options given to you, like --raw, --patch,
specifies.
We however didn't have a good idea how to represent such a
comparison between parents and the child, so we chose the most
redundant, verbose, and obvious, which is N pairwise patches with
each of N parents to the child (for a N-parent patch).
Later "--cc" and "-c" came as an alternative way to represent
comparison between parents and the child.
Given that I, together with Linus, invented "--cc" and "-c", taking
inspiration from how Paul Mackerras showed a merge in his 'gitk'
tool, and made the design decision not to require "-m" to get the
output in the format they specify when the "git log" traversal shows
merge commits, I do not know what to say when you repeat that "--cc"
does not imply "-m". It simply is not true.
I think this is the second time you claimed the above after I
explained the same to you, if I am not mistaken. If you do not want
to be corrected, that is fine, and I'll stop wasting my time trying
to correct you.
But I still have to make sure that you (or anybody else) do not
spread misinformation to other users by writing incorrect statements
in documentation patches.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] diff-merges: introduce '-d' option
2023-09-12 7:59 ` Sergey Organov
@ 2023-09-14 22:17 ` Junio C Hamano
2023-09-14 23:56 ` Sergey Organov
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2023-09-14 22:17 UTC (permalink / raw)
To: Sergey Organov; +Cc: git
Sergey Organov <sorganov@gmail.com> writes:
>> 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.
>
> It doesn't, and I believe it's a good thing, as primary function of
> --first-parent is to change history traversal rules, and if -d did that,
> it would be extremely confusing.
I am not sure about that.
> Also, --first-parent is correctly documented as implying
> --diff-merges=first-parent, not as defeating -m.
Yes, exactly. That makes me even more convinced that the intuitive
behaviour, when we say "we have this great short-hand option that
lets your 'git log' to do the first-parent thing with patch output",
is to do the first-parent traversal _and_ show first-parent patches.
"-d" is documented as a short-hand for "--diff-merges=first-parent
--patch" and not for "--first-parent --patch", so the behaviour may
correctly match documentation, but that does not make the documented
behaviour an intuitive one. And a behaviour that is not intuitive
is confusing.
> If we read resulting documentation with a fresh eye, -d is similar to
> --cc, and -c, just producing yet another kind of output, so I think all
> this fits together quite nicely and shouldn't cause confusion.
Another thing is that showing first-parent patch for merges while
letting the traversal also visit the second-parent chain is not as
useful an option as it could be, even though it is not so bad as the
original "-m -p" that also showed second-parent patch for merges as
well. People would have to say "log --first-parent -p" to get the
first-parent traversal with first-parent patch output, and they
would not behefit from having "-d".
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] diff-merges: introduce '-d' option
2023-09-14 22:17 ` Junio C Hamano
@ 2023-09-14 23:56 ` Sergey Organov
2023-09-15 17:24 ` Junio C Hamano
0 siblings, 1 reply; 18+ messages in thread
From: Sergey Organov @ 2023-09-14 23:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com> writes:
>
>>> 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.
>>
>> It doesn't, and I believe it's a good thing, as primary function of
>> --first-parent is to change history traversal rules, and if -d did that,
>> it would be extremely confusing.
>
> I am not sure about that.
>
>> Also, --first-parent is correctly documented as implying
>> --diff-merges=first-parent, not as defeating -m.
>
> Yes, exactly. That makes me even more convinced that the intuitive
> behaviour, when we say "we have this great short-hand option that
> lets your 'git log' to do the first-parent thing with patch output",
> is to do the first-parent traversal _and_ show first-parent patches.
>
> "-d" is documented as a short-hand for "--diff-merges=first-parent
> --patch" and not for "--first-parent --patch", so the behaviour may
> correctly match documentation, but that does not make the documented
> behaviour an intuitive one. And a behaviour that is not intuitive
> is confusing.
I think both behaviors make sense, provided they are correctly
documented. I just prefer the one that is more basic, yet allows to
achieve things that another one does not.
>
>> If we read resulting documentation with a fresh eye, -d is similar to
>> --cc, and -c, just producing yet another kind of output, so I think all
>> this fits together quite nicely and shouldn't cause confusion.
>
> Another thing is that showing first-parent patch for merges while
> letting the traversal also visit the second-parent chain is not as
> useful an option as it could be, even though it is not so bad as the
> original "-m -p" that also showed second-parent patch for merges as
> well.
I don't see why desire to look at diff-to-first-parent on "side"
branches is any different from desire to look at them on "primary"
branch, sorry, so I still don't want "-d" to affect traversal or other
commit filtering rules. We do have --first-parent as well as a few
others for that.
> People would have to say "log --first-parent -p" to get the
> first-parent traversal with first-parent patch output, and they
> would not behefit from having "-d".
Well, at least they can now say "log --first-parent -d" as well ;)
Honestly, the "log --first-parent -p" (without "-m") suddenly producing
diffs for merge commits is already unnatural, needs yet another
special-casing in documentation, and then, finally, this relatively new
behavior was introduced exactly because there were no "-d" at that time,
to save typing "-m". The latter is yet another example of why "-d" in
its current form is a good idea.
That said, if you feel like there is place for a short-cut for this
particular use-case, it'd be fine with me, say:
--fpd:
short-cut for "--first-parent -d"
would fit quite nicely into the picture, I think.
Thanks,
-- Sergey Organov
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] diff-merges: introduce '-d' option
2023-09-14 23:56 ` Sergey Organov
@ 2023-09-15 17:24 ` Junio C Hamano
2023-09-16 18:37 ` Sergey Organov
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2023-09-15 17:24 UTC (permalink / raw)
To: Sergey Organov; +Cc: git
Sergey Organov <sorganov@gmail.com> writes:
> I don't see why desire to look at diff-to-first-parent on "side"
> branches is any different from desire to look at them on "primary"
> branch
Yeah, but that is not what I meant. The above argues for why
"--diff-merges=first-parent" should exist independently from the
"--first-parent" traversal *and* display option. I am not saying
it should not exist.
But I view that the desire to look at any commits and its changes on
the "side" branch at all *is* at odds with the wish to look at
first-parent change for merge commits. Once you decide to look at
first-parent change for a merge commit, then every change you see
for each commit on the "side" branch, whether it is shown as
first-parent diff or N pairwise diffs, is what you have already seen
in the change in the merge commit, because "git log" goes newer to
older, and the commits on the side branches appear after the merge
that brings them to the mainline.
Making "log -d" mean "log --diff-merges=first-parent --patch" lets
that less useful combination ("show first-parent patches but
traverse side branches as well") squat on the short and sweet "-d"
that could be used for more useful "log --first-parent --patch",
which would also be more common and intuitive to users, and that is
what I suspect will become problematic in the longer run.
Thanks.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] diff-merges: introduce '-d' option
2023-09-15 17:24 ` Junio C Hamano
@ 2023-09-16 18:37 ` Sergey Organov
0 siblings, 0 replies; 18+ messages in thread
From: Sergey Organov @ 2023-09-16 18:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com> writes:
>
>> I don't see why desire to look at diff-to-first-parent on "side"
>> branches is any different from desire to look at them on "primary"
>> branch
>
> Yeah, but that is not what I meant. The above argues for why
> "--diff-merges=first-parent" should exist independently from the
> "--first-parent" traversal *and* display option. I am not saying
> it should not exist.
I was not assuming you were saying this, as it has been discussed and
agreed upon when --diff-merges=first-parent was introduced, though I
think I now see your point more clearly.
>
> But I view that the desire to look at any commits and its changes on
> the "side" branch at all *is* at odds with the wish to look at
> first-parent change for merge commits.
I think I do now understand what you mean, yet I have alternative view
on the issue.
> Once you decide to look at first-parent change for a merge commit,
> then every change you see for each commit on the "side" branch,
> whether it is shown as first-parent diff or N pairwise diffs, is what
> you have already seen in the change in the merge commit,
Actually, this happens to be exactly one of intended use-cases for "-d".
It's useful to see how some change introduced by the merge looked in the
context of the original commit, or to figure where the change came from.
> because "git log" goes newer to older, and the commits on the side
> branches appear after the merge that brings them to the mainline.
The exact order is orthogonal to the issue at hands, I think.
> Making "log -d" mean "log --diff-merges=first-parent --patch" lets
> that less useful combination ("show first-parent patches but
> traverse side branches as well") squat on the short and sweet "-d"
> that could be used for more useful "log --first-parent --patch",
> which would also be more common and intuitive to users, and that is
> what I suspect will become problematic in the longer run.
Sorry, "-d ≡ --first-parent --patch" you suggest contradicts my view on
the whole scheme of things, for several reasons:
* I still find it problematic if -d, intended to fit nicely among --cc,
-c, -d, -m, -p, --remerge-diff options, suddenly implies --first-parent.
This would bring yet another inconsistency, and I don't want to be the
one who introduced it.
* In its current state -d conveniently means: "gimme simple diff output
for everything", where --first-parent you suggest doesn't fit at all.
* Current -d implementation is semantically as close to -p as possible,
tweaking exactly one thing compared to -p: the format of output for
merge commits, so is simpler than what you suggest from all angles, as
--first-parent tweaks more than one thing.
* To me what you argue for looks mostly like a desire to have a
short-cut for "--first-parent --patch", and my patch in question does
not seem to contradict this desire, as it'd be very surprising if
somebody came up with the name "-d" for such a short-cut. Definitely not
me.
* Finally, if -d becomes "--patch --first-parent", how do I get back
useful "--patch --diff-merges=first-parent" part of it, provided
--first-parent is unreversable? And even if it were reversable, then
git log -d --no-first-parent =
git log --patch --first-parent --no-first-parent =
git log --patch
is definitely not what is needed, nor frequent demand to revert implied
things indicates optimal design. Compare this to
git log -d --first-parent
that current -d provides for you to get what you need, and that
unambiguously reads: "gimme *d*iff for all commits while following
*first parent* through the history" (while, unlike, -p not requiring
--first-parent to implicitly tweak diff for merges output).
Overall, after considering your concern, I'd still prefer to leave "-d"
semantics as implemented, consistent with the rest of similar options,
and let somebody else define more shortcuts for their frequent use-cases
if they feel like it.
Thanks,
-- Sergey Organov
P.S. I also figure that maybe our divergence comes from the fact that I
consider merge commits to be primarily commits (introducing particular
set of changes, and then having reference to the source of the changes),
whereas you consider them primarily merges (joining two histories, and
then maybe some artificial changes that make merges "evil"). That's why
we often end up agreeing to disagree, as both these points of view seem
pretty valid.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] diff-merges: improve --diff-merges documentation
2023-09-13 0:22 ` Junio C Hamano
@ 2023-09-18 16:20 ` Sergey Organov
2023-09-19 16:38 ` Junio C Hamano
0 siblings, 1 reply; 18+ messages in thread
From: Sergey Organov @ 2023-09-18 16:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com> writes:
>
>>> It is more like that `-p` does not imply `-m` (which used to mean
>>> "consider showing the comparison between parent(s) and the child,
>>> even for merge commits"), even though newer options like `-c`,
>>> `--cc` and others do imply `-m` (simply because they do not make
>>> much sense if they are not allowed to work on merges) that may make
>>> new people confused.
>>
>> No, neither --cc nor -c imply -m.
>
> I was only trying to help you polish the text you added to explain
> what you called the "legacy feature" to reflect the reason behind
> that legacy. As you obviously were not there back then when I made
> "--cc" imply "-m" while keeping "-p" not to imply "-m".
Your help is appreciated, yet unfortunately I still can't figure how to
improve the text based on your advice.
Your "I made --cc imply -m" does not explain why later, when you made
--cc imply -p (did you, or was it somebody else?), you didn't make -m
imply -p at the same time, and then "while keeping -p not to imply -m"
sounds out of place as we rather try to figure why "-m not implies -p".
The "--c imply -m" part of the help raises yet another question: if --cc
implied -m, why it was not -m that was made to imply -p instead of --cc
(and -c)? Then both --cc and -c would imply -p automatically as a
side-effect of implication of -p by -m (do not confuse with agreed
non-implication of -m by -p), and then all the relevant options were
consistent. This consideration renders current situation more surprising
instead of clarifying it, I'm afraid.
"-p does not imply -m" fact is fine with me and is not the cause of user
confusion I'm trying to address. How does it help us to explain why "-m
does not imply -p" though?
[...]
>
> Given that I, together with Linus, invented "--cc" and "-c", taking
> inspiration from how Paul Mackerras showed a merge in his 'gitk'
> tool, and made the design decision not to require "-m" to get the
> output in the format they specify when the "git log" traversal shows
> merge commits, I do not know what to say when you repeat that "--cc"
> does not imply "-m". It simply is not true.
I keep saying "--cc does not imply -m" because it does not seem to,
unless you either use some vague meaning of "imply", or mean some other
"-m", not the one used in "git log". Please check:
$ cd src/git
$ git --version
git version 2.42.0.111.gd814540bb75b
$ git describe
v2.42.0-111-gd814540bb75b
$ git log 74a2e88700efc -n1 -p --cc > diff.actual
$ git log 74a2e88700efc -n1 -p --cc -m > diff.expected
$ cmp diff.expected diff.actual
diff.expected diff.actual differ: byte 706, line 18
$
This test tells us that "--c" is not the same as "--cc -m", that for me
in turn reads "--cc does not imply -m", and that's what I continue to
say.
>
> I think this is the second time you claimed the above after I
> explained the same to you, if I am not mistaken. If you do not want
> to be corrected, that is fine, and I'll stop wasting my time trying
> to correct you.
I'd love to be corrected, but I think I carefully checked my grounds
before saying that --cc does not imply -m, please consider:
1. "--cc implies -m" is not documented. Please point to the
documentation in case I missed it.
2. Git does not behave as if "--cc implied -m", see the test-case above.
If it's neither documented nor matches actual behavior, it's not there,
at least from the POV of random user, to whom my original clarification
of "why -m does not imply -p?" has been addressed.
On top of that, I even can't figure why we argue about it in the first
place, as it seems to be irrelevant to the issue at hand: explain why -m
does not imply -p?
>
> But I still have to make sure that you (or anybody else) do not
> spread misinformation to other users by writing incorrect statements
> in documentation patches.
I'm all against spreading misinformation, and try my best to avoid it
myself. I still fail to see what misinformation, exactly, you find in
this particular explanation by me:
" Note: This option [`-m`] not implying `-p` is legacy feature that is
preserved for the sake of backward compatibility. "
That's exactly what I figured out from a lot of discussions over my
multiple attempts to make `-m` behave more usefully. Is it that "legacy
feature" somehow sounds offensive, or what?
As, despite your help, I fail to come up with better edition of the
note, please, if you feel like it, suggest your own variant of
explanation to the user why `-m` is left inconsistent with the rest of
diff for merges options, provided current matching documentation reads
roughly like this (from more recent options to oldest):
--remrege-diff: produces "remerge" output. Implies -p.
--cc: produces dense combined output. Implies -p.
-c: produces combined output. Implies -p.
-m: produces separate output, provided -p is given as well (?!).
and so why
git log -m
surprisingly has no visible effect, and then the user needs to
type:
git log -m -p
That's all I wanted to explain to the user in a few words with the note
you argue against.
Thanks,
-- Sergey Organov
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] diff-merges: improve --diff-merges documentation
2023-09-18 16:20 ` Sergey Organov
@ 2023-09-19 16:38 ` Junio C Hamano
2023-09-19 19:52 ` Sergey Organov
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2023-09-19 16:38 UTC (permalink / raw)
To: Sergey Organov; +Cc: git
Sergey Organov <sorganov@gmail.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> I was only trying to help you polish the text you added to explain
>> what you called the "legacy feature" to reflect the reason behind
>> that legacy. As you obviously were not there back then when I made
>> "--cc" imply "-m" while keeping "-p" not to imply "-m".
>
> Your help is appreciated, yet unfortunately I still can't figure how to
> improve the text based on your advice.
If I were doing this patch, I would start from something like this:
-m::
By default, comparisons between parent commits and the child
commit are not shown for merge commits, but with the `-m`
option, `git log` can be told to show comparisons for merges
in chosen formats (e.g. `--raw`, `-p`, `--stat`). When
output formats (e.g. `--cc`) that are specifically designed
to show better comparisons for merges are given, this option
is implied; in other words, you do not have to say e.g. `git
log -m --cc`. `git log --cc` suffices.
The rest is a tangent that is not related to the above. I suspect
that this also applies to newer `--remerge-diff`, as it also targets
to show merges better than the original "pairwise patches" that were
largely useless, but the right way to view what `--cc` and other
formats do for non-merge commits is *not* to think that they "imply"
`-p`. It is more like that the output from these formats on
non-merge commits happen to be identical to what `-p` would produce.
You could say that the "magic" these options know to show merge
commits better degenerates to what `-p` gives when applied to
non-merge commits.
Another way to look at it is that `--cc` and friends, even though
they are meant as improvements for showing merges over "-m -p" that
gives human-unreadable pair-wise diffs, do not imply "--merges"
(i.e. show only merge commits)---hence they have to show something
for non-merge commits. Because output formats for all of them were
modeled loosely [*] after "-p" output, we happened to pick it as the
format they fall back to when they are not showing comparisons for
merge commits.
[Footnote]
* Here, `-p` roughly means "what GNU patch and `git apply` take".
Output from `-c` and `--cc` on merge commits do not qualify, but
they are loosely modeled after it.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] diff-merges: improve --diff-merges documentation
2023-09-19 16:38 ` Junio C Hamano
@ 2023-09-19 19:52 ` Sergey Organov
0 siblings, 0 replies; 18+ messages in thread
From: Sergey Organov @ 2023-09-19 19:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> I was only trying to help you polish the text you added to explain
>>> what you called the "legacy feature" to reflect the reason behind
>>> that legacy. As you obviously were not there back then when I made
>>> "--cc" imply "-m" while keeping "-p" not to imply "-m".
>>
>> Your help is appreciated, yet unfortunately I still can't figure how to
>> improve the text based on your advice.
>
> If I were doing this patch, I would start from something like this:
>
> -m::
> By default, comparisons between parent commits and the child
> commit are not shown for merge commits, but with the `-m`
> option, `git log` can be told to show comparisons for merges
> in chosen formats (e.g. `--raw`, `-p`, `--stat`). When
> output formats (e.g. `--cc`) that are specifically designed
> to show better comparisons for merges are given, this option
> is implied; in other words, you do not have to say e.g. `git
> log -m --cc`. `git log --cc` suffices.
Well, to me this piece looks much harder to understand than current Git
documentation, and then seemingly contradicts current Git behavior and
implementation, as "log --cc -m" is not the same as "log --cc" in the
current Git (so we can't say that --cc implies -m), and "log -m --cc" is
the same as "log --cc" due to absolutely different reason: -m and --cc
are mutually exclusive options, so the last one simply takes precedence.
In the current Git, as documented, -m just produces separate diff with
respect to every parent. Simple and straightforward. Users don't need to
learn about --cc, -c, --raw, --stat... to figure what -m does and if
it's what they need. Unfortunately they still need to learn about -p,
but I'm already done trying to promote this simple change.
>
> The rest is a tangent that is not related to the above. I suspect
> that this also applies to newer `--remerge-diff`, as it also targets
> to show merges better than the original "pairwise patches" that were
> largely useless, but the right way to view what `--cc` and other
> formats do for non-merge commits is *not* to think that they "imply"
> `-p`. It is more like that the output from these formats on
> non-merge commits happen to be identical to what `-p` would produce.
> You could say that the "magic" these options know to show merge
> commits better degenerates to what `-p` gives when applied to
> non-merge commits.
>
> Another way to look at it is that `--cc` and friends, even though
> they are meant as improvements for showing merges over "-m -p" that
> gives human-unreadable pair-wise diffs, do not imply "--merges"
> (i.e. show only merge commits)---hence they have to show something
> for non-merge commits. Because output formats for all of them were
> modeled loosely [*] after "-p" output, we happened to pick it as the
> format they fall back to when they are not showing comparisons for
> merge commits.
I admit you are very creative producing these views,, but currently
these options just imply -p. Simple to understand, useful, works.
Overall, as you don't like my simple clarification, and I don't like the
direction(s) you propose, I figure I rather withdraw the part of patch
causing contention in the re-roll.
Thanks,
-- Sergey Organov
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/2] diff-merges: introduce '-d' option
2023-09-09 12:54 [PATCH 0/2] diff-merges: introduce '-d' option Sergey Organov
2023-09-09 12:54 ` [PATCH 1/2] diff-merges: improve --diff-merges documentation Sergey Organov
2023-09-09 12:54 ` [PATCH 2/2] diff-merges: introduce '-d' option Sergey Organov
@ 2023-09-20 15:02 ` Sergey Organov
2023-09-20 15:02 ` [PATCH v2 1/2] diff-merges: improve --diff-merges documentation Sergey Organov
2023-09-20 15:02 ` [PATCH v2 2/2] diff-merges: introduce '-d' option Sergey Organov
2 siblings, 2 replies; 18+ messages in thread
From: Sergey Organov @ 2023-09-20 15:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Sergey Organov
This new convenience option requests full diff with respect to first
parent, so that
git log -d
will output diff with respect to first parent for every commit,
universally, no matter how many parents the commit turns out to have.
It's implemented as pure synonym for
--diff-merges=first-parent --patch
The first commit in the series tweaks diff-merges documentation a bit,
and is valuable by itself. It's put here as '-d' implementation commit
depends on it in its documentation part.
Note: the need for this new convenience option mostly emerged from
denial by the community of patches that modify '-m' behavior to imply
'-p' as the rest of similar options (such as --cc) do.
Updates in v2:
* Reordered documentation for diff-merges formats in accordance with
Junio recommendation.
* Removed clarification of surprising -m behavior due to controversy
with Junio on how exactly it should look like.
Sergey Organov (2):
diff-merges: improve --diff-merges documentation
diff-merges: introduce '-d' option
Documentation/diff-options.txt | 102 ++++++++++++++++++---------------
Documentation/git-log.txt | 4 +-
diff-merges.c | 3 +
t/t4013-diff-various.sh | 8 +++
4 files changed, 70 insertions(+), 47 deletions(-)
Interdiff against v1:
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index d773dafcb10a..19bb78ff6652 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -47,9 +47,6 @@ ifdef::git-log[]
Show diffs for merge commits in the default format. This is
similar to '--diff-merges=on' (which see) except `-m` will
produce no output unless `-p` is given as well.
-+
-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.
@@ -96,16 +93,6 @@ 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).
-+
combined, c::
Show differences from each of the parents to the merge
result simultaneously instead of showing pairwise diff between
@@ -117,6 +104,16 @@ dense-combined, cc::
by omitting uninteresting hunks whose contents in the parents
have only two variants and the merge result picks one of them
without modification.
++
+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).
--
--combined-all-paths::
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 1/2] diff-merges: improve --diff-merges documentation
2023-09-20 15:02 ` [PATCH v2 0/2] " Sergey Organov
@ 2023-09-20 15:02 ` Sergey Organov
2023-09-20 15:02 ` [PATCH v2 2/2] diff-merges: introduce '-d' option Sergey Organov
1 sibling, 0 replies; 18+ messages in thread
From: Sergey Organov @ 2023-09-20 15:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Sergey Organov
* Put descriptions of convenience shortcuts first, so they are the
first things reader observes rather than lengthy detailed stuff.
* Get rid of very long line containing all the --diff-merges formats
by replacing them with <format>, and putting each supported format
on its own line.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
Documentation/diff-options.txt | 98 ++++++++++++++++++----------------
Documentation/git-log.txt | 2 +-
2 files changed, 54 insertions(+), 46 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9f33f887711d..8035210c1418 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -43,66 +43,74 @@ 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)::
+-m::
+ Show diffs for merge commits in the default format. This is
+ similar to '--diff-merges=on' (which see) except `-m` will
+ produce no output unless `-p` is given as well.
+
+-c::
+ Produce combined diff output for merge commits.
+ Shortcut for '--diff-merges=combined -p'.
+
+--cc::
+ Produce dense combined diff output for merge commits.
+ Shortcut for '--diff-merges=dense-combined -p'.
+
+--remerge-diff::
+ Produce diff against re-merge.
+ Shortcut for '--diff-merges=remerge -p'.
+
--no-diff-merges::
+ Synonym for '--diff-merges=off'.
+
+--diff-merges=<format>::
Specify diff format 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.
+ {diff-merges-default} unless `--first-parent` is in use, in
+ which case `first-parent` is the default.
+
---diff-merges=(off|none):::
---no-diff-merges:::
+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:::
- 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
+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`.
+
---diff-merges=first-parent:::
---diff-merges=1:::
- This option makes merge commits show the full diff with
- respect to the first parent only.
+first-parent, 1::
+ Show full diff with respect to first parent. This is the same
+ format as `--patch` produces for non-merge commits.
+
---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.
+separate::
+ Show full diff with respect to each of 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.
+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.
++
+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.
++
+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`.
-+
---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`.
+--
--combined-all-paths::
This flag causes combined diffs (used for merge commits) to
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 2a66cf888074..9b7ec96e767a 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -124,7 +124,7 @@ Note that unless one of `--diff-merges` variants (including short
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
-the default format.
+the default format for merge commits.
:git-log: 1
:diff-merges-default: `off`
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/2] diff-merges: introduce '-d' option
2023-09-20 15:02 ` [PATCH v2 0/2] " Sergey Organov
2023-09-20 15:02 ` [PATCH v2 1/2] diff-merges: improve --diff-merges documentation Sergey Organov
@ 2023-09-20 15:02 ` Sergey Organov
1 sibling, 0 replies; 18+ messages in thread
From: Sergey Organov @ 2023-09-20 15:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Sergey Organov
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>
---
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 8035210c1418..19bb78ff6652 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -48,6 +48,10 @@ ifdef::git-log[]
similar to '--diff-merges=on' (which see) except `-m` will
produce no output unless `-p` is given as well.
+-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
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2023-09-20 15:03 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-09 12:54 [PATCH 0/2] diff-merges: introduce '-d' option Sergey Organov
2023-09-09 12:54 ` [PATCH 1/2] diff-merges: improve --diff-merges documentation Sergey Organov
2023-09-11 21:12 ` Junio C Hamano
2023-09-12 7:37 ` Sergey Organov
2023-09-13 0:22 ` Junio C Hamano
2023-09-18 16:20 ` Sergey Organov
2023-09-19 16:38 ` Junio C Hamano
2023-09-19 19:52 ` Sergey Organov
2023-09-09 12:54 ` [PATCH 2/2] diff-merges: introduce '-d' option Sergey Organov
2023-09-11 21:01 ` Junio C Hamano
2023-09-12 7:59 ` Sergey Organov
2023-09-14 22:17 ` Junio C Hamano
2023-09-14 23:56 ` Sergey Organov
2023-09-15 17:24 ` Junio C Hamano
2023-09-16 18:37 ` Sergey Organov
2023-09-20 15:02 ` [PATCH v2 0/2] " Sergey Organov
2023-09-20 15:02 ` [PATCH v2 1/2] diff-merges: improve --diff-merges documentation Sergey Organov
2023-09-20 15:02 ` [PATCH v2 2/2] diff-merges: introduce '-d' option Sergey Organov
Code repositories for project(s) associated with this public inbox
https://80x24.org/pub/scm/git/git.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).