Re: [PATCH 1/4] Add color_fwrite(), a function coloring each line individually

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sun, 11 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux