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

Johannes Schindelin [off-list ref] 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.

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).

Hi,

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

Johannes Schindelin [off-list ref] writes:

quoted

+/*
+ * 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.

quoted

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

We have to set the color before every line and reset it before every
newline.  Add a function color_fwrite() which does that for us.

Signed-off-by: Johannes Schindelin <redacted>
---
 color.c |   28 ++++++++++++++++++++++++++++
 color.h |    1 +
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/color.c b/color.c
index fc0b72a..b028880 100644
--- a/color.c
+++ b/color.c

@@ -191,3 +191,31 @@ int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...)
 	va_end(args);
 	return r;
 }
+
+/*
+ * This function splits the buffer by newlines and colors the lines individually.
+ *
+ * Returns 0 on success.
+ */
+int color_fwrite_lines(FILE *fp, const char *color,
+		size_t count, const char *buf)
+{
+	if (!*color)
+		return fwrite(buf, count, 1, fp) != 1;
+	while (count) {
+		char *p = memchr(buf, '\n', count);
+		if (fputs(color, fp) < 0 ||
+				fwrite(buf, p ? p - buf : count, 1, fp) != 1 ||
+				fputs(COLOR_RESET, fp) < 0)
+			return -1;
+		if (!p)
+			return 0;
+		if (fputc('\n', fp) < 0)
+			return -1;
+		count -= p + 1 - buf;
+		buf = p + 1;
+	}
+	return 0;
+}
+
+

diff --git a/color.h b/color.h
index 6cf5c88..cd5c985 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, ...);
+int color_fwrite_lines(FILE *fp, const char *color, size_t count, const char *buf);
 
 #endif /* COLOR_H */

-- 
1.6.1.223.g50c8f

Johannes Schindelin wrote:

We have to set the color before every line and reset it before every
newline.  Add a function color_fwrite() which does that for us.

color_fwrite_lines(), but I guess Junio can correct this himself.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git