git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/3 v5] diff --stat: use the full terminal width

From
Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date
Feb 15, 2012, 11:03 UTC
Message-ID
<1329303808-16989-2-git-send-email-zbyszek@in.waw.pl>
In-Reply-To
<1329303808-16989-1-git-send-email-zbyszek@in.waw.pl>

Use as many columns as necessary for the filenames and up to 40 columns for the graph.

Some projects (especially in Java), have long filename paths, with nested directories or long individual filenames. When files are renamed, the filename part in 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 graph part 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 graph, 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.

The --stat output in tests is not affected.
Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
This is the main patch. Presented with the two options (either
hardcode number_width=4 or remove fprintf(options->file, "
%*"PRIuMAX"%s", number_width, ...)), I've taken the middle way:
number_width=4 is hardcoded, but the fprintf is reverted to the previous
version. I think that this way the code is most readable, independently
if later changes.
Junio suggested to:
 - contain a change to the test suite somewhere, probably t/test-lib.sh,
   to set COLUMNS=80 and export it, to make sure that the existing test
   won't be broken when the number of columns learned from ioctl(1) is
   different from 80; and
 - add a new test that explicitly sets wider COLUMNS and makes sure you
   get a wider diffstat graph.

I haven't done this, because $COLUMNS and the actual terminal width is always ignored in tests. There's even a test to verify that COLUMNS=200 doesn't mess up git format-patch output.

A test to check that diff --stat responds to terminal size would be nice, but I don't know how to force git-diff to use the real terminal size in tests.

v5:
- tests are moved to an earlier patch
- using decimal_width(change count) is moved to a later patch
- "histogram" is really not used
v4:
- comments are updated and the word "histogram" is banished
- "mopping up" is removed (but the minimum width are guaranteed)
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                         | 88 +++++++++++++++++++++++++++------------
 2 files changed, 69 insertions(+), 33 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9f7cba2..36e4ee3 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 part. 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..be6d40b 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 = 4, count;
 	const char *reset, *add_c, *del_c;
 	const char *line_prefix = "";
 	int extra_shown = 0;
@@ -1341,25 +1341,15 @@ 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);
 
+	/*
+	 * Find the longest filename and max number of changes
+	 */
 	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 +1370,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 the empty column at the end, altogether
+	 * 6 + decimal_width(max_change).
+	 *
+	 * If there's not enough space, we will use the smaller of
+	 * stat_name_width (if set) and 5/8*width for the filename,
+	 * and the rest for constant elements + graph part, but no more
+	 * than 40 for the graph part.
+	 * (5/8 gives 50 for filename and 30 for the constant parts + graph
+	 * 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();
+
+	/*
+	 * Guarantee 3/8*16==6 for the graph part
+	 * and 5/8*16==10 for the filename part
+	 */
+	if (width < 16 + 6 + number_width)
+		width = 16 + 6 + number_width;
+
+	/*
+	 * First assign sizes that are wanted, ignoring available width.
+	 */
+	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;
+
+	/*
+	 * Adjust adjustable widths not to exceed maximum width
+	 */
+	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;
+	}
+
+	/*
+	 * 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,9 +1482,9 @@ 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);

-- 
1.7.9.5.g91d5
Previous: Zbigniew Jędrzejewski-SzmekNext: Junio C Hamano
Message 34 of 46 in “(unknown)”
  1. Zbigniew Jędrzejewski-SzmekFeb 9, 2012
  2. 1/4 Move git_version_string to help.c in preparation for diff changesZbigniew Jędrzejewski-Szmek, Feb 9, 2012
  3. Junio C HamanoFeb 10, 2012
  4. 2/4 help.c: make term_columns() cached and export itZbigniew Jędrzejewski-Szmek, Feb 9, 2012
  5. Junio C HamanoFeb 10, 2012
  6. 3/4 diff --stat: use the real terminal widthZbigniew Jędrzejewski-Szmek, Feb 9, 2012
  7. Junio C HamanoFeb 10, 2012
  8. Nguyen Thai Ngoc DuyFeb 10, 2012
  9. Zbigniew Jędrzejewski-SzmekFeb 10, 2012
  10. Nguyen Thai Ngoc DuyFeb 10, 2012
  11. 0/3 diff --stat: use the real terminal widthZbigniew Jędrzejewski-Szmek, Feb 10, 2012
  12. 1/3 Move git_version_string to help.c before diff changesZbigniew Jędrzejewski-Szmek, Feb 10, 2012
  13. Junio C HamanoFeb 10, 2012
  14. 2/3 help.c: make term_columns() cached and export itZbigniew Jędrzejewski-Szmek, Feb 10, 2012
  15. Nguyen Thai Ngoc DuyFeb 11, 2012
  16. Zbigniew Jędrzejewski-SzmekFeb 11, 2012
  17. Junio C HamanoFeb 12, 2012
  18. 1/2 Save terminal width before setting up pager and export term_columns()Zbigniew Jędrzejewski-Szmek, Feb 12, 2012
  19. Junio C HamanoFeb 13, 2012
  20. Nguyen Thai Ngoc DuyFeb 14, 2012
  21. Zbigniew Jędrzejewski-SzmekFeb 14, 2012
  22. 2/2 Rename lineno_width to decimal_width and export itZbigniew Jędrzejewski-Szmek, Feb 12, 2012
  23. Junio C HamanoFeb 13, 2012
  24. make lineno_width() from blame reusable for othersZbigniew Jędrzejewski-Szmek, Feb 14, 2012
  25. 3/3 diff --stat: use the real terminal widthZbigniew Jędrzejewski-Szmek, Feb 10, 2012
  26. Junio C HamanoFeb 10, 2012
  27. diff --stat: use the full terminal widthZbigniew Jędrzejewski-Szmek, Feb 12, 2012
  28. Junio C HamanoFeb 14, 2012
  29. 1/3 diff --stat: use the full terminal widthZbigniew Jędrzejewski-Szmek, Feb 14, 2012
  30. 2/3 diff --stat: better alignment for binary filesZbigniew Jędrzejewski-Szmek, Feb 14, 2012
  31. 3/3 Update diff --stat output in tests and tutorialZbigniew Jędrzejewski-Szmek, Feb 14, 2012
  32. Junio C HamanoFeb 15, 2012
  33. 1/3 diff --stat: tests for long filenames and big change countsZbigniew Jędrzejewski-Szmek, Feb 15, 2012
  34. 2/3 diff --stat: use the full terminal widthZbigniew Jędrzejewski-Szmek, Feb 15, 2012
  35. Junio C HamanoFeb 15, 2012
  36. 3/3 diff --stat: use less columns for change countsZbigniew Jędrzejewski-Szmek, Feb 15, 2012
  37. Nguyen Thai Ngoc DuyFeb 15, 2012
  38. Junio C HamanoFeb 15, 2012
  39. Junio C HamanoFeb 15, 2012
  40. Zbigniew Jędrzejewski-SzmekFeb 16, 2012
  41. Junio C HamanoFeb 16, 2012
  42. Junio C HamanoFeb 15, 2012
  43. Junio C HamanoFeb 15, 2012
  44. Johannes SixtFeb 15, 2012
  45. 4/4 diff --stat: use most of the space for file namesZbigniew Jędrzejewski-Szmek, Feb 9, 2012
  46. Junio C HamanoFeb 10, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.