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
Cc: "Michael J Gruber" <git@drmicha.warpmail.net>,
	pclouds@gmail.com,
	"Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>
Subject: [PATCH 2/3 v4] diff --stat: better alignment for binary files
Date: Wed, 15 Feb 2012 00:45:45 +0100	[thread overview]
Message-ID: <1329263146-19215-2-git-send-email-zbyszek@in.waw.pl> (raw)
In-Reply-To: <1329263146-19215-1-git-send-email-zbyszek@in.waw.pl>

The output for binary and unmerged files was not updated in
'diff --stat': use the full terminal width'. This fixes this
omission and modifies the graph width logic to include enough
space for "Bin XXX -> YYY bytes".

If changes to binary files are mixed with changes to text files,
change counts are padded to take at least three columns. And the other
way around, if change counts require more than three columns, then
"Bin"s is padded to align with the change count. This way, the +- part
starts in the same column as "XXX -> YYY" part for binary files. This
makes the graph easier to parse visually thanks to the empty column.
This mimics the old layout of diff --stat.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 diff.c                 | 39 ++++++++++++++++++++++++++++++++-------
 t/t4012-diff-binary.sh | 17 +++++++++++++++++
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/diff.c b/diff.c
index 4dc2b5c..0732623 100644
--- a/diff.c
+++ b/diff.c
@@ -1326,8 +1326,8 @@ 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, graph_width, number_width, count;
+	int total_files = data->nr, count;
+	int width, name_width, graph_width, number_width = 0, bin_width = 0;
 	const char *reset, *add_c, *del_c;
 	const char *line_prefix = "";
 	int extra_shown = 0;
@@ -1363,8 +1363,21 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		if (max_len < len)
 			max_len = len;
 
-		if (file->is_binary || file->is_unmerged)
+		if (file->is_unmerged) {
+			/* "Unmerged" is 8 characters */
+			bin_width = bin_width < 8 ? 8 : bin_width;
 			continue;
+		}
+		if (file->is_binary) {
+			/* "Bin XXX -> YYY bytes" */
+			int w = 14 + decimal_width(file->added)
+				+ decimal_width(file->deleted);
+			bin_width = bin_width < w ? w : bin_width;
+			/* Display change counts aligned with "Bin" */
+			number_width = 3;
+			continue;
+		}
+
 		if (max_change < change)
 			max_change = change;
 	}
@@ -1389,10 +1402,19 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	 * stat_name_width fixes the maximum width of the filename,
 	 * and is also used to divide available columns if there
 	 * aren't enough.
+	 *
+	 * Binary files are displayed with "Bin XXX -> YYY bytes"
+	 * instead of the change count and graph. This part is treated
+	 * similarly to the graph part, except that it is not
+	 * "scaled". If total width is too small to accomodate the
+	 * guaranteed minimum width of the filename part and the
+	 * separators and this message, this message will "overflow"
+	 * making the line longer than the maximum width.
 	 */
 
 	width = options->stat_width ? options->stat_width : term_columns();
-	number_width = decimal_width(max_change);
+	number_width = decimal_width(max_change) > number_width ?
+		decimal_width(max_change) : number_width;
 
 	/*
 	 * Guarantee 3/8*16==6 for the graph part
@@ -1403,8 +1425,11 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 
 	/*
 	 * First assign sizes that are wanted, ignoring available width.
+	 * (strlen("Bin XXX -> YYY bytes") is bin_width + 4)
 	 */
-	graph_width = max_change < 40 ? max_change : 40;
+	graph_width = max_change + 4 > bin_width ? max_change : bin_width - 4;
+	if (graph_width > 40)
+		graph_width = 40;
 	name_width = (options->stat_name_width > 0 &&
 		      options->stat_name_width < max_len) ?
 		options->stat_name_width : max_len;
@@ -1458,7 +1483,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		if (data->files[i]->is_binary) {
 			fprintf(options->file, "%s", line_prefix);
 			show_name(options->file, prefix, name, len);
-			fprintf(options->file, "  Bin ");
+			fprintf(options->file, " %*s ", number_width, "Bin");
 			fprintf(options->file, "%s%"PRIuMAX"%s",
 				del_c, deleted, reset);
 			fprintf(options->file, " -> ");
@@ -1471,7 +1496,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		else if (data->files[i]->is_unmerged) {
 			fprintf(options->file, "%s", line_prefix);
 			show_name(options->file, prefix, name, len);
-			fprintf(options->file, "  Unmerged\n");
+			fprintf(options->file, " Unmerged\n");
 			continue;
 		}
 
diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 2d9f9a0..ea0b376 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -90,4 +90,21 @@ test_expect_success 'diff --no-index with binary creation' '
 	test_cmp expected actual
 '
 
+cat >expect <<EOF
+ binfile  |   Bin 0 -> 1026 bytes
+ textfile | 10000 ++++++++++++++++++++++++++++++++++++++++
+EOF
+
+test_expect_success 'diff with binary files and big change count
+
+	Verify that "Bin" and the change count are properly aligned when
+	change count is big' '
+	echo X | dd of=binfile bs=1k seek=1 &&
+	git add binfile &&
+	seq 10000 > textfile &&
+	git add textfile &&
+	git diff --cached --stat binfile textfile | grep " | " > actual
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.9.6.ga1838.dirty

  reply	other threads:[~2012-02-14 23:46 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         ` [PATCH 3/3] diff --stat: use the real terminal width Zbigniew Jędrzejewski-Szmek
2012-02-10 18:24         ` [PATCH 0/3 v2] " 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                 ` Zbigniew Jędrzejewski-Szmek [this message]
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=1329263146-19215-2-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).