commit c75e74214f40cfb13542a5331afaf32280f708fa
from: Mark Jamsek <mark@jamsek.dev>
via: Thomas Adam <thomas@xteddy.org>
date: Wed Sep 04 19:31:59 2024 UTC

tog: expand diffstat to all diff views

Suggested by naddy: display diffstat in diff views of arbitrary commits.
For the sake of consistency, show it in tree and blob diffs now too.
Adjust and grow regress to cover the change.

ok stsp@

commit - 282924c1e6cd7db9281df73757d81406513c0d42
commit + c75e74214f40cfb13542a5331afaf32280f708fa
blob - 9e9c1f1986ac507f2ccf5449d989cd59ff4bf43c
blob + 528451906a8ec3ab90d51ead482cb3f1dc70fa3e
--- regress/tog/diff.sh
+++ regress/tog/diff.sh
@@ -76,7 +76,7 @@ EOF
 
 test_diff_arbitrary_commits()
 {
-	test_init diff_arbitrary_commits 80 18
+	test_init diff_arbitrary_commits
 
 	local commit_id1=`git_show_head $testroot/repo`
 	local alpha_id_old=`get_blob_id $testroot/repo "" alpha`
@@ -99,7 +99,12 @@ SCREENDUMP
 EOF
 
 	cat <<EOF >$testroot/view.expected
-$(trim 80 "[1/16] diff $commit_id1 $head_id_truncated")
+$(trim 80 "[1/21] diff $commit_id1 $head_id_truncated")
+M  alpha  |  1+  1-
+A  new    |  1+  0-
+
+2 files changed, 2 insertions(+), 1 deletion(-)
+
 commit - $commit_id1
 commit + $head_id
 blob - $alpha_id_old
@@ -116,6 +121,7 @@ $(trim 80 "blob + $new_id (mode 644)")
 @@ -0,0 +1 @@
 +new file
 
+
 (END)
 EOF
 
@@ -404,7 +410,11 @@ test_diff_commit_keywords()
 	rhs_id=$(pop_idx 8 $ids)
 
 	cat <<-EOF >$testroot/view.expected
-	$(trim 120 "[1/10] diff $lhs_id $rhs_id")
+	$(trim 120 "[1/14] diff $lhs_id $rhs_id")
+	M  alpha  |  1+  1-
+
+	1 file changed, 1 insertion(+), 1 deletion(-)
+
 	commit - $lhs_id
 	commit + $rhs_id
 	blob - $(pop_idx 5 $alpha_ids)
@@ -423,10 +433,6 @@ test_diff_commit_keywords()
 
 
 
-
-
-
-
 	(END)
 	EOF
 
@@ -621,6 +627,11 @@ EOF
 	fi
 
 	cat <<EOF >$testroot/content.expected
+M  alpha  |  1+  1-
+M  beta   |  1+  1-
+
+2 files changed, 2 insertions(+), 2 deletions(-)
+
 commit - $id_root
 commit + $id_head
 blob - $alpha_root
@@ -639,7 +650,7 @@ blob + $beta_head
 +modified beta
 EOF
 
-	# test diff of arbitrary commits
+	# test diff of arbitrary commits displays diffstat in patch file
 	cd $testroot/repo && tog diff :head:-2 :head
 	patchpath=$(tail -1 $testroot/view | cut -d' ' -f 5)
 
@@ -667,7 +678,99 @@ EOF
 
 	test_done "$testroot" $ret
 }
+
+test_diff_blobs()
+{
+	test_init diff_blobs 148 14
 
+	local blob_alpha_root=$(get_blob_id $testroot/repo "" alpha)
+
+	echo "new alpha" > $testroot/repo/alpha
+	git_commit $testroot/repo -m "new alpha"
+
+	local blob_alpha_head=$(get_blob_id $testroot/repo "" alpha)
+
+	cat <<EOF >$TOG_TEST_SCRIPT
+SCREENDUMP
+EOF
+
+	cat <<EOF >$testroot/view.expected
+[1/12] diff $blob_alpha_root $blob_alpha_head
+M  $blob_alpha_head  |  1+  1-
+
+1 file changed, 1 insertion(+), 1 deletion(-)
+
+blob - $blob_alpha_root
+blob + $blob_alpha_head
+--- $blob_alpha_root
++++ $blob_alpha_head
+@@ -1 +1 @@
+-alpha
++new alpha
+
+(END)
+EOF
+
+	cd $testroot/repo && tog diff $blob_alpha_root $blob_alpha_head
+	cmp -s $testroot/view.expected $testroot/view
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/view.expected $testroot/view
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	test_done "$testroot" $ret
+}
+
+test_diff_trees()
+{
+	test_init diff_trees 148 16
+
+	local tree_gamma_root=$(get_blob_id $testroot/repo "" gamma/)
+	local blob_delta_root=$(get_blob_id $testroot/repo gamma delta)
+
+	echo "new delta" > $testroot/repo/gamma/delta
+	git_commit $testroot/repo -m "new delta"
+
+	local tree_gamma_head=$(get_blob_id $testroot/repo "" gamma/)
+	local blob_delta_head=$(get_blob_id $testroot/repo gamma delta)
+
+	cat <<EOF >$TOG_TEST_SCRIPT
+SCREENDUMP
+EOF
+
+	cat <<EOF >$testroot/view.expected
+[1/14] diff $tree_gamma_root $tree_gamma_head
+M  delta  |  1+  1-
+
+1 file changed, 1 insertion(+), 1 deletion(-)
+
+tree - $tree_gamma_root
+tree + $tree_gamma_head
+blob - $blob_delta_root
+blob + $blob_delta_head
+--- delta
++++ delta
+@@ -1 +1 @@
+-delta
++new delta
+
+(END)
+EOF
+
+	cd $testroot/repo && tog diff $tree_gamma_root $tree_gamma_head
+	cmp -s $testroot/view.expected $testroot/view
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/view.expected $testroot/view
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	test_done "$testroot" $ret
+}
+
 test_parseargs "$@"
 run_test test_diff_contiguous_commits
 run_test test_diff_arbitrary_commits
@@ -675,3 +778,5 @@ run_test test_diff_J_keymap
 run_test test_diff_commit_keywords
 run_test test_diff_horizontal_scroll
 run_test test_diff_p_keymap
+run_test test_diff_blobs
+run_test test_diff_trees
blob - 67bde569cd6803d9fc6f03e82a82db50051d61c3
blob + 0b8323f8cea5abb4777743d902c28ee9388c52f4
--- regress/tog/log.sh
+++ regress/tog/log.sh
@@ -728,7 +728,7 @@ test_log_search()
 
 test_log_mark_keymap()
 {
-	test_init log_mark_keymap 141 10
+	test_init log_mark_keymap 141 14
 
 	local repo="$testroot/repo"
 	local wt="$testroot/wt"
@@ -774,6 +774,10 @@ test_log_mark_keymap()
 	commit $id_root [2/2]
 	$ymd_head $prefix_head flan_hacker ~[master] new alpha
 	$ymd_root $prefix_root flan_hacker >adding the test tree
+
+
+
+
 
 
 
@@ -804,6 +808,10 @@ test_log_mark_keymap()
 	commit $id_root [2/2]
 	$ymd_head $prefix_head flan_hacker ~[master] new alpha
 	$ymd_root $prefix_root flan_hacker  adding the test tree
+
+
+
+
 
 
 
@@ -839,6 +847,10 @@ test_log_mark_keymap()
 
 
 
+
+
+
+
 	EOF
 
 	tog log
@@ -860,7 +872,11 @@ test_log_mark_keymap()
 	EOF
 
 	cat <<-EOF >$testroot/view.expected
-	[1/10] diff $id_head $id_root
+	[1/14] diff $id_head $id_root
+	M  alpha  |  1+  1-
+
+	1 file changed, 1 insertion(+), 1 deletion(-)
+
 	commit - $id_head
 	commit + $id_root
 	blob - $alpha_head
blob - aef5e53ec70381644eb56e603bd13c6f3e721e5e
blob + 25ca48971a9ae7a2b798a438e632875584b4f8a9
--- tog/tog.c
+++ tog/tog.c
@@ -5191,6 +5191,67 @@ cat_diff(FILE *dst, FILE *src, struct got_diff_line **
 	*d_nlines += s_nlines;
 
 	return NULL;
+}
+
+static const struct got_error *
+write_diffstat(FILE *outfile, struct got_diff_line **lines, size_t *nlines,
+    struct got_diffstat_cb_arg *dsa)
+{
+	const struct got_error		*err;
+	struct got_pathlist_entry	*pe;
+	off_t				 offset;
+	int				 n;
+
+	if (*nlines == 0) {
+		err = add_line_metadata(lines, nlines, 0, GOT_DIFF_LINE_NONE);
+		if (err != NULL)
+			return err;
+		offset = 0;
+	} else
+		offset = (*lines)[*nlines - 1].offset;
+
+	TAILQ_FOREACH(pe, dsa->paths, entry) {
+		struct got_diff_changed_path *cp = pe->data;
+		int pad = dsa->max_path_len - pe->path_len + 1;
+
+		n = fprintf(outfile, "%c  %s%*c | %*d+ %*d-\n", cp->status,
+		    pe->path, pad, ' ', dsa->add_cols + 1, cp->add,
+		    dsa->rm_cols + 1, cp->rm);
+		if (n < 0)
+			return got_error_from_errno("fprintf");
+
+		offset += n;
+		err = add_line_metadata(lines, nlines, offset,
+		    GOT_DIFF_LINE_CHANGES);
+		if (err != NULL)
+			return err;
+	}
+
+	if (fputc('\n', outfile) == EOF)
+		return got_error_from_errno("fputc");
+
+	offset++;
+	err = add_line_metadata(lines, nlines, offset, GOT_DIFF_LINE_NONE);
+	if (err != NULL)
+		return err;
+
+	n = fprintf(outfile,
+	    "%d file%s changed, %d insertion%s(+), %d deletion%s(-)\n",
+	    dsa->nfiles, dsa->nfiles > 1 ? "s" : "", dsa->ins,
+	    dsa->ins != 1 ? "s" : "", dsa->del, dsa->del != 1 ? "s" : "");
+	if (n < 0)
+		return got_error_from_errno("fprintf");
+
+	offset += n;
+	err = add_line_metadata(lines, nlines, offset, GOT_DIFF_LINE_NONE);
+	if (err != NULL)
+		return err;
+
+	if (fputc('\n', outfile) == EOF)
+		return got_error_from_errno("fputc");
+
+	offset++;
+	return add_line_metadata(lines, nlines, offset, GOT_DIFF_LINE_NONE);
 }
 
 static const struct got_error *
@@ -5206,7 +5267,6 @@ write_commit_info(struct got_diff_line **lines, size_t
 	time_t committer_time;
 	const char *author, *committer;
 	char *refs_str = NULL;
-	struct got_pathlist_entry *pe;
 	off_t outoff = 0;
 	int n;
 
@@ -5317,48 +5377,8 @@ write_commit_info(struct got_diff_line **lines, size_t
 		    GOT_DIFF_LINE_LOGMSG);
 		if (err)
 			goto done;
-	}
-
-	TAILQ_FOREACH(pe, dsa->paths, entry) {
-		struct got_diff_changed_path *cp = pe->data;
-		int pad = dsa->max_path_len - pe->path_len + 1;
-
-		n = fprintf(outfile, "%c  %s%*c | %*d+ %*d-\n", cp->status,
-		    pe->path, pad, ' ', dsa->add_cols + 1, cp->add,
-		    dsa->rm_cols + 1, cp->rm);
-		if (n < 0) {
-			err = got_error_from_errno("fprintf");
-			goto done;
-		}
-		outoff += n;
-		err = add_line_metadata(lines, nlines, outoff,
-		    GOT_DIFF_LINE_CHANGES);
-		if (err)
-			goto done;
-	}
-
-	fputc('\n', outfile);
-	outoff++;
-	err = add_line_metadata(lines, nlines, outoff, GOT_DIFF_LINE_NONE);
-	if (err)
-		goto done;
-
-	n = fprintf(outfile,
-	    "%d file%s changed, %d insertion%s(+), %d deletion%s(-)\n",
-	    dsa->nfiles, dsa->nfiles > 1 ? "s" : "", dsa->ins,
-	    dsa->ins != 1 ? "s" : "", dsa->del, dsa->del != 1 ? "s" : "");
-	if (n < 0) {
-		err = got_error_from_errno("fprintf");
-		goto done;
 	}
-	outoff += n;
-	err = add_line_metadata(lines, nlines, outoff, GOT_DIFF_LINE_NONE);
-	if (err)
-		goto done;
 
-	fputc('\n', outfile);
-	outoff++;
-	err = add_line_metadata(lines, nlines, outoff, GOT_DIFF_LINE_NONE);
 done:
 	free(id_str);
 	free(logmsg);
@@ -5376,8 +5396,15 @@ create_diff(struct tog_diff_view_state *s)
 	struct got_diff_line *lines = NULL;
 	struct got_pathlist_head changed_paths;
 	struct got_commit_object *commit2 = NULL;
+	struct got_diffstat_cb_arg dsa;
+	size_t nlines = 0;
 
 	TAILQ_INIT(&changed_paths);
+	memset(&dsa, 0, sizeof(dsa));
+	dsa.paths = &changed_paths;
+	dsa.diff_algo = tog_diff_algo;
+	dsa.force_text = s->force_text_diff;
+	dsa.ignore_ws = s->ignore_whitespace;
 
 	free(s->lines);
 	s->lines = malloc(sizeof(*s->lines));
@@ -5394,10 +5421,23 @@ create_diff(struct tog_diff_view_state *s)
 	if (s->f == NULL)
 		return got_error_from_errno("got_opentemp");
 
+	/*
+	 * The diffstat requires the diff to be built first, but we want the
+	 * diffstat to prepend the diff when displayed. Build the diff first
+	 * in the temporary file and write the diffstat and/or commit info to
+	 * the persistent file (s->f) from which views are drawn, then append
+	 * the diff from the temp file to the diffstat/commit info in s->f.
+	 */
 	tmp_diff_file = got_opentemp();
 	if (tmp_diff_file == NULL)
 		return got_error_from_errno("got_opentemp");
 
+	lines = malloc(sizeof(*lines));
+	if (lines == NULL) {
+		err = got_error_from_errno("malloc");
+		goto done;
+	}
+
 	if (s->id1)
 		err = got_object_get_type(&obj_type, s->repo, s->id1);
 	else
@@ -5407,44 +5447,33 @@ create_diff(struct tog_diff_view_state *s)
 
 	switch (obj_type) {
 	case GOT_OBJ_TYPE_BLOB:
-		err = got_diff_objects_as_blobs(&s->lines, &s->nlines,
+		err = got_diff_objects_as_blobs(&lines, &nlines,
 		    s->f1, s->f2, s->fd1, s->fd2, s->id1, s->id2,
 		    s->label1, s->label2, tog_diff_algo, s->diff_context,
-		    s->ignore_whitespace, s->force_text_diff, NULL, s->repo,
-		    s->f);
+		    s->ignore_whitespace, s->force_text_diff, &dsa, s->repo,
+		    tmp_diff_file);
+		if (err != NULL)
+			goto done;
 		break;
 	case GOT_OBJ_TYPE_TREE:
-		err = got_diff_objects_as_trees(&s->lines, &s->nlines,
+		err = got_diff_objects_as_trees(&lines, &nlines,
 		    s->f1, s->f2, s->fd1, s->fd2, s->id1, s->id2, NULL, "", "",
 		    tog_diff_algo, s->diff_context, s->ignore_whitespace,
-		    s->force_text_diff, NULL, s->repo, s->f);
+		    s->force_text_diff, &dsa, s->repo, tmp_diff_file);
+		if (err != NULL)
+			goto done;
 		break;
 	case GOT_OBJ_TYPE_COMMIT: {
 		const struct got_object_id_queue *parent_ids;
 		struct got_object_qid *pid;
 		struct got_reflist_head *refs;
-		size_t nlines = 0;
-		struct got_diffstat_cb_arg dsa = {
-			0, 0, 0, 0, 0, 0,
-			&changed_paths,
-			s->ignore_whitespace,
-			s->force_text_diff,
-			tog_diff_algo
-		};
 
-		lines = malloc(sizeof(*lines));
-		if (lines == NULL) {
-			err = got_error_from_errno("malloc");
-			goto done;
-		}
-
-		/* build diff first in tmp file then append to commit info */
 		err = got_diff_objects_as_commits(&lines, &nlines,
 		    s->f1, s->f2, s->fd1, s->fd2, s->id1, s->id2, NULL,
 		    tog_diff_algo, s->diff_context, s->ignore_whitespace,
 		    s->force_text_diff, &dsa, s->repo, tmp_diff_file);
 		if (err)
-			break;
+			goto done;
 
 		refs = got_reflist_object_id_map_lookup(tog_refs_idmap, s->id2);
 		/* Show commit info if we're diffing to a parent/root commit. */
@@ -5473,15 +5502,20 @@ create_diff(struct tog_diff_view_state *s)
 				}
 			}
 		}
-
-		err = cat_diff(s->f, tmp_diff_file, &s->lines, &s->nlines,
-		    lines, nlines);
 		break;
 	}
 	default:
 		err = got_error(GOT_ERR_OBJ_TYPE);
-		break;
+		goto done;
 	}
+
+	err = write_diffstat(s->f, &s->lines, &s->nlines, &dsa);
+	if (err != NULL)
+		goto done;
+
+	err = cat_diff(s->f, tmp_diff_file, &s->lines, &s->nlines,
+	    lines, nlines);
+
 done:
 	free(lines);
 	if (commit2 != NULL)