Download raw body.
tog diff lacks diffstat
Mark Jamsek <mark@jamsek.com> wrote:
> Christian Weisgerber <naddy@mips.inka.de> wrote:
> > I don't use "tog diff" in practice, so it's only with the new mark
> > feature that I have noticed that there is no diffstat if the diff
> > doesn't correspond to a single commit. Mark, can you add this?
>
> Sure! I'll do it tonight. Thanks for pointing it out :)
The below diff adds the diffstat to diff views of arbitrary commits, not
just parent:child commits. I figured we should also show it in diffs of
blobs and trees too, so have added that; regress has been adapted and
expanded to cover the change and is passing in sha1 and sha256 mode.
The diff is rebased on main but is on top of the tog diff 'p' keymap
commit in my tree thought it should still apply without it.
Thanks again for highlighting its absence, naddy!
M regress/tog/diff.sh | 107+ 7-
M regress/tog/log.sh | 18+ 2-
M tog/tog.c | 106+ 65-
3 files changed, 231 insertions(+), 74 deletions(-)
commit - 1420a6d42d9ffe5e2fcf77760d2068bfaf9c8757
commit + eba95b8a4f303376c3833adad398feb8e300854a
blob - c31964ba473354a0a9622d6c1724f4705d6d3dcd
blob + d8d0d214f46d115de70295102f416c8efa700d8d
--- 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
@@ -546,9 +552,103 @@ 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
run_test test_diff_J_keymap
run_test test_diff_commit_keywords
run_test test_diff_horizontal_scroll
+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"
@@ -781,6 +781,10 @@ test_log_mark_keymap()
+
+
+
+
EOF
tog log
@@ -811,6 +815,10 @@ test_log_mark_keymap()
+
+
+
+
EOF
tog log
@@ -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 - 13586fd308da97f395a46278d336699dc1aa3eac
blob + 3c2a7c4f7c4a6865f154ca10ac9c0ba1e7775be1
--- tog/tog.c
+++ tog/tog.c
@@ -5190,6 +5190,67 @@ cat_diff(FILE *dst, FILE *src, struct got_diff_line **
}
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 *
write_commit_info(struct got_diff_line **lines, size_t *nlines,
struct got_object_id *commit_id, struct got_reflist_head *refs,
struct got_repository *repo, int ignore_ws, int force_text_diff,
@@ -5202,7 +5263,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;
@@ -5315,46 +5375,8 @@ write_commit_info(struct got_diff_line **lines, size_t
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;
+ err = write_diffstat(outfile, lines, nlines, dsa);
- 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);
@@ -5377,8 +5399,16 @@ 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;
+ int do_diffstat = 1;
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));
@@ -5395,10 +5425,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
@@ -5408,44 +5451,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. */
@@ -5455,6 +5487,7 @@ create_diff(struct tog_diff_view_state *s)
s->force_text_diff, &dsa, s->f);
if (err)
goto done;
+ do_diffstat = 0;
} else {
err = got_object_open_as_commit(&commit2, s->repo,
s->id2);
@@ -5470,19 +5503,27 @@ create_diff(struct tog_diff_view_state *s)
s->force_text_diff, &dsa, s->f);
if (err)
goto done;
+ do_diffstat = 0;
break;
}
}
}
-
- 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;
}
+
+ if (do_diffstat) {
+ 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)
--
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
tog diff lacks diffstat