text in --stat is colored like color.diff.plain

4 messages, 2 authors, 2016-06-15 · open the first message on its own page

text in --stat is colored like color.diff.plain

From: Markus Heidelberg <hidden>
Date: 2016-06-15 22:46:38

I noticed, that the filenames and the text "X files changed, Y
insertions(+), Z deletions(-)" in the --stat output ended with an ESC[m,
although they were not colored. Further investigation showed, that
these parts of the output indeed can use colors from color.diff.plain.

Is this intention? Commit 785f743 (diff --stat: color output.,
2006-09-26) introduced it, but only mentioned colored output of the
+++++++---- diffstat graph in the commit message. color.diff.plain is
documented for coloring context text and I don't think it looks good
either as it is now, nor is this really "context text".
Note: --shortstat and --summary don't use colors like --stat. Or is this
really intention and I don't see the reason for it?

If not, should we remove the colors for --stat?
After that maybe we could do some more color.diff.old/new coloring for
"Y insertions(+)", "Z deletions(-)", "create mode ...", "delete mode ..."

Markus

[PATCH 1/2] diff: do not color --stat output like patch context

From: Markus Heidelberg <hidden>
Date: 2016-06-15 22:46:39

The diffstat used the color.diff.plain slot (context text) for coloring
filenames and the whole summary line. This didn't look nice and the
affected text isn't patch context at all.

Signed-off-by: Markus Heidelberg <redacted>
---

    Maybe it's easier to begin with a patch :)

 diff.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/diff.c b/diff.c
index 3ac7168..d581d4d 100644
--- a/diff.c
+++ b/diff.c

@@ -839,10 +839,9 @@ static int scale_linear(int it, int width, int max_change)
 }
 
 static void show_name(FILE *file,
-		      const char *prefix, const char *name, int len,
-		      const char *reset, const char *set)
+		      const char *prefix, const char *name, int len)
 {
-	fprintf(file, " %s%s%-*s%s |", set, prefix, len, name, reset);
+	fprintf(file, " %s%-*s |", prefix, len, name);
 }
 
 static void show_graph(FILE *file, char ch, int cnt, const char *set, const char *reset)

@@ -956,7 +955,7 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
 		}
 
 		if (data->files[i]->is_binary) {
-			show_name(options->file, prefix, name, len, reset, set);
+			show_name(options->file, prefix, name, len);
 			fprintf(options->file, "  Bin ");
 			fprintf(options->file, "%s%d%s", del_c, deleted, reset);
 			fprintf(options->file, " -> ");

@@ -966,7 +965,7 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
 			continue;
 		}
 		else if (data->files[i]->is_unmerged) {
-			show_name(options->file, prefix, name, len, reset, set);
+			show_name(options->file, prefix, name, len);
 			fprintf(options->file, "  Unmerged\n");
 			continue;
 		}

@@ -988,7 +987,7 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
 			add = scale_linear(add, width, max_change);
 			del = scale_linear(del, width, max_change);
 		}
-		show_name(options->file, prefix, name, len, reset, set);
+		show_name(options->file, prefix, name, len);
 		fprintf(options->file, "%5d%s", added + deleted,
 				added + deleted ? " " : "");
 		show_graph(options->file, '+', add, add_c, reset);

@@ -996,8 +995,8 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
 		fprintf(options->file, "\n");
 	}
 	fprintf(options->file,
-	       "%s %d files changed, %d insertions(+), %d deletions(-)%s\n",
-	       set, total_files, adds, dels, reset);
+	       " %d files changed, %d insertions(+), %d deletions(-)\n",
+	       total_files, adds, dels);
 }
 
 static void show_shortstats(struct diffstat_t* data, struct diff_options *options)

-- 
1.6.3.rc1.84.g1036b

[PATCH 2/2] diff: color statistics (stat, shortstat, numstat)

From: Markus Heidelberg <hidden>
Date: 2016-06-15 22:46:39

stat/shortstat:
Color added and removed lines and the corresponding signs ('+' and '-')
in the summary.

numstat:
Color added and removed lines per file.

Signed-off-by: Markus Heidelberg <redacted>
---

    I didn't consider --summary and --name-status. Also in --stat it
    would be possible to color filenames in renames and copies and the
    mode in mode changes. Not sure if it would be nice or distracting.
    At least it wouldn't look so consistent, I think.

 diff.c |   26 ++++++++++++++++++++------
 1 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/diff.c b/diff.c
index d581d4d..bc8377d 100644
--- a/diff.c
+++ b/diff.c

@@ -994,18 +994,24 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
 		show_graph(options->file, '-', del, del_c, reset);
 		fprintf(options->file, "\n");
 	}
-	fprintf(options->file,
-	       " %d files changed, %d insertions(+), %d deletions(-)\n",
-	       total_files, adds, dels);
+	fprintf(options->file, " %d files changed, %s%d%s insertions(%s+%s), "
+						  "%s%d%s deletions(%s-%s)\n",
+				total_files, add_c, adds, reset, add_c, reset,
+					     del_c, dels, reset, del_c, reset);
 }
 
 static void show_shortstats(struct diffstat_t* data, struct diff_options *options)
 {
 	int i, adds = 0, dels = 0, total_files = data->nr;
+	const char *reset, *add_c, *del_c;
 
 	if (data->nr == 0)
 		return;
 
+	reset = diff_get_color_opt(options, DIFF_RESET);
+	add_c = diff_get_color_opt(options, DIFF_FILE_NEW);
+	del_c = diff_get_color_opt(options, DIFF_FILE_OLD);
+
 	for (i = 0; i < data->nr; i++) {
 		if (!data->files[i]->is_binary &&
 		    !data->files[i]->is_unmerged) {

@@ -1020,17 +1026,24 @@ static void show_shortstats(struct diffstat_t* data, struct diff_options *option
 			}
 		}
 	}
-	fprintf(options->file, " %d files changed, %d insertions(+), %d deletions(-)\n",
-	       total_files, adds, dels);
+	fprintf(options->file, " %d files changed, %s%d%s insertions(%s+%s), "
+						  "%s%d%s deletions(%s-%s)\n",
+				total_files, add_c, adds, reset, add_c, reset,
+					     del_c, dels, reset, del_c, reset);
 }
 
 static void show_numstat(struct diffstat_t* data, struct diff_options *options)
 {
 	int i;
+	const char *reset, *add_c, *del_c;
 
 	if (data->nr == 0)
 		return;
 
+	reset = diff_get_color_opt(options, DIFF_RESET);
+	add_c = diff_get_color_opt(options, DIFF_FILE_NEW);
+	del_c = diff_get_color_opt(options, DIFF_FILE_OLD);
+
 	for (i = 0; i < data->nr; i++) {
 		struct diffstat_file *file = data->files[i];

@@ -1038,7 +1051,8 @@ static void show_numstat(struct diffstat_t* data, struct diff_options *options)
 			fprintf(options->file, "-\t-\t");
 		else
 			fprintf(options->file,
-				"%d\t%d\t", file->added, file->deleted);
+				"%s%d%s\t%s%d%s\t", add_c, file->added, reset,
+						    del_c, file->deleted, reset);
 		if (options->line_termination) {
 			fill_print_name(file);
 			if (!file->is_renamed)

-- 
1.6.3.rc1.84.g1036b

Re: [PATCH 2/2] diff: color statistics (stat, shortstat, numstat)

From: Jeff King <hidden>
Date: 2016-06-15 22:46:39

On Sat, Apr 25, 2009 at 12:06:48AM +0200, Markus Heidelberg wrote:

numstat:
Color added and removed lines per file.

Somehow I thought numstat was about being easy to parse for plumbing.
Are people really using it for human-readable output?

-Peff

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help