git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>
To: git@vger.kernel.org, gitster@pobox.com, pclouds@gmail.com
Cc: "Michael J Gruber" <git@drmicha.warpmail.net>,
	"Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>
Subject: [PATCH 3/3] diff --stat: use the real terminal width
Date: Fri, 10 Feb 2012 17:39:32 +0100	[thread overview]
Message-ID: <1328891972-23695-4-git-send-email-zbyszek@in.waw.pl> (raw)
In-Reply-To: <1328891972-23695-1-git-send-email-zbyszek@in.waw.pl>

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 use the full terminal width.

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

Three tests are added: the first one check that --stat output for a
long file name is the same as previously (the test should pass
independently of this patch). The last two check that --stat output
with a massive change uses only 40 columns for +- in graph output.
Those two tests are dependent on this patch.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 diff.c                  |   26 ++++++++++++++------------
 t/t4014-format-patch.sh |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/diff.c b/diff.c
index 84780fd..8176d74 100644
--- a/diff.c
+++ b/diff.c
@@ -7,6 +7,7 @@
 #include "diffcore.h"
 #include "delta.h"
 #include "xdiff-interface.h"
+#include "help.h"
 #include "color.h"
 #include "attr.h"
 #include "run-command.h"
@@ -1376,7 +1377,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, count;
 	const char *reset, *add_c, *del_c;
 	const char *line_prefix = "";
 	int extra_shown = 0;
@@ -1390,8 +1391,9 @@ 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;
+	width = options->stat_width ? options->stat_width : term_columns();
+	name_width = options->stat_name_width ? options->stat_name_width
+		: width * 5/8; /* this gives 50 if not on tty */
 	count = options->stat_count ? options->stat_count : data->nr;
 
 	/* Sanity: give at least 5 columns to the graph,
@@ -1433,14 +1435,14 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	 * 10 is for one blank at the beginning of the line plus
 	 * " | count " between the name and the graph.
 	 *
-	 * From here on, name_width is the width of the name area,
-	 * and width is the width of the graph area.
+	 * 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.
 	 */
 	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;
+	graph_width = max_change < 40 ? max_change : 40;
+	if (width < (name_width + 10) + graph_width)
+		graph_width = width - (name_width + 10);
 
 	for (i = 0; i < count; i++) {
 		const char *prefix = "";
@@ -1497,9 +1499,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);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 7dfe716..bd3002b 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -894,4 +894,44 @@ test_expect_success 'format patch ignores color.ui' '
 	test_cmp expect actual
 '
 
+cat >expect <<'EOF'
+ ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa |	 1 +
+EOF
+name=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+test_expect_success 'format patch filename width is limited' "
+	> ${name} &&
+	git add ${name} &&
+	git commit -m message &&
+	echo a > ${name} &&
+	git commit -m message ${name} &&
+	git format-patch --stat --stdout -1 |
+		grep -m 1 aaaaa > actual &&
+	test_cmp expect actual
+"
+
+cat >expect <<'EOF'
+ abcd | 1000 ++++++++++++++++++++++++++++++++++++++++
+EOF
+test_expect_success 'format patch graph width is 40 columns' '
+	> abcd &&
+	git add abcd &&
+	git commit -m message &&
+	seq 1000 > abcd &&
+	git commit -m message abcd &&
+	git format-patch --stat --stdout -1 |
+		grep -m 1 abcd > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'format patch ignores COLUMNS' '
+	> abcd &&
+	git add abcd &&
+	git commit -m message &&
+	seq 1000 > abcd &&
+	git commit -m message abcd &&
+	COLUMNS=200 git format-patch --stat --stdout -1 |
+		grep -m 1 abcd > actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.9.263.g4be11.dirty

  parent reply	other threads:[~2012-02-10 16:40 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-09 23:58 (unknown), Zbigniew Jędrzejewski-Szmek
2012-02-09 23:58 ` [PATCH 1/4] Move git_version_string to help.c in preparation for diff changes Zbigniew Jędrzejewski-Szmek
2012-02-10  0:46   ` Junio C Hamano
2012-02-09 23:58 ` [PATCH 2/4] help.c: make term_columns() cached and export it Zbigniew Jędrzejewski-Szmek
2012-02-10  0:50   ` Junio C Hamano
2012-02-09 23:58 ` [PATCH 3/4] diff --stat: use the real terminal width Zbigniew Jędrzejewski-Szmek
2012-02-10  0:54   ` Junio C Hamano
2012-02-10  6:15   ` Nguyen Thai Ngoc Duy
2012-02-10 11:25     ` Zbigniew Jędrzejewski-Szmek
2012-02-10 13:00       ` Nguyen Thai Ngoc Duy
2012-02-10 16:39       ` [PATCH 0/3 v2] " Zbigniew Jędrzejewski-Szmek
2012-02-10 16:39         ` [PATCH 1/3] Move git_version_string to help.c before diff changes Zbigniew Jędrzejewski-Szmek
2012-02-10 17:58           ` Junio C Hamano
2012-02-10 16:39         ` [PATCH 2/3] help.c: make term_columns() cached and export it Zbigniew Jędrzejewski-Szmek
2012-02-11  4:36           ` Nguyen Thai Ngoc Duy
2012-02-11 10:49             ` Zbigniew Jędrzejewski-Szmek
2012-02-12  9:40               ` Junio C Hamano
2012-02-12 14:12                 ` [PATCH 1/2] Save terminal width before setting up pager and export term_columns() Zbigniew Jędrzejewski-Szmek
2012-02-13 23:00                   ` Junio C Hamano
2012-02-14 11:44                   ` Nguyen Thai Ngoc Duy
2012-02-14 11:53                     ` Zbigniew Jędrzejewski-Szmek
2012-02-12 14:16                 ` [PATCH 2/2] Rename lineno_width to decimal_width and export it Zbigniew Jędrzejewski-Szmek
2012-02-13 23:29                   ` Junio C Hamano
2012-02-14 12:24                     ` [PATCH v2] make lineno_width() from blame reusable for others Zbigniew Jędrzejewski-Szmek
2012-02-10 16:39         ` Zbigniew Jędrzejewski-Szmek [this message]
2012-02-10 18:24         ` [PATCH 0/3 v2] diff --stat: use the real terminal width Junio C Hamano
2012-02-12 14:30           ` [PATCH v3] diff --stat: use the full " Zbigniew Jędrzejewski-Szmek
2012-02-14  1:08             ` Junio C Hamano
2012-02-14 23:45               ` [PATCH 1/3 v4] " Zbigniew Jędrzejewski-Szmek
2012-02-14 23:45                 ` [PATCH 2/3 v4] diff --stat: better alignment for binary files Zbigniew Jędrzejewski-Szmek
2012-02-14 23:45                 ` [PATCH 3/3 v4] Update diff --stat output in tests and tutorial Zbigniew Jędrzejewski-Szmek
2012-02-15  1:21                   ` Junio C Hamano
2012-02-15 11:03                     ` [PATCH 1/3 v5] diff --stat: tests for long filenames and big change counts Zbigniew Jędrzejewski-Szmek
2012-02-15 11:03                       ` [PATCH 2/3 v5] diff --stat: use the full terminal width Zbigniew Jędrzejewski-Szmek
2012-02-15 18:07                         ` Junio C Hamano
2012-02-15 11:03                       ` [PATCH 3/3 v5] diff --stat: use less columns for change counts Zbigniew Jędrzejewski-Szmek
2012-02-15 12:12                         ` Nguyen Thai Ngoc Duy
2012-02-15 17:12                       ` [PATCH 1/3 v5] diff --stat: tests for long filenames and big " Junio C Hamano
2012-02-15 17:33                         ` Junio C Hamano
2012-02-16  9:57                         ` Zbigniew Jędrzejewski-Szmek
2012-02-16 20:01                           ` Junio C Hamano
2012-02-15  0:07                 ` [PATCH 1/3 v4] diff --stat: use the full terminal width Junio C Hamano
2012-02-15  1:18                 ` Junio C Hamano
2012-02-15  7:39                 ` Johannes Sixt
2012-02-09 23:58 ` [PATCH 4/4] diff --stat: use most of the space for file names Zbigniew Jędrzejewski-Szmek
2012-02-10  0:55   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1328891972-23695-4-git-send-email-zbyszek@in.waw.pl \
    --to=zbyszek@in.waw.pl \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).