Re: [PATCH 17/26] diff.c: convert show_stats to use emit_diff_symbol

[Brandon Williams <bmwill@google.com>]

On 06/19, Stefan Beller wrote:
> We call print_stat_summary from builtin/apply, so we still
> need the version with a file pointer, so introduce
> print_stat_summary_0 that uses emit_string machinery and
> keep print_stat_summary with the same arguments around.
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  diff.c | 118 +++++++++++++++++++++++++++++++++++++++++------------------------
>  diff.h |   4 +--
>  2 files changed, 77 insertions(+), 45 deletions(-)
> 
> diff --git a/diff.c b/diff.c
> index e6ade5fde0..0a5ff2500f 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -586,7 +586,10 @@ enum diff_symbol {
>  	DIFF_SYMBOL_SUBMODULE_HEADER,
>  	DIFF_SYMBOL_SUBMODULE_ERROR,
>  	DIFF_SYMBOL_SUBMODULE_PIPETHROUGH,
> -
> +	DIFF_SYMBOL_STATS_SUMMARY_NO_FILES,
> +	DIFF_SYMBOL_STATS_SUMMARY_ABBREV,
> +	DIFF_SYMBOL_STATS_SUMMARY_INSERTS_DELETES,
> +	DIFF_SYMBOL_STATS_LINE,
>  };
>  /*
>   * Flags for content lines:
> @@ -628,6 +631,7 @@ static void emit_diff_symbol(struct diff_options *o, enum diff_symbol s,
>  {
>  	static const char *nneof = " No newline at end of file\n";
>  	const char *context, *reset, *set, *meta, *fraginfo;
> +	struct strbuf sb = STRBUF_INIT;
>  	switch (s) {
>  	case DIFF_SYMBOL_SEPARATOR:
>  		fprintf(o->file, "%s%c",
> @@ -756,9 +760,22 @@ static void emit_diff_symbol(struct diff_options *o, enum diff_symbol s,
>  	case DIFF_SYMBOL_SUBMODULE_PIPETHROUGH:
>  		emit_line(o, "", "", line, len);
>  		break;
> +	case DIFF_SYMBOL_STATS_SUMMARY_NO_FILES:
> +		fprintf(o->file, " 0 files changed\n");

Why is this one a fprintf call instead of an emit_line call?  I'm sure
this is intended It just stands out to me.

> +		break;
> +	case DIFF_SYMBOL_STATS_SUMMARY_ABBREV:
> +		emit_line(o, "", "", " ...\n", strlen(" ...\n"));
> +		break;
> +	case DIFF_SYMBOL_STATS_LINE:
> +		emit_line(o, "", "", line, len);
> +		break;
> +	case DIFF_SYMBOL_STATS_SUMMARY_INSERTS_DELETES:
> +		emit_line(o, "", "", line, len);
> +		break;
>  	default:
>  		die("BUG: unknown diff symbol");
>  	}
> +	strbuf_release(&sb);
>  }
>  
>  void diff_emit_submodule_del(struct diff_options *o, const char *line)
> @@ -1712,20 +1729,14 @@ static int scale_linear(int it, int width, int max_change)
>  	return 1 + (it * (width - 1) / max_change);
>  }
>  
> -static void show_name(FILE *file,
> -		      const char *prefix, const char *name, int len)
> -{
> -	fprintf(file, " %s%-*s |", prefix, len, name);
> -}
> -
> -static void show_graph(FILE *file, char ch, int cnt, const char *set, const char *reset)
> +static void show_graph(struct strbuf *out, char ch, int cnt,
> +		       const char *set, const char *reset)
>  {
>  	if (cnt <= 0)
>  		return;
> -	fprintf(file, "%s", set);
> -	while (cnt--)
> -		putc(ch, file);
> -	fprintf(file, "%s", reset);
> +	strbuf_addstr(out, set);
> +	strbuf_addchars(out, ch, cnt);
> +	strbuf_addstr(out, reset);
>  }
>  
>  static void fill_print_name(struct diffstat_file *file)
> @@ -1749,14 +1760,16 @@ static void fill_print_name(struct diffstat_file *file)
>  	file->print_name = pname;
>  }
>  
> -int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
> +static void print_stat_summary_inserts_deletes(struct diff_options *options,
> +		int files, int insertions, int deletions)
>  {
>  	struct strbuf sb = STRBUF_INIT;
> -	int ret;
>  
>  	if (!files) {
>  		assert(insertions == 0 && deletions == 0);
> -		return fprintf(fp, "%s\n", " 0 files changed");
> +		emit_diff_symbol(options, DIFF_SYMBOL_STATS_SUMMARY_NO_FILES,
> +				 NULL, 0, 0);
> +		return;
>  	}
>  
>  	strbuf_addf(&sb,
> @@ -1783,9 +1796,19 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
>  			    deletions);
>  	}
>  	strbuf_addch(&sb, '\n');
> -	ret = fputs(sb.buf, fp);
> +	emit_diff_symbol(options, DIFF_SYMBOL_STATS_SUMMARY_INSERTS_DELETES,
> +			 sb.buf, sb.len, 0);
>  	strbuf_release(&sb);
> -	return ret;
> +}
> +
> +void print_stat_summary(FILE *fp, int files,
> +			int insertions, int deletions)
> +{
> +	struct diff_options o;
> +	memset(&o, 0, sizeof(o));
> +	o.file = fp;
> +
> +	print_stat_summary_inserts_deletes(&o, files, insertions, deletions);
>  }
>  
>  static void show_stats(struct diffstat_t *data, struct diff_options *options)
> @@ -1795,13 +1818,13 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>  	int total_files = data->nr, count;
>  	int width, name_width, graph_width, number_width = 0, bin_width = 0;
>  	const char *reset, *add_c, *del_c;
> -	const char *line_prefix = "";
>  	int extra_shown = 0;
> +	const char *line_prefix = diff_line_prefix(options);
> +	struct strbuf out = STRBUF_INIT;
>  
>  	if (data->nr == 0)
>  		return;
>  
> -	line_prefix = diff_line_prefix(options);
>  	count = options->stat_count ? options->stat_count : data->nr;
>  
>  	reset = diff_get_color_opt(options, DIFF_RESET);
> @@ -1955,26 +1978,32 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>  		}
>  
>  		if (file->is_binary) {
> -			fprintf(options->file, "%s", line_prefix);
> -			show_name(options->file, prefix, name, len);
> -			fprintf(options->file, " %*s", number_width, "Bin");
> +			strbuf_addf(&out, " %s%-*s |", prefix, len, name);
> +			strbuf_addf(&out, " %*s", number_width, "Bin");
>  			if (!added && !deleted) {
> -				putc('\n', options->file);
> +				strbuf_addch(&out, '\n');
> +				emit_diff_symbol(options, DIFF_SYMBOL_STATS_LINE,
> +						 out.buf, out.len, 0);
> +				strbuf_reset(&out);
>  				continue;
>  			}
> -			fprintf(options->file, " %s%"PRIuMAX"%s",
> +			strbuf_addf(&out, " %s%"PRIuMAX"%s",
>  				del_c, deleted, reset);
> -			fprintf(options->file, " -> ");
> -			fprintf(options->file, "%s%"PRIuMAX"%s",
> +			strbuf_addstr(&out, " -> ");
> +			strbuf_addf(&out, "%s%"PRIuMAX"%s",
>  				add_c, added, reset);
> -			fprintf(options->file, " bytes");
> -			fprintf(options->file, "\n");
> +			strbuf_addstr(&out, " bytes\n");
> +			emit_diff_symbol(options, DIFF_SYMBOL_STATS_LINE,
> +					 out.buf, out.len, 0);
> +			strbuf_reset(&out);
>  			continue;
>  		}
>  		else if (file->is_unmerged) {
> -			fprintf(options->file, "%s", line_prefix);
> -			show_name(options->file, prefix, name, len);
> -			fprintf(options->file, " Unmerged\n");
> +			strbuf_addf(&out, " %s%-*s |", prefix, len, name);
> +			strbuf_addstr(&out, " Unmerged\n");
> +			emit_diff_symbol(options, DIFF_SYMBOL_STATS_LINE,
> +					 out.buf, out.len, 0);
> +			strbuf_reset(&out);
>  			continue;
>  		}
>  
> @@ -1997,14 +2026,16 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>  				add = total - del;
>  			}
>  		}
> -		fprintf(options->file, "%s", line_prefix);
> -		show_name(options->file, prefix, name, len);
> -		fprintf(options->file, " %*"PRIuMAX"%s",
> +		strbuf_addf(&out, " %s%-*s |", prefix, len, name);
> +		strbuf_addf(&out, " %*"PRIuMAX"%s",
>  			number_width, added + deleted,
>  			added + deleted ? " " : "");
> -		show_graph(options->file, '+', add, add_c, reset);
> -		show_graph(options->file, '-', del, del_c, reset);
> -		fprintf(options->file, "\n");
> +		show_graph(&out, '+', add, add_c, reset);
> +		show_graph(&out, '-', del, del_c, reset);
> +		strbuf_addch(&out, '\n');
> +		emit_diff_symbol(options, DIFF_SYMBOL_STATS_LINE,
> +				 out.buf, out.len, 0);
> +		strbuf_reset(&out);
>  	}
>  
>  	for (i = 0; i < data->nr; i++) {
> @@ -2025,11 +2056,13 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>  		if (i < count)
>  			continue;
>  		if (!extra_shown)
> -			fprintf(options->file, "%s ...\n", line_prefix);
> +			emit_diff_symbol(options,
> +					 DIFF_SYMBOL_STATS_SUMMARY_ABBREV,
> +					 NULL, 0, 0);
>  		extra_shown = 1;
>  	}
> -	fprintf(options->file, "%s", line_prefix);
> -	print_stat_summary(options->file, total_files, adds, dels);
> +
> +	print_stat_summary_inserts_deletes(options, total_files, adds, dels);
>  }
>  
>  static void show_shortstats(struct diffstat_t *data, struct diff_options *options)
> @@ -2041,7 +2074,7 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option
>  
>  	for (i = 0; i < data->nr; i++) {
>  		int added = data->files[i]->added;
> -		int deleted= data->files[i]->deleted;
> +		int deleted = data->files[i]->deleted;
>  
>  		if (data->files[i]->is_unmerged ||
>  		    (!data->files[i]->is_interesting && (added + deleted == 0))) {
> @@ -2051,8 +2084,7 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option
>  			dels += deleted;
>  		}
>  	}
> -	fprintf(options->file, "%s", diff_line_prefix(options));
> -	print_stat_summary(options->file, total_files, adds, dels);
> +	print_stat_summary_inserts_deletes(options, total_files, adds, dels);
>  }
>  
>  static void show_numstat(struct diffstat_t *data, struct diff_options *options)
> diff --git a/diff.h b/diff.h
> index 2ee0ef3908..b73f7a1e75 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -398,8 +398,8 @@ extern int parse_rename_score(const char **cp_p);
>  
>  extern long parse_algorithm_value(const char *value);
>  
> -extern int print_stat_summary(FILE *fp, int files,
> -			      int insertions, int deletions);
> +extern void print_stat_summary(FILE *fp, int files,
> +			       int insertions, int deletions);
>  extern void setup_diff_pager(struct diff_options *);
>  
>  #endif /* DIFF_H */
> -- 
> 2.12.2.575.gb14f27f917
> 

-- 
Brandon Williams