Re: [RFC PATCH 1/5] format-patch: allow additional generated content in make_cover_letter()

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Eric,

On Wed, 30 May 2018, Eric Sunshine wrote:

> make_cover_letter() returns early when it lacks sufficient state to emit
> a diffstat, which makes it difficult to extend the function to reliably
> emit additional generated content. Work around this shortcoming by
> factoring diffstat-printing logic out to its own function and calling it
> as needed without otherwise inhibiting normal control flow.
> 
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>

Makes sense.

Ciao,
Dscho

> ---
>  builtin/log.c | 43 +++++++++++++++++++++++--------------------
>  1 file changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/builtin/log.c b/builtin/log.c
> index 71f68a3e4f..e01a256c11 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -992,6 +992,26 @@ static char *find_branch_name(struct rev_info *rev)
>  	return branch;
>  }
>  
> +static void emit_diffstat(struct rev_info *rev,
> +			  struct commit *origin, struct commit *head)
> +{
> +	struct diff_options opts;
> +
> +	memcpy(&opts, &rev->diffopt, sizeof(opts));
> +	opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
> +	opts.stat_width = MAIL_DEFAULT_WRAP;
> +
> +	diff_setup_done(&opts);
> +
> +	diff_tree_oid(&origin->tree->object.oid,
> +		      &head->tree->object.oid,
> +		      "", &opts);
> +	diffcore_std(&opts);
> +	diff_flush(&opts);
> +
> +	fprintf(rev->diffopt.file, "\n");
> +}
> +
>  static void make_cover_letter(struct rev_info *rev, int use_stdout,
>  			      struct commit *origin,
>  			      int nr, struct commit **list,
> @@ -1005,7 +1025,6 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
>  	struct strbuf sb = STRBUF_INIT;
>  	int i;
>  	const char *encoding = "UTF-8";
> -	struct diff_options opts;
>  	int need_8bit_cte = 0;
>  	struct pretty_print_context pp = {0};
>  	struct commit *head = list[0];
> @@ -1055,25 +1074,9 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
>  
>  	shortlog_output(&log);
>  
> -	/*
> -	 * We can only do diffstat with a unique reference point
> -	 */
> -	if (!origin)
> -		return;
> -
> -	memcpy(&opts, &rev->diffopt, sizeof(opts));
> -	opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
> -	opts.stat_width = MAIL_DEFAULT_WRAP;
> -
> -	diff_setup_done(&opts);
> -
> -	diff_tree_oid(&origin->tree->object.oid,
> -		      &head->tree->object.oid,
> -		      "", &opts);
> -	diffcore_std(&opts);
> -	diff_flush(&opts);
> -
> -	fprintf(rev->diffopt.file, "\n");
> +	/* We can only do diffstat with a unique reference point */
> +	if (origin)
> +		emit_diffstat(rev, origin, head);
>  }
>  
>  static const char *clean_message_id(const char *msg_id)
> -- 
> 2.17.1.1235.ge295dfb56e
> 
>