From: Sergey Organov <sorganov@gmail.com>
Date: Tue, 04 Aug 2020 20:56:25 GMT
Subject: Re: [PATCH v2 0/7] making log --first-parent imply -m
Message-ID: <873652166e.fsf@osv.gnss.ru>
In-Reply-To: <20200804195830.GA2014743@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Aug 04, 2020 at 08:50:16PM +0300, Sergey Organov wrote:
>
>> Attached is rather minimal incompatible change to --diff-merges that'd
>> allow extensions in the future, to get out of urge for the discussed
>> changes. I'm going to follow-up with actual improvements and I'm aware
>> it lacks documentation changes.
>
> Thanks, I like the direction here. Definitely it would need
> documentation, but also tests (probably in t4013 alongside the ones my
> series added; in fact you'd probably need to adjust my tests for the
> non-optional argument).
>
>> diff --git a/revision.c b/revision.c
>> index 669bc856694f..dcdff59bc36a 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -2323,10 +2323,31 @@ 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 ((argcount = parse_long_opt("diff-merges", argv, &optarg))) {
>>  		revs->ignore_merges = 0;
>> +		if (!strcmp(optarg, "off")) {
>> +			revs->ignore_merges = 1;
>> +		} else if (!strcmp(optarg, "all")) {
>> +			revs->diff = 0;
>
> Should this be revs->ignore_merges = 0?

It's 4 lines above, as it's in fact common for all the cases but the
first one.

-- Sergey