From f168da3a3c5d6ce9b7c1ea036ca184c20729153e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 30 Apr 2018 23:54:13 +0200 Subject: [PATCH 01/21] linear-assignment: a function to solve least-cost assignment problems The problem solved by the code introduced in this commit goes like this: given two sets of items, and a cost matrix which says how much it "costs" to assign any given item of the first set to any given item of the second, assign all items (except when the sets have different size) in the cheapest way. We use the Jonker-Volgenant algorithm to solve the assignment problem to answer questions such as: given two different versions of a topic branch (or iterations of a patch series), what is the best pairing of commits/patches between the different versions? Signed-off-by: Johannes Schindelin --- Makefile | 1 + linear-assignment.c | 201 ++++++++++++++++++++++++++++++++++++++++++++ linear-assignment.h | 22 +++++ 3 files changed, 224 insertions(+) create mode 100644 linear-assignment.c create mode 100644 linear-assignment.h diff --git a/Makefile b/Makefile index bc4fc8eeab2186..1af719b448b359 100644 --- a/Makefile +++ b/Makefile @@ -870,6 +870,7 @@ LIB_OBJS += gpg-interface.o LIB_OBJS += graph.o LIB_OBJS += grep.o LIB_OBJS += hashmap.o +LIB_OBJS += linear-assignment.o LIB_OBJS += help.o LIB_OBJS += hex.o LIB_OBJS += ident.o diff --git a/linear-assignment.c b/linear-assignment.c new file mode 100644 index 00000000000000..9b3e56e283ccb9 --- /dev/null +++ b/linear-assignment.c @@ -0,0 +1,201 @@ +/* + * Based on: Jonker, R., & Volgenant, A. (1987). A shortest augmenting path + * algorithm for dense and sparse linear assignment problems. Computing, + * 38(4), 325-340. + */ +#include "cache.h" +#include "linear-assignment.h" + +#define COST(column, row) cost[(column) + column_count * (row)] + +/* + * The parameter `cost` is the cost matrix: the cost to assign column j to row + * i is `cost[j + column_count * i]. + */ +void compute_assignment(int column_count, int row_count, int *cost, + int *column2row, int *row2column) +{ + int *v, *d; + int *free_row, free_count = 0, saved_free_count, *pred, *col; + int i, j, phase; + + memset(column2row, -1, sizeof(int) * column_count); + memset(row2column, -1, sizeof(int) * row_count); + ALLOC_ARRAY(v, column_count); + + /* column reduction */ + for (j = column_count - 1; j >= 0; j--) { + int i1 = 0; + + for (i = 1; i < row_count; i++) + if (COST(j, i1) > COST(j, i)) + i1 = i; + v[j] = COST(j, i1); + if (row2column[i1] == -1) { + /* row i1 unassigned */ + row2column[i1] = j; + column2row[j] = i1; + } else { + if (row2column[i1] >= 0) + row2column[i1] = -2 - row2column[i1]; + column2row[j] = -1; + } + } + + /* reduction transfer */ + ALLOC_ARRAY(free_row, row_count); + for (i = 0; i < row_count; i++) { + int j1 = row2column[i]; + if (j1 == -1) + free_row[free_count++] = i; + else if (j1 < -1) + row2column[i] = -2 - j1; + else { + int min = COST(!j1, i) - v[!j1]; + for (j = 1; j < column_count; j++) + if (j != j1 && min > COST(j, i) - v[j]) + min = COST(j, i) - v[j]; + v[j1] -= min; + } + } + + if (free_count == + (column_count < row_count ? row_count - column_count : 0)) { + free(v); + free(free_row); + return; + } + + /* augmenting row reduction */ + for (phase = 0; phase < 2; phase++) { + int k = 0; + + saved_free_count = free_count; + free_count = 0; + while (k < saved_free_count) { + int u1, u2; + int j1 = 0, j2, i0; + + i = free_row[k++]; + u1 = COST(j1, i) - v[j1]; + j2 = -1; + u2 = INT_MAX; + for (j = 1; j < column_count; j++) { + int c = COST(j, i) - v[j]; + if (u2 > c) { + if (u1 < c) { + u2 = c; + j2 = j; + } else { + u2 = u1; + u1 = c; + j2 = j1; + j1 = j; + } + } + } + if (j2 < 0) { + j2 = j1; + u2 = u1; + } + + i0 = column2row[j1]; + if (u1 < u2) + v[j1] -= u2 - u1; + else if (i0 >= 0) { + j1 = j2; + i0 = column2row[j1]; + } + + if (i0 >= 0) { + if (u1 < u2) + free_row[--k] = i0; + else + free_row[free_count++] = i0; + } + row2column[i] = j1; + column2row[j1] = i; + } + } + + /* augmentation */ + saved_free_count = free_count; + ALLOC_ARRAY(d, column_count); + ALLOC_ARRAY(pred, column_count); + ALLOC_ARRAY(col, column_count); + for (free_count = 0; free_count < saved_free_count; free_count++) { + int i1 = free_row[free_count], low = 0, up = 0, last, k; + int min, c, u1; + + for (j = 0; j < column_count; j++) { + d[j] = COST(j, i1) - v[j]; + pred[j] = i1; + col[j] = j; + } + + j = -1; + do { + last = low; + min = d[col[up++]]; + for (k = up; k < column_count; k++) { + j = col[k]; + c = d[j]; + if (c <= min) { + if (c < min) { + up = low; + min = c; + } + col[k] = col[up]; + col[up++] = j; + } + } + for (k = low; k < up; k++) + if (column2row[col[k]] == -1) + goto update; + + /* scan a row */ + do { + int j1 = col[low++]; + + i = column2row[j1]; + u1 = COST(j1, i) - v[j1] - min; + for (k = up; k < column_count; k++) { + j = col[k]; + c = COST(j, i) - v[j] - u1; + if (c < d[j]) { + d[j] = c; + pred[j] = i; + if (c == min) { + if (column2row[j] == -1) + goto update; + col[k] = col[up]; + col[up++] = j; + } + } + } + } while (low != up); + } while (low == up); + +update: + /* updating of the column pieces */ + for (k = 0; k < last; k++) { + int j1 = col[k]; + v[j1] += d[j1] - min; + } + + /* augmentation */ + do { + if (j < 0) + BUG("negative j: %d", j); + i = pred[j]; + column2row[j] = i; + SWAP(j, row2column[i]); + } while (i1 != i); + } + + free(col); + free(pred); + free(d); + free(v); + free(free_row); +} diff --git a/linear-assignment.h b/linear-assignment.h new file mode 100644 index 00000000000000..1dfea766290d9d --- /dev/null +++ b/linear-assignment.h @@ -0,0 +1,22 @@ +#ifndef LINEAR_ASSIGNMENT_H +#define LINEAR_ASSIGNMENT_H + +/* + * Compute an assignment of columns -> rows (and vice versa) such that every + * column is assigned to at most one row (and vice versa) minimizing the + * overall cost. + * + * The parameter `cost` is the cost matrix: the cost to assign column j to row + * i is `cost[j + column_count * i]. + * + * The arrays column2row and row2column will be populated with the respective + * assignments (-1 for unassigned, which can happen only if column_count != + * row_count). + */ +void compute_assignment(int column_count, int row_count, int *cost, + int *column2row, int *row2column); + +/* The maximal cost in the cost matrix (to prevent integer overflows). */ +#define COST_MAX (1<<16) + +#endif From 33758f361c4cbe47bca50d10d61ff36bf6da2b5a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 1 May 2018 21:42:28 +0200 Subject: [PATCH 02/21] Introduce `range-diff` to compare iterations of a topic branch This command does not do a whole lot so far, apart from showing a usage that is oddly similar to that of `git tbdiff`. And for a good reason: the next commits will turn `range-branch` into a full-blown replacement for `tbdiff`. At this point, we ignore tbdiff's color options, as they will all be implemented later using diff_options. Since f318d739159 (generate-cmds.sh: export all commands to command-list.h, 2018-05-10), every new command *requires* a man page to build right away, so let's also add a blank man page, too. Signed-off-by: Johannes Schindelin --- .gitignore | 1 + Documentation/git-range-diff.txt | 10 ++++++++++ Makefile | 1 + builtin.h | 1 + builtin/range-diff.c | 25 +++++++++++++++++++++++++ command-list.txt | 1 + git.c | 1 + 7 files changed, 40 insertions(+) create mode 100644 Documentation/git-range-diff.txt create mode 100644 builtin/range-diff.c diff --git a/.gitignore b/.gitignore index 3284a1e9b1e80e..cc0ad74b4beb2b 100644 --- a/.gitignore +++ b/.gitignore @@ -113,6 +113,7 @@ /git-pull /git-push /git-quiltimport +/git-range-diff /git-read-tree /git-rebase /git-rebase--am diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt new file mode 100644 index 00000000000000..49f717db8b7ca2 --- /dev/null +++ b/Documentation/git-range-diff.txt @@ -0,0 +1,10 @@ +git-range-diff(1) +================= + +NAME +---- +git-range-diff - Compare two commit ranges (e.g. two versions of a branch) + +GIT +--- +Part of the linkgit:git[1] suite diff --git a/Makefile b/Makefile index 1af719b448b359..7ff7eba42bf2dd 100644 --- a/Makefile +++ b/Makefile @@ -1063,6 +1063,7 @@ BUILTIN_OBJS += builtin/prune-packed.o BUILTIN_OBJS += builtin/prune.o BUILTIN_OBJS += builtin/pull.o BUILTIN_OBJS += builtin/push.o +BUILTIN_OBJS += builtin/range-diff.o BUILTIN_OBJS += builtin/read-tree.o BUILTIN_OBJS += builtin/rebase--helper.o BUILTIN_OBJS += builtin/receive-pack.o diff --git a/builtin.h b/builtin.h index 0362f1ce25fb64..99206df4bd43fc 100644 --- a/builtin.h +++ b/builtin.h @@ -201,6 +201,7 @@ extern int cmd_prune(int argc, const char **argv, const char *prefix); extern int cmd_prune_packed(int argc, const char **argv, const char *prefix); extern int cmd_pull(int argc, const char **argv, const char *prefix); extern int cmd_push(int argc, const char **argv, const char *prefix); +extern int cmd_range_diff(int argc, const char **argv, const char *prefix); extern int cmd_read_tree(int argc, const char **argv, const char *prefix); extern int cmd_rebase__helper(int argc, const char **argv, const char *prefix); extern int cmd_receive_pack(int argc, const char **argv, const char *prefix); diff --git a/builtin/range-diff.c b/builtin/range-diff.c new file mode 100644 index 00000000000000..36788ea4f2b7a7 --- /dev/null +++ b/builtin/range-diff.c @@ -0,0 +1,25 @@ +#include "cache.h" +#include "builtin.h" +#include "parse-options.h" + +static const char * const builtin_range_diff_usage[] = { +N_("git range-diff [] .. .."), +N_("git range-diff [] ..."), +N_("git range-diff [] "), +NULL +}; + +int cmd_range_diff(int argc, const char **argv, const char *prefix) +{ + int creation_factor = 60; + struct option options[] = { + OPT_INTEGER(0, "creation-factor", &creation_factor, + N_("Percentage by which creation is weighted")), + OPT_END() + }; + + argc = parse_options(argc, argv, NULL, options, + builtin_range_diff_usage, 0); + + return 0; +} diff --git a/command-list.txt b/command-list.txt index e1c26c1bb7e618..a9dda3b8af6a75 100644 --- a/command-list.txt +++ b/command-list.txt @@ -139,6 +139,7 @@ git-prune-packed plumbingmanipulators git-pull mainporcelain remote git-push mainporcelain remote git-quiltimport foreignscminterface +git-range-diff mainporcelain git-read-tree plumbingmanipulators git-rebase mainporcelain history git-receive-pack synchelpers diff --git a/git.c b/git.c index fc7d15d549e491..5b48cac3a95c64 100644 --- a/git.c +++ b/git.c @@ -520,6 +520,7 @@ static struct cmd_struct commands[] = { { "prune-packed", cmd_prune_packed, RUN_SETUP }, { "pull", cmd_pull, RUN_SETUP | NEED_WORK_TREE }, { "push", cmd_push, RUN_SETUP }, + { "range-diff", cmd_range_diff, RUN_SETUP | USE_PAGER }, { "read-tree", cmd_read_tree, RUN_SETUP | SUPPORT_SUPER_PREFIX}, { "rebase--helper", cmd_rebase__helper, RUN_SETUP | NEED_WORK_TREE }, { "receive-pack", cmd_receive_pack }, From 08b8c3fc45253737ef6ca860e6cbe3ee6211d7a6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 2 May 2018 02:34:01 +0200 Subject: [PATCH 03/21] range-diff: first rudimentary implementation At this stage, `git range-diff` can determine corresponding commits of two related commit ranges. This makes use of the recently introduced implementation of the linear assignment algorithm. The core of this patch is a straight port of the ideas of tbdiff, the apparently dormant project at https://github.com/trast/tbdiff. The output does not at all match `tbdiff`'s output yet, as this patch really concentrates on getting the patch matching part right. Note: due to differences in the diff algorithm (`tbdiff` uses the Python module `difflib`, Git uses its xdiff fork), the cost matrix calculated by `range-diff` is different (but very similar) to the one calculated by `tbdiff`. Therefore, it is possible that they find different matching commits in corner cases (e.g. when a patch was split into two patches of roughly equal length). Signed-off-by: Johannes Schindelin --- Makefile | 1 + builtin/range-diff.c | 45 ++++++- range-diff.c | 311 +++++++++++++++++++++++++++++++++++++++++++ range-diff.h | 7 + 4 files changed, 363 insertions(+), 1 deletion(-) create mode 100644 range-diff.c create mode 100644 range-diff.h diff --git a/Makefile b/Makefile index 7ff7eba42bf2dd..72f16882e61dfc 100644 --- a/Makefile +++ b/Makefile @@ -925,6 +925,7 @@ LIB_OBJS += progress.o LIB_OBJS += prompt.o LIB_OBJS += protocol.o LIB_OBJS += quote.o +LIB_OBJS += range-diff.o LIB_OBJS += reachable.o LIB_OBJS += read-cache.o LIB_OBJS += reflog-walk.o diff --git a/builtin/range-diff.c b/builtin/range-diff.c index 36788ea4f2b7a7..94c1f362cc4fee 100644 --- a/builtin/range-diff.c +++ b/builtin/range-diff.c @@ -1,6 +1,7 @@ #include "cache.h" #include "builtin.h" #include "parse-options.h" +#include "range-diff.h" static const char * const builtin_range_diff_usage[] = { N_("git range-diff [] .. .."), @@ -17,9 +18,51 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) N_("Percentage by which creation is weighted")), OPT_END() }; + int res = 0; + struct strbuf range1 = STRBUF_INIT, range2 = STRBUF_INIT; argc = parse_options(argc, argv, NULL, options, builtin_range_diff_usage, 0); - return 0; + if (argc == 2) { + if (!strstr(argv[0], "..")) + die(_("no .. in range: '%s'"), argv[0]); + strbuf_addstr(&range1, argv[0]); + + if (!strstr(argv[1], "..")) + die(_("no .. in range: '%s'"), argv[1]); + strbuf_addstr(&range2, argv[1]); + } else if (argc == 3) { + strbuf_addf(&range1, "%s..%s", argv[0], argv[1]); + strbuf_addf(&range2, "%s..%s", argv[0], argv[2]); + } else if (argc == 1) { + const char *b = strstr(argv[0], "..."), *a = argv[0]; + int a_len; + + if (!b) { + error(_("single arg format must be symmetric range")); + usage_with_options(builtin_range_diff_usage, options); + } + + a_len = (int)(b - a); + if (!a_len) { + a = "HEAD"; + a_len = strlen(a); + } + b += 3; + if (!*b) + b = "HEAD"; + strbuf_addf(&range1, "%s..%.*s", b, a_len, a); + strbuf_addf(&range2, "%.*s..%s", a_len, a, b); + } else { + error(_("need two commit ranges")); + usage_with_options(builtin_range_diff_usage, options); + } + + res = show_range_diff(range1.buf, range2.buf, creation_factor); + + strbuf_release(&range1); + strbuf_release(&range2); + + return res; } diff --git a/range-diff.c b/range-diff.c new file mode 100644 index 00000000000000..15d418afa674c2 --- /dev/null +++ b/range-diff.c @@ -0,0 +1,311 @@ +#include "cache.h" +#include "range-diff.h" +#include "string-list.h" +#include "run-command.h" +#include "argv-array.h" +#include "hashmap.h" +#include "xdiff-interface.h" +#include "linear-assignment.h" + +struct patch_util { + /* For the search for an exact match */ + struct hashmap_entry e; + const char *diff, *patch; + + int i; + int diffsize; + size_t diff_offset; + /* the index of the matching item in the other branch, or -1 */ + int matching; + struct object_id oid; +}; + +/* + * Reads the patches into a string list, with the `util` field being populated + * as struct object_id (will need to be free()d). + */ +static int read_patches(const char *range, struct string_list *list) +{ + struct child_process cp = CHILD_PROCESS_INIT; + FILE *in; + struct strbuf buf = STRBUF_INIT, line = STRBUF_INIT; + struct patch_util *util = NULL; + int in_header = 1; + + argv_array_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges", + "--reverse", "--date-order", "--decorate=no", + "--no-abbrev-commit", range, + NULL); + cp.out = -1; + cp.no_stdin = 1; + cp.git_cmd = 1; + + if (start_command(&cp)) + return error_errno(_("could not start `log`")); + in = fdopen(cp.out, "r"); + if (!in) { + error_errno(_("could not read `log` output")); + finish_command(&cp); + return -1; + } + + while (strbuf_getline(&line, in) != EOF) { + const char *p; + + if (skip_prefix(line.buf, "commit ", &p)) { + if (util) { + string_list_append(list, buf.buf)->util = util; + strbuf_reset(&buf); + } + util = xcalloc(sizeof(*util), 1); + if (get_oid(p, &util->oid)) { + error(_("could not parse commit '%s'"), p); + free(util); + string_list_clear(list, 1); + strbuf_release(&buf); + strbuf_release(&line); + fclose(in); + finish_command(&cp); + return -1; + } + util->matching = -1; + in_header = 1; + continue; + } + + if (starts_with(line.buf, "diff --git")) { + in_header = 0; + strbuf_addch(&buf, '\n'); + if (!util->diff_offset) + util->diff_offset = buf.len; + strbuf_addbuf(&buf, &line); + } else if (in_header) { + if (starts_with(line.buf, "Author: ")) { + strbuf_addbuf(&buf, &line); + strbuf_addstr(&buf, "\n\n"); + } else if (starts_with(line.buf, " ")) { + strbuf_addbuf(&buf, &line); + strbuf_addch(&buf, '\n'); + } + continue; + } else if (starts_with(line.buf, "@@ ")) + strbuf_addstr(&buf, "@@"); + else if (!line.buf[0] || starts_with(line.buf, "index ")) + /* + * A completely blank (not ' \n', which is context) + * line is not valid in a diff. We skip it + * silently, because this neatly handles the blank + * separator line between commits in git-log + * output. + * + * We also want to ignore the diff's `index` lines + * because they contain exact blob hashes in which + * we are not interested. + */ + continue; + else + strbuf_addbuf(&buf, &line); + + strbuf_addch(&buf, '\n'); + util->diffsize++; + } + fclose(in); + strbuf_release(&line); + + if (util) + string_list_append(list, buf.buf)->util = util; + strbuf_release(&buf); + + if (finish_command(&cp)) + return -1; + + return 0; +} + +static int patch_util_cmp(const void *dummy, const struct patch_util *a, + const struct patch_util *b, const char *keydata) +{ + return strcmp(a->diff, keydata ? keydata : b->diff); +} + +static void find_exact_matches(struct string_list *a, struct string_list *b) +{ + struct hashmap map; + int i; + + hashmap_init(&map, (hashmap_cmp_fn)patch_util_cmp, NULL, 0); + + /* First, add the patches of a to a hash map */ + for (i = 0; i < a->nr; i++) { + struct patch_util *util = a->items[i].util; + + util->i = i; + util->patch = a->items[i].string; + util->diff = util->patch + util->diff_offset; + hashmap_entry_init(util, strhash(util->diff)); + hashmap_add(&map, util); + } + + /* Now try to find exact matches in b */ + for (i = 0; i < b->nr; i++) { + struct patch_util *util = b->items[i].util, *other; + + util->i = i; + util->patch = b->items[i].string; + util->diff = util->patch + util->diff_offset; + hashmap_entry_init(util, strhash(util->diff)); + other = hashmap_remove(&map, util, NULL); + if (other) { + if (other->matching >= 0) + BUG("already assigned!"); + + other->matching = i; + util->matching = other->i; + } + } + + hashmap_free(&map, 0); +} + +static void diffsize_consume(void *data, char *line, unsigned long len) +{ + (*(int *)data)++; +} + +static int diffsize(const char *a, const char *b) +{ + xpparam_t pp = { 0 }; + xdemitconf_t cfg = { 0 }; + mmfile_t mf1, mf2; + int count = 0; + + mf1.ptr = (char *)a; + mf1.size = strlen(a); + mf2.ptr = (char *)b; + mf2.size = strlen(b); + + cfg.ctxlen = 3; + if (!xdi_diff_outf(&mf1, &mf2, diffsize_consume, &count, &pp, &cfg)) + return count; + + error(_("failed to generate diff")); + return COST_MAX; +} + +static void get_correspondences(struct string_list *a, struct string_list *b, + int creation_factor) +{ + int n = a->nr + b->nr; + int *cost, c, *a2b, *b2a; + int i, j; + + ALLOC_ARRAY(cost, st_mult(n, n)); + ALLOC_ARRAY(a2b, n); + ALLOC_ARRAY(b2a, n); + + for (i = 0; i < a->nr; i++) { + struct patch_util *a_util = a->items[i].util; + + for (j = 0; j < b->nr; j++) { + struct patch_util *b_util = b->items[j].util; + + if (a_util->matching == j) + c = 0; + else if (a_util->matching < 0 && b_util->matching < 0) + c = diffsize(a_util->diff, b_util->diff); + else + c = COST_MAX; + cost[i + n * j] = c; + } + + c = a_util->matching < 0 ? + a_util->diffsize * creation_factor / 100 : COST_MAX; + for (j = b->nr; j < n; j++) + cost[i + n * j] = c; + } + + for (j = 0; j < b->nr; j++) { + struct patch_util *util = b->items[j].util; + + c = util->matching < 0 ? + util->diffsize * creation_factor / 100 : COST_MAX; + for (i = a->nr; i < n; i++) + cost[i + n * j] = c; + } + + for (i = a->nr; i < n; i++) + for (j = b->nr; j < n; j++) + cost[i + n * j] = 0; + + compute_assignment(n, n, cost, a2b, b2a); + + for (i = 0; i < a->nr; i++) + if (a2b[i] >= 0 && a2b[i] < b->nr) { + struct patch_util *a_util = a->items[i].util; + struct patch_util *b_util = b->items[a2b[i]].util; + + a_util->matching = a2b[i]; + b_util->matching = i; + } + + free(cost); + free(a2b); + free(b2a); +} + +static const char *short_oid(struct patch_util *util) +{ + return find_unique_abbrev(&util->oid, DEFAULT_ABBREV); +} + +static void output(struct string_list *a, struct string_list *b) +{ + int i; + + for (i = 0; i < b->nr; i++) { + struct patch_util *util = b->items[i].util, *prev; + + if (util->matching < 0) + printf("-: -------- > %d: %s\n", + i + 1, short_oid(util)); + else { + prev = a->items[util->matching].util; + printf("%d: %s ! %d: %s\n", + util->matching + 1, short_oid(prev), + i + 1, short_oid(util)); + } + } + + for (i = 0; i < a->nr; i++) { + struct patch_util *util = a->items[i].util; + + if (util->matching < 0) + printf("%d: %s < -: --------\n", + i + 1, short_oid(util)); + } +} + +int show_range_diff(const char *range1, const char *range2, + int creation_factor) +{ + int res = 0; + + struct string_list branch1 = STRING_LIST_INIT_DUP; + struct string_list branch2 = STRING_LIST_INIT_DUP; + + if (read_patches(range1, &branch1)) + res = error(_("could not parse log for '%s'"), range1); + if (!res && read_patches(range2, &branch2)) + res = error(_("could not parse log for '%s'"), range2); + + if (!res) { + find_exact_matches(&branch1, &branch2); + get_correspondences(&branch1, &branch2, creation_factor); + output(&branch1, &branch2); + } + + string_list_clear(&branch1, 1); + string_list_clear(&branch2, 1); + + return res; +} diff --git a/range-diff.h b/range-diff.h new file mode 100644 index 00000000000000..7b6eef303f5efb --- /dev/null +++ b/range-diff.h @@ -0,0 +1,7 @@ +#ifndef RANGE_DIFF_H +#define RANGE_DIFF_H + +int show_range_diff(const char *range1, const char *range2, + int creation_factor); + +#endif From 7b90919685d1e94b50f5278ec1b57a59cba1f8e5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 2 May 2018 12:22:29 +0200 Subject: [PATCH 04/21] range-diff: improve the order of the shown commits This patch lets `git range-diff` use the same order as tbdiff. The idea is simple: for left-to-right readers, it is natural to assume that the `git range-diff` is performed between an older vs a newer version of the branch. As such, the user is probably more interested in the question "where did this come from?" rather than "where did that one go?". To that end, we list the commits in the order of the second commit range ("the newer version"), inserting the unmatched commits of the first commit range as soon as all their predecessors have been shown. Signed-off-by: Johannes Schindelin --- range-diff.c | 59 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/range-diff.c b/range-diff.c index 15d418afa674c2..2d94200d3034b1 100644 --- a/range-diff.c +++ b/range-diff.c @@ -12,7 +12,7 @@ struct patch_util { struct hashmap_entry e; const char *diff, *patch; - int i; + int i, shown; int diffsize; size_t diff_offset; /* the index of the matching item in the other branch, or -1 */ @@ -260,28 +260,49 @@ static const char *short_oid(struct patch_util *util) static void output(struct string_list *a, struct string_list *b) { - int i; - - for (i = 0; i < b->nr; i++) { - struct patch_util *util = b->items[i].util, *prev; + int i = 0, j = 0; + + /* + * We assume the user is really more interested in the second argument + * ("newer" version). To that end, we print the output in the order of + * the RHS (the `b` parameter). To put the LHS (the `a` parameter) + * commits that are no longer in the RHS into a good place, we place + * them once we have shown all of their predecessors in the LHS. + */ + + while (i < a->nr || j < b->nr) { + struct patch_util *a_util, *b_util; + a_util = i < a->nr ? a->items[i].util : NULL; + b_util = j < b->nr ? b->items[j].util : NULL; + + /* Skip all the already-shown commits from the LHS. */ + while (i < a->nr && a_util->shown) + a_util = ++i < a->nr ? a->items[i].util : NULL; + + /* Show unmatched LHS commit whose predecessors were shown. */ + if (i < a->nr && a_util->matching < 0) { + printf("%d: %s < -: --------\n", + i + 1, short_oid(a_util)); + i++; + continue; + } - if (util->matching < 0) + /* Show unmatched RHS commits. */ + while (j < b->nr && b_util->matching < 0) { printf("-: -------- > %d: %s\n", - i + 1, short_oid(util)); - else { - prev = a->items[util->matching].util; - printf("%d: %s ! %d: %s\n", - util->matching + 1, short_oid(prev), - i + 1, short_oid(util)); + j + 1, short_oid(b_util)); + b_util = ++j < b->nr ? b->items[j].util : NULL; } - } - - for (i = 0; i < a->nr; i++) { - struct patch_util *util = a->items[i].util; - if (util->matching < 0) - printf("%d: %s < -: --------\n", - i + 1, short_oid(util)); + /* Show matching LHS/RHS pair. */ + if (j < b->nr) { + a_util = a->items[b_util->matching].util; + printf("%d: %s ! %d: %s\n", + b_util->matching + 1, short_oid(a_util), + j + 1, short_oid(b_util)); + a_util->shown = 1; + j++; + } } } From 8515d2f75c5439bcc38ea1a9f5a57b89cd8945d7 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 6 May 2018 17:26:19 +0200 Subject: [PATCH 05/21] range-diff: also show the diff between patches Just like tbdiff, we now show the diff between matching patches. This is a "diff of two diffs", so it can be a bit daunting to read for the beginner. An alternative would be to display an interdiff, i.e. the hypothetical diff which is the result of first reverting the old diff and then applying the new diff. Especially when rebasing frequently, an interdiff is often not feasible, though: if the old diff cannot be applied in reverse (due to a moving upstream), an interdiff can simply not be inferred. This commit brings `range-diff` closer to feature parity with regard to tbdiff. To make `git range-diff` respect e.g. color.diff.* settings, we have to adjust git_branch_config() accordingly. Note: while we now parse diff options such as --color, the effect is not yet the same as in tbdiff, where also the commit pairs would be colored. This is left for a later commit. Note also: while tbdiff accepts the `--no-patches` option to suppress these diffs between patches, we prefer the `-s` (or `--no-patch`) option that is automatically supported via our use of diff_opt_parse(). And finally note: to support diff options, we have to call `parse_options()` such that it keeps unknown options, and then loop over those and let `diff_opt_parse()` handle them. After that loop, we have to call `parse_options()` again, to make sure that no unknown options are left. Helped-by: Thomas Gummerer Helped-by: Eric Sunshine Signed-off-by: Johannes Schindelin --- builtin/range-diff.c | 31 +++++++++++++++++++++++++++++-- range-diff.c | 34 +++++++++++++++++++++++++++++++--- range-diff.h | 4 +++- 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/builtin/range-diff.c b/builtin/range-diff.c index 94c1f362cc4fee..3b06ed9449a391 100644 --- a/builtin/range-diff.c +++ b/builtin/range-diff.c @@ -2,6 +2,7 @@ #include "builtin.h" #include "parse-options.h" #include "range-diff.h" +#include "config.h" static const char * const builtin_range_diff_usage[] = { N_("git range-diff [] .. .."), @@ -13,15 +14,40 @@ NULL int cmd_range_diff(int argc, const char **argv, const char *prefix) { int creation_factor = 60; + struct diff_options diffopt = { NULL }; struct option options[] = { OPT_INTEGER(0, "creation-factor", &creation_factor, N_("Percentage by which creation is weighted")), OPT_END() }; - int res = 0; + int i, j, res = 0; struct strbuf range1 = STRBUF_INIT, range2 = STRBUF_INIT; + git_config(git_diff_ui_config, NULL); + + diff_setup(&diffopt); + diffopt.output_format = DIFF_FORMAT_PATCH; + argc = parse_options(argc, argv, NULL, options, + builtin_range_diff_usage, PARSE_OPT_KEEP_UNKNOWN | + PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0); + + for (i = j = 1; i < argc && strcmp("--", argv[i]); ) { + int c = diff_opt_parse(&diffopt, argv + i, argc - i, prefix); + + if (!c) + argv[j++] = argv[i++]; + else + i += c; + } + while (i < argc) + argv[j++] = argv[i++]; + argc = j; + diff_setup_done(&diffopt); + + /* Make sure that there are no unparsed options */ + argc = parse_options(argc, argv, NULL, + options + ARRAY_SIZE(options) - 1, /* OPT_END */ builtin_range_diff_usage, 0); if (argc == 2) { @@ -59,7 +85,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) usage_with_options(builtin_range_diff_usage, options); } - res = show_range_diff(range1.buf, range2.buf, creation_factor); + res = show_range_diff(range1.buf, range2.buf, creation_factor, + &diffopt); strbuf_release(&range1); strbuf_release(&range2); diff --git a/range-diff.c b/range-diff.c index 2d94200d3034b1..71883a4b7d06dd 100644 --- a/range-diff.c +++ b/range-diff.c @@ -6,6 +6,7 @@ #include "hashmap.h" #include "xdiff-interface.h" #include "linear-assignment.h" +#include "diffcore.h" struct patch_util { /* For the search for an exact match */ @@ -258,7 +259,31 @@ static const char *short_oid(struct patch_util *util) return find_unique_abbrev(&util->oid, DEFAULT_ABBREV); } -static void output(struct string_list *a, struct string_list *b) +static struct diff_filespec *get_filespec(const char *name, const char *p) +{ + struct diff_filespec *spec = alloc_filespec(name); + + fill_filespec(spec, &null_oid, 0, 0644); + spec->data = (char *)p; + spec->size = strlen(p); + spec->should_munmap = 0; + spec->is_stdin = 1; + + return spec; +} + +static void patch_diff(const char *a, const char *b, + struct diff_options *diffopt) +{ + diff_queue(&diff_queued_diff, + get_filespec("a", a), get_filespec("b", b)); + + diffcore_std(diffopt); + diff_flush(diffopt); +} + +static void output(struct string_list *a, struct string_list *b, + struct diff_options *diffopt) { int i = 0, j = 0; @@ -300,6 +325,9 @@ static void output(struct string_list *a, struct string_list *b) printf("%d: %s ! %d: %s\n", b_util->matching + 1, short_oid(a_util), j + 1, short_oid(b_util)); + if (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT)) + patch_diff(a->items[b_util->matching].string, + b->items[j].string, diffopt); a_util->shown = 1; j++; } @@ -307,7 +335,7 @@ static void output(struct string_list *a, struct string_list *b) } int show_range_diff(const char *range1, const char *range2, - int creation_factor) + int creation_factor, struct diff_options *diffopt) { int res = 0; @@ -322,7 +350,7 @@ int show_range_diff(const char *range1, const char *range2, if (!res) { find_exact_matches(&branch1, &branch2); get_correspondences(&branch1, &branch2, creation_factor); - output(&branch1, &branch2); + output(&branch1, &branch2, diffopt); } string_list_clear(&branch1, 1); diff --git a/range-diff.h b/range-diff.h index 7b6eef303f5efb..2407d46a301e55 100644 --- a/range-diff.h +++ b/range-diff.h @@ -1,7 +1,9 @@ #ifndef RANGE_DIFF_H #define RANGE_DIFF_H +#include "diff.h" + int show_range_diff(const char *range1, const char *range2, - int creation_factor); + int creation_factor, struct diff_options *diffopt); #endif From a10ca01633c65bdae66f10af93075fe0400ccb5b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 2 May 2018 16:49:09 +0200 Subject: [PATCH 06/21] range-diff: right-trim commit messages When comparing commit messages, we need to keep in mind that they are indented by four spaces. That is, empty lines are no longer empty, but have "trailing whitespace". When displaying them in color, that results in those nagging red lines. Let's just right-trim the lines in the commit message, it's not like trailing white-space in the commit messages are important enough to care about in `git range-diff`. Signed-off-by: Johannes Schindelin --- range-diff.c | 1 + 1 file changed, 1 insertion(+) diff --git a/range-diff.c b/range-diff.c index 71883a4b7d06dd..1ecee2c09c43e3 100644 --- a/range-diff.c +++ b/range-diff.c @@ -85,6 +85,7 @@ static int read_patches(const char *range, struct string_list *list) strbuf_addbuf(&buf, &line); strbuf_addstr(&buf, "\n\n"); } else if (starts_with(line.buf, " ")) { + strbuf_rtrim(&line); strbuf_addbuf(&buf, &line); strbuf_addch(&buf, '\n'); } From f81cbef2c7946bac5e3f6c08a9b937eb05450cae Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 2 May 2018 16:52:21 +0200 Subject: [PATCH 07/21] range-diff: indent the diffs just like tbdiff The main information in the `range-diff` view comes from the list of matching and non-matching commits, the diffs are additional information. Indenting them helps with the reading flow. Signed-off-by: Johannes Schindelin --- builtin/range-diff.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/builtin/range-diff.c b/builtin/range-diff.c index 3b06ed9449a391..f0598005aece52 100644 --- a/builtin/range-diff.c +++ b/builtin/range-diff.c @@ -11,6 +11,11 @@ N_("git range-diff [] "), NULL }; +static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data) +{ + return data; +} + int cmd_range_diff(int argc, const char **argv, const char *prefix) { int creation_factor = 60; @@ -21,12 +26,16 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) OPT_END() }; int i, j, res = 0; + struct strbuf four_spaces = STRBUF_INIT; struct strbuf range1 = STRBUF_INIT, range2 = STRBUF_INIT; git_config(git_diff_ui_config, NULL); diff_setup(&diffopt); diffopt.output_format = DIFF_FORMAT_PATCH; + diffopt.output_prefix = output_prefix_cb; + strbuf_addstr(&four_spaces, " "); + diffopt.output_prefix_data = &four_spaces; argc = parse_options(argc, argv, NULL, options, builtin_range_diff_usage, PARSE_OPT_KEEP_UNKNOWN | @@ -90,6 +99,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) strbuf_release(&range1); strbuf_release(&range2); + strbuf_release(&four_spaces); return res; } From 458090ffd23115545c999aeef952e2e29ee628f0 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 2 May 2018 16:53:50 +0200 Subject: [PATCH 08/21] range-diff: suppress the diff headers When showing the diff between corresponding patches of the two branch versions, we have to make up a fake filename to run the diff machinery. That filename does not carry any meaningful information, hence tbdiff suppresses it. So we should, too. Signed-off-by: Johannes Schindelin --- builtin/range-diff.c | 1 + diff.c | 5 ++++- diff.h | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/builtin/range-diff.c b/builtin/range-diff.c index f0598005aece52..76659d0b3f0385 100644 --- a/builtin/range-diff.c +++ b/builtin/range-diff.c @@ -33,6 +33,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) diff_setup(&diffopt); diffopt.output_format = DIFF_FORMAT_PATCH; + diffopt.flags.suppress_diff_headers = 1; diffopt.output_prefix = output_prefix_cb; strbuf_addstr(&four_spaces, " "); diffopt.output_prefix_data = &four_spaces; diff --git a/diff.c b/diff.c index 04d044bbb67b77..9c4bd9fa11d658 100644 --- a/diff.c +++ b/diff.c @@ -3395,13 +3395,16 @@ static void builtin_diff(const char *name_a, memset(&xpp, 0, sizeof(xpp)); memset(&xecfg, 0, sizeof(xecfg)); memset(&ecbdata, 0, sizeof(ecbdata)); + if (o->flags.suppress_diff_headers) + lbl[0] = NULL; ecbdata.label_path = lbl; ecbdata.color_diff = want_color(o->use_color); ecbdata.ws_rule = whitespace_rule(name_b); if (ecbdata.ws_rule & WS_BLANK_AT_EOF) check_blank_at_eof(&mf1, &mf2, &ecbdata); ecbdata.opt = o; - ecbdata.header = header.len ? &header : NULL; + if (header.len && !o->flags.suppress_diff_headers) + ecbdata.header = &header; xpp.flags = o->xdl_opts; xpp.anchors = o->anchors; xpp.anchors_nr = o->anchors_nr; diff --git a/diff.h b/diff.h index a14895bb824f2c..d88ceb3570fda3 100644 --- a/diff.h +++ b/diff.h @@ -94,6 +94,7 @@ struct diff_flags { unsigned funccontext:1; unsigned default_follow_renames:1; unsigned stat_with_summary:1; + unsigned suppress_diff_headers:1; }; static inline void diff_flags_or(struct diff_flags *a, From d3be03a446a40d2176b6ffdd9e095d97873042c2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 2 May 2018 23:35:48 +0200 Subject: [PATCH 09/21] range-diff: adjust the output of the commit pairs This not only uses "dashed stand-ins" for "pairs" where one side is missing (i.e. unmatched commits that are present only in one of the two commit ranges), but also adds onelines for the reader's pleasure. This change brings `git range-diff` yet another step closer to feature parity with tbdiff: it now shows the oneline, too, and indicates with `=` when the commits have identical diffs. Signed-off-by: Johannes Schindelin --- range-diff.c | 59 ++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 9 deletions(-) diff --git a/range-diff.c b/range-diff.c index 1ecee2c09c43e3..23aa61af594189 100644 --- a/range-diff.c +++ b/range-diff.c @@ -7,6 +7,8 @@ #include "xdiff-interface.h" #include "linear-assignment.h" #include "diffcore.h" +#include "commit.h" +#include "pretty.h" struct patch_util { /* For the search for an exact match */ @@ -255,9 +257,49 @@ static void get_correspondences(struct string_list *a, struct string_list *b, free(b2a); } -static const char *short_oid(struct patch_util *util) +static void output_pair_header(struct strbuf *buf, + struct strbuf *dashes, + struct patch_util *a_util, + struct patch_util *b_util) { - return find_unique_abbrev(&util->oid, DEFAULT_ABBREV); + struct object_id *oid = a_util ? &a_util->oid : &b_util->oid; + struct commit *commit; + + if (!dashes->len) + strbuf_addchars(dashes, '-', + strlen(find_unique_abbrev(oid, + DEFAULT_ABBREV))); + + strbuf_reset(buf); + if (!a_util) + strbuf_addf(buf, "-: %s ", dashes->buf); + else + strbuf_addf(buf, "%d: %s ", a_util->i + 1, + find_unique_abbrev(&a_util->oid, DEFAULT_ABBREV)); + + if (!a_util) + strbuf_addch(buf, '>'); + else if (!b_util) + strbuf_addch(buf, '<'); + else if (strcmp(a_util->patch, b_util->patch)) + strbuf_addch(buf, '!'); + else + strbuf_addch(buf, '='); + + if (!b_util) + strbuf_addf(buf, " -: %s", dashes->buf); + else + strbuf_addf(buf, " %d: %s", b_util->i + 1, + find_unique_abbrev(&b_util->oid, DEFAULT_ABBREV)); + + commit = lookup_commit_reference(the_repository, oid); + if (commit) { + strbuf_addch(buf, ' '); + pp_commit_easy(CMIT_FMT_ONELINE, commit, buf); + } + strbuf_addch(buf, '\n'); + + fwrite(buf->buf, buf->len, 1, stdout); } static struct diff_filespec *get_filespec(const char *name, const char *p) @@ -286,6 +328,7 @@ static void patch_diff(const char *a, const char *b, static void output(struct string_list *a, struct string_list *b, struct diff_options *diffopt) { + struct strbuf buf = STRBUF_INIT, dashes = STRBUF_INIT; int i = 0, j = 0; /* @@ -307,25 +350,21 @@ static void output(struct string_list *a, struct string_list *b, /* Show unmatched LHS commit whose predecessors were shown. */ if (i < a->nr && a_util->matching < 0) { - printf("%d: %s < -: --------\n", - i + 1, short_oid(a_util)); + output_pair_header(&buf, &dashes, a_util, NULL); i++; continue; } /* Show unmatched RHS commits. */ while (j < b->nr && b_util->matching < 0) { - printf("-: -------- > %d: %s\n", - j + 1, short_oid(b_util)); + output_pair_header(&buf, &dashes, NULL, b_util); b_util = ++j < b->nr ? b->items[j].util : NULL; } /* Show matching LHS/RHS pair. */ if (j < b->nr) { a_util = a->items[b_util->matching].util; - printf("%d: %s ! %d: %s\n", - b_util->matching + 1, short_oid(a_util), - j + 1, short_oid(b_util)); + output_pair_header(&buf, &dashes, a_util, b_util); if (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT)) patch_diff(a->items[b_util->matching].string, b->items[j].string, diffopt); @@ -333,6 +372,8 @@ static void output(struct string_list *a, struct string_list *b, j++; } } + strbuf_release(&buf); + strbuf_release(&dashes); } int show_range_diff(const char *range1, const char *range2, From 94b44dfe6859fd79803d682885a6a52ad800eedd Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 6 May 2018 17:35:03 +0200 Subject: [PATCH 10/21] range-diff: do not show "function names" in hunk headers We are comparing complete, formatted commit messages with patches. There are no function names here, so stop looking for them. Signed-off-by: Johannes Schindelin --- range-diff.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/range-diff.c b/range-diff.c index 23aa61af594189..6d75563f466d55 100644 --- a/range-diff.c +++ b/range-diff.c @@ -9,6 +9,7 @@ #include "diffcore.h" #include "commit.h" #include "pretty.h" +#include "userdiff.h" struct patch_util { /* For the search for an exact match */ @@ -302,6 +303,10 @@ static void output_pair_header(struct strbuf *buf, fwrite(buf->buf, buf->len, 1, stdout); } +static struct userdiff_driver no_func_name = { + .funcname = { "$^", 0 } +}; + static struct diff_filespec *get_filespec(const char *name, const char *p) { struct diff_filespec *spec = alloc_filespec(name); @@ -311,6 +316,7 @@ static struct diff_filespec *get_filespec(const char *name, const char *p) spec->size = strlen(p); spec->should_munmap = 0; spec->is_stdin = 1; + spec->driver = &no_func_name; return spec; } From 1477c58e4c0c9e1d5836ffcb9bcb7360b1a5df0d Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Wed, 2 May 2018 17:19:37 +0200 Subject: [PATCH 11/21] range-diff: add tests These are essentially lifted from https://github.com/trast/tbdiff, with light touch-ups to account for the command now being named `git range-diff`. Apart from renaming `tbdiff` to `range-diff`, only one test case needed to be adjusted: 11 - 'changed message'. The underlying reason it had to be adjusted is that diff generation is sometimes ambiguous. In this case, a comment line and an empty line are added, but it is ambiguous whether they were added after the existing empty line, or whether an empty line and the comment line are added *before* the existing empty line. And apparently xdiff picks a different option here than Python's difflib. Signed-off-by: Johannes Schindelin --- t/.gitattributes | 1 + t/t3206-range-diff.sh | 145 ++++++++++ t/t3206/history.export | 604 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 750 insertions(+) create mode 100755 t/t3206-range-diff.sh create mode 100644 t/t3206/history.export diff --git a/t/.gitattributes b/t/.gitattributes index 3bd959ae523cff..b17bf71b84e12c 100644 --- a/t/.gitattributes +++ b/t/.gitattributes @@ -1,6 +1,7 @@ t[0-9][0-9][0-9][0-9]/* -whitespace /diff-lib/* eol=lf /t0110/url-* binary +/t3206/* eol=lf /t3900/*.txt eol=lf /t3901/*.txt eol=lf /t4034/*/* eol=lf diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh new file mode 100755 index 00000000000000..2237c7f4af9464 --- /dev/null +++ b/t/t3206-range-diff.sh @@ -0,0 +1,145 @@ +#!/bin/sh + +test_description='range-diff tests' + +. ./test-lib.sh + +# Note that because of the range-diff's heuristics, test_commit does more +# harm than good. We need some real history. + +test_expect_success 'setup' ' + git fast-import < "$TEST_DIRECTORY"/t3206/history.export +' + +test_expect_success 'simple A..B A..C (unmodified)' ' + git range-diff --no-color master..topic master..unmodified \ + >actual && + cat >expected <<-EOF && + 1: 4de457d = 1: 35b9b25 s/5/A/ + 2: fccce22 = 2: de345ab s/4/A/ + 3: 147e64e = 3: 9af6654 s/11/B/ + 4: a63e992 = 4: 2901f77 s/12/B/ + EOF + test_cmp expected actual +' + +test_expect_success 'simple B...C (unmodified)' ' + git range-diff --no-color topic...unmodified >actual && + # same "expected" as above + test_cmp expected actual +' + +test_expect_success 'simple A B C (unmodified)' ' + git range-diff --no-color master topic unmodified >actual && + # same "expected" as above + test_cmp expected actual +' + +test_expect_success 'trivial reordering' ' + git range-diff --no-color master topic reordered >actual && + cat >expected <<-EOF && + 1: 4de457d = 1: aca177a s/5/A/ + 3: 147e64e = 2: 14ad629 s/11/B/ + 4: a63e992 = 3: ee58208 s/12/B/ + 2: fccce22 = 4: 307b27a s/4/A/ + EOF + test_cmp expected actual +' + +test_expect_success 'removed a commit' ' + git range-diff --no-color master topic removed >actual && + cat >expected <<-EOF && + 1: 4de457d = 1: 7657159 s/5/A/ + 2: fccce22 < -: ------- s/4/A/ + 3: 147e64e = 2: 43d84d3 s/11/B/ + 4: a63e992 = 3: a740396 s/12/B/ + EOF + test_cmp expected actual +' + +test_expect_success 'added a commit' ' + git range-diff --no-color master topic added >actual && + cat >expected <<-EOF && + 1: 4de457d = 1: 2716022 s/5/A/ + 2: fccce22 = 2: b62accd s/4/A/ + -: ------- > 3: df46cfa s/6/A/ + 3: 147e64e = 4: 3e64548 s/11/B/ + 4: a63e992 = 5: 12b4063 s/12/B/ + EOF + test_cmp expected actual +' + +test_expect_success 'new base, A B C' ' + git range-diff --no-color master topic rebased >actual && + cat >expected <<-EOF && + 1: 4de457d = 1: cc9c443 s/5/A/ + 2: fccce22 = 2: c5d9641 s/4/A/ + 3: 147e64e = 3: 28cc2b6 s/11/B/ + 4: a63e992 = 4: 5628ab7 s/12/B/ + EOF + test_cmp expected actual +' + +test_expect_success 'new base, B...C' ' + # this syntax includes the commits from master! + git range-diff --no-color topic...rebased >actual && + cat >expected <<-EOF && + -: ------- > 1: a31b12e unrelated + 1: 4de457d = 2: cc9c443 s/5/A/ + 2: fccce22 = 3: c5d9641 s/4/A/ + 3: 147e64e = 4: 28cc2b6 s/11/B/ + 4: a63e992 = 5: 5628ab7 s/12/B/ + EOF + test_cmp expected actual +' + +test_expect_success 'changed commit' ' + git range-diff --no-color topic...changed >actual && + cat >expected <<-EOF && + 1: 4de457d = 1: a4b3333 s/5/A/ + 2: fccce22 = 2: f51d370 s/4/A/ + 3: 147e64e ! 3: 0559556 s/11/B/ + @@ -10,7 +10,7 @@ + 9 + 10 + -11 + -+B + ++BB + 12 + 13 + 14 + 4: a63e992 ! 4: d966c5c s/12/B/ + @@ -8,7 +8,7 @@ + @@ + 9 + 10 + - B + + BB + -12 + +B + 13 + EOF + test_cmp expected actual +' + +test_expect_success 'changed message' ' + git range-diff --no-color topic...changed-message >actual && + sed s/Z/\ /g >expected <<-EOF && + 1: 4de457d = 1: f686024 s/5/A/ + 2: fccce22 ! 2: 4ab067d s/4/A/ + @@ -2,6 +2,8 @@ + Z + Z s/4/A/ + Z + + Also a silly comment here! + + + Zdiff --git a/file b/file + Z--- a/file + Z+++ b/file + 3: 147e64e = 3: b9cb956 s/11/B/ + 4: a63e992 = 4: 8add5f1 s/12/B/ + EOF + test_cmp expected actual +' + +test_done diff --git a/t/t3206/history.export b/t/t3206/history.export new file mode 100644 index 00000000000000..b8ffff0940d6f1 --- /dev/null +++ b/t/t3206/history.export @@ -0,0 +1,604 @@ +blob +mark :1 +data 51 +1 +2 +3 +4 +5 +6 +7 +8 +9 +10 +11 +12 +13 +14 +15 +16 +17 +18 +19 +20 + +reset refs/heads/removed +commit refs/heads/removed +mark :2 +author Thomas Rast 1374424921 +0200 +committer Thomas Rast 1374484724 +0200 +data 8 +initial +M 100644 :1 file + +blob +mark :3 +data 51 +1 +2 +3 +4 +A +6 +7 +8 +9 +10 +11 +12 +13 +14 +15 +16 +17 +18 +19 +20 + +commit refs/heads/topic +mark :4 +author Thomas Rast 1374485014 +0200 +committer Thomas Rast 1374485014 +0200 +data 7 +s/5/A/ +from :2 +M 100644 :3 file + +blob +mark :5 +data 51 +1 +2 +3 +A +A +6 +7 +8 +9 +10 +11 +12 +13 +14 +15 +16 +17 +18 +19 +20 + +commit refs/heads/topic +mark :6 +author Thomas Rast 1374485024 +0200 +committer Thomas Rast 1374485024 +0200 +data 7 +s/4/A/ +from :4 +M 100644 :5 file + +blob +mark :7 +data 50 +1 +2 +3 +A +A +6 +7 +8 +9 +10 +B +12 +13 +14 +15 +16 +17 +18 +19 +20 + +commit refs/heads/topic +mark :8 +author Thomas Rast 1374485036 +0200 +committer Thomas Rast 1374485036 +0200 +data 8 +s/11/B/ +from :6 +M 100644 :7 file + +blob +mark :9 +data 49 +1 +2 +3 +A +A +6 +7 +8 +9 +10 +B +B +13 +14 +15 +16 +17 +18 +19 +20 + +commit refs/heads/topic +mark :10 +author Thomas Rast 1374485044 +0200 +committer Thomas Rast 1374485044 +0200 +data 8 +s/12/B/ +from :8 +M 100644 :9 file + +blob +mark :11 +data 10 +unrelated + +commit refs/heads/master +mark :12 +author Thomas Rast 1374485127 +0200 +committer Thomas Rast 1374485127 +0200 +data 10 +unrelated +from :2 +M 100644 :11 otherfile + +commit refs/heads/rebased +mark :13 +author Thomas Rast 1374485014 +0200 +committer Thomas Rast 1374485137 +0200 +data 7 +s/5/A/ +from :12 +M 100644 :3 file + +commit refs/heads/rebased +mark :14 +author Thomas Rast 1374485024 +0200 +committer Thomas Rast 1374485138 +0200 +data 7 +s/4/A/ +from :13 +M 100644 :5 file + +commit refs/heads/rebased +mark :15 +author Thomas Rast 1374485036 +0200 +committer Thomas Rast 1374485138 +0200 +data 8 +s/11/B/ +from :14 +M 100644 :7 file + +commit refs/heads/rebased +mark :16 +author Thomas Rast 1374485044 +0200 +committer Thomas Rast 1374485138 +0200 +data 8 +s/12/B/ +from :15 +M 100644 :9 file + +commit refs/heads/added +mark :17 +author Thomas Rast 1374485014 +0200 +committer Thomas Rast 1374485341 +0200 +data 7 +s/5/A/ +from :2 +M 100644 :3 file + +commit refs/heads/added +mark :18 +author Thomas Rast 1374485024 +0200 +committer Thomas Rast 1374485341 +0200 +data 7 +s/4/A/ +from :17 +M 100644 :5 file + +blob +mark :19 +data 51 +1 +2 +3 +A +A +A +7 +8 +9 +10 +11 +12 +13 +14 +15 +16 +17 +18 +19 +20 + +commit refs/heads/added +mark :20 +author Thomas Rast 1374485186 +0200 +committer Thomas Rast 1374485341 +0200 +data 7 +s/6/A/ +from :18 +M 100644 :19 file + +blob +mark :21 +data 50 +1 +2 +3 +A +A +A +7 +8 +9 +10 +B +12 +13 +14 +15 +16 +17 +18 +19 +20 + +commit refs/heads/added +mark :22 +author Thomas Rast 1374485036 +0200 +committer Thomas Rast 1374485341 +0200 +data 8 +s/11/B/ +from :20 +M 100644 :21 file + +blob +mark :23 +data 49 +1 +2 +3 +A +A +A +7 +8 +9 +10 +B +B +13 +14 +15 +16 +17 +18 +19 +20 + +commit refs/heads/added +mark :24 +author Thomas Rast 1374485044 +0200 +committer Thomas Rast 1374485341 +0200 +data 8 +s/12/B/ +from :22 +M 100644 :23 file + +commit refs/heads/reordered +mark :25 +author Thomas Rast 1374485014 +0200 +committer Thomas Rast 1374485350 +0200 +data 7 +s/5/A/ +from :2 +M 100644 :3 file + +blob +mark :26 +data 50 +1 +2 +3 +4 +A +6 +7 +8 +9 +10 +B +12 +13 +14 +15 +16 +17 +18 +19 +20 + +commit refs/heads/reordered +mark :27 +author Thomas Rast 1374485036 +0200 +committer Thomas Rast 1374485350 +0200 +data 8 +s/11/B/ +from :25 +M 100644 :26 file + +blob +mark :28 +data 49 +1 +2 +3 +4 +A +6 +7 +8 +9 +10 +B +B +13 +14 +15 +16 +17 +18 +19 +20 + +commit refs/heads/reordered +mark :29 +author Thomas Rast 1374485044 +0200 +committer Thomas Rast 1374485350 +0200 +data 8 +s/12/B/ +from :27 +M 100644 :28 file + +commit refs/heads/reordered +mark :30 +author Thomas Rast 1374485024 +0200 +committer Thomas Rast 1374485350 +0200 +data 7 +s/4/A/ +from :29 +M 100644 :9 file + +commit refs/heads/changed +mark :31 +author Thomas Rast 1374485014 +0200 +committer Thomas Rast 1374485507 +0200 +data 7 +s/5/A/ +from :2 +M 100644 :3 file + +commit refs/heads/changed +mark :32 +author Thomas Rast 1374485024 +0200 +committer Thomas Rast 1374485507 +0200 +data 7 +s/4/A/ +from :31 +M 100644 :5 file + +blob +mark :33 +data 51 +1 +2 +3 +A +A +6 +7 +8 +9 +10 +BB +12 +13 +14 +15 +16 +17 +18 +19 +20 + +commit refs/heads/changed +mark :34 +author Thomas Rast 1374485036 +0200 +committer Thomas Rast 1374485507 +0200 +data 8 +s/11/B/ +from :32 +M 100644 :33 file + +blob +mark :35 +data 50 +1 +2 +3 +A +A +6 +7 +8 +9 +10 +BB +B +13 +14 +15 +16 +17 +18 +19 +20 + +commit refs/heads/changed +mark :36 +author Thomas Rast 1374485044 +0200 +committer Thomas Rast 1374485507 +0200 +data 8 +s/12/B/ +from :34 +M 100644 :35 file + +commit refs/heads/changed-message +mark :37 +author Thomas Rast 1374485014 +0200 +committer Thomas Rast 1374485530 +0200 +data 7 +s/5/A/ +from :2 +M 100644 :3 file + +commit refs/heads/changed-message +mark :38 +author Thomas Rast 1374485024 +0200 +committer Thomas Rast 1374485530 +0200 +data 35 +s/4/A/ + +Also a silly comment here! +from :37 +M 100644 :5 file + +commit refs/heads/changed-message +mark :39 +author Thomas Rast 1374485036 +0200 +committer Thomas Rast 1374485536 +0200 +data 8 +s/11/B/ +from :38 +M 100644 :7 file + +commit refs/heads/changed-message +mark :40 +author Thomas Rast 1374485044 +0200 +committer Thomas Rast 1374485536 +0200 +data 8 +s/12/B/ +from :39 +M 100644 :9 file + +commit refs/heads/unmodified +mark :41 +author Thomas Rast 1374485014 +0200 +committer Thomas Rast 1374485631 +0200 +data 7 +s/5/A/ +from :2 +M 100644 :3 file + +commit refs/heads/unmodified +mark :42 +author Thomas Rast 1374485024 +0200 +committer Thomas Rast 1374485631 +0200 +data 7 +s/4/A/ +from :41 +M 100644 :5 file + +commit refs/heads/unmodified +mark :43 +author Thomas Rast 1374485036 +0200 +committer Thomas Rast 1374485632 +0200 +data 8 +s/11/B/ +from :42 +M 100644 :7 file + +commit refs/heads/unmodified +mark :44 +author Thomas Rast 1374485044 +0200 +committer Thomas Rast 1374485632 +0200 +data 8 +s/12/B/ +from :43 +M 100644 :9 file + +commit refs/heads/removed +mark :45 +author Thomas Rast 1374485014 +0200 +committer Thomas Rast 1374486061 +0200 +data 7 +s/5/A/ +from :2 +M 100644 :3 file + +commit refs/heads/removed +mark :46 +author Thomas Rast 1374485036 +0200 +committer Thomas Rast 1374486061 +0200 +data 8 +s/11/B/ +from :45 +M 100644 :26 file + +commit refs/heads/removed +mark :47 +author Thomas Rast 1374485044 +0200 +committer Thomas Rast 1374486061 +0200 +data 8 +s/12/B/ +from :46 +M 100644 :28 file + +reset refs/heads/removed +from :47 + From 32492c159b4fc02d690a3034d1a5deb21fcdd2ed Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 May 2018 01:32:19 +0200 Subject: [PATCH 12/21] range-diff: use color for the commit pairs Arguably the most important part of `git range-diff`'s output is the list of commits in the two branches, together with their relationships. For that reason, tbdiff introduced color-coding that is pretty intuitive, especially for unchanged patches (all dim yellow, like the first line in `git show`'s output) vs modified patches (old commit is red, new commit is green). Let's imitate that color scheme. Signed-off-by: Johannes Schindelin --- range-diff.c | 51 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/range-diff.c b/range-diff.c index 6d75563f466d55..b1663da7cd3068 100644 --- a/range-diff.c +++ b/range-diff.c @@ -258,34 +258,53 @@ static void get_correspondences(struct string_list *a, struct string_list *b, free(b2a); } -static void output_pair_header(struct strbuf *buf, +static void output_pair_header(struct diff_options *diffopt, + struct strbuf *buf, struct strbuf *dashes, struct patch_util *a_util, struct patch_util *b_util) { struct object_id *oid = a_util ? &a_util->oid : &b_util->oid; struct commit *commit; + char status; + const char *color_reset = diff_get_color_opt(diffopt, DIFF_RESET); + const char *color_old = diff_get_color_opt(diffopt, DIFF_FILE_OLD); + const char *color_new = diff_get_color_opt(diffopt, DIFF_FILE_NEW); + const char *color_commit = diff_get_color_opt(diffopt, DIFF_COMMIT); + const char *color; if (!dashes->len) strbuf_addchars(dashes, '-', strlen(find_unique_abbrev(oid, DEFAULT_ABBREV))); + if (!b_util) { + color = color_old; + status = '<'; + } else if (!a_util) { + color = color_new; + status = '>'; + } else if (strcmp(a_util->patch, b_util->patch)) { + color = color_commit; + status = '!'; + } else { + color = color_commit; + status = '='; + } + strbuf_reset(buf); + strbuf_addstr(buf, status == '!' ? color_old : color); if (!a_util) strbuf_addf(buf, "-: %s ", dashes->buf); else strbuf_addf(buf, "%d: %s ", a_util->i + 1, find_unique_abbrev(&a_util->oid, DEFAULT_ABBREV)); - if (!a_util) - strbuf_addch(buf, '>'); - else if (!b_util) - strbuf_addch(buf, '<'); - else if (strcmp(a_util->patch, b_util->patch)) - strbuf_addch(buf, '!'); - else - strbuf_addch(buf, '='); + if (status == '!') + strbuf_addf(buf, "%s%s", color_reset, color); + strbuf_addch(buf, status); + if (status == '!') + strbuf_addf(buf, "%s%s", color_reset, color_new); if (!b_util) strbuf_addf(buf, " -: %s", dashes->buf); @@ -295,10 +314,13 @@ static void output_pair_header(struct strbuf *buf, commit = lookup_commit_reference(the_repository, oid); if (commit) { + if (status == '!') + strbuf_addf(buf, "%s%s", color_reset, color); + strbuf_addch(buf, ' '); pp_commit_easy(CMIT_FMT_ONELINE, commit, buf); } - strbuf_addch(buf, '\n'); + strbuf_addf(buf, "%s\n", color_reset); fwrite(buf->buf, buf->len, 1, stdout); } @@ -356,21 +378,24 @@ static void output(struct string_list *a, struct string_list *b, /* Show unmatched LHS commit whose predecessors were shown. */ if (i < a->nr && a_util->matching < 0) { - output_pair_header(&buf, &dashes, a_util, NULL); + output_pair_header(diffopt, + &buf, &dashes, a_util, NULL); i++; continue; } /* Show unmatched RHS commits. */ while (j < b->nr && b_util->matching < 0) { - output_pair_header(&buf, &dashes, NULL, b_util); + output_pair_header(diffopt, + &buf, &dashes, NULL, b_util); b_util = ++j < b->nr ? b->items[j].util : NULL; } /* Show matching LHS/RHS pair. */ if (j < b->nr) { a_util = a->items[b_util->matching].util; - output_pair_header(&buf, &dashes, a_util, b_util); + output_pair_header(diffopt, + &buf, &dashes, a_util, b_util); if (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT)) patch_diff(a->items[b_util->matching].string, b->items[j].string, diffopt); From 969a196f48be31e72803d673981a1fa8ad1e3f4b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 May 2018 02:14:31 +0200 Subject: [PATCH 13/21] color: add the meta color GIT_COLOR_REVERSE This "color" simply reverts background and foreground. It will be used in the upcoming "dual color" mode of `git range-diff`, where we will reverse colors for the -/+ markers and the fragment headers of the "outer" diff. Signed-off-by: Johannes Schindelin --- color.h | 1 + 1 file changed, 1 insertion(+) diff --git a/color.h b/color.h index 5b744e1bc68617..33e786342a7747 100644 --- a/color.h +++ b/color.h @@ -44,6 +44,7 @@ struct strbuf; #define GIT_COLOR_BG_CYAN "\033[46m" #define GIT_COLOR_FAINT "\033[2m" #define GIT_COLOR_FAINT_ITALIC "\033[2;3m" +#define GIT_COLOR_REVERSE "\033[7m" /* A special value meaning "no color selected" */ #define GIT_COLOR_NIL "NIL" From f1c86f606e018c34230dc016ffdc25a3c70bf840 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 May 2018 02:17:02 +0200 Subject: [PATCH 14/21] diff: add an internal option to dual-color diffs of diffs When diffing diffs, it can be quite daunting to figure out what the heck is going on, as there are nested +/- signs. Let's make this easier by adding a flag in diff_options that allows color-coding the outer diff sign with inverted colors, so that the preimage and postimage is colored like the diff it is. Of course, this really only makes sense when the preimage and postimage *are* diffs. So let's not expose this flag via a command-line option for now. This is a feature that was invented by git-tbdiff, and it will be used by `git range-diff` in the next commit, by offering it via a new option: `--dual-color`. Signed-off-by: Johannes Schindelin --- diff.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++----------- diff.h | 1 + 2 files changed, 69 insertions(+), 15 deletions(-) diff --git a/diff.c b/diff.c index 9c4bd9fa11d658..e6c857abf4e12e 100644 --- a/diff.c +++ b/diff.c @@ -609,14 +609,18 @@ static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, ecbdata->blank_at_eof_in_postimage = (at - l2) + 1; } -static void emit_line_0(struct diff_options *o, const char *set, const char *reset, +static void emit_line_0(struct diff_options *o, + const char *set, unsigned reverse, const char *reset, int first, const char *line, int len) { int has_trailing_newline, has_trailing_carriage_return; int nofirst; FILE *file = o->file; - fputs(diff_line_prefix(o), file); + if (first) + fputs(diff_line_prefix(o), file); + else if (!len) + return; if (len == 0) { has_trailing_newline = (first == '\n'); @@ -634,8 +638,10 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res } if (len || !nofirst) { + if (reverse && want_color(o->use_color)) + fputs(GIT_COLOR_REVERSE, file); fputs(set, file); - if (!nofirst) + if (first && !nofirst) fputc(first, file); fwrite(line, len, 1, file); fputs(reset, file); @@ -649,7 +655,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res static void emit_line(struct diff_options *o, const char *set, const char *reset, const char *line, int len) { - emit_line_0(o, set, reset, line[0], line+1, len-1); + emit_line_0(o, set, 0, reset, line[0], line+1, len-1); } enum diff_symbol { @@ -1168,7 +1174,8 @@ static void dim_moved_lines(struct diff_options *o) static void emit_line_ws_markup(struct diff_options *o, const char *set, const char *reset, - const char *line, int len, char sign, + const char *line, int len, + const char *set_sign, char sign, unsigned ws_rule, int blank_at_eof) { const char *ws = NULL; @@ -1179,14 +1186,20 @@ static void emit_line_ws_markup(struct diff_options *o, ws = NULL; } - if (!ws) - emit_line_0(o, set, reset, sign, line, len); - else if (blank_at_eof) + if (!ws && !set_sign) + emit_line_0(o, set, 0, reset, sign, line, len); + else if (!ws) { + /* Emit just the prefix, then the rest. */ + emit_line_0(o, set_sign ? set_sign : set, !!set_sign, reset, + sign, "", 0); + emit_line_0(o, set, 0, reset, 0, line, len); + } else if (blank_at_eof) /* Blank line at EOF - paint '+' as well */ - emit_line_0(o, ws, reset, sign, line, len); + emit_line_0(o, ws, 0, reset, sign, line, len); else { /* Emit just the prefix, then the rest. */ - emit_line_0(o, set, reset, sign, "", 0); + emit_line_0(o, set_sign ? set_sign : set, !!set_sign, reset, + sign, "", 0); ws_check_emit(line, len, ws_rule, o->file, set, reset, ws); } @@ -1196,7 +1209,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o, struct emitted_diff_symbol *eds) { static const char *nneof = " No newline at end of file\n"; - const char *context, *reset, *set, *meta, *fraginfo; + const char *context, *reset, *set, *set_sign, *meta, *fraginfo; struct strbuf sb = STRBUF_INIT; enum diff_symbol s = eds->s; @@ -1209,7 +1222,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o, context = diff_get_color_opt(o, DIFF_CONTEXT); reset = diff_get_color_opt(o, DIFF_RESET); putc('\n', o->file); - emit_line_0(o, context, reset, '\\', + emit_line_0(o, context, 0, reset, '\\', nneof, strlen(nneof)); break; case DIFF_SYMBOL_SUBMODULE_HEADER: @@ -1236,7 +1249,18 @@ static void emit_diff_symbol_from_struct(struct diff_options *o, case DIFF_SYMBOL_CONTEXT: set = diff_get_color_opt(o, DIFF_CONTEXT); reset = diff_get_color_opt(o, DIFF_RESET); - emit_line_ws_markup(o, set, reset, line, len, ' ', + set_sign = NULL; + if (o->flags.dual_color_diffed_diffs) { + char c = !len ? 0 : line[0]; + + if (c == '+') + set = diff_get_color_opt(o, DIFF_FILE_NEW); + else if (c == '@') + set = diff_get_color_opt(o, DIFF_FRAGINFO); + else if (c == '-') + set = diff_get_color_opt(o, DIFF_FILE_OLD); + } + emit_line_ws_markup(o, set, reset, line, len, set_sign, ' ', flags & (DIFF_SYMBOL_CONTENT_WS_MASK), 0); break; case DIFF_SYMBOL_PLUS: @@ -1263,7 +1287,20 @@ static void emit_diff_symbol_from_struct(struct diff_options *o, set = diff_get_color_opt(o, DIFF_FILE_NEW); } reset = diff_get_color_opt(o, DIFF_RESET); - emit_line_ws_markup(o, set, reset, line, len, '+', + if (!o->flags.dual_color_diffed_diffs) + set_sign = NULL; + else { + char c = !len ? 0 : line[0]; + + set_sign = set; + if (c == '-') + set = diff_get_color_opt(o, DIFF_FILE_OLD); + else if (c == '@') + set = diff_get_color_opt(o, DIFF_FRAGINFO); + else if (c != '+') + set = diff_get_color_opt(o, DIFF_CONTEXT); + } + emit_line_ws_markup(o, set, reset, line, len, set_sign, '+', flags & DIFF_SYMBOL_CONTENT_WS_MASK, flags & DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF); break; @@ -1291,7 +1328,20 @@ static void emit_diff_symbol_from_struct(struct diff_options *o, set = diff_get_color_opt(o, DIFF_FILE_OLD); } reset = diff_get_color_opt(o, DIFF_RESET); - emit_line_ws_markup(o, set, reset, line, len, '-', + if (!o->flags.dual_color_diffed_diffs) + set_sign = NULL; + else { + char c = !len ? 0 : line[0]; + + set_sign = set; + if (c == '+') + set = diff_get_color_opt(o, DIFF_FILE_NEW); + else if (c == '@') + set = diff_get_color_opt(o, DIFF_FRAGINFO); + else if (c != '-') + set = diff_get_color_opt(o, DIFF_CONTEXT); + } + emit_line_ws_markup(o, set, reset, line, len, set_sign, '-', flags & DIFF_SYMBOL_CONTENT_WS_MASK, 0); break; case DIFF_SYMBOL_WORDS_PORCELAIN: @@ -1482,6 +1532,7 @@ static void emit_hunk_header(struct emit_callback *ecbdata, const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO); const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO); const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET); + const char *reverse = ecbdata->color_diff ? GIT_COLOR_REVERSE : ""; static const char atat[2] = { '@', '@' }; const char *cp, *ep; struct strbuf msgbuf = STRBUF_INIT; @@ -1502,6 +1553,8 @@ static void emit_hunk_header(struct emit_callback *ecbdata, ep += 2; /* skip over @@ */ /* The hunk header in fraginfo color */ + if (ecbdata->opt->flags.dual_color_diffed_diffs) + strbuf_addstr(&msgbuf, reverse); strbuf_addstr(&msgbuf, frag); strbuf_add(&msgbuf, line, ep - line); strbuf_addstr(&msgbuf, reset); diff --git a/diff.h b/diff.h index d88ceb3570fda3..cca4f9d6c92cc7 100644 --- a/diff.h +++ b/diff.h @@ -95,6 +95,7 @@ struct diff_flags { unsigned default_follow_renames:1; unsigned stat_with_summary:1; unsigned suppress_diff_headers:1; + unsigned dual_color_diffed_diffs:1; }; static inline void diff_flags_or(struct diff_flags *a, From 3c7b9f339258fee12a1ea1184dc078ca5eca1cba Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 May 2018 03:01:00 +0200 Subject: [PATCH 15/21] range-diff: offer to dual-color the diffs When showing what changed between old and new commits, we show a diff of the patches. This diff is a diff between diffs, therefore there are nested +/- signs, and it can be relatively hard to understand what is going on. With the --dual-color option, the preimage and the postimage are colored like the diffs they are, and the *outer* +/- sign is inverted for clarity. Signed-off-by: Johannes Schindelin --- builtin/range-diff.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/builtin/range-diff.c b/builtin/range-diff.c index 76659d0b3f0385..5a9ad82fb88830 100644 --- a/builtin/range-diff.c +++ b/builtin/range-diff.c @@ -20,9 +20,12 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) { int creation_factor = 60; struct diff_options diffopt = { NULL }; + int dual_color = 0; struct option options[] = { OPT_INTEGER(0, "creation-factor", &creation_factor, N_("Percentage by which creation is weighted")), + OPT_BOOL(0, "dual-color", &dual_color, + N_("color both diff and diff-between-diffs")), OPT_END() }; int i, j, res = 0; @@ -60,6 +63,11 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) options + ARRAY_SIZE(options) - 1, /* OPT_END */ builtin_range_diff_usage, 0); + if (dual_color) { + diffopt.use_color = 1; + diffopt.flags.dual_color_diffed_diffs = 1; + } + if (argc == 2) { if (!strstr(argv[0], "..")) die(_("no .. in range: '%s'"), argv[0]); From c56c51c8bb470ec9a498e984854f5154acf86ef2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 May 2018 03:11:58 +0200 Subject: [PATCH 16/21] range-diff --dual-color: skip white-space warnings When displaying a diff of diffs, it is possible that there is an outer `+` before a context line. That happens when the context changed between old and new commit. When that context line starts with a tab (after the space that marks it as context line), our diff machinery spits out a white-space error (space before tab), but in this case, that is incorrect. Rather than adding a specific whitespace flag that specifically ignores the first space in the output (and might miss other problems with the white-space warnings), let's just skip handling white-space errors in dual color mode to begin with. Signed-off-by: Johannes Schindelin --- diff.c | 1 + 1 file changed, 1 insertion(+) diff --git a/diff.c b/diff.c index e6c857abf4e12e..ea8ecae0417b76 100644 --- a/diff.c +++ b/diff.c @@ -1299,6 +1299,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o, set = diff_get_color_opt(o, DIFF_FRAGINFO); else if (c != '+') set = diff_get_color_opt(o, DIFF_CONTEXT); + flags &= ~DIFF_SYMBOL_CONTENT_WS_MASK; } emit_line_ws_markup(o, set, reset, line, len, set_sign, '+', flags & DIFF_SYMBOL_CONTENT_WS_MASK, From 8c5543a0667fffe0cb0684427f726fdfb75b28d0 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 May 2018 15:50:53 +0200 Subject: [PATCH 17/21] range-diff: populate the man page The bulk of this patch consists of a heavily butchered version of tbdiff's README written by Thomas Rast and Thomas Gummerer, lifted from https://github.com/trast/tbdiff. Signed-off-by: Johannes Schindelin --- Documentation/git-range-diff.txt | 229 +++++++++++++++++++++++++++++++ 1 file changed, 229 insertions(+) diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt index 49f717db8b7ca2..bebb47d4290cb4 100644 --- a/Documentation/git-range-diff.txt +++ b/Documentation/git-range-diff.txt @@ -5,6 +5,235 @@ NAME ---- git-range-diff - Compare two commit ranges (e.g. two versions of a branch) +SYNOPSIS +-------- +[verse] +'git range-diff' [--color=[]] [--no-color] [] + [--dual-color] [--creation-factor=] + ( | ... | ) + +DESCRIPTION +----------- + +This command shows the differences between two versions of a patch +series, or more generally, two commit ranges (ignoring merge commits). + +To that end, it first finds pairs of commits from both commit ranges +that correspond with each other. Two commits are said to correspond when +the diff between their patches (i.e. the author information, the commit +message and the commit diff) is reasonably small compared to the +patches' size. See ``Algorithm`` below for details. + +Finally, the list of matching commits is shown in the order of the +second commit range, with unmatched commits being inserted just after +all of their ancestors have been shown. + + +OPTIONS +------- +--dual-color:: + When the commit diffs differ, recreate the original diffs' + coloring, and add outer -/+ diff markers with the *background* + being red/green to make it easier to see e.g. when there was a + change in what exact lines were added. + +--creation-factor=:: + Set the creation/deletion cost fudge factor to ``. + Defaults to 60. Try a larger value if `git range-diff` erroneously + considers a large change a total rewrite (deletion of one commit + and addition of another), and a smaller one in the reverse case. + See the ``Algorithm`` section below for an explanation why this is + needed. + + :: + Compare the commits specified by the two ranges, where + `` is considered an older version of ``. + +...:: + Equivalent to passing `..` and `..`. + + :: + Equivalent to passing `..` and `..`. + Note that `` does not need to be the exact branch point + of the branches. Example: after rebasing a branch `my-topic`, + `git range-diff my-topic@{u} my-topic@{1} my-topic` would + show the differences introduced by the rebase. + +`git range-diff` also accepts the regular diff options (see +linkgit:git-diff[1]), most notably the `--color=[]` and +`--no-color` options. These options are used when generating the "diff +between patches", i.e. to compare the author, commit message and diff of +corresponding old/new commits. There is currently no means to tweak the +diff options passed to `git log` when generating those patches. + + +CONFIGURATION +------------- +This command uses the `diff.color.*` and `pager.range-diff` settings +(the latter is on by default). +See linkgit:git-config[1]. + + +EXAMPLES +-------- + +When a rebase required merge conflicts to be resolved, compare the changes +introduced by the rebase directly afterwards using: + +------------ +$ git range-diff @{u} @{1} @ +------------ + + +A typical output of `git range-diff` would look like this: + +------------ +-: ------- > 1: 0ddba11 Prepare for the inevitable! +1: c0debee = 2: cab005e Add a helpful message at the start +2: f00dbal ! 3: decafe1 Describe a bug + @@ -1,3 +1,3 @@ + Author: A U Thor + + -TODO: Describe a bug + +Describe a bug + @@ -324,5 +324,6 + This is expected. + + -+What is unexpected is that it will also crash. + ++Unexpectedly, it also crashes. This is a bug, and the jury is + ++still out there how to fix it best. See ticket #314 for details. + + Contact +3: bedead < -: ------- TO-UNDO +------------ + +In this example, there are 3 old and 3 new commits, where the developer +removed the 3rd, added a new one before the first two, and modified the +commit message of the 2nd commit as well its diff. + +When the output goes to a terminal, it is color-coded by default, just +like regular `git diff`'s output. In addition, the first line (adding a +commit) is green, the last line (deleting a commit) is red, the second +line (with a perfect match) is yellow like the commit header of `git +show`'s output, and the third line colors the old commit red, the new +one green and the rest like `git show`'s commit header. + +The color-coded diff is actually a bit hard to read, though, as it +colors the entire lines red or green. The line that added "What is +unexpected" in the old commit, for example, is completely red, even if +the intent of the old commit was to add something. + +To help with that, use the `--dual-color` mode. In this mode, the diff +of diffs will retain the original diff colors, and prefix the lines with +-/+ markers that have their *background* red or green, to make it more +obvious that they describe how the diff itself changed. + + +Algorithm +--------- + +The general idea is this: we generate a cost matrix between the commits +in both commit ranges, then solve the least-cost assignment. + +The cost matrix is populated thusly: for each pair of commits, both +diffs are generated and the "diff of diffs" is generated, with 3 context +lines, then the number of lines in that diff is used as cost. + +To avoid false positives (e.g. when a patch has been removed, and an +unrelated patch has been added between two iterations of the same patch +series), the cost matrix is extended to allow for that, by adding +fixed-cost entries for wholesale deletes/adds. + +Example: Let commits `1--2` be the first iteration of a patch series and +`A--C` the second iteration. Let's assume that `A` is a cherry-pick of +`2,` and `C` is a cherry-pick of `1` but with a small modification (say, +a fixed typo). Visualize the commits as a bipartite graph: + +------------ + 1 A + + 2 B + + C +------------ + +We are looking for a "best" explanation of the new series in terms of +the old one. We can represent an "explanation" as an edge in the graph: + + +------------ + 1 A + / + 2 --------' B + + C +------------ + +This explanation comes for "free" because there was no change. Similarly +`C` could be explained using `1`, but that comes at some cost c>0 +because of the modification: + +------------ + 1 ----. A + | / + 2 ----+---' B + | + `----- C + c>0 +------------ + +In mathematical terms, what we are looking for is some sort of a minimum +cost bipartite matching; `1` is matched to `C` at some cost, etc. The +underlying graph is in fact a complete bipartite graph; the cost we +associate with every edge is the size of the diff between the two +commits' patches. To explain also new commits, we introduce dummy nodes +on both sides: + +------------ + 1 ----. A + | / + 2 ----+---' B + | + o `----- C + c>0 + o o + + o o +------------ + +The cost of an edge `o--C` is the size of `C`'s diff, modified by a +fudge factor that should be smaller than 100%. The cost of an edge +`o--o` is free. The fudge factor is necessary because even if `1` and +`C` have nothing in common, they may still share a few empty lines and +such, possibly making the assignment `1--C`, `o--o` slightly cheaper +than `1--o`, `o--C` even if `1` and `C` have nothing in common. With the +fudge factor we require a much larger common part to consider patches as +corresponding. + +The overall time needed to compute this algorithm is the time needed to +compute n+m commit diffs and then n*m diffs of patches, plus the time +needed to compute the least-cost assigment between n and m diffs. Git +uses an implementation of the Jonker-Volgenant algorithm to solve the +assignment problem, which has cubic runtime complexity. The matching +found in this case will look like this: + +------------ + 1 ----. A + | / + 2 ----+---' B + .--+-----' + o -' `----- C + c>0 + o ---------- o + + o ---------- o +------------ + + +SEE ALSO +-------- +linkgit:git-log[1] + GIT --- Part of the linkgit:git[1] suite From 16e3cf27b515ba94412c48b0f58059f1f30cc08d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 May 2018 16:44:25 +0200 Subject: [PATCH 18/21] completion: support `git range-diff` Tab completion of `git range-diff` is very convenient, especially given that the revision arguments to specify the commit ranges to compare are typically more complex than, say, what is normally passed to `git log`. Signed-off-by: Johannes Schindelin --- contrib/completion/git-completion.bash | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 94c95516ebec7f..3d4ec34323e871 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1976,6 +1976,20 @@ _git_push () __git_complete_remote_or_refspec } +_git_range_diff () +{ + case "$cur" in + --*) + __gitcomp " + --creation-factor= --dual-color + $__git_diff_common_options + " + return + ;; + esac + __git_complete_revlist +} + _git_rebase () { __git_find_repo_path From d9b09abcfd0b8ca51ad1f5bbc473a02139d48890 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 5 May 2018 21:52:20 +0200 Subject: [PATCH 19/21] range-diff: left-pad patch numbers As pointed out by Elijah Newren, tbdiff has this neat little alignment trick where it outputs the commit pairs with patch numbers that are padded to the maximal patch number's width: 1: cafedead = 1: acefade first patch [...] 314: beefeada < 314: facecab up to PI! Let's do the same in range-diff, too. Signed-off-by: Johannes Schindelin --- range-diff.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/range-diff.c b/range-diff.c index b1663da7cd3068..b6b9abac266f3a 100644 --- a/range-diff.c +++ b/range-diff.c @@ -259,6 +259,7 @@ static void get_correspondences(struct string_list *a, struct string_list *b, } static void output_pair_header(struct diff_options *diffopt, + int patch_no_width, struct strbuf *buf, struct strbuf *dashes, struct patch_util *a_util, @@ -295,9 +296,9 @@ static void output_pair_header(struct diff_options *diffopt, strbuf_reset(buf); strbuf_addstr(buf, status == '!' ? color_old : color); if (!a_util) - strbuf_addf(buf, "-: %s ", dashes->buf); + strbuf_addf(buf, "%*s: %s ", patch_no_width, "-", dashes->buf); else - strbuf_addf(buf, "%d: %s ", a_util->i + 1, + strbuf_addf(buf, "%*d: %s ", patch_no_width, a_util->i + 1, find_unique_abbrev(&a_util->oid, DEFAULT_ABBREV)); if (status == '!') @@ -307,9 +308,9 @@ static void output_pair_header(struct diff_options *diffopt, strbuf_addf(buf, "%s%s", color_reset, color_new); if (!b_util) - strbuf_addf(buf, " -: %s", dashes->buf); + strbuf_addf(buf, " %*s: %s", patch_no_width, "-", dashes->buf); else - strbuf_addf(buf, " %d: %s", b_util->i + 1, + strbuf_addf(buf, " %*d: %s", patch_no_width, b_util->i + 1, find_unique_abbrev(&b_util->oid, DEFAULT_ABBREV)); commit = lookup_commit_reference(the_repository, oid); @@ -357,6 +358,7 @@ static void output(struct string_list *a, struct string_list *b, struct diff_options *diffopt) { struct strbuf buf = STRBUF_INIT, dashes = STRBUF_INIT; + int patch_no_width = decimal_width(1 + (a->nr > b->nr ? a->nr : b->nr)); int i = 0, j = 0; /* @@ -378,7 +380,7 @@ static void output(struct string_list *a, struct string_list *b, /* Show unmatched LHS commit whose predecessors were shown. */ if (i < a->nr && a_util->matching < 0) { - output_pair_header(diffopt, + output_pair_header(diffopt, patch_no_width, &buf, &dashes, a_util, NULL); i++; continue; @@ -386,7 +388,7 @@ static void output(struct string_list *a, struct string_list *b, /* Show unmatched RHS commits. */ while (j < b->nr && b_util->matching < 0) { - output_pair_header(diffopt, + output_pair_header(diffopt, patch_no_width, &buf, &dashes, NULL, b_util); b_util = ++j < b->nr ? b->items[j].util : NULL; } @@ -394,7 +396,7 @@ static void output(struct string_list *a, struct string_list *b, /* Show matching LHS/RHS pair. */ if (j < b->nr) { a_util = a->items[b_util->matching].util; - output_pair_header(diffopt, + output_pair_header(diffopt, patch_no_width, &buf, &dashes, a_util, b_util); if (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT)) patch_diff(a->items[b_util->matching].string, From f6fd3955eba9cf0f647c53bead92a810b4dd70fa Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 30 Jun 2018 22:41:41 +0200 Subject: [PATCH 20/21] range-diff: make --dual-color the default mode After using this command extensively for the last two months, this developer came to the conclusion that even if the dual color mode still leaves a lot of room for confusion about what was actually changed, the non-dual color mode is substantially worse in that regard. Therefore, we really want to make the dual color mode the default. Signed-off-by: Johannes Schindelin --- Documentation/git-range-diff.txt | 32 +++++++++++++++----------- builtin/range-diff.c | 10 ++++---- contrib/completion/git-completion.bash | 2 +- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt index bebb47d4290cb4..82c71c682936e7 100644 --- a/Documentation/git-range-diff.txt +++ b/Documentation/git-range-diff.txt @@ -9,7 +9,7 @@ SYNOPSIS -------- [verse] 'git range-diff' [--color=[]] [--no-color] [] - [--dual-color] [--creation-factor=] + [--no-dual-color] [--creation-factor=] ( | ... | ) DESCRIPTION @@ -31,11 +31,14 @@ all of their ancestors have been shown. OPTIONS ------- ---dual-color:: - When the commit diffs differ, recreate the original diffs' - coloring, and add outer -/+ diff markers with the *background* - being red/green to make it easier to see e.g. when there was a - change in what exact lines were added. +--no-dual-color:: + When the commit diffs differ, `git range-diff` recreates the + original diffs' coloring, and adds outer -/+ diff markers with + the *background* being red/green to make it easier to see e.g. + when there was a change in what exact lines were added. This is + known to `range-diff` as "dual coloring". Use `--no-dual-color` + to revert to color all lines according to the outer diff markers + (and completely ignore the inner diff when it comes to color). --creation-factor=:: Set the creation/deletion cost fudge factor to ``. @@ -118,15 +121,16 @@ line (with a perfect match) is yellow like the commit header of `git show`'s output, and the third line colors the old commit red, the new one green and the rest like `git show`'s commit header. -The color-coded diff is actually a bit hard to read, though, as it -colors the entire lines red or green. The line that added "What is -unexpected" in the old commit, for example, is completely red, even if -the intent of the old commit was to add something. +A naive color-coded diff of diffs is actually a bit hard to read, +though, as it colors the entire lines red or green. The line that added +"What is unexpected" in the old commit, for example, is completely red, +even if the intent of the old commit was to add something. -To help with that, use the `--dual-color` mode. In this mode, the diff -of diffs will retain the original diff colors, and prefix the lines with --/+ markers that have their *background* red or green, to make it more -obvious that they describe how the diff itself changed. +To help with that, `range` uses the `--dual-color` mode by default. In +this mode, the diff of diffs will retain the original diff colors, and +prefix the lines with -/+ markers that have their *background* red or +green, to make it more obvious that they describe how the diff itself +changed. Algorithm diff --git a/builtin/range-diff.c b/builtin/range-diff.c index 5a9ad82fb88830..f52d45d9d61444 100644 --- a/builtin/range-diff.c +++ b/builtin/range-diff.c @@ -20,11 +20,11 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) { int creation_factor = 60; struct diff_options diffopt = { NULL }; - int dual_color = 0; + int simple_color = -1; struct option options[] = { OPT_INTEGER(0, "creation-factor", &creation_factor, N_("Percentage by which creation is weighted")), - OPT_BOOL(0, "dual-color", &dual_color, + OPT_BOOL(0, "no-dual-color", &simple_color, N_("color both diff and diff-between-diffs")), OPT_END() }; @@ -63,8 +63,10 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) options + ARRAY_SIZE(options) - 1, /* OPT_END */ builtin_range_diff_usage, 0); - if (dual_color) { - diffopt.use_color = 1; + if (simple_color < 1) { + if (!simple_color) + /* force color when --dual-color was used */ + diffopt.use_color = 1; diffopt.flags.dual_color_diffed_diffs = 1; } diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 3d4ec34323e871..d63d2dffd4e121 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1981,7 +1981,7 @@ _git_range_diff () case "$cur" in --*) __gitcomp " - --creation-factor= --dual-color + --creation-factor= --no-dual-color $__git_diff_common_options " return From 699cd712e2aa56ad7bb1356b72caf8d4738a18e5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 21 Jul 2018 13:06:03 +0200 Subject: [PATCH 21/21] range-diff: use dim/bold cues to improve dual color mode It *is* a confusing thing to look at a diff of diffs. All too easy is it to mix up whether the -/+ markers refer to the "inner" or the "outer" diff, i.e. whether a `+` indicates that a line was added by either the old or the new diff (or both), or whether the new diff does something different than the old diff. To make things easier to process for normal developers, we introduced the dual color mode which colors the lines according to the commit diff, i.e. lines that are added by a commit (whether old, new, or both) are colored in green. In non-dual color mode, the lines would be colored according to the outer diff: if the old commit added a line, it would be colored red (because that line addition is only present in the first commit range that was specified on the command-line, i.e. the "old" commit, but not in the second commit range, i.e. the "new" commit). However, this dual color mode is still not making things clear enough, as we are looking at two levels of diffs, and we still only pick a color according to *one* of them (the outer diff marker is colored differently, of course, but in particular with deep indentation, it is easy to lose track of that outer diff marker's background color). Therefore, let's add another dimension to the mix. Still use green/red/normal according to the commit diffs, but now also dim the lines that were only in the old commit, and use bold face for the lines that are only in the new commit. That way, it is much easier not to lose track of, say, when we are looking at a line that was added in the previous iteration of a patch series but the new iteration adds a slightly different version: the obsolete change will be dimmed, the current version of the patch will be bold. At least this developer has a much easier time reading the range-diffs that way. Signed-off-by: Johannes Schindelin --- Documentation/config.txt | 6 ++++-- Documentation/git-range-diff.txt | 17 +++++++++++++---- color.h | 6 ++++++ diff.c | 28 ++++++++++++++++++++++------ diff.h | 8 +++++++- 5 files changed, 52 insertions(+), 13 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 63365dcf3db6f5..90241ed77cb104 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1193,8 +1193,10 @@ color.diff.:: (highlighting whitespace errors), `oldMoved` (deleted lines), `newMoved` (added lines), `oldMovedDimmed`, `oldMovedAlternative`, `oldMovedAlternativeDimmed`, `newMovedDimmed`, `newMovedAlternative` - and `newMovedAlternativeDimmed` (See the '' - setting of '--color-moved' in linkgit:git-diff[1] for details). + `newMovedAlternativeDimmed` (See the '' + setting of '--color-moved' in linkgit:git-diff[1] for details), + `contextDimmed`, `oldDimmed`, `newDimmed`, `contextBold`, + `oldBold`, and `newBold` (see linkgit:git-range-diff[1] for details). color.decorate.:: Use customized color for 'git log --decorate' output. `` is one diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt index 82c71c682936e7..f693930fdb502a 100644 --- a/Documentation/git-range-diff.txt +++ b/Documentation/git-range-diff.txt @@ -35,10 +35,19 @@ OPTIONS When the commit diffs differ, `git range-diff` recreates the original diffs' coloring, and adds outer -/+ diff markers with the *background* being red/green to make it easier to see e.g. - when there was a change in what exact lines were added. This is - known to `range-diff` as "dual coloring". Use `--no-dual-color` - to revert to color all lines according to the outer diff markers - (and completely ignore the inner diff when it comes to color). + when there was a change in what exact lines were added. ++ +Additionally, the commit diff lines that are only present in the first commit +range are shown "dimmed" (this can be overridden using the `color.diff.` +config setting where `` is one of `contextDimmed`, `oldDimmed` and +`newDimmed`), and the commit diff lines that are only present in the second +commit range are shown in bold (which can be overridden using the config +settings `color.diff.` with `` being one of `contextBold`, +`oldBold` or `newBold`). ++ +This is known to `range-diff` as "dual coloring". Use `--no-dual-color` +to revert to color all lines according to the outer diff markers +(and completely ignore the inner diff when it comes to color). --creation-factor=:: Set the creation/deletion cost fudge factor to ``. diff --git a/color.h b/color.h index 33e786342a7747..98894d6a17563d 100644 --- a/color.h +++ b/color.h @@ -36,6 +36,12 @@ struct strbuf; #define GIT_COLOR_BOLD_BLUE "\033[1;34m" #define GIT_COLOR_BOLD_MAGENTA "\033[1;35m" #define GIT_COLOR_BOLD_CYAN "\033[1;36m" +#define GIT_COLOR_FAINT_RED "\033[2;31m" +#define GIT_COLOR_FAINT_GREEN "\033[2;32m" +#define GIT_COLOR_FAINT_YELLOW "\033[2;33m" +#define GIT_COLOR_FAINT_BLUE "\033[2;34m" +#define GIT_COLOR_FAINT_MAGENTA "\033[2;35m" +#define GIT_COLOR_FAINT_CYAN "\033[2;36m" #define GIT_COLOR_BG_RED "\033[41m" #define GIT_COLOR_BG_GREEN "\033[42m" #define GIT_COLOR_BG_YELLOW "\033[43m" diff --git a/diff.c b/diff.c index ea8ecae0417b76..ae131495216a93 100644 --- a/diff.c +++ b/diff.c @@ -70,6 +70,12 @@ static char diff_colors[][COLOR_MAXLEN] = { GIT_COLOR_BOLD_YELLOW, /* NEW_MOVED ALTERNATIVE */ GIT_COLOR_FAINT, /* NEW_MOVED_DIM */ GIT_COLOR_FAINT_ITALIC, /* NEW_MOVED_ALTERNATIVE_DIM */ + GIT_COLOR_FAINT, /* CONTEXT_DIM */ + GIT_COLOR_FAINT_RED, /* OLD_DIM */ + GIT_COLOR_FAINT_GREEN, /* NEW_DIM */ + GIT_COLOR_BOLD, /* CONTEXT_BOLD */ + GIT_COLOR_BOLD_RED, /* OLD_BOLD */ + GIT_COLOR_BOLD_GREEN, /* NEW_BOLD */ }; static const char *color_diff_slots[] = { @@ -89,6 +95,12 @@ static const char *color_diff_slots[] = { [DIFF_FILE_NEW_MOVED_ALT] = "newMovedAlternative", [DIFF_FILE_NEW_MOVED_DIM] = "newMovedDimmed", [DIFF_FILE_NEW_MOVED_ALT_DIM] = "newMovedAlternativeDimmed", + [DIFF_CONTEXT_DIM] = "contextDimmed", + [DIFF_FILE_OLD_DIM] = "oldDimmed", + [DIFF_FILE_NEW_DIM] = "newDimmed", + [DIFF_CONTEXT_BOLD] = "contextBold", + [DIFF_FILE_OLD_BOLD] = "oldBold", + [DIFF_FILE_NEW_BOLD] = "newBold", }; static NORETURN void die_want_option(const char *option_name) @@ -1294,11 +1306,13 @@ static void emit_diff_symbol_from_struct(struct diff_options *o, set_sign = set; if (c == '-') - set = diff_get_color_opt(o, DIFF_FILE_OLD); + set = diff_get_color_opt(o, DIFF_FILE_OLD_BOLD); else if (c == '@') set = diff_get_color_opt(o, DIFF_FRAGINFO); - else if (c != '+') - set = diff_get_color_opt(o, DIFF_CONTEXT); + else if (c == '+') + set = diff_get_color_opt(o, DIFF_FILE_NEW_BOLD); + else + set = diff_get_color_opt(o, DIFF_CONTEXT_BOLD); flags &= ~DIFF_SYMBOL_CONTENT_WS_MASK; } emit_line_ws_markup(o, set, reset, line, len, set_sign, '+', @@ -1336,11 +1350,13 @@ static void emit_diff_symbol_from_struct(struct diff_options *o, set_sign = set; if (c == '+') - set = diff_get_color_opt(o, DIFF_FILE_NEW); + set = diff_get_color_opt(o, DIFF_FILE_NEW_DIM); else if (c == '@') set = diff_get_color_opt(o, DIFF_FRAGINFO); - else if (c != '-') - set = diff_get_color_opt(o, DIFF_CONTEXT); + else if (c == '-') + set = diff_get_color_opt(o, DIFF_FILE_OLD_DIM); + else + set = diff_get_color_opt(o, DIFF_CONTEXT_DIM); } emit_line_ws_markup(o, set, reset, line, len, set_sign, '-', flags & DIFF_SYMBOL_CONTENT_WS_MASK, 0); diff --git a/diff.h b/diff.h index cca4f9d6c92cc7..e1e54256c180a5 100644 --- a/diff.h +++ b/diff.h @@ -248,7 +248,13 @@ enum color_diff { DIFF_FILE_NEW_MOVED = 13, DIFF_FILE_NEW_MOVED_ALT = 14, DIFF_FILE_NEW_MOVED_DIM = 15, - DIFF_FILE_NEW_MOVED_ALT_DIM = 16 + DIFF_FILE_NEW_MOVED_ALT_DIM = 16, + DIFF_CONTEXT_DIM = 17, + DIFF_FILE_OLD_DIM = 18, + DIFF_FILE_NEW_DIM = 19, + DIFF_CONTEXT_BOLD = 20, + DIFF_FILE_OLD_BOLD = 21, + DIFF_FILE_NEW_BOLD = 22, }; const char *diff_get_color(int diff_use_color, enum color_diff ix); #define diff_get_color_opt(o, ix) \