From 348ae56cb2266d3294611112ae0368386124d720 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 13 Aug 2018 04:33:02 -0700 Subject: 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 Signed-off-by: Junio C Hamano --- builtin/range-diff.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 builtin/range-diff.c (limited to 'builtin/range-diff.c') diff --git a/builtin/range-diff.c b/builtin/range-diff.c new file mode 100644 index 0000000000..36788ea4f2 --- /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; +} -- cgit v1.2.3 From d9c66f0b5bfdf3fc2898b7baad1bb9a72bfd7bf7 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 13 Aug 2018 04:33:04 -0700 Subject: 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 Signed-off-by: Junio C Hamano --- 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 (limited to 'builtin/range-diff.c') diff --git a/Makefile b/Makefile index 7ff7eba42b..72f16882e6 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 36788ea4f2..94c1f362cc 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 0000000000..15d418afa6 --- /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 0000000000..7b6eef303f --- /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 -- cgit v1.2.3 From c8c5e43ac3f9a5b785faf16f10bb8e2c493606a4 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 13 Aug 2018 04:33:07 -0700 Subject: 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 Signed-off-by: Junio C Hamano --- builtin/range-diff.c | 31 +++++++++++++++++++++++++++++-- range-diff.c | 34 +++++++++++++++++++++++++++++++--- range-diff.h | 4 +++- 3 files changed, 63 insertions(+), 6 deletions(-) (limited to 'builtin/range-diff.c') diff --git a/builtin/range-diff.c b/builtin/range-diff.c index 94c1f362cc..3b06ed9449 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 2d94200d30..71883a4b7d 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 7b6eef303f..2407d46a30 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 -- cgit v1.2.3 From 5e242e63d0e9955a37268cc6b93b4e8fdedde07f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 13 Aug 2018 04:33:10 -0700 Subject: 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 Signed-off-by: Junio C Hamano --- builtin/range-diff.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'builtin/range-diff.c') diff --git a/builtin/range-diff.c b/builtin/range-diff.c index 3b06ed9449..f0598005ae 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; } -- cgit v1.2.3 From 1cdde296a5d3b0fd17a3fcd7a0869550b064fdce Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 13 Aug 2018 04:33:11 -0700 Subject: 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 Signed-off-by: Junio C Hamano --- builtin/range-diff.c | 1 + diff.c | 5 ++++- diff.h | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) (limited to 'builtin/range-diff.c') diff --git a/builtin/range-diff.c b/builtin/range-diff.c index f0598005ae..76659d0b3f 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 04d044bbb6..9c4bd9fa11 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 a14895bb82..d88ceb3570 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, -- cgit v1.2.3 From 31cf61a08008eb152247b16ee96811220f019fbd Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 13 Aug 2018 04:33:22 -0700 Subject: 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 Signed-off-by: Junio C Hamano --- builtin/range-diff.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'builtin/range-diff.c') diff --git a/builtin/range-diff.c b/builtin/range-diff.c index 76659d0b3f..5a9ad82fb8 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]); -- cgit v1.2.3 From 275267937bdbb8611e8872d64adebe7587c6fa5a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 13 Aug 2018 04:33:30 -0700 Subject: 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 Signed-off-by: Junio C Hamano --- Documentation/git-range-diff.txt | 32 ++++++++++++++++++-------------- builtin/range-diff.c | 10 ++++++---- contrib/completion/git-completion.bash | 2 +- 3 files changed, 25 insertions(+), 19 deletions(-) (limited to 'builtin/range-diff.c') diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt index bebb47d429..82c71c6829 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 5a9ad82fb8..f52d45d9d6 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 3d4ec34323..d63d2dffd4 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 -- cgit v1.2.3