Re: [RFC/PATCH 2/4] textconv: make diff_options accessible from blame

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 4, 2010, 05:48 UTC
Message-ID: <7vvd9z5owr.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1275562038-7468-3-git-send-email-axel.bonnet@ensimag.imag.fr>

Axel Bonnet <axel.bonnet@ensimag.imag.fr> writes:

Show 6 quoted lines

> Diff_options specify whether conversion is activated or not. Blame needs
> to access these options in order to concert files with external drivers
>
> Signed-off-by: Diane Gasselin <diane.gasselin@ensimag.imag.fr>
> Signed-off-by: ClÃ©ment Poulain <clement.poulain@ensimag.imag.fr>
> Signed-off-by: Axel Bonnet <axel.bonnet@ensimag.imag.fr>

The name of Clément is spelled correctly on the mail header while S-o-b line is corrupt. Perhaps you have recorded your commits in UTF-8 but allowed your MUA to send in 8859-1? This comment applies to all the patches in the series.

Show 14 quoted lines

> diff --git a/builtin/blame.c b/builtin/blame.c
> index fc15863..63b497c 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -89,7 +89,8 @@ struct origin {
>   * Given an origin, prepare mmfile_t structure to be used by the
>   * diff machinery
>   */
> -static void fill_origin_blob(struct origin *o, mmfile_t *file)
> +static void fill_origin_blob(struct diff_options opt,
> +			     struct origin *o, mmfile_t *file)
>  {
>  	if (!o->file.ptr) {
>  		enum object_type type;

Two points.

 * Generally we do not want to pass structures by value.  It is especially
   true when the structure is bigger than one word, and accesses to the
   variable in the callee is read-only.

 * The callee does not seem to use the new parameter yet.  You might want
   to defer this change until fill-origin-blob actually starts using it.