From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 11 Jan 2009 22:43:41 GMT
Subject: Re: [PATCH 1/4] Add color_fwrite(), a function coloring each line individually
Message-ID: <7vwsd1o44i.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.DEB.1.00.0901112058570.3586@pacific.mpi-cbg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> +/*
> + * This function splits the buffer by newlines and colors the lines individually.
> + */
> +void color_fwrite(FILE *f, const char *color, size_t count, const char *buf)

Is it just me that this is grossly misnamed?  It is not about fwrite of
count bytes starting at buf in the specified color.  At list it should be
called color_fwrite_lines() or something like that.

> diff --git a/color.h b/color.h
> index 6cf5c88..9fb58f5 100644
> --- a/color.h
> +++ b/color.h
> @@ -19,5 +19,6 @@ int git_config_colorbool(const char *var, const char *value, int stdout_is_tty);
>  void color_parse(const char *var, const char *value, char *dst);
>  int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
>  int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
> +void color_fwrite(FILE *f, const char *color, size_t count, const char *buf);

Also if other functions in the family all return int to indicate errors
and name the FILE * argument fp, I find it a very bad taste not to follow
their patterns without having a good reason (which I do not see).