Re: [PATCH v3 2/2] range-diff: fix regression in passing along diff options

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Nov 8, 2018, 17:08 UTC
Message-ID: <CAPig+cTeNcn_raJWfEK_C_du1v1KYQFTVmWA9B4DNHAXoFHw-w@mail.gmail.com>
In-Reply-To: <20181107122202.1813-3-avarab@gmail.com>

On Wed, Nov 7, 2018 at 7:22 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

> In 73a834e9e2 ("range-diff: relieve callers of low-level configuration
> burden", 2018-07-22) we broke passing down options like --no-patch,
> --stat etc. Fix that regression, and add a test for some of these
> options being passed down.

Thanks both (Ævar and Dscho) for cleaning up my mess, and sorry for not responding sooner; I only just found time to read the discussion thread. One comment below...

Show 8 quoted lines

> 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,
>                 memcpy(&opts, diffopt, sizeof(opts));
> -               opts.output_format = DIFF_FORMAT_PATCH;
> +               if (!opts.output_format)
> +                       opts.output_format = DIFF_FORMAT_PATCH;

Looking at diff.c:parse_diff_opt() and enable_patch_output(), rather than introducing this new conditional, I'm thinking that a more correct fix would be:

    opts.output_format |= DIFF_FORMAT_PATCH;

(note the '|=' operator). This would result in 'opts.output_format' containing (DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT), just as it did prior to 73a834e9e2 when --no-patch was specified.