Re: [PATCH 2/5] diff: allow --patch to override -s/--no-patch

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 15, 2013, 15:02 UTC
Message-ID: <7v61wbeuix.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1373893639-13413-3-git-send-email-Matthieu.Moy@imag.fr>

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

Show 22 quoted lines

> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  diff.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 6bd821d..66a6877 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3515,9 +3515,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
>  	int argcount;
>  
>  	/* Output format options */
> -	if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch"))
> +	if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch")) {
> +		options->output_format &= ~DIFF_FORMAT_NO_OUTPUT;
>  		options->output_format |= DIFF_FORMAT_PATCH;
> -	else if (opt_arg(arg, 'U', "unified", &options->context))
> +	} else if (opt_arg(arg, 'U', "unified", &options->context))
>  		options->output_format |= DIFF_FORMAT_PATCH;
>  	else if (!strcmp(arg, "--raw"))
>  		options->output_format |= DIFF_FORMAT_RAW;

I am wondering if the difference after this patch between "-p" and "-U8" is deliberate, or just an accident coming from the way the original was written in ee1e5412 (git diff: support "-U" and "--unified" options properly, 2006-05-13).

If the original were written in this way:

	if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch") ||
            opt_arg(arg, 'U', "unified", &options->context))
  		options->output_format |= DIFF_FORMAT_PATCH;

I suspect you would have given the additional

		options->output_format &= ~DIFF_FORMAT_NO_OUTPUT;

to the whole thing, without singling out "-U8". Or am I missing some good reason why "--no-patch -U8" should not produce a patch, while "--no-patch -p" and "-U8" should both should?