Re: [PATCH v4 3/3] range-diff: make diff option behavior (e.g. --stat) consistent

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 12, 2018, 03:17 UTC
Message-ID: <xmqqk1lj3ry1.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAPig+cTWgmApseaz4cZqUU4U1NJrcu3vq0FzVwHO6icT7nWcPw@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 14 quoted lines

> On Fri, Nov 9, 2018 at 5:18 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> Make the behavior when diff options (e.g. "--stat") are passed
>> consistent with how "diff" behaves.
>> [...]
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>> diff --git a/range-diff.c b/range-diff.c
>> @@ -453,7 +453,8 @@ int show_range_diff(const char *range1, const char *range2,
>> -               opts.output_format |= DIFF_FORMAT_PATCH;
>> +               if (!opts.output_format)
>> +                       opts.output_format |= DIFF_FORMAT_PATCH;
>
> I think this can just be '=' now instead of '|=' (to avoid confusing
> the reader, even if it's functionally equivalent).

Hmph, could the condition in the future change to

	-	if (!opts.output_format)
	+	if (! (opts.output_format & DIFF_FORMAT_MASK))
			opts.output_format |= DIFF_FORMAT_PATCH

if we ever gain a new "output_format" bit that does not affect how we show the diff in a major way, and that is on by default? If so, I think "|=" is more future-proof. Otherwise, "=" is indeed more clear way to spell the intention.