Re: [PATCH v2 0/7] making log --first-parent imply -m

[Sergey Organov <sorganov@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1216 bytes --]

Jeff King <peff@peff.net> 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-revision-change-diff-merges-option-to-require-parame.patch --]
[-- Type: text/x-patch, Size: 1417 bytes --]

From 34cba8c49e2c40fbc228b9904491633939792b6d Mon Sep 17 00:00:00 2001
From: Sergey Organov <sorganov@gmail.com>
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@gmail.com>
---
 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