[PATCH v3] diff --stat: use the full terminal width

["Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>]

Use as many columns as necessary for filenames, as few columns as
necessary for change counts, and up to 40 columns for the histogram.

Some projects (especially in Java), have long filename paths, with
nested directories or long individual filenames. When files are
renamed, the stat output can be almost useless. If the middle part
between { and } is long (because the file was moved to a completely
different directory), then most of the path would be truncated.

It makes sense to detect and use the full terminal width and display
full filenames if possible.

If commits changing a lot of lines are displayed in a wide terminal
window (200 or more columns), and the +- graph would use the full
width, the output would look bad. Messages wrapped to about 80
columns would be interspersed with very long +- lines. It makes
sense to limit the width of the histogram to a fixed value, even if more
columns are available. This fixed value is subjectively hard-coded to
be 40 columns, which seems to work well for git.git and linux-2.6.git and
some other repositories.

If there isn't enough columns to print both the filename and the histogram,
at least 5/8 of available space is devoted to filenames. On a standard 80 column
terminal, or if not connected to a terminal and using the default of 80 columns,
this gives the same partition as before.

Number of columns required for change counts is computed based on
the maximum number of changed lines. This means that usually a few more
columns will be available for the filenames and the histogram.

Tests are added for various combinations of long filename and big change
count and ways to specify widths.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
Hi,
this is v3 (reduced to one patch) on top of:
- Save terminal width before setting up pager and export term_columns(),
- Rename lineno_width to decimal_width and export it.

The logic is described in a comment in show_stats() in diff.c. In case
of a column 80 terminal window, the output will not be identical,
because of using decimal_width, but it'll be very close to what was
there previously.

v3:
- use decimal_width(max_change) to calculate number of columns
  required for change counts
- rework the logic to divide columns
- document the logic in comments, update docs
- add more tests

v2:
- style fixes
- some tests for git-format-patch added
- patches 3 and 4 squashed together, since they touch the same lines
- graph width is limited to 40 columns, even if there's more space
- patch descriptions extended and cleared up

 Documentation/diff-options.txt | 14 +++---
 diff.c                         | 93 +++++++++++++++++++++++++------------
 t/t4014-format-patch.sh        | 98 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 168 insertions(+), 37 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9f7cba2..35dfdfb 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -53,13 +53,15 @@ endif::git-format-patch[]
 	Generate a diff using the "patience diff" algorithm.
 
 --stat[=<width>[,<name-width>[,<count>]]]::
-	Generate a diffstat.  You can override the default
-	output width for 80-column terminal by `--stat=<width>`.
-	The width of the filename part can be controlled by
-	giving another width to it separated by a comma.
+	Generate a diffstat. By default, as much space as necessary
+	will be used for the filename part, and up to 40 columns for
+	the graph histogram. Maximum width defaults to terminal width,
+	or 80 columns if not connected to a terminal, and can be
+	overriden by `<width>`. The width of the filename part can be
+	limited by giving another width `<name-width>` after a comma.
 	By giving a third parameter `<count>`, you can limit the
-	output to the first `<count>` lines, followed by
-	`...` if there are more.
+	output to the first `<count>` lines, followed by `...` if
+	there are more.
 +
 These parameters can also be set individually with `--stat-width=<width>`,
 `--stat-name-width=<name-width>` and `--stat-count=<count>`.
diff --git a/diff.c b/diff.c
index 7e15426..7abcbe9 100644
--- a/diff.c
+++ b/diff.c
@@ -1327,7 +1327,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	int i, len, add, del, adds = 0, dels = 0;
 	uintmax_t max_change = 0, max_len = 0;
 	int total_files = data->nr;
-	int width, name_width, count;
+	int width, name_width, graph_width, number_width, count;
 	const char *reset, *add_c, *del_c;
 	const char *line_prefix = "";
 	int extra_shown = 0;
@@ -1341,25 +1341,13 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		line_prefix = msg->buf;
 	}
 
-	width = options->stat_width ? options->stat_width : 80;
-	name_width = options->stat_name_width ? options->stat_name_width : 50;
-	count = options->stat_count ? options->stat_count : data->nr;
-
-	/* Sanity: give at least 5 columns to the graph,
-	 * but leave at least 10 columns for the name.
-	 */
-	if (width < 25)
-		width = 25;
-	if (name_width < 10)
-		name_width = 10;
-	else if (width < name_width + 15)
-		name_width = width - 15;
-
 	/* Find the longest filename and max number of changes */
 	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);
 
+	count = options->stat_count ? options->stat_count : data->nr;
+
 	for (i = 0; (i < count) && (i < data->nr); i++) {
 		struct diffstat_file *file = data->files[i];
 		uintmax_t change = file->added + file->deleted;
@@ -1380,19 +1368,63 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	}
 	count = i; /* min(count, data->nr) */
 
-	/* Compute the width of the graph part;
-	 * 10 is for one blank at the beginning of the line plus
-	 * " | count " between the name and the graph.
+	/* We have width = stat_width or term_columns() columns total.
+	 * We want a maximum of min(max_len, stat_name_width) for the name part.
+	 * We want a maximum of min(max_change, 40) for the +- part.
+	 * We also need 1 for " " and 4 + decimal_width(max_change)
+	 * for " | NNNN " and one for " " at the end, altogether
+	 * 6 + decimal_width(max_change).
+	 *
+	 * If there's not enough space, we will use stat_name_width
+	 * or 5/8*width for filename, and the rest for constant
+	 * elements + histogram, but no more than 40 for the histogram.
+	 * (5/8 gives 50 for filename and 30 for constant parts and
+	 * histogram for the standard terminal size).
 	 *
-	 * From here on, name_width is the width of the name area,
-	 * and width is the width of the graph area.
+	 * In other words: stat_width limits the maximum width, and
+	 * stat_name_width fixes the maximum width of the filename,
+	 * and is also used to divide available columns if there
+	 * aren't enough.
 	 */
-	name_width = (name_width < max_len) ? name_width : max_len;
-	if (width < (name_width + 10) + max_change)
-		width = width - (name_width + 10);
-	else
-		width = max_change;
+	width = options->stat_width ? options->stat_width : term_columns();
+	number_width = decimal_width(max_change);
+	/* first sizes that are wanted */
+	graph_width = max_change < 40 ? max_change : 40;
+	name_width = (options->stat_name_width > 0 &&
+		      options->stat_name_width < max_len) ?
+		options->stat_name_width : max_len;
+
+	/* sanity: guarantee a minimum and maximum width */
+	if (width < 25)
+		width = 25;
+
+	if (name_width + number_width + 6 + graph_width > width) {
+		if (graph_width > width * 3/8 - number_width - 6)
+			graph_width = width * 3/8 - number_width - 6;
+		if (graph_width > 40)
+			graph_width =  40;
+		if (name_width > width - number_width - 6 - graph_width)
+			name_width = width - number_width - 6 - graph_width;
+		else
+			graph_width = width - number_width - 6 - name_width;
+	}
 
+	/* More sanity: give at least 5 columns to the graph,
+	 * but leave at least 10 columns for the name.
+	 *
+	 * This should already be satisfied, unless max_change is
+	 * really huge. If the window is extemely narrow, this might
+	 * overflow available columns.
+	 */
+	if (name_width < 10 && max_len >= 10)
+		name_width = 10;
+	if (graph_width < 5 && max_change >= 5)
+		graph_width = 5;
+
+	/* From here name_width is the width of the name area,
+	 * and graph_width is the width of the graph area.
+	 * max_change is used to scale graph properly.
+	 */
 	for (i = 0; i < count; i++) {
 		const char *prefix = "";
 		char *name = data->files[i]->print_name;
@@ -1448,14 +1480,15 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		adds += add;
 		dels += del;
 
-		if (width <= max_change) {
-			add = scale_linear(add, width, max_change);
-			del = scale_linear(del, width, max_change);
+		if (graph_width <= max_change) {
+			add = scale_linear(add, graph_width, max_change);
+			del = scale_linear(del, graph_width, max_change);
 		}
 		fprintf(options->file, "%s", line_prefix);
 		show_name(options->file, prefix, name, len);
-		fprintf(options->file, "%5"PRIuMAX"%s", added + deleted,
-				added + deleted ? " " : "");
+		fprintf(options->file, " %*"PRIuMAX"%s",
+			number_width, added + deleted,
+			added + deleted ? " " : "");
 		show_graph(options->file, '+', add, add_c, reset);
 		show_graph(options->file, '-', del, del_c, reset);
 		fprintf(options->file, "\n");
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 6797512..91be989 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -519,7 +519,7 @@ test_expect_success 'shortlog of cover-letter wraps overly-long onelines' '
 
 cat > expect << EOF
 ---
- file |   16 ++++++++++++++++
+ file | 16 ++++++++++++++++
  1 files changed, 16 insertions(+), 0 deletions(-)
 
 diff --git a/file b/file
@@ -894,4 +894,100 @@ test_expect_success 'format patch ignores color.ui' '
 	test_cmp expect actual
 '
 
+name=aaaaaaaaaa
+name=$name$name$name$name$name$name$name$name$name$name$name$name
+test_expect_success 'preparation' "
+	> ${name} &&
+	git add ${name} &&
+	git commit -m message &&
+	echo a > ${name} &&
+	git commit -m message ${name}
+"
+
+cat >expect <<'EOF'
+ ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
+EOF
+test_expect_success 'format patch graph width is 80 columns' '
+	git format-patch --stat --stdout -1 |
+		grep -m 1 aaaaa > actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+ ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
+EOF
+test_expect_success 'format patch --stat=width with long name' '
+	git format-patch --stat=40 --stdout -1 |
+		grep -m 1 aaaa > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'format patch --stat-width=width works long name' '
+	git format-patch --stat-width=40 --stdout -1 |
+		grep -m 1 aaaa > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'format patch --stat=...,name-width with long name' '
+	git format-patch --stat=60,32 --stdout -1 |
+		grep -m 1 aaaa > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'format patch --stat-name-width with long name' '
+	git format-patch --stat-name-width=32 --stdout -1 |
+		grep -m 1 aaaa > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'preparation' '
+	> abcd &&
+	git add abcd &&
+	git commit -m message &&
+	seq 1000 > abcd &&
+	git commit -m message abcd
+'
+
+cat >expect <<'EOF'
+ abcd | 1000 ++++++++++++++++++++++++++++++++++++++++
+EOF
+test_expect_success 'format patch graph width is 40 columns' '
+	git format-patch --stat --stdout -1 |
+		grep -m 1 abcd > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'format patch ignores COLUMNS' '
+	COLUMNS=200 git format-patch --stat --stdout -1 |
+		grep -m 1 abcd > actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+ abcd | 1000 ++++++++++++++++++++++++++
+EOF
+test_expect_success 'format patch --stat=width with big change' '
+	git format-patch --stat=40 --stdout -1 |
+		grep -m 1 abcd > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'format patch --stat-width=width with big change' '
+	git format-patch --stat-width=40 --stdout -1 |
+		grep -m 1 abcd > actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+ ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++
+EOF
+test_expect_success 'format patch --stat=width with big change and long name' '
+	cp abcd aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
+	git add aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
+	git commit -m message &&
+	git format-patch --stat-width=60 --stdout -1 |
+		grep -m 1 aaaa > actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.9.3.g2429d.dirty