From: Johannes Schindelin Date: Sun, 11 Jan 2009 23:49:03 GMT Subject: Re: [PATCH 1/4] Add color_fwrite(), a function coloring each line individually Message-ID: In-Reply-To: <7vwsd1o44i.fsf@gitster.siamese.dyndns.org> Hi, On Sun, 11 Jan 2009, Junio C Hamano wrote: > Johannes Schindelin 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). Valid points. Sorry, Dscho