Jeff King <peff@xxxxxxxx> writes:
> On Tue, Aug 04, 2020 at 12:42:45PM -0700, Junio C Hamano wrote:
>
>> As a minimum patch, I think it is OK to have just 'all' and 'none'
>> (not even c or cc, let alone the one with ultra-long name whose
>> effect is mystery) before we let the result graduate to 'master'.
>> Others can be added on top, as the primary focus of Peff's series is
>> to make sure "-m" can be countermanded, for which being able to say
>> "no" is sufficient, and the primary reason why we are further
>> futzing with the series with this addition is to leave the door open
>> for later additions of different "modes" in which how
>> "--diff-merges" option can operate (iow, Peff's was merely on/off,
>> but you are making sure others such as <num> can be added over
>> time).
>
> I like that suggestion very much. It solves the "optional arguments are
> evil" problem without having to worry about the other bits.
Here is the patch reduced to absolute minimum, both functionally and
textually. I removed even 'all', as it has its own subtleties that need
further discussion, so the patch only introduces --diff-merges=off.
If it looks OK, I'll do documentation and tests parts.
Thanks,
-- Sergey
>From 34cba8c49e2c40fbc228b9904491633939792b6d Mon Sep 17 00:00:00 2001
From: Sergey Organov <sorganov@xxxxxxxxx>
Date: Tue, 4 Aug 2020 16:48:27 +0300
Subject: [PATCH RFC v2] revision: change "--diff-merges" option to require
parameter
--diff-merges=off is the only accepted form for now, a synonym for
--no-diff-merges.
This patch is a preparation for adding more values, as well as supporting
--diff-merges=<parent>, where <parent> is single parent number to output diff
against.
Signed-off-by: Sergey Organov <sorganov@xxxxxxxxx>
---
revision.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/revision.c b/revision.c
index 669bc856694f..88a0bfdb4a4c 100644
--- a/revision.c
+++ b/revision.c
@@ -2323,8 +2323,15 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->diff = 1;
revs->diffopt.flags.recursive = 1;
revs->diffopt.flags.tree_in_recursive = 1;
- } else if (!strcmp(arg, "-m") || !strcmp(arg, "--diff-merges")) {
+ } else if (!strcmp(arg, "-m")) {
revs->ignore_merges = 0;
+ } else if ((argcount = parse_long_opt("diff-merges", argv, &optarg))) {
+ if (!strcmp(optarg, "off")) {
+ revs->ignore_merges = 1;
+ } else {
+ die("--diff-merges: unknown value '%s'.", optarg);
+ }
+ return argcount;
} else if (!strcmp(arg, "--no-diff-merges")) {
revs->ignore_merges = 1;
} else if (!strcmp(arg, "-c")) {
--
2.20.1
