Subject: Re: [PATCH v2 26/33] diff-merges: let new options enable diff without -p

From: Sergey Organov <[email-masked]>
Date: Sun, 20 Dec 2020 22:07:14 +0300

Sergey Organov <[email-masked]> writes:

> Elijah Newren <[email-masked]> writes:
>
>> On Wed, Dec 16, 2020 at 10:50 AM Sergey Organov <[email-masked]> wrote:
>>>

[...]

>>
>>>         else if (!strcmp(optarg, "first") || !strcmp(optarg, "first-parent"))
>>>                 set_first_parent(revs);
>>>         else if (!strcmp(optarg, "sep")   || !strcmp(optarg, "separate"))
>>> @@ -64,6 +67,7 @@ static void set_diff_merges(struct rev_info *revs, const char *optarg)
>>>                 set_dense_combined(revs);
>>>         else
>>>                 die(_("unknown value for --diff-merges: %s"), optarg);
>>> +       revs->merges_need_diff = 1;
>>
>> I'd put this above the if-else-else block, to make it clearer why you
>> are returning early for the "off"/"none" case.
>
> Yeah, makes sense, thanks!

In fact this can't be done as it changes the outcome, as set_xxx()
functions clear the flag.

I rather added clarification comments to the code.

Thanks,
-- Sergey