From: Mark Jamsek Subject: Re: got: add diffstat to got diffg To: Omar Polo Cc: Game of Trees Date: Tue, 10 Jan 2023 00:11:12 +1100 On 23-01-09 01:17PM, Omar Polo wrote: > On 2023/01/09 22:38:37 +1100, Mark Jamsek wrote: > > As mentioned on irc, you convinced me we should always display the diff > > with -d, so the below diff makes this change from the previous revision. > > > > We now output the diffstat first, which is followed by the diff. One > > positive result is that, unlike tog and got log, we only compute the > > diff once, so this improves performance a little. We also now need to > > use a temp file, though, because we want the diffstat output first, but > > we need the entire diff to get our field widths and totals. As such, we > > can't print to stdout as we build the diff like we previously did. But > > this isn't a big deal. > > > > Sorry for the large diff; if you prefer, I can send a diff of changes > > from the previous as it's a bit smaller. Just let me know! > > It breaks godwebd. In gotwebd/got_operations.c the calls to > got_diff_objects_as_* needs to be tweaked. I'd say (for the moment at > least) to just disable the diffstat there; if wanted we can enable it > but the styling would need to be tweaked too and my diff to > templateify that part is still pending. > > (actually my templatefication of gotweb_render_diff is very likely to > be broken by this; not a big deal, i'll rebase it after this gets in.) Ah, I didn't even think about gotwebd--that was foolish. Thanks! I've just disabled diffstat there for now. I think it would be nice to display it eventually, but I'll leave that call to you :) And sorry to break your diff! > Some comments inline based on a quick read; I don't have a strong > opinion on having got showing a diffstat (I actually have my own > standalone diffstat script that I sometimes use for fun) but otherwise > I think the diff reads fine. I've made all suggested changes except for moving 'path' into function scope because (1) with your other brilliant suggestion to allocate 'change' in get_diffstat() (I can't believe I didn't think of that!), we now only have one free(path) on failure; and (2) I actually have a diff in my tree making some minor changes to diff_blobs() doing much the same as you mentioned regarding failure leaks. I didn't want to mix that in with this diff though. So I might hoist path up to function scope in that along with the other cleanup. Thanks, op! diffstat refs/remotes/origin/main 6257b0224ff6ba4aa3451337d0df8e02c357a9dd M got/got.1 | 5+ 1- M got/got.c | 161+ 38- M gotwebd/got_operations.c | 3+ 3- M include/got_diff.h | 14+ 7- M lib/diff.c | 170+ 86- M lib/worktree.c | 3+ 3- M tog/tog.c | 4+ 3- 7 files changed, 360 insertions(+), 141 deletions(-) diff refs/remotes/origin/main 6257b0224ff6ba4aa3451337d0df8e02c357a9dd commit - 243a8f150f5f79c7530b415ca9df09bdbebd0cf9 commit + 6257b0224ff6ba4aa3451337d0df8e02c357a9dd blob - c86779cca9d917f301a8ac66e723282c6c0bcb57 blob + 3014446b87c1bbbc0c5271ec13deecb93f5ccc0c --- got/got.1 +++ got/got.1 @@ -904,7 +904,7 @@ is never traversed. .Tg di .It Xo .Cm diff -.Op Fl aPsw +.Op Fl adPsw .Op Fl C Ar number .Op Fl c Ar commit .Op Fl r Ar repository-path @@ -962,6 +962,10 @@ option. Cannot be used together with the .Fl P option. +.It Fl d +Display diffstat of changes before the actual diff by annotating each file path +or blob hash being diffed with the total number of lines added and removed. +A summary line will display the total number of changes across all files. .It Fl P Interpret all arguments as paths only. This option can be used to resolve ambiguity in cases where paths blob - 9a89d5b62f1635fb4b385534abdd217ee4a06611 blob + 9c142650fd13686238b78aa2d167eaa7b856cffd --- got/got.c +++ got/got.c @@ -3624,7 +3624,8 @@ diff_blobs(struct got_object_id *blob_id1, struct got_ static const struct got_error * diff_blobs(struct got_object_id *blob_id1, struct got_object_id *blob_id2, const char *path, int diff_context, int ignore_whitespace, - int force_text_diff, struct got_repository *repo, FILE *outfile) + int force_text_diff, int show_diffstat, struct got_repository *repo, + FILE *outfile) { const struct got_error *err = NULL; struct got_blob_object *blob1 = NULL, *blob2 = NULL; @@ -3666,7 +3667,7 @@ diff_blobs(struct got_object_id *blob_id1, struct got_ path++; err = got_diff_blob(NULL, NULL, blob1, blob2, f1, f2, path, path, GOT_DIFF_ALGORITHM_PATIENCE, diff_context, ignore_whitespace, - force_text_diff, outfile); + force_text_diff, show_diffstat, NULL, outfile); done: if (fd1 != -1 && close(fd1) == -1 && err == NULL) err = got_error_from_errno("close"); @@ -3727,6 +3728,8 @@ diff_trees(struct got_object_id *tree_id1, struct got_ arg.diff_context = diff_context; arg.ignore_whitespace = ignore_whitespace; arg.force_text_diff = force_text_diff; + arg.show_diffstat = 0; + arg.diffstat = NULL; arg.diff_algo = GOT_DIFF_ALGORITHM_PATIENCE; arg.outfile = outfile; arg.lines = NULL; @@ -3891,7 +3894,7 @@ print_patch(struct got_commit_object *commit, struct g switch (obj_type) { case GOT_OBJ_TYPE_BLOB: err = diff_blobs(obj_id1, obj_id2, path, diff_context, - 0, 0, repo, outfile); + 0, 0, 0, repo, outfile); break; case GOT_OBJ_TYPE_TREE: err = diff_trees(obj_id1, obj_id2, path, diff_context, @@ -4164,6 +4167,31 @@ print_commit(struct got_commit_object *commit, struct } static const struct got_error * +print_diffstat(struct got_diffstat_cb_arg *dsa, struct got_pathlist_head *paths, + const char *header) +{ + struct got_pathlist_entry *pe; + + if (header != NULL) + printf("%s\n", header); + + TAILQ_FOREACH(pe, paths, entry) { + struct got_diff_changed_path *cp = pe->data; + int pad = dsa->max_path_len - pe->path_len + 1; + + printf(" %c %s%*c | %*d+ %*d-\n", cp->status, pe->path, pad, + ' ', dsa->add_cols + 1, cp->add, dsa->rm_cols + 1, cp->rm); + } + printf("\n%d file%s changed, %d insertions(+), %d deletions(-)\n\n", + dsa->nfiles, dsa->nfiles > 1 ? "s" : "", dsa->ins, dsa->del); + + if (fflush(stdout) != 0) + return got_error_from_errno("fflush"); + + return NULL; +} + +static const struct got_error * print_commit(struct got_commit_object *commit, struct got_object_id *id, struct got_repository *repo, const char *path, struct got_pathlist_head *changed_paths, struct got_diffstat_cb_arg *dsa, @@ -4237,31 +4265,18 @@ print_commit(struct got_commit_object *commit, struct } while (line); free(logmsg0); - if (changed_paths) { + if (dsa && changed_paths) { + err = print_diffstat(dsa, changed_paths, NULL); + if (err) + goto done; + } else if (changed_paths) { struct got_pathlist_entry *pe; TAILQ_FOREACH(pe, changed_paths, entry) { struct got_diff_changed_path *cp = pe->data; - char *stat = NULL; - if (dsa) { - int pad = dsa->max_path_len - pe->path_len + 1; - - if (asprintf(&stat, "%*c | %*d+ %*d-", - pad, ' ', dsa->add_cols + 1, cp->add, - dsa->rm_cols + 1, cp->rm) == -1) { - err = got_error_from_errno("asprintf"); - goto done; - } - } - printf(" %c %s%s\n", cp->status, pe->path, - stat ? stat : ""); - free(stat); + printf(" %c %s\n", cp->status, pe->path); } - if (dsa) - printf("\n%d file%s changed, %d insertions(+), " - "%d deletions(-)\n", dsa->nfiles, - dsa->nfiles > 1 ? "s" : "", dsa->ins, dsa->del); printf("\n"); } if (show_patch) { @@ -4727,7 +4742,7 @@ usage_diff(void) __dead static void usage_diff(void) { - fprintf(stderr, "usage: %s diff [-aPsw] [-C number] [-c commit] " + fprintf(stderr, "usage: %s diff [-adPsw] [-C number] [-c commit] " "[-r repository-path] [object1 object2 | path ...]\n", getprogname()); exit(1); @@ -4736,6 +4751,7 @@ struct print_diff_arg { struct print_diff_arg { struct got_repository *repo; struct got_worktree *worktree; + struct got_diffstat_cb_arg *diffstat; int diff_context; const char *id_str; int header_shown; @@ -4743,8 +4759,10 @@ struct print_diff_arg { enum got_diff_algorithm diff_algo; int ignore_whitespace; int force_text_diff; + int show_diffstat; FILE *f1; FILE *f2; + FILE *outfile; }; /* @@ -4836,12 +4854,22 @@ print_diff(void *arg, unsigned char status, unsigned c return got_error_from_errno("got_opentemp_truncate"); if (!a->header_shown) { - printf("diff %s%s\n", a->diff_staged ? "-s " : "", - got_worktree_get_root_path(a->worktree)); - printf("commit - %s\n", a->id_str); - printf("path + %s%s\n", + if (fprintf(a->outfile, "diff %s%s\n", + a->diff_staged ? "-s " : "", + got_worktree_get_root_path(a->worktree)) < 0) { + err = got_error_from_errno("fprintf"); + goto done; + } + if (fprintf(a->outfile, "commit - %s\n", a->id_str) < 0) { + err = got_error_from_errno("fprintf"); + goto done; + } + if (fprintf(a->outfile, "path + %s%s\n", got_worktree_get_root_path(a->worktree), - a->diff_staged ? " (staged changes)" : ""); + a->diff_staged ? " (staged changes)" : "") < 0) { + err = got_error_from_errno("fprintf"); + goto done; + } a->header_shown = 1; } @@ -4874,7 +4902,8 @@ print_diff(void *arg, unsigned char status, unsigned c err = got_diff_objects_as_blobs(NULL, NULL, a->f1, a->f2, fd1, fd2, blob_id, staged_blob_id, label1, label2, a->diff_algo, a->diff_context, a->ignore_whitespace, - a->force_text_diff, a->repo, stdout); + a->force_text_diff, a->show_diffstat, a->diffstat, a->repo, + a->outfile); goto done; } @@ -4964,7 +4993,8 @@ print_diff(void *arg, unsigned char status, unsigned c err = got_diff_blob_file(blob1, a->f1, size1, label1, f2 ? f2 : a->f2, f2_exists, &sb, path, GOT_DIFF_ALGORITHM_PATIENCE, a->diff_context, - a->ignore_whitespace, a->force_text_diff, stdout); + a->ignore_whitespace, a->force_text_diff, a->show_diffstat, + a->diffstat, a->outfile); done: if (fd1 != -1 && close(fd1) == -1 && err == NULL) err = got_error_from_errno("close"); @@ -4981,6 +5011,30 @@ cmd_diff(int argc, char *argv[]) } static const struct got_error * +printfile(FILE *f) +{ + char buf[8192]; + size_t r; + + if (fseeko(f, 0L, SEEK_SET) == -1) + return got_error_from_errno("fseek"); + + for (;;) { + r = fread(buf, 1, sizeof(buf), f); + if (r == 0) { + if (ferror(f)) + return got_error_from_errno("fread"); + if (feof(f)) + break; + } + if (fwrite(buf, 1, r, stdout) != r) + return got_ferror(stdout, GOT_ERR_IO); + } + + return NULL; +} + +static const struct got_error * cmd_diff(int argc, char *argv[]) { const struct got_error *error; @@ -4993,17 +5047,21 @@ cmd_diff(int argc, char *argv[]) char *labels[2] = { NULL, NULL }; int type1 = GOT_OBJ_TYPE_ANY, type2 = GOT_OBJ_TYPE_ANY; int diff_context = 3, diff_staged = 0, ignore_whitespace = 0, ch, i; - int force_text_diff = 0, force_path = 0, rflag = 0; + int force_text_diff = 0, force_path = 0, rflag = 0, show_diffstat = 0; const char *errstr; struct got_reflist_head refs; - struct got_pathlist_head paths; + struct got_pathlist_head diffstat_paths, paths; struct got_pathlist_entry *pe; - FILE *f1 = NULL, *f2 = NULL; + FILE *f1 = NULL, *f2 = NULL, *outfile = NULL; int fd1 = -1, fd2 = -1; int *pack_fds = NULL; + struct got_diffstat_cb_arg dsa; + memset(&dsa, 0, sizeof(dsa)); + TAILQ_INIT(&refs); TAILQ_INIT(&paths); + TAILQ_INIT(&diffstat_paths); #ifndef PROFILE if (pledge("stdio rpath wpath cpath flock proc exec sendfd unveil", @@ -5011,7 +5069,7 @@ cmd_diff(int argc, char *argv[]) err(1, "pledge"); #endif - while ((ch = getopt(argc, argv, "aC:c:Pr:sw")) != -1) { + while ((ch = getopt(argc, argv, "aC:c:dPr:sw")) != -1) { switch (ch) { case 'a': force_text_diff = 1; @@ -5028,6 +5086,9 @@ cmd_diff(int argc, char *argv[]) errx(1, "too many -c options used"); commit_args[ncommit_args++] = optarg; break; + case 'd': + show_diffstat = 1; + break; case 'P': force_path = 1; break; @@ -5091,6 +5152,13 @@ cmd_diff(int argc, char *argv[]) if (error != NULL) goto done; + if (show_diffstat) { + dsa.paths = &diffstat_paths; + dsa.force_text = force_text_diff; + dsa.ignore_ws = ignore_whitespace; + dsa.diff_algo = GOT_DIFF_ALGORITHM_PATIENCE; + } + if (rflag || worktree == NULL || ncommit_args > 0) { if (force_path) { error = got_error_msg(GOT_ERR_NOT_IMPL, @@ -5150,6 +5218,12 @@ cmd_diff(int argc, char *argv[]) goto done; } + outfile = got_opentemp(); + if (outfile == NULL) { + error = got_error_from_errno("got_opentemp"); + goto done; + } + if (ncommit_args == 0 && (ids[0] == NULL || ids[1] == NULL)) { struct print_diff_arg arg; char *id_str; @@ -5189,12 +5263,33 @@ cmd_diff(int argc, char *argv[]) arg.diff_staged = diff_staged; arg.ignore_whitespace = ignore_whitespace; arg.force_text_diff = force_text_diff; + arg.show_diffstat = show_diffstat; + arg.diffstat = &dsa; arg.f1 = f1; arg.f2 = f2; + arg.outfile = outfile; error = got_worktree_status(worktree, &paths, repo, 0, print_diff, &arg, check_cancelled, NULL); free(id_str); + if (error) + goto done; + + if (show_diffstat && dsa.nfiles > 0) { + char *header; + + if (asprintf(&header, "diffstat %s%s", + diff_staged ? "-s " : "", + got_worktree_get_root_path(worktree)) == -1) + goto done; + + error = print_diffstat(&dsa, &diffstat_paths, header); + free(header); + if (error) + goto done; + } + + error = printfile(outfile); goto done; } @@ -5329,24 +5424,45 @@ cmd_diff(int argc, char *argv[]) error = got_diff_objects_as_blobs(NULL, NULL, f1, f2, fd1, fd2, ids[0], ids[1], NULL, NULL, GOT_DIFF_ALGORITHM_PATIENCE, diff_context, - ignore_whitespace, force_text_diff, repo, stdout); + ignore_whitespace, force_text_diff, show_diffstat, + show_diffstat ? &dsa : NULL, repo, outfile); break; case GOT_OBJ_TYPE_TREE: error = got_diff_objects_as_trees(NULL, NULL, f1, f2, fd1, fd2, ids[0], ids[1], &paths, "", "", GOT_DIFF_ALGORITHM_PATIENCE, diff_context, - ignore_whitespace, force_text_diff, repo, stdout); + ignore_whitespace, force_text_diff, show_diffstat, + show_diffstat ? &dsa : NULL, repo, outfile); break; case GOT_OBJ_TYPE_COMMIT: - printf("diff %s %s\n", labels[0], labels[1]); + fprintf(outfile, "diff %s %s\n", labels[0], labels[1]); error = got_diff_objects_as_commits(NULL, NULL, f1, f2, fd1, fd2, ids[0], ids[1], &paths, GOT_DIFF_ALGORITHM_PATIENCE, diff_context, - ignore_whitespace, force_text_diff, repo, stdout); + ignore_whitespace, force_text_diff, show_diffstat, + show_diffstat ? &dsa : NULL, repo, outfile); break; default: error = got_error(GOT_ERR_OBJ_TYPE); } + if (error) + goto done; + + if (show_diffstat && dsa.nfiles > 0) { + char *header = NULL; + + if (asprintf(&header, "diffstat %s %s", + labels[0], labels[1]) == -1) + goto done; + + error = print_diffstat(&dsa, &diffstat_paths, header); + free(header); + if (error) + goto done; + } + + error = printfile(outfile); + done: free(labels[0]); free(labels[1]); @@ -5368,7 +5484,14 @@ done: TAILQ_FOREACH(pe, &paths, entry) free((char *)pe->path); got_pathlist_free(&paths); + TAILQ_FOREACH(pe, &diffstat_paths, entry) { + free((char *)pe->path); + free(pe->data); + } + got_pathlist_free(&diffstat_paths); got_ref_list_free(&refs); + if (outfile && fclose(outfile) == EOF && error == NULL) + error = got_error_from_errno("fclose"); if (f1 && fclose(f1) == EOF && error == NULL) error = got_error_from_errno("fclose"); if (f2 && fclose(f2) == EOF && error == NULL) blob - f8b05cf0a3f3223ffcb6a59216871982fd980bc9 blob + da6c79a3e639404327d4a27ce17468d32e06c119 --- gotwebd/got_operations.c +++ gotwebd/got_operations.c @@ -1393,17 +1393,17 @@ got_output_repo_diff(struct request *c) case GOT_OBJ_TYPE_BLOB: error = got_diff_objects_as_blobs(NULL, NULL, f1, f2, fd4, fd5, id1, id2, NULL, NULL, GOT_DIFF_ALGORITHM_MYERS, 3, 0, 0, - repo, f3); + 0, NULL, repo, f3); break; case GOT_OBJ_TYPE_TREE: error = got_diff_objects_as_trees(NULL, NULL, f1, f2, fd4, fd5, id1, id2, NULL, "", "", GOT_DIFF_ALGORITHM_MYERS, 3, 0, 0, - repo, f3); + 0, NULL, repo, f3); break; case GOT_OBJ_TYPE_COMMIT: error = got_diff_objects_as_commits(NULL, NULL, f1, f2, fd4, fd5, id1, id2, NULL, GOT_DIFF_ALGORITHM_MYERS, 3, 0, 0, - repo, f3); + 0, NULL, repo, f3); break; default: error = got_error(GOT_ERR_OBJ_TYPE); blob - 18fe6a19ccc9b5b2450e313f81d9246ce51b630d blob + ba881aed5067fd49ac9a5f284d1df97cc01b1def --- include/got_diff.h +++ include/got_diff.h @@ -44,6 +44,8 @@ struct got_diff_line { uint8_t type; }; +struct got_diffstat_cb_arg; + /* * Compute the differences between two blobs and write unified diff text * to the provided output file. Two open temporary files must be provided @@ -61,8 +63,8 @@ const struct got_error *got_diff_blob(struct got_diff_ */ const struct got_error *got_diff_blob(struct got_diff_line **, size_t *, struct got_blob_object *, struct got_blob_object *, FILE *, FILE *, - const char *, const char *, enum got_diff_algorithm, int, int, int, - FILE *); + const char *, const char *, enum got_diff_algorithm, int, int, int, int, + struct got_diffstat_cb_arg *, FILE *); /* * Compute the differences between a blob and a file and write unified diff @@ -75,7 +77,8 @@ const struct got_error *got_diff_blob_file(struct got_ */ const struct got_error *got_diff_blob_file(struct got_blob_object *, FILE *, off_t, const char *, FILE *, int, struct stat *, const char *, - enum got_diff_algorithm, int, int, int, FILE *); + enum got_diff_algorithm, int, int, int, int, struct got_diffstat_cb_arg *, + FILE *); /* * A callback function invoked to handle the differences between two blobs @@ -105,6 +108,8 @@ struct got_diff_blob_output_unidiff_arg { int diff_context; /* Sets the number of context lines. */ int ignore_whitespace; /* Ignore whitespace differences. */ int force_text_diff; /* Assume text even if binary data detected. */ + int show_diffstat; /* Compute diffstat of changes */ + struct got_diffstat_cb_arg *diffstat; enum got_diff_algorithm diff_algo; /* Diffing algorithm to use. */ /* @@ -204,7 +209,8 @@ const struct got_error *got_diff_objects_as_blobs(stru const struct got_error *got_diff_objects_as_blobs(struct got_diff_line **, size_t *, FILE *, FILE *, int, int, struct got_object_id *, struct got_object_id *, const char *, const char *, enum got_diff_algorithm, - int, int, int, struct got_repository *, FILE *); + int, int, int, int, struct got_diffstat_cb_arg *, struct got_repository *, + FILE *); struct got_pathlist_head; @@ -226,8 +232,8 @@ const struct got_error *got_diff_objects_as_trees(stru const struct got_error *got_diff_objects_as_trees(struct got_diff_line **, size_t *, FILE *, FILE *, int, int, struct got_object_id *, struct got_object_id *, struct got_pathlist_head *, const char *, - const char *, enum got_diff_algorithm, int, int, int, - struct got_repository *, FILE *); + const char *, enum got_diff_algorithm, int, int, int, int, + struct got_diffstat_cb_arg *, struct got_repository *, FILE *); /* * Diff two objects, assuming both objects are commits. @@ -244,6 +250,7 @@ const struct got_error *got_diff_objects_as_commits(st const struct got_error *got_diff_objects_as_commits(struct got_diff_line **, size_t *, FILE *, FILE *, int, int, struct got_object_id *, struct got_object_id *, struct got_pathlist_head *, enum got_diff_algorithm, - int, int, int, struct got_repository *, FILE *); + int, int, int, int, struct got_diffstat_cb_arg *, struct got_repository *, + FILE *); #define GOT_DIFF_MAX_CONTEXT 64 blob - 5e9010e76c3aa5de6de83e6b5cd54a4749c4f51a blob + b5900dcf76baf3e7af894d4817aa447491a287fc --- lib/diff.c +++ lib/diff.c @@ -59,12 +59,78 @@ static const struct got_error * return NULL; } +static void +diffstat_field_width(size_t *maxlen, int *add_cols, int *rm_cols, size_t len, + uint32_t add, uint32_t rm) +{ + int d1 = 1, d2 = 1; + + if (maxlen) + *maxlen = MAX(*maxlen, len); + + while (add /= 10) + ++d1; + *add_cols = MAX(*add_cols, d1); + + while (rm /= 10) + ++d2; + *rm_cols = MAX(*rm_cols, d2); +} + static const struct got_error * +get_diffstat(struct got_diffstat_cb_arg *ds, const char *path, + struct diff_result *r, int force_text, int status) +{ + const struct got_error *err; + struct got_pathlist_entry *pe; + struct got_diff_changed_path *change = NULL; + int flags = (r->left->atomizer_flags | r->right->atomizer_flags); + int isbin = (flags & DIFF_ATOMIZER_FOUND_BINARY_DATA); + int i; + + change = calloc(1, sizeof(*change)); + if (change == NULL) + return got_error_from_errno("malloc"); + + if (!isbin || force_text) { + for (i = 0; i < r->chunks.len; ++i) { + struct diff_chunk *c; + int clc, crc; + + c = diff_chunk_get(r, i); + clc = diff_chunk_get_left_count(c); + crc = diff_chunk_get_right_count(c); + + if (crc && !clc) + change->add += crc; + if (clc && !crc) + change->rm += clc; + } + } + + change->status = status; + ds->ins += change->add; + ds->del += change->rm; + ++ds->nfiles; + + err = got_pathlist_append(ds->paths, path, change); + if (err) + return err; + + pe = TAILQ_LAST(ds->paths, got_pathlist_head); + diffstat_field_width(&ds->max_path_len, &ds->add_cols, &ds->rm_cols, + pe->path_len, change->add, change->rm); + + return NULL; +} + +static const struct got_error * diff_blobs(struct got_diff_line **lines, size_t *nlines, struct got_diffreg_result **resultp, struct got_blob_object *blob1, struct got_blob_object *blob2, FILE *f1, FILE *f2, const char *label1, const char *label2, mode_t mode1, mode_t mode2, - int diff_context, int ignore_whitespace, int force_text_diff, FILE *outfile, + int diff_context, int ignore_whitespace, int force_text_diff, + int show_diffstat, struct got_diffstat_cb_arg *ds, FILE *outfile, enum got_diff_algorithm diff_algo) { const struct got_error *err = NULL, *free_err; @@ -170,11 +236,51 @@ diff_blobs(struct got_diff_line **lines, size_t *nline free(modestr1); free(modestr2); } + err = got_diffreg(&result, f1, f2, diff_algo, ignore_whitespace, force_text_diff); if (err) goto done; + if (show_diffstat) { + char *path = NULL; + int status = GOT_STATUS_NO_CHANGE; + + if (label1 == NULL && label2 == NULL) { + /* diffstat of blobs, show hash instead of path */ + if (asprintf(&path, "%.10s -> %.10s", + idstr1, idstr2) == -1) { + err = got_error_from_errno("asprintf"); + goto done; + } + } else { + path = strdup(label2 ? label2 : label1); + if (path == NULL) { + err = got_error_from_errno("malloc"); + goto done; + } + } + + /* + * Ignore 'm'ode status change: if there's no accompanying + * content change, there'll be no diffstat, and if there + * are actual changes, 'M'odified takes precedence. + */ + if (blob1 == NULL) + status = GOT_STATUS_ADD; + else if (blob2 == NULL) + status = GOT_STATUS_DELETE; + else + status = GOT_STATUS_MODIFY; + + err = get_diffstat(ds, path, result->result, force_text_diff, + status); + if (err) { + free(path); + goto done; + } + } + if (outfile) { err = got_diffreg_output(lines, nlines, result, blob1 != NULL, blob2 != NULL, @@ -208,7 +314,8 @@ got_diff_blob_output_unidiff(void *arg, struct got_blo return diff_blobs(&a->lines, &a->nlines, NULL, blob1, blob2, f1, f2, label1, label2, mode1, mode2, a->diff_context, - a->ignore_whitespace, a->force_text_diff, a->outfile, a->diff_algo); + a->ignore_whitespace, a->force_text_diff, a->show_diffstat, + a->diffstat, a->outfile, a->diff_algo); } const struct got_error * @@ -216,11 +323,12 @@ got_diff_blob(struct got_diff_line **lines, size_t*nli struct got_blob_object *blob1, struct got_blob_object *blob2, FILE *f1, FILE *f2, const char *label1, const char *label2, enum got_diff_algorithm diff_algo, int diff_context, - int ignore_whitespace, int force_text_diff, FILE *outfile) + int ignore_whitespace, int force_text_diff, int show_diffstat, + struct got_diffstat_cb_arg *ds, FILE *outfile) { return diff_blobs(lines, nlines, NULL, blob1, blob2, f1, f2, label1, label2, 0, 0, diff_context, ignore_whitespace, - force_text_diff, outfile, diff_algo); + force_text_diff, show_diffstat, ds, outfile, diff_algo); } static const struct got_error * @@ -228,7 +336,8 @@ diff_blob_file(struct got_diffreg_result **resultp, struct got_blob_object *blob1, FILE *f1, off_t size1, const char *label1, FILE *f2, int f2_exists, struct stat *sb2, const char *label2, enum got_diff_algorithm diff_algo, int diff_context, int ignore_whitespace, - int force_text_diff, FILE *outfile) + int force_text_diff, int show_diffstat, struct got_diffstat_cb_arg *ds, + FILE *outfile) { const struct got_error *err = NULL, *free_err; char hex1[SHA1_DIGEST_STRING_LENGTH]; @@ -277,6 +386,36 @@ done: goto done; } + if (show_diffstat) { + char *path = NULL; + int status = GOT_STATUS_NO_CHANGE; + + path = strdup(label2 ? label2 : label1); + if (path == NULL) { + err = got_error_from_errno("malloc"); + goto done; + } + + /* + * Ignore 'm'ode status change: if there's no accompanying + * content change, there'll be no diffstat, and if there + * are actual changes, 'M'odified takes precedence. + */ + if (blob1 == NULL) + status = GOT_STATUS_ADD; + else if (!f2_exists) + status = GOT_STATUS_DELETE; + else + status = GOT_STATUS_MODIFY; + + err = get_diffstat(ds, path, result->result, force_text_diff, + status); + if (err) { + free(path); + goto done; + } + } + done: if (resultp && err == NULL) *resultp = result; @@ -292,11 +431,12 @@ got_diff_blob_file(struct got_blob_object *blob1, FILE got_diff_blob_file(struct got_blob_object *blob1, FILE *f1, off_t size1, const char *label1, FILE *f2, int f2_exists, struct stat *sb2, const char *label2, enum got_diff_algorithm diff_algo, int diff_context, - int ignore_whitespace, int force_text_diff, FILE *outfile) + int ignore_whitespace, int force_text_diff, int show_diffstat, + struct got_diffstat_cb_arg *ds, FILE *outfile) { return diff_blob_file(NULL, blob1, f1, size1, label1, f2, f2_exists, sb2, label2, diff_algo, diff_context, ignore_whitespace, - force_text_diff, outfile); + force_text_diff, show_diffstat, ds, outfile); } static const struct got_error * @@ -611,23 +751,6 @@ static void NULL, label2, 0, te2->mode, repo); } -static void -diffstat_field_width(size_t *maxlen, int *add_cols, int *rm_cols, size_t len, - uint32_t add, uint32_t rm) -{ - int d1 = 1, d2 = 1; - - *maxlen = MAX(*maxlen, len); - - while (add /= 10) - ++d1; - *add_cols = MAX(*add_cols, d1); - - while (rm /= 10) - ++d2; - *rm_cols = MAX(*rm_cols, d2); -} - const struct got_error * got_diff_tree_compute_diffstat(void *arg, struct got_blob_object *blob1, struct got_blob_object *blob2, FILE *f1, FILE *f2, @@ -637,35 +760,23 @@ got_diff_tree_compute_diffstat(void *arg, struct got_b { const struct got_error *err = NULL; struct got_diffreg_result *result = NULL; - struct diff_result *r; - struct got_diff_changed_path *change = NULL; struct got_diffstat_cb_arg *a = arg; - struct got_pathlist_entry *pe; char *path = NULL; - int i; + int status = GOT_STATUS_NO_CHANGE; path = strdup(label2 ? label2 : label1); if (path == NULL) return got_error_from_errno("malloc"); - change = malloc(sizeof(*change)); - if (change == NULL) { - err = got_error_from_errno("malloc"); - goto done; - } - - change->add = 0; - change->rm = 0; - change->status = GOT_STATUS_NO_CHANGE; if (id1 == NULL) - change->status = GOT_STATUS_ADD; + status = GOT_STATUS_ADD; else if (id2 == NULL) - change->status = GOT_STATUS_DELETE; + status = GOT_STATUS_DELETE; else { if (got_object_id_cmp(id1, id2) != 0) - change->status = GOT_STATUS_MODIFY; + status = GOT_STATUS_MODIFY; else if (mode1 != mode2) - change->status = GOT_STATUS_MODE_CHANGE; + status = GOT_STATUS_MODE_CHANGE; } if (f1) { @@ -697,36 +808,8 @@ got_diff_tree_compute_diffstat(void *arg, struct got_b if (err) goto done; - for (i = 0, r = result->result; i < r->chunks.len; ++i) { - int flags = (r->left->atomizer_flags | r->right->atomizer_flags); - int isbin = (flags & DIFF_ATOMIZER_FOUND_BINARY_DATA); + err = get_diffstat(a, path, result->result, a->force_text, status); - if (!isbin || a->force_text) { - struct diff_chunk *c; - int clc, crc; - - c = diff_chunk_get(r, i); - clc = diff_chunk_get_left_count(c); - crc = diff_chunk_get_right_count(c); - - if (clc && !crc) - change->rm += clc; - else if (crc && !clc) - change->add += crc; - } - } - - err = got_pathlist_append(a->paths, path, change); - if (err) - goto done; - - pe = TAILQ_LAST(a->paths, got_pathlist_head); - diffstat_field_width(&a->max_path_len, &a->add_cols, &a->rm_cols, - pe->path_len, change->add, change->rm); - a->ins += change->add; - a->del += change->rm; - ++a->nfiles; - done: if (result) { const struct got_error *free_err; @@ -735,10 +818,8 @@ done: if (free_err && err == NULL) err = free_err; } - if (err) { + if (err) free(path); - free(change); - } return err; } @@ -885,8 +966,8 @@ got_diff_objects_as_blobs(struct got_diff_line **lines struct got_object_id *id1, struct got_object_id *id2, const char *label1, const char *label2, enum got_diff_algorithm diff_algo, int diff_context, - int ignore_whitespace, int force_text_diff, - struct got_repository *repo, FILE *outfile) + int ignore_whitespace, int force_text_diff, int show_diffstat, + struct got_diffstat_cb_arg *ds, struct got_repository *repo, FILE *outfile) { const struct got_error *err; struct got_blob_object *blob1 = NULL, *blob2 = NULL; @@ -906,7 +987,7 @@ got_diff_objects_as_blobs(struct got_diff_line **lines } err = got_diff_blob(lines, nlines, blob1, blob2, f1, f2, label1, label2, diff_algo, diff_context, ignore_whitespace, force_text_diff, - outfile); + show_diffstat, ds, outfile); done: if (blob1) got_object_blob_close(blob1); @@ -1080,6 +1161,7 @@ diff_objects_as_trees(struct got_diff_line **lines, si struct got_object_id *id1, struct got_object_id *id2, struct got_pathlist_head *paths, const char *label1, const char *label2, int diff_context, int ignore_whitespace, int force_text_diff, + int show_diffstat, struct got_diffstat_cb_arg *dsa, struct got_repository *repo, FILE *outfile, enum got_diff_algorithm diff_algo) { @@ -1106,6 +1188,8 @@ diff_objects_as_trees(struct got_diff_line **lines, si arg.diff_context = diff_context; arg.ignore_whitespace = ignore_whitespace; arg.force_text_diff = force_text_diff; + arg.show_diffstat = show_diffstat; + arg.diffstat = dsa; arg.outfile = outfile; if (want_linemeta) { arg.lines = *lines; @@ -1114,14 +1198,12 @@ diff_objects_as_trees(struct got_diff_line **lines, si arg.lines = NULL; arg.nlines = 0; } - if (paths == NULL || TAILQ_EMPTY(paths)) { - err = got_diff_tree(tree1, tree2, f1, f2, fd1, fd2, - label1, label2, repo, - got_diff_blob_output_unidiff, &arg, 1); - } else { + if (paths == NULL || TAILQ_EMPTY(paths)) + err = got_diff_tree(tree1, tree2, f1, f2, fd1, fd2, label1, + label2, repo, got_diff_blob_output_unidiff, &arg, 1); + else err = diff_paths(tree1, tree2, f1, f2, fd1, fd2, paths, repo, got_diff_blob_output_unidiff, &arg); - } if (want_linemeta) { *lines = arg.lines; /* was likely re-allocated */ *nlines = arg.nlines; @@ -1140,7 +1222,8 @@ got_diff_objects_as_trees(struct got_diff_line **lines struct got_object_id *id1, struct got_object_id *id2, struct got_pathlist_head *paths, const char *label1, const char *label2, enum got_diff_algorithm diff_algo, int diff_context, int ignore_whitespace, - int force_text_diff, struct got_repository *repo, FILE *outfile) + int force_text_diff, int show_diffstat, struct got_diffstat_cb_arg *dsa, + struct got_repository *repo, FILE *outfile) { const struct got_error *err; char *idstr = NULL; @@ -1182,7 +1265,7 @@ got_diff_objects_as_trees(struct got_diff_line **lines err = diff_objects_as_trees(lines, nlines, f1, f2, fd1, fd2, id1, id2, paths, label1, label2, diff_context, ignore_whitespace, - force_text_diff, repo, outfile, diff_algo); + force_text_diff, show_diffstat, dsa, repo, outfile, diff_algo); done: free(idstr); return err; @@ -1194,6 +1277,7 @@ got_diff_objects_as_commits(struct got_diff_line **lin struct got_object_id *id1, struct got_object_id *id2, struct got_pathlist_head *paths, enum got_diff_algorithm diff_algo, int diff_context, int ignore_whitespace, int force_text_diff, + int show_diffstat, struct got_diffstat_cb_arg *dsa, struct got_repository *repo, FILE *outfile) { const struct got_error *err; @@ -1237,8 +1321,8 @@ got_diff_objects_as_commits(struct got_diff_line **lin err = diff_objects_as_trees(lines, nlines, f1, f2, fd1, fd2, commit1 ? got_object_commit_get_tree_id(commit1) : NULL, got_object_commit_get_tree_id(commit2), paths, "", "", - diff_context, ignore_whitespace, force_text_diff, repo, outfile, - diff_algo); + diff_context, ignore_whitespace, force_text_diff, show_diffstat, + dsa, repo, outfile, diff_algo); done: if (commit1) got_object_commit_close(commit1); blob - b40f67cf87889e4cd271c0870db2a357160443ad blob + e92fe5aa6a9932cfb59504fd20073ebc1fb4f6d4 --- lib/worktree.c +++ lib/worktree.c @@ -5083,8 +5083,8 @@ append_ct_diff(struct got_commitable *ct, int *diff_he } err = got_diff_objects_as_blobs(NULL, NULL, f1, f2, fd1, fd2, ct->base_blob_id, ct->staged_blob_id, - label1, label2, GOT_DIFF_ALGORITHM_PATIENCE, 3, 0, 0, - repo, diff_outfile); + label1, label2, GOT_DIFF_ALGORITHM_PATIENCE, 3, 0, 0, 0, + NULL, repo, diff_outfile); goto done; } @@ -5154,7 +5154,7 @@ append_ct_diff(struct got_commitable *ct, int *diff_he err = got_diff_blob_file(blob1, f1, size1, label1, ondisk_file ? ondisk_file : f2, f2_exists, &sb, ct->path, - GOT_DIFF_ALGORITHM_PATIENCE, 3, 0, 0, diff_outfile); + GOT_DIFF_ALGORITHM_PATIENCE, 3, 0, 0, 0, NULL, diff_outfile); done: if (fd1 != -1 && close(fd1) == -1 && err == NULL) err = got_error_from_errno("close"); blob - 7a91723cf327c438d7801b8d486b165bd84ba1ea blob + e05667c5078d9ea47470c917b489eef99e99c0a7 --- tog/tog.c +++ tog/tog.c @@ -4782,13 +4782,14 @@ create_diff(struct tog_diff_view_state *s) err = got_diff_objects_as_blobs(&s->lines, &s->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, s->repo, s->f); + s->ignore_whitespace, s->force_text_diff, 0, NULL, s->repo, + s->f); break; case GOT_OBJ_TYPE_TREE: err = got_diff_objects_as_trees(&s->lines, &s->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, s->repo, s->f); + s->force_text_diff, 0, NULL, s->repo, s->f); break; case GOT_OBJ_TYPE_COMMIT: { const struct got_object_id_queue *parent_ids; @@ -4826,7 +4827,7 @@ create_diff(struct tog_diff_view_state *s) err = got_diff_objects_as_commits(&s->lines, &s->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, s->repo, s->f); + s->force_text_diff, 0, NULL, s->repo, s->f); break; } default: -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68