| Message ID | 20230909125446.142715-2-sorganov@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | diff-merges: introduce '-d' option | expand |
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").
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
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.
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
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.
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
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`
* 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(-)