From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 04 Aug 2020 21:27:58 GMT
Subject: Re: [PATCH v2 0/7] making log --first-parent imply -m
Message-ID: <xmqq7duerti9.fsf@gitster.c.googlers.com>
In-Reply-To: <873652166e.fsf@osv.gnss.ru>

Sergey Organov <sorganov@gmail.com> writes:

> Jeff King <peff@peff.net> writes:
>
>>> +	} 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.

I may be mistaken, but I thought Peff was asking about turning
revs->diff off.  I somehow thought that the equivalence planned for
the short term is:

            (new)               (peff's)         (master)
	diff-merges=none == --no-diff-merges == ! -m
	diff-merges=all  == --diff-merges    == -m

and future extension would add equivalents to --cc and -c, in
addition to <parentNum>, which is not something we can do with the
current UI and machinery.

So, shouldn't the body of that else if clause for "all" be a no-op?
i.e.

		} else if (!strcmp(optarg, "all")) {
			; /* nothing */
		}