Re: [PATCH v3 2/4] diff: make diff_populate_filespec_options struct

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 7, 2020, 23:44 UTC
Message-ID: <xmqqpncirhd9.fsf@gitster.c.googlers.com>
In-Reply-To: <c1973fd6308109b0cc99544500d8932222b66726.1586296510.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

> The behavior of diff_populate_filespec() currently can be customized
> through a bitflag, but a subsequent patch requires it to support a
> non-boolean option. Replace the bitflag with an options struct.

Hmph, clever :-).

> +	struct diff_populate_filespec_options dpf_options = {
> +		.check_size_only = 1,
> +	};

I would have called this instance of d-p-f-o "check_size_only", which would make the site that uses it ...

>  	if (!DIFF_FILE_VALID(one))
>  		return 0;
> -	diff_populate_filespec(r, one, CHECK_SIZE_ONLY);
> +	diff_populate_filespec(r, one, &dpf_options);

... easier to understand, especially if we can made it constant, but that would probably contradict the plan to add more fields to the structure, so let's see how it goes.

Show 8 quoted lines

> @@ -3339,13 +3346,17 @@ static void emit_binary_diff(struct diff_options *o,
>  int diff_filespec_is_binary(struct repository *r,
>  			    struct diff_filespec *one)
>  {
> +	struct diff_populate_filespec_options dpf_options = {
> +		.check_binary = 1,
> +	};
> +

The same comment applies to here, too.