Re: [PATCH 2/3] revision: add "--ignore-merges" option to counteract "-m"
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 28, 2020, 21:01 UTC
- Message-ID
- <xmqqv9i7cq1k.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20200728163853.GB2650252@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 46 quoted lines
> The "-m" option sets revs->ignore_merges to "0", but there's no way to > undo it. This probably isn't something anybody overly cares about, since > "1" is already the default, but it will serve as an escape hatch when we > flip the default for ignore_merges to "0" in more situations. > > We'll also add a few extra niceties: > > - initialize the value to "-1" to indicate "not set", and then resolve > it to the normal 0/1 bool in setup_revisions(). This lets any tweak > functions, as well as setup_revisions() itself, avoid clobbering the > user's preference (which until now they couldn't actually express). > > - since we now have --ignore-merges, let's add the matching > --no-ignore-merges, which is just a synonym for "-m". In fact, it's > simpler to just document --no-ignore-merges alongside "-m", and > leave it implied that its opposite countermands it. > > The new test shows that this behaves just the same as the current > behavior without "-m". > > Signed-off-by: Jeff King <peff@peff.net> > --- > I pulled the option name from the rev_info field name. It might be too > broad (we are not ignoring merges during the traversal, only for the > diff). It could be "--no-diff-merges" or something, but that would > involve flipping the sense of the boolean (but that would just be in the > code, not user-visible, so not that big a deal). > > Documentation/rev-list-options.txt | 1 + > builtin/log.c | 4 +- > revision.c | 10 ++- > revision.h | 2 +- > t/t4013-diff-various.sh | 1 + > ...g_--ignore-merges_-p_--first-parent_master | 78 +++++++++++++++++++ > 6 files changed, 90 insertions(+), 6 deletions(-) > create mode 100644 t/t4013/diff.log_--ignore-merges_-p_--first-parent_master > > diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt > index b01b2b6773..fbd8fa0381 100644 > --- a/Documentation/rev-list-options.txt > +++ b/Documentation/rev-list-options.txt > @@ -1148,6 +1148,7 @@ options may be given. See linkgit:git-diff-files[1] for more options. > rename or copy detection have been requested). > > -m:: > +--no-ignore-merges::
This invites a natural "does --ignore-merges exist, and if so what does it do?" Why not to have "--[no-]ignore-merges" as a separate entry immediately after the existing "-m" instead?
> This flag makes the merge commits show the full diff like > regular commits; for each merge parent, a separate log entry > and diff is generated. An exception is that only diff against
That is,
--[no-]ignore-merges::
`--no-ignore-merges` is a synonym to `-m`. `--ignore-merges` countermands an earlier `-m` that is either explicitly or implicitly given.
or something along the line, perhaps?
Show 9 quoted lines
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh > index 43267d6024..8f9181316f 100755 > --- a/t/t4013-diff-various.sh > +++ b/t/t4013-diff-various.sh > @@ -297,6 +297,7 @@ log --root --patch-with-stat --summary master > log --root -c --patch-with-stat --summary master > # improved by Timo's patch > log --root --cc --patch-with-stat --summary master > +log --ignore-merges -p --first-parent master
This explicitly says "I do not want to see diff for merges" ...
Show 16 quoted lines
> log -p --first-parent master > log -m -p --first-parent master > log -m -p master > diff --git a/t/t4013/diff.log_--ignore-merges_-p_--first-parent_master b/t/t4013/diff.log_--ignore-merges_-p_--first-parent_master > new file mode 100644 > index 0000000000..fa0cdc8a23 > --- /dev/null > +++ b/t/t4013/diff.log_--ignore-merges_-p_--first-parent_master > @@ -0,0 +1,78 @@ > +$ git log --ignore-merges -p --first-parent master > +commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 > +Merge: 9a6d494 c7a2ab9 > +Author: A U Thor <author@example.com> > +Date: Mon Jun 26 00:04:00 2006 +0000 > + > + Merge branch 'side' into master
... and that is honored here? Good.