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 --- builtin/range-diff.c | 45 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) (limited to 'builtin/range-diff.c') 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; } -- 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 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 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); -- 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 + 1 file changed, 1 insertion(+) (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; -- 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 --- builtin/range-diff.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'builtin/range-diff.c') 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; } -- cgit v1.2.3