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

From: Sergey Organov <sorganov@gmail.com>
Date: Dec 20, 2020, 19:07 UTC
Message-ID: <87k0tcthy5.fsf@osv.gnss.ru>
In-Reply-To: <87wnxfb2gt.fsf@osv.gnss.ru>

Sergey Organov <sorganov@gmail.com> writes:

> Elijah Newren <newren@gmail.com> writes:
>
>> On Wed, Dec 16, 2020 at 10:50 AM Sergey Organov <sorganov@gmail.com> wrote:
>>>

[...]

Show 14 quoted lines

>>
>>>         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