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

[PATCH 4/5] diff --stat: move the "total count" logic to the last loop

From
Junio C Hamano <gitster@pobox.com>
Date
Nov 27, 2012, 21:21 UTC
Message-ID
<1354051310-29093-5-git-send-email-gitster@pobox.com>
In-Reply-To
<1354051310-29093-1-git-send-email-gitster@pobox.com>

The diffstat generation logic, with --stat-count limit, is implemented as three loops.

 - The first counts the width necessary to show stats up to
   specified number of entries, and notes up to how many entries in
   the data we need to iterate to show the graph;
 - The second iterates that many times to draw the graph, adjusts
   the number of "total modified files", and counts the total
   added/deleted lines for the part that was shown in the graph;
 - The third iterates over the remainder and only does the part to
   count "total added/deleted lines" and to adjust "total modified
   files" without drawing anything.

Move the logic to count added/deleted lines and modified files from the second loop to the third loop.

This incidentally fixes a bug. The third loop was not filtering binary changes (counted in bytes) from the total added/deleted as it should. The second loop implemented this correctly, so if a binary change appeared earlier than the --stat-count cutoff, the code counted number of added/deleted lines correctly, but if it appeared beyond the cutoff, the number of lines would have mixed with the byte count in the buggy third loop.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c                     | 21 ++++++++++++---------
 t/t4049-diff-stat-count.sh |  2 +-
 2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/diff.c b/diff.c
index e4e70e5..4105260 100644
--- a/diff.c
+++ b/diff.c
@@ -1498,7 +1498,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		if (max_change < change)
 			max_change = change;
 	}
-	count = i; /* min(count, data->nr) */
+	count = i; /* where we can stop scanning in data->files[] */
 
 	/*
 	 * We have width = stat_width or term_columns() columns total.
@@ -1592,10 +1592,9 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		uintmax_t deleted = file->deleted;
 		int name_len;
 
-		if (!file->is_interesting && (added + deleted == 0)) {
-			total_files--;
+		if (!file->is_interesting && (added + deleted == 0))
 			continue;
-		}
+
 		/*
 		 * "scale" the filename
 		 */
@@ -1640,8 +1639,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		 */
 		add = added;
 		del = deleted;
-		adds += add;
-		dels += del;
 
 		if (graph_width <= max_change) {
 			int total = add + del;
@@ -1667,7 +1664,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		show_graph(options->file, '-', del, del_c, reset);
 		fprintf(options->file, "\n");
 	}
-	for (i = count; i < data->nr; i++) {
+
+	for (i = 0; i < data->nr; i++) {
 		struct diffstat_file *file = data->files[i];
 		uintmax_t added = file->added;
 		uintmax_t deleted = file->deleted;
@@ -1675,8 +1673,13 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 			total_files--;
 			continue;
 		}
-		adds += added;
-		dels += deleted;
+
+		if (!file->is_binary && !file->is_unmerged) {
+			adds += added;
+			dels += deleted;
+		}
+		if (i < count)
+			continue;
 		if (!extra_shown)
 			fprintf(options->file, "%s ...\n", line_prefix);
 		extra_shown = 1;
diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
index e212b11..70ee073 100755
--- a/t/t4049-diff-stat-count.sh
+++ b/t/t4049-diff-stat-count.sh
@@ -28,7 +28,7 @@ test_expect_success 'limit output to 2 (simple)' '
 	test_i18ncmp expect actual
 '
 
-test_expect_failure 'binary changes do not count in lines' '
+test_expect_success 'binary changes do not count in lines' '
 	git reset --hard &&
 	chmod +x c d &&
 	echo a >a &&
-- 
1.8.0.1.331.g808d2af
Previous: Junio C HamanoNext: Junio C Hamano
Message 5 of 15 in “"diff --stat" counting fixes”
  1. 0/5 "diff --stat" counting fixesJunio C Hamano, Nov 27, 2012
  2. 1/5 test: add failing tests for "diff --stat" to t4049Junio C Hamano, Nov 27, 2012
  3. 2/5 diff --stat: status of unmodified pair in diff-q is not zeroJunio C Hamano, Nov 27, 2012
  4. 3/5 diff --stat: use "file" temporary variable to refer to data->files[i]Junio C Hamano, Nov 27, 2012
  5. 4/5 diff --stat: move the "total count" logic to the last loopJunio C Hamano, Nov 27, 2012
  6. 5/5 diff --stat: do not count "unmerged" entriesJunio C Hamano, Nov 27, 2012
  7. 6/5 diff --shortstat: do not count "unmerged" entriesJunio C Hamano, Nov 27, 2012
  8. t4049: avoid test failures on filemode challenged file systems (Windows)Johannes Sixt, Nov 29, 2012
  9. Junio C HamanoNov 29, 2012
  10. Junio C HamanoNov 29, 2012
  11. Junio C HamanoNov 29, 2012
  12. Johannes SixtNov 30, 2012
  13. Junio C HamanoDec 2, 2012
  14. Antoine PelisseDec 1, 2012
  15. Junio C HamanoDec 2, 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.