Download raw body.
tog diff lacks diffstat
Mark Jamsek <mark@jamsek.com> wrote:
> 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.
Updated/actual diff.
Previous diff sent was the wrong commit: after adding the diffstat to
blobs and trees, we don't need the do_diffstat flag as the diffstat is
now shown in all diff types so written unconditionally.
M regress/tog/diff.sh | 107+ 7-
M regress/tog/log.sh | 18+ 2-
M tog/tog.c | 100+ 66-
3 files changed, 225 insertions(+), 75 deletions(-)
commit - a0eec9526dea03232daf4c7fc4e84ea24a06d786
commit + 7ea98496b009d71032480f2f134304929b9e7f6d
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 - 0f0c5142c308899d47b16880bf3b26d868b72c92
blob + e15f571c7eea4eb3da1738e1d2233258b41c72db
--- 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,6 @@ 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;
-
- 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);
@@ -5372,8 +5392,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));
@@ -5390,10 +5417,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
@@ -5403,44 +5443,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. */
@@ -5469,15 +5498,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)
--
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
tog diff lacks diffstat