[PATCH 2/2] rebase --root: fix amending root commit messages
From: Johannes Schindelin
The code path that triggered that "BUG" really does not want to run
without an explicit commit message. In the case where we want to amend a
commit message, we have an *implicit* commit message, though: the one of
the commit to amend. Therefore, this code path should not even be
entered.
Signed-off-by: Johannes Schindelin
---
sequencer.c | 2 +-
t/t3404-rebase-interactive.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index cca968043..4034c0461 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -784,7 +784,7 @@ static int run_git_commit(const char *defmsg, struct
replay_opts *opts,
struct child_process cmd = CHILD_PROCESS_INIT;
const char *value;
- if (flags & CREATE_ROOT_COMMIT) {
+ if ((flags & CREATE_ROOT_COMMIT) && !(flags & AMEND_MSG)) {
struct strbuf msg = STRBUF_INIT, script = STRBUF_INIT;
const char *author = is_rebase_i(opts) ?
read_author_ident() : NULL;
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ca94c688d..e500d7c32 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -971,7 +971,7 @@ test_expect_success 'rebase -i --root fixup root commit' '
test 0 = $(git cat-file commit HEAD | grep -c ^parent\ )
'
-test_expect_failure 'rebase -i --root reword root commit' '
+test_expect_success 'rebase -i --root reword root commit' '
test_when_finished "test_might_fail git rebase --abort" &&
git checkout -b reword-root-branch master &&
set_fake_editor &&
--
2.17.0.windows.1
[PATCH 1/2] rebase --root: demonstrate a bug while amending root commit messages
From: Todd Zullinger When splitting a repository, running `git rebase -i --root` to reword the initial commit, Git dies with BUG: sequencer.c:795: root commit without message. Signed-off-by: Todd Zullinger Signed-off-by: Johannes Schindelin --- t/t3404-rebase-interactive.sh | 9 + 1 file changed, 9 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index c65826dda..ca94c688d 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -971,6 +971,15 @@ test_expect_success 'rebase -i --root fixup root commit' ' test 0 = $(git cat-file commit HEAD | grep -c ^parent\ ) ' +test_expect_failure 'rebase -i --root reword root commit' ' + test_when_finished "test_might_fail git rebase --abort" && + git checkout -b reword-root-branch master && + set_fake_editor && + FAKE_LINES="reword 1 2" FAKE_COMMIT_MESSAGE="A changed" \ + git rebase -i --root && + git show HEAD^ | grep "A changed" +' + test_expect_success C_LOCALE_OUTPUT 'rebase --edit-todo does not work on non-interactive rebase' ' git reset --hard && git checkout conflict-branch && -- 2.17.0.windows.1
[PATCH 0/2] rebase --root: fix `reword` on a root commit
From: GitGitGadget Todd Zullinger reported this bug in https://public-inbox.org/git/20180615043111.gs3...@zaya.teonanacatl.net/: when calling git rebase --root and trying to reword the root commit's message, a BUG is reported. This fixes that. IMO the bug fix is trivial enough to qualify for inclusion into v2.18.0, still. Johannes Schindelin (1): rebase --root: fix amending root commit messages Todd Zullinger (1): rebase --root: demonstrate a bug while amending root commit messages sequencer.c | 2 +- t/t3404-rebase-interactive.sh | 9 + 2 files changed, 10 insertions(+), 1 deletion(-) base-commit: 68372c88794aba15f853542008cda39def768372 -- 2.17.0.windows.1
[PATCH v3 02/20] Introduce `range-diff` to compare iterations of a topic branch
From: Johannes Schindelin
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.
Signed-off-by: Johannes Schindelin
---
.gitignore | 1 +
Makefile | 1 +
builtin.h| 1 +
builtin/range-diff.c | 25 +
command-list.txt | 1 +
git.c| 1 +
6 files changed, 30 insertions(+)
create mode 100644 builtin/range-diff.c
diff --git a/.gitignore b/.gitignore
index 3284a1e9b..cc0ad74b4 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/Makefile b/Makefile
index c5ba124f1..190384cae 100644
--- a/Makefile
+++ b/Makefile
@@ -1059,6 +1059,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 0362f1ce2..99206df4b 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 0..36788ea4f
--- /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", _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 e1c26c1bb..a9dda3b8a 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -139,6 +139,7 @@ git-prune-packedplumbingmanipulators
git-pullmainporcelain remote
git-pushmainporcelain remote
git-quiltimport foreignscminterface
+git-range-diff mainporcelain
git-read-tree plumbingmanipulators
git-rebase mainporcelain history
git-receive-packsynchelpers
diff --git a/git.c b/git.c
index 9dbe6ffaa..13e37f1e3 100644
--- a/git.c
+++ b/git.c
@@ -517,6 +517,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 },
--
gitgitgadget
[PATCH v3 03/20] range-diff: first rudimentary implementation
From: Johannes Schindelin
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 Hungarian 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 | 47 ++-
range-diff.c | 307 +++
range-diff.h | 7 +
4 files changed, 359 insertions(+), 3 deletions(-)
create mode 100644 range-diff.c
create mode 100644 range-diff.h
diff --git a/Makefile b/Makefile
index 190384cae..f20126e11 100644
--- a/Makefile
+++ b/Makefile
@@ -921,6 +921,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 36788ea4f..c37a72100 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,49 @@ 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);
+ argc = parse_options(argc, argv, NULL, options,
builtin_range_diff_usage,
+0);
- return 0;
+ if (argc == 2) {
+ if (!strstr(argv[0], ".."))
+ warning(_("no .. in range: '%s'"), argv[0]);
+ strbuf_addstr(, argv[0]);
+
+ if (!strstr(argv[1], ".."))
+ warning(_("no .. in range: '%s'"), argv[1]);
+ strbuf_addstr(, argv[1]);
+ } else if (argc == 3) {
+ strbuf_addf(, "%s..%s", argv[0], argv[1]);
+ strbuf_addf(, "%s..%s", argv[0], argv[2]);
+ } else if (argc == 1) {
+ const char *b = strstr(argv[0], "..."), *a = argv[0];
+ int a_len;
+
+ if (!b)
+ die(_("single arg format requires a symmetric range"));
+
+ a_len = (int)(b - a);
+ if (!a_len) {
+ a = "HEAD";
+ a_len = strlen(a);
+ }
+ b += 3;
+ if (!*b)
+ b = "HEAD";
+ strbuf_addf(, "%s..%.*s", b, a_len, a);
+ strbuf_addf(, "%.*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();
+ strbuf_release();
+
+ return res;
}
diff --git a/range-diff.c b/range-diff.c
new file mode 100644
index 0..c374333a4
--- /dev/null
+++ b/range-diff.c
@@ -0,0 +1,307 @@
+#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(, "log", "--no-color", "-p", "--no-merges",
+ "--reverse", "--date-order",
[PATCH v3 15/20] range-diff: offer to dual-color the diffs
From: Johannes Schindelin
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 7c0967220..e8f7fe452 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", _factor,
N_("Percentage by which creation is weighted")),
+ OPT_BOOL(0, "dual-color", _color,
+ N_("color both diff and diff-between-diffs")),
OPT_END()
};
int i, j, res = 0;
@@ -50,6 +53,11 @@ int cmd_range_diff(int argc, const char **argv, const char
*prefix)
argc = j;
diff_setup_done();
+ if (dual_color) {
+ diffopt.use_color = 1;
+ diffopt.flags.dual_color_diffed_diffs = 1;
+ }
+
if (argc == 2) {
if (!strstr(argv[0], ".."))
warning(_("no .. in range: '%s'"), argv[0]);
--
gitgitgadget
[PATCH v3 14/20] diff: add an internal option to dual-color diffs of diffs
From: Johannes Schindelin
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 8c568cbe0..26445ffa1 100644
--- a/diff.c
+++ b/diff.c
@@ -562,14 +562,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');
@@ -587,8 +591,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);
@@ -602,7 +608,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 {
@@ -962,7 +968,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;
@@ -973,14 +980,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);
}
@@ -990,7 +1003,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;
@@ -1003,7 +1016,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, '\\',
+
[PATCH v3 00/20] Add `range-diff`, a `tbdiff` lookalike
The incredibly useful `git-tbdiff` tool to compare patch series (say, to see
what changed between two iterations sent to the Git mailing list) is slightly
less useful for this developer due to the fact that it requires the `hungarian`
and `numpy` Python packages which are for some reason really hard to build in
MSYS2. So hard that I even had to give up, because it was simply easier to
reimplement the whole shebang as a builtin command.
The project at https://github.com/trast/tbdiff seems to be dormant, anyway.
Funny (and true) story: I looked at the open Pull Requests to see how active
that project is, only to find to my surprise that I had submitted one in August
2015, and that it was still unanswered let alone merged.
While at it, I forward-ported AEvar's patch to force `--decorate=no` because
`git -p tbdiff` would fail otherwise.
Side note: I work on implementing branch-diff not only to make life easier for
reviewers who have to suffer through v2, v3, ... of my patch series, but also
to verify my changes before submitting a new iteraion. And also, maybe even
more importantly, I plan to use it to verify my merging-rebases of Git for
Windows (for which I previously used to redirect the pre-rebase/post-rebase
diffs vs upstream and then compare them using `git diff --no-index`). And of
course any interested person can see what changes were necessary e.g. in the
merging-rebase of Git for Windows onto v2.17.0 by running a command like:
```sh
base=^{/Start.the.merging-rebase}
tag=v2.17.0.windows.1
pre=$tag$base^2
git branch-diff --dual-color $pre$base..$pre $tag$base..$tag
```
The --dual-color mode will identify the many changes that are solely due to
different diff context lines (where otherwise uncolored lines start with a
background-colored -/+ marker), i.e. merge conflicts I had to resolve.
Changes since v2:
- Right-aligned the patch numbers in the commit pairs.
- Used ALLOC_ARRAY() in hungarian.c instead of xmalloc(sizeof()*size).
- Changed compute_assignment()s return type from int to void, as it always
succeeds.
- Changed the Hungarian Algorithm to use an integer cost matrix.
- Changed the --creation-weight option to --creation-factor
where is an integer.
- Retitled 1/19 and 2/19 to better conform with the current conventions, as
pointed out (and suggested) by Junio.
- Shut up Coverity, and at the same time avoided passing the unnecessary `i`
and `j` parameters to output_pair_header().
- Removed support for the `--no-patches` option: we inherit diff_options'
support for `-s` already (and much more).
- Removed the ugly `_INV` enum values, and introduced a beautiful
GIT_COLOR_REVERSE instead. This way, whatever the user configured as
color.diff.new (or .old) will be used in reverse in the dual color mode.
- Instead of overriding the fragment header color, the dual color mode will now
reverse the "outer" fragment headers, too.
- Turned the stand-alone branch-diff command into the `--diff` option of `git
branch`. Adjusted pretty much *all* commit messages to account for this. This
change should no longer be visible: see below.
- Pretty much re-wrote the completion, to support the new --diff mode of
git-branch. See below: it was reverted for range-diff.
- Renamed t7910 to t3206, to be closer to the git-branch tests.
- Ensured that git_diff_ui_config() gets called, and therefore color.diff.*
respected.
- Avoided leaking `four_spaces`.
- Fixed a declaration in a for (;;) statement (which Junio had as a fixup! that
I almost missed).
- Renamed `branch --diff`, which had been renamed from `branch-diff` (which was
picked to avoid re-using `tbdiff`) to `range-diff`.
- Renamed `hungarian.c` and its header to `linear-assignment.c`
- Made `--dual-color` the default, and changed it to still auto-detect whether
color should be used rather than forcing it
Johannes Schindelin (19):
linear-assignment: a function to solve least-cost assignment problems
Introduce `range-diff` to compare iterations of a topic branch
range-diff: first rudimentary implementation
range-diff: improve the order of the shown commits
range-diff: also show the diff between patches
range-diff: right-trim commit messages
range-diff: indent the diffs just like tbdiff
range-diff: suppress the diff headers
range-diff: adjust the output of the commit pairs
range-diff: do not show "function names" in hunk headers
range-diff: use color for the commit pairs
color: add the meta color GIT_COLOR_REVERSE
diff: add an internal option to dual-color diffs of diffs
range-diff: offer to dual-color the diffs
range-diff --dual-color: work around bogus white-space warning
range-diff: add a man page
completion: support `git range-diff`
range-diff: left-pad patch numbers
range-diff: make --dual-color the default mode
Thomas Rast (1):
range-diff: add tests
.gitignore | 1 +
Documentation/git-range-diff.txt | 238
[PATCH v3 19/20] range-diff: left-pad patch numbers
From: Johannes Schindelin
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 | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/range-diff.c b/range-diff.c
index 870c3680c..d3e51bf36 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -254,7 +254,8 @@ static void get_correspondences(struct string_list *a,
struct string_list *b,
free(b2a);
}
-static void output_pair_header(struct diff_options *diffopt, struct strbuf
*buf,
+static void output_pair_header(struct diff_options *diffopt, int
patch_no_width,
+ struct strbuf *buf,
struct patch_util *a_util,
struct patch_util *b_util)
{
@@ -293,9 +294,9 @@ static void output_pair_header(struct diff_options
*diffopt, struct strbuf *buf,
strbuf_reset(buf);
strbuf_addstr(buf, status == '!' ? color_old : color);
if (!a_util)
- strbuf_addf(buf, "-: %s ", dashes);
+ strbuf_addf(buf, "%*s: %s ", patch_no_width, "-", dashes);
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(_util->oid, DEFAULT_ABBREV));
if (status == '!')
@@ -305,9 +306,9 @@ static void output_pair_header(struct diff_options
*diffopt, struct strbuf *buf,
strbuf_addf(buf, "%s%s", color_reset, color_new);
if (!b_util)
- strbuf_addf(buf, " -: %s", dashes);
+ strbuf_addf(buf, " %*s: %s", patch_no_width, "-", dashes);
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(_util->oid, DEFAULT_ABBREV));
commit = lookup_commit_reference(oid);
@@ -360,6 +361,7 @@ static void output(struct string_list *a, struct
string_list *b,
struct diff_options *diffopt)
{
struct strbuf buf = STRBUF_INIT;
+ int patch_no_width = decimal_width(1 + (a->nr > b->nr ? a->nr : b->nr));
int i = 0, j = 0;
/*
@@ -381,21 +383,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(diffopt, , a_util, NULL);
+ output_pair_header(diffopt, patch_no_width, ,
+ a_util, NULL);
i++;
continue;
}
/* Show unmatched RHS commits. */
while (j < b->nr && b_util->matching < 0) {
- output_pair_header(diffopt, , NULL, b_util);
+ output_pair_header(diffopt, patch_no_width, ,
+ 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(diffopt, , a_util, b_util);
+ output_pair_header(diffopt, patch_no_width, ,
+ 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);
--
gitgitgadget
[PATCH v3 12/20] range-diff: use color for the commit pairs
From: Johannes Schindelin
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 | 47 ++-
1 file changed, 34 insertions(+), 13 deletions(-)
diff --git a/range-diff.c b/range-diff.c
index 8df73da4e..870c3680c 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -254,13 +254,19 @@ 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 patch_util *a_util,
struct patch_util *b_util)
{
static char *dashes;
struct object_id *oid = a_util ? _util->oid : _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) {
char *p;
@@ -270,21 +276,33 @@ static void output_pair_header(struct strbuf *buf,
*p = '-';
}
+ 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);
else
strbuf_addf(buf, "%d: %s ", a_util->i + 1,
find_unique_abbrev(_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);
@@ -297,12 +315,15 @@ static void output_pair_header(struct strbuf *buf,
const char *commit_buffer = get_commit_buffer(commit, NULL);
const char *subject;
+ if (status == '!')
+ strbuf_addf(buf, "%s%s", color_reset, color);
+
find_commit_subject(commit_buffer, );
strbuf_addch(buf, ' ');
format_subject(buf, subject, " ");
unuse_commit_buffer(commit, commit_buffer);
}
- strbuf_addch(buf, '\n');
+ strbuf_addf(buf, "%s\n", color_reset);
fwrite(buf->buf, buf->len, 1, stdout);
}
@@ -360,21 +381,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) {
- output_pair_header(, a_util, NULL);
+ output_pair_header(diffopt, , a_util, NULL);
i++;
continue;
}
/* Show unmatched RHS commits. */
while (j < b->nr && b_util->matching < 0) {
- output_pair_header(, NULL, b_util);
+ output_pair_header(diffopt, , 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(, a_util, b_util);
+ output_pair_header(diffopt, , 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);
--
gitgitgadget
[PATCH v3 17/20] range-diff: add a man page
From: Johannes Schindelin
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 | 235 +++
1 file changed, 235 insertions(+)
create mode 100644 Documentation/git-range-diff.txt
diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
new file mode 100644
index 0..189236cc6
--- /dev/null
+++ b/Documentation/git-range-diff.txt
@@ -0,0 +1,235 @@
+git-range-diff(1)
+==
+
+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 merges).
+
+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
[PATCH v3 18/20] completion: support `git range-diff`
From: Johannes Schindelin
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, your grandfather's `git
log` arguments.
Signed-off-by: Johannes Schindelin
squash! WIP completion: support `git range-diff`
Revert "WIP completion: support `git range-diff`"
This reverts commit 2e7af652af9e53a19fd947f8ebe37a78043afa49.
---
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 94c95516e..402490673 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
--
gitgitgadget
[PATCH v3 09/20] range-diff: adjust the output of the commit pairs
From: Johannes Schindelin
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 | 66 +---
1 file changed, 57 insertions(+), 9 deletions(-)
diff --git a/range-diff.c b/range-diff.c
index 8d3b96455..0e0e77106 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 */
@@ -251,9 +253,57 @@ 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 patch_util *a_util,
+ struct patch_util *b_util)
{
- return find_unique_abbrev(>oid, DEFAULT_ABBREV);
+ static char *dashes;
+ struct object_id *oid = a_util ? _util->oid : _util->oid;
+ struct commit *commit;
+
+ if (!dashes) {
+ char *p;
+
+ dashes = xstrdup(find_unique_abbrev(oid, DEFAULT_ABBREV));
+ for (p = dashes; *p; p++)
+ *p = '-';
+ }
+
+ strbuf_reset(buf);
+ if (!a_util)
+ strbuf_addf(buf, "-: %s ", dashes);
+ else
+ strbuf_addf(buf, "%d: %s ", a_util->i + 1,
+ find_unique_abbrev(_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);
+ else
+ strbuf_addf(buf, " %d: %s", b_util->i + 1,
+ find_unique_abbrev(_util->oid, DEFAULT_ABBREV));
+
+ commit = lookup_commit_reference(oid);
+ if (commit) {
+ const char *commit_buffer = get_commit_buffer(commit, NULL);
+ const char *subject;
+
+ find_commit_subject(commit_buffer, );
+ strbuf_addch(buf, ' ');
+ format_subject(buf, subject, " ");
+ unuse_commit_buffer(commit, commit_buffer);
+ }
+ strbuf_addch(buf, '\n');
+
+ fwrite(buf->buf, buf->len, 1, stdout);
}
static struct diff_filespec *get_filespec(const char *name, const char *p)
@@ -282,6 +332,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;
int i = 0, j = 0;
/*
@@ -303,25 +354,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(, 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(, 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(, 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);
@@ -329,6 +376,7 @@ static void output(struct string_list *a, struct
string_list *b,
j++;
}
}
+ strbuf_release();
}
int show_range_diff(const char *range1, const char *range2,
--
gitgitgadget
[PATCH v3 20/20] range-diff: make --dual-color the default mode
From: Johannes Schindelin
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 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 | 13 -
builtin/range-diff.c | 10 ++
contrib/completion/git-completion.bash | 2 +-
3 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index 189236cc6..02d33ac43 100644
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt
@@ -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 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. 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/builtin/range-diff.c b/builtin/range-diff.c
index e8f7fe452..6cee0c73a 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", _factor,
N_("Percentage by which creation is weighted")),
- OPT_BOOL(0, "dual-color", _color,
+ OPT_BOOL(0, "no-dual-color", _color,
N_("color both diff and diff-between-diffs")),
OPT_END()
};
@@ -53,8 +53,10 @@ int cmd_range_diff(int argc, const char **argv, const char
*prefix)
argc = j;
diff_setup_done();
- 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 402490673..e35fc28fc 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
--
gitgitgadget
[PATCH v3 07/20] range-diff: indent the diffs just like tbdiff
From: Johannes Schindelin
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 5f12bbfa9..660e1f961 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.output_format = DIFF_FORMAT_PATCH;
+ diffopt.output_prefix = output_prefix_cb;
+ strbuf_addstr(_spaces, "");
+ diffopt.output_prefix_data = _spaces;
argc = parse_options(argc, argv, NULL, options,
builtin_range_diff_usage, PARSE_OPT_KEEP_UNKNOWN);
@@ -78,6 +87,7 @@ int cmd_range_diff(int argc, const char **argv, const char
*prefix)
strbuf_release();
strbuf_release();
+ strbuf_release(_spaces);
return res;
}
--
gitgitgadget
[PATCH v3 13/20] color: add the meta color GIT_COLOR_REVERSE
From: Johannes Schindelin 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 5b744e1bc..33e786342 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" -- gitgitgadget
[PATCH v3 10/20] range-diff: do not show "function names" in hunk headers
From: Johannes Schindelin
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 0e0e77106..8df73da4e 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 */
@@ -306,6 +307,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);
@@ -315,6 +320,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 = _func_name;
return spec;
}
--
gitgitgadget
[PATCH v3 06/20] range-diff: right-trim commit messages
From: Johannes Schindelin
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 530f2fc32..8d3b96455 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(, );
strbuf_addstr(, "\n\n");
} else if (starts_with(line.buf, "")) {
+ strbuf_rtrim();
strbuf_addbuf(, );
strbuf_addch(, '\n');
}
--
gitgitgadget
[PATCH v3 08/20] range-diff: suppress the diff headers
From: Johannes Schindelin
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 660e1f961..7c0967220 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.output_format = DIFF_FORMAT_PATCH;
+ diffopt.flags.suppress_diff_headers = 1;
diffopt.output_prefix = output_prefix_cb;
strbuf_addstr(_spaces, "");
diffopt.output_prefix_data = _spaces;
diff --git a/diff.c b/diff.c
index 639eb646b..8c568cbe0 100644
--- a/diff.c
+++ b/diff.c
@@ -3189,13 +3189,16 @@ static void builtin_diff(const char *name_a,
memset(, 0, sizeof(xpp));
memset(, 0, sizeof(xecfg));
memset(, 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(, , );
ecbdata.opt = o;
- ecbdata.header = header.len ? : NULL;
+ if (header.len && !o->flags.suppress_diff_headers)
+ ecbdata.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 dedac472c..928f48995 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,
--
gitgitgadget
[PATCH v3 16/20] range-diff --dual-color: work around bogus white-space warning
From: Johannes Schindelin
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.
Work around this by detecting that situation and simply *not* printing
the space in that case.
This is slightly improper a fix because it is conceivable that an
output_prefix might be configured with *just* the right length to let
that tab jump to a different tab stop depending whether we emit that
space or not.
However, the proper fix would be relatively ugly and intrusive because
it would have to *weaken* the WS_SPACE_BEFORE_TAB option in ws.c.
Besides, we do not expose the --dual-color option in cases other than
the `git range-diff` command, which only uses a hard-coded
output_prefix of four spaces (which misses the problem by one
column... ;-)).
Signed-off-by: Johannes Schindelin
---
diff.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/diff.c b/diff.c
index 26445ffa1..325007167 100644
--- a/diff.c
+++ b/diff.c
@@ -1093,6 +1093,12 @@ 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);
+ /* Avoid space-before-tab warning */
+ if (c == ' ' && (len < 2 || line[1] == '\t' ||
+line[1] == '\r' || line[1] == '\n')) {
+ line++;
+ len--;
+ }
}
emit_line_ws_markup(o, set, reset, line, len, set_sign, '+',
flags & DIFF_SYMBOL_CONTENT_WS_MASK,
--
gitgitgadget
[PATCH v3 01/20] linear-assignment: a function to solve least-cost assignment problems
From: Johannes Schindelin
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 | 203
linear-assignment.h | 22 +
3 files changed, 226 insertions(+)
create mode 100644 linear-assignment.c
create mode 100644 linear-assignment.h
diff --git a/Makefile b/Makefile
index 0cb6590f2..c5ba124f1 100644
--- a/Makefile
+++ b/Makefile
@@ -868,6 +868,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 0..0b0344b5f
--- /dev/null
+++ b/linear-assignment.c
@@ -0,0 +1,203 @@
+/*
+ * 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;
+
[PATCH v3 05/20] range-diff: also show the diff between patches
From: Johannes Schindelin
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 often, 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` option that is
automatically supported via our use of diff_opt_parse().
Signed-off-by: Johannes Schindelin
---
builtin/range-diff.c | 25 +
range-diff.c | 34 +++---
range-diff.h | 4 +++-
3 files changed, 55 insertions(+), 8 deletions(-)
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index c37a72100..5f12bbfa9 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,16 +14,31 @@ 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", _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;
- argc = parse_options(argc, argv, NULL, options,
builtin_range_diff_usage,
-0);
+ git_config(git_diff_ui_config, NULL);
+
+ diff_setup();
+ diffopt.output_format = DIFF_FORMAT_PATCH;
+
+ argc = parse_options(argc, argv, NULL, options,
+ builtin_range_diff_usage, PARSE_OPT_KEEP_UNKNOWN);
+
+ for (i = j = 0; i < argc; i++) {
+ int c = diff_opt_parse(, argv + i, argc - i, prefix);
+
+ if (!c)
+ argv[j++] = argv[i];
+ }
+ argc = j;
+ diff_setup_done();
if (argc == 2) {
if (!strstr(argv[0], ".."))
@@ -57,7 +73,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,
+ );
strbuf_release();
strbuf_release();
diff --git a/range-diff.c b/range-diff.c
index e71cf0ba7..530f2fc32 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 */
@@ -254,7 +255,31 @@ static const char *short_oid(struct patch_util *util)
return find_unique_abbrev(>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, _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(_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;
@@ -296,6 +321,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,
+
[PATCH v3 04/20] range-diff: improve the order of the shown commits
From: Johannes Schindelin
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 c374333a4..e71cf0ba7 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 */
@@ -256,28 +256,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++;
+ }
}
}
--
gitgitgadget
[PATCH 1/2] t3430: demonstrate what -r, --autosquash & --exec should do
From: Johannes Schindelin The --exec option's implementation is not really well-prepared for --rebase-merges. Demonstrate this. Signed-off-by: Johannes Schindelin --- t/t3430-rebase-merges.sh | 17 + 1 file changed, 17 insertions(+) diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 9e6229727..0bf5eaa37 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -363,4 +363,21 @@ test_expect_success 'octopus merges' ' EOF ' +test_expect_failure 'with --autosquash and --exec' ' + git checkout -b with-exec H && + echo Booh >B.t && + test_tick && + git commit --fixup B B.t && + write_script show.sh <<-\EOF && + subject="$(git show -s --format=%s HEAD)" + content="$(git diff HEAD^! | tail -n 1)" + echo "$subject: $content" + EOF + test_tick && + git rebase -ir --autosquash --exec ./show.sh A >actual && + grep "B: +Booh" actual && + grep "E: +Booh" actual && + grep "G: +G" actual +' + test_done -- gitgitgadget
[PATCH 0/2] Make git rebase work with --rebase-merges and --exec
It was reported via IRC that the exec lines are inserted in the wrong spots when using --rebase-merges. The reason is that we used a simple, incorrect implementation that happened to work as long as the generated todo list only contains pick, fixup and squash commands. Which is not the case with--rebase-merges. Fix this issue by using a correct, if longer and slightly more complex implementation instead. Johannes Schindelin (2): t3430: demonstrate what -r, --autosquash & --exec should do rebase --exec: make it work with --rebase-merges sequencer.c | 59 t/t3430-rebase-merges.sh | 17 2 files changed, 65 insertions(+), 11 deletions(-) base-commit: 1d89318c48d233d52f1db230cf622935ac3c69fa Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-13%2Fdscho%2Frebase-merges-and-exec-commands-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-13/dscho/rebase-merges-and-exec-commands-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/13 -- gitgitgadget
[PATCH 2/2] rebase --exec: make it work with --rebase-merges
From: Johannes Schindelin
The idea of `--exec` is to append an `exec` call after each `pick`.
Since the introduction of fixup!/squash! commits, this idea was extended
to apply to "pick, possibly followed by a fixup/squash chain", i.e. an
exec would not be inserted between a `pick` and any of its corresponding
`fixup` or `squash` lines.
The current implementation uses a dirty trick to achieve that: it
assumes that there are only pick/fixup/squash commands, and then
*inserts* the `exec` lines before any `pick` but the first, and appends
a final one.
With the todo lists generated by `git rebase --rebase-merges`, this
simple implementation shows its problems: it produces the exact wrong
thing when there are `label`, `reset` and `merge` commands.
Let's change the implementation to do exactly what we want: look for
`pick` lines, skip any fixup/squash chains, and then insert the `exec`
line. Lather, rinse, repeat.
While at it, also add `exec` lines after `merge` commands, because they
are similar in spirit to `pick` commands: they add new commits.
Signed-off-by: Johannes Schindelin
---
sequencer.c | 59
t/t3430-rebase-merges.sh | 2 +-
2 files changed, 49 insertions(+), 12 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 31038472f..dda5cdbba 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4244,10 +4244,9 @@ int sequencer_add_exec_commands(const char *commands)
{
const char *todo_file = rebase_path_todo();
struct todo_list todo_list = TODO_LIST_INIT;
- struct todo_item *item;
struct strbuf *buf = _list.buf;
size_t offset = 0, commands_len = strlen(commands);
- int i, first;
+ int i, insert_final_commands;
if (strbuf_read_file(_list.buf, todo_file, 0) < 0)
return error(_("could not read '%s'."), todo_file);
@@ -4257,19 +4256,57 @@ int sequencer_add_exec_commands(const char *commands)
return error(_("unusable todo list: '%s'"), todo_file);
}
- first = 1;
- /* insert before every pick except the first one */
- for (item = todo_list.items, i = 0; i < todo_list.nr; i++, item++) {
- if (item->command == TODO_PICK && !first) {
- strbuf_insert(buf, item->offset_in_buf + offset,
- commands, commands_len);
- offset += commands_len;
+ /*
+* Insert after every pick. Here, fixup/squash chains
+* are considered part of the pick, so we insert the commands *after*
+* those chains if there are any.
+*/
+ insert_final_commands = 1;
+ for (i = 0; i < todo_list.nr; ) {
+ enum todo_command command = todo_list.items[i].command;
+ int j = 0;
+
+ if (command != TODO_PICK && command != TODO_MERGE) {
+ i++;
+ continue;
+ }
+
+ /* skip fixup/squash chain, if any */
+ for (i++; i < todo_list.nr; i++, j = 0) {
+ command = todo_list.items[i].command;
+
+ if (is_fixup(command))
+ continue;
+
+ if (command != TODO_COMMENT)
+ break;
+
+ /* skip comment if followed by any fixup/squash */
+ for (j = i + 1; j < todo_list.nr; j++)
+ if (todo_list.items[j].command != TODO_COMMENT)
+ break;
+ if (j < todo_list.nr &&
+ is_fixup(todo_list.items[j].command)) {
+ i = j;
+ continue;
+ }
+ break;
}
- first = 0;
+
+ if (i >= todo_list.nr) {
+ insert_final_commands = 1;
+ break;
+ }
+
+ strbuf_insert(buf, todo_list.items[i].offset_in_buf + offset,
+ commands, commands_len);
+ offset += commands_len;
+ insert_final_commands = 0;
}
/* append final */
- strbuf_add(buf, commands, commands_len);
+ if (insert_final_commands)
+ strbuf_add(buf, commands, commands_len);
i = write_message(buf->buf, buf->len, todo_file, 0);
todo_list_release(_list);
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 0bf5eaa37..90ae613e2 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -363,7 +363,7 @@ test_expect_success 'octopus merges' '
EOF
'
-test_expect_failure 'with --autosquash and --exec' '
+test_expect_success 'with --autosquash and --exec' '
git checkout -b with-exec H &&
echo Booh >B.t &&
test_tick &&
--
gitgitgadget
[PATCH v2 8/9] vscode: add a dictionary for cSpell
From: Johannes Schindelin
The quite useful cSpell extension allows VS Code to have "squiggly"
lines under spelling mistakes. By default, this would add too much
clutter, though, because so much of Git's source code uses words that
would trigger cSpell.
Let's add a few words to make the spell checking more useful by reducing
the number of false positives.
Signed-off-by: Johannes Schindelin
---
contrib/vscode/init.sh | 169 -
1 file changed, 168 insertions(+), 1 deletion(-)
diff --git a/contrib/vscode/init.sh b/contrib/vscode/init.sh
index 29f2a729d..a134cb4c5 100755
--- a/contrib/vscode/init.sh
+++ b/contrib/vscode/init.sh
@@ -32,7 +32,174 @@ cat >.vscode/settings.json.new <<\EOF ||
"files.associations": {
"*.h": "c",
"*.c": "c"
-}
+},
+"cSpell.words": [
+"DATAW",
+"DBCACHED",
+"DFCHECK",
+"DTYPE",
+"Hamano",
+"HCAST",
+"HEXSZ",
+"HKEY",
+"HKLM",
+"IFGITLINK",
+"IFINVALID",
+"ISBROKEN",
+"ISGITLINK",
+"ISSYMREF",
+"Junio",
+"LPDWORD",
+"LPPROC",
+"LPWSTR",
+"MSVCRT",
+"NOARG",
+"NOCOMPLETE",
+"NOINHERIT",
+"RENORMALIZE",
+"STARTF",
+"STARTUPINFOEXW",
+"Schindelin",
+"UCRT",
+"YESNO",
+"argcp",
+"beginthreadex",
+"committish",
+"contentp",
+"cpath",
+"cpidx",
+"ctim",
+"dequote",
+"envw",
+"ewah",
+"fdata",
+"fherr",
+"fhin",
+"fhout",
+"fragp",
+"fsmonitor",
+"hnsec",
+"idents",
+"includeif",
+"interpr",
+"iprog",
+"isexe",
+"iskeychar",
+"kompare",
+"mksnpath",
+"mktag",
+"mktree",
+"mmblob",
+"mmbuffer",
+"mmfile",
+"noenv",
+"nparents",
+"ntpath",
+"ondisk",
+"ooid",
+"oplen",
+"osdl",
+"pnew",
+"pold",
+"ppinfo",
+"pushf",
+"pushv",
+"rawsz",
+"rebasing",
+"reencode",
+"repo",
+"rerere",
+"scld",
+"sharedrepo",
+"spawnv",
+"spawnve",
+"spawnvpe",
+"strdup'ing",
+"submodule",
+"submodules",
+"topath",
+"topo",
+"tpatch",
+"unexecutable",
+"unhide",
+"unkc",
+"unkv",
+"unmark",
+"unmatch",
+"unsets",
+"unshown",
+"untracked",
+"untrackedcache",
+"unuse",
+"upos",
+"uval",
+"vreportf",
+"wargs",
+"wargv",
+"wbuffer",
+"wcmd",
+"wcsnicmp",
+"wcstoutfdup",
+"wdeltaenv",
+"wdir",
+"wenv",
+"wenvblk",
+"wenvcmp",
+"wenviron",
+"wenvpos",
+"wenvsz",
+"wfile",
+"wfilename",
+"wfopen",
+"wfreopen",
+"wfullpath",
+"which'll",
+"wlink",
+"wmain",
+"wmkdir",
+"wmktemp",
+"wnewpath",
+"wotype",
+"wpath",
+"wpathname",
+"wpgmptr",
+"wpnew",
+"wpointer",
+"wpold",
+"wpos",
+"wputenv",
+"wrmdir",
+"wship",
+"wtarget",
+"wtemplate",
+"wunlink",
+"xcalloc",
+"xgetcwd",
+"xmallocz",
+"xmemdupz",
+"xmmap",
+"xopts",
+"xrealloc",
+"xsnprintf",
+"xutftowcs",
+"xutftowcsn",
+"xwcstoutf"
+],
+"cSpell.ignoreRegExpList": [
+"\\\"(DIRC|FSMN|REUC|UNTR)\\\"",
+"u[0-9a-fA-Fx]{4}\\b",
+"\\b(filfre|frotz|xyzzy)\\b",
+"\\bCMIT_FMT_DEFAULT\\b",
+"\\bde-munge\\b",
+"\\bGET_OID_DISAMBIGUATORS\\b",
+"\\bHASH_RENORMALIZE\\b",
+"\\bTREESAMEness\\b",
+"\\bUSE_STDEV\\b",
+"\\Wchar *\\*\\W*utfs\\W",
+"cURL's",
+"nedmalloc'ed",
+"ntifs\\.h",
+],
}
EOF
die "Could not write settings.json"
--
gitgitgadget
[PATCH v2 7/9] vscode: use 8-space tabs, no trailing ws, etc for Git's source code
From: Johannes Schindelin
This adds a couple settings for the .c/.h files so that it is easier to
conform to Git's conventions while editing the source code.
Signed-off-by: Johannes Schindelin
---
contrib/vscode/init.sh | 8
1 file changed, 8 insertions(+)
diff --git a/contrib/vscode/init.sh b/contrib/vscode/init.sh
index face115e8..29f2a729d 100755
--- a/contrib/vscode/init.sh
+++ b/contrib/vscode/init.sh
@@ -21,6 +21,14 @@ cat >.vscode/settings.json.new <<\EOF ||
"editor.wordWrap": "wordWrapColumn",
"editor.wordWrapColumn": 72
},
+"[c]": {
+"editor.detectIndentation": false,
+"editor.insertSpaces": false,
+"editor.tabSize": 8,
+"editor.wordWrap": "wordWrapColumn",
+"editor.wordWrapColumn": 80,
+"files.trimTrailingWhitespace": true
+},
"files.associations": {
"*.h": "c",
"*.c": "c"
--
gitgitgadget
[PATCH v2 4/9] mingw: define WIN32 explicitly
From: Johannes Schindelin This helps VS Code's intellisense to figure out that we want to include windows.h, and that we want to define the minimum target Windows version as Windows Vista/2008R2. Signed-off-by: Johannes Schindelin --- config.mak.uname | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.mak.uname b/config.mak.uname index 684fc5bf0..2be2f1981 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -528,7 +528,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) COMPAT_OBJS += compat/mingw.o compat/winansi.o \ compat/win32/pthread.o compat/win32/syslog.o \ compat/win32/dirent.o - BASIC_CFLAGS += -DPROTECT_NTFS_DEFAULT=1 + BASIC_CFLAGS += -DWIN32 -DPROTECT_NTFS_DEFAULT=1 EXTLIBS += -lws2_32 GITLIBS += git.res PTHREAD_LIBS = -- gitgitgadget
[PATCH v2 1/9] contrib: add a script to initialize VS Code configuration
From: Johannes Schindelin
VS Code is a lightweight but powerful source code editor which runs on
your desktop and is available for Windows, macOS and Linux. Among other
languages, it has support for C/C++ via an extension, which offers to
not only build and debug the code, but also Intellisense, i.e.
code-aware completion and similar niceties.
This patch adds a script that helps set up the environment to work
effectively with VS Code: simply run the Unix shell script
contrib/vscode/init.sh, which creates the relevant files, and open the
top level folder of Git's source code in VS Code.
Signed-off-by: Johannes Schindelin
---
.gitignore| 1 +
contrib/vscode/.gitattributes | 1 +
contrib/vscode/README.md | 14 +++
contrib/vscode/init.sh| 165 ++
4 files changed, 181 insertions(+)
create mode 100644 contrib/vscode/.gitattributes
create mode 100644 contrib/vscode/README.md
create mode 100755 contrib/vscode/init.sh
diff --git a/.gitignore b/.gitignore
index 388cc4bee..592e8f879 100644
--- a/.gitignore
+++ b/.gitignore
@@ -206,6 +206,7 @@
/config.mak.autogen
/config.mak.append
/configure
+/.vscode/
/tags
/TAGS
/cscope*
diff --git a/contrib/vscode/.gitattributes b/contrib/vscode/.gitattributes
new file mode 100644
index 0..e89f2236e
--- /dev/null
+++ b/contrib/vscode/.gitattributes
@@ -0,0 +1 @@
+init.sh whitespace=-indent-with-non-tab
diff --git a/contrib/vscode/README.md b/contrib/vscode/README.md
new file mode 100644
index 0..8202d6203
--- /dev/null
+++ b/contrib/vscode/README.md
@@ -0,0 +1,14 @@
+Configuration for VS Code
+=
+
+[VS Code](https://code.visualstudio.com/) is a lightweight but powerful source
+code editor which runs on your desktop and is available for
+[Windows](https://code.visualstudio.com/docs/setup/windows),
+[macOS](https://code.visualstudio.com/docs/setup/mac) and
+[Linux](https://code.visualstudio.com/docs/setup/linux). Among other languages,
+it has [support for C/C++ via an
extension](https://github.com/Microsoft/vscode-cpptools).
+
+To start developing Git with VS Code, simply run the Unix shell script called
+`init.sh` in this directory, which creates the configuration files in
+`.vscode/` that VS Code consumes. `init.sh` needs access to `make` and `gcc`,
+so run the script in a Git SDK shell if you are using Windows.
diff --git a/contrib/vscode/init.sh b/contrib/vscode/init.sh
new file mode 100755
index 0..3cc93243f
--- /dev/null
+++ b/contrib/vscode/init.sh
@@ -0,0 +1,165 @@
+#!/bin/sh
+
+die () {
+ echo "$*" >&2
+ exit 1
+}
+
+cd "$(dirname "$0")"/../.. ||
+die "Could not cd to top-level directory"
+
+mkdir -p .vscode ||
+die "Could not create .vscode/"
+
+# General settings
+
+cat >.vscode/settings.json <<\EOF ||
+{
+"C_Cpp.intelliSenseEngine": "Default",
+"C_Cpp.intelliSenseEngineFallback": "Disabled",
+"files.associations": {
+"*.h": "c",
+"*.c": "c"
+}
+}
+EOF
+die "Could not write settings.json"
+
+# Infer some setup-specific locations/names
+
+GCCPATH="$(which gcc)"
+GDBPATH="$(which gdb)"
+MAKECOMMAND="make -j5 DEVELOPER=1"
+OSNAME=
+X=
+case "$(uname -s)" in
+MINGW*)
+ GCCPATH="$(cygpath -am "$GCCPATH")"
+ GDBPATH="$(cygpath -am "$GDBPATH")"
+ MAKE_BASH="$(cygpath -am /git-cmd.exe) --command=usrbinbash.exe"
+ MAKECOMMAND="$MAKE_BASH -lc \\\"$MAKECOMMAND\\\""
+ OSNAME=Win32
+ X=.exe
+ ;;
+Linux)
+ OSNAME=Linux
+ ;;
+Darwin)
+ OSNAME=macOS
+ ;;
+esac
+
+# Default build task
+
+cat >.vscode/tasks.json .vscode/launch.json
[PATCH v2 3/9] cache.h: extract enum declaration from inside a struct declaration
From: Johannes Schindelin
While it is technically possible, it is confusing. Not only the user,
but also VS Code's intellisense.
Signed-off-by: Johannes Schindelin
---
cache.h | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/cache.h b/cache.h
index 89a107a7f..2380136f6 100644
--- a/cache.h
+++ b/cache.h
@@ -1425,18 +1425,20 @@ extern void *read_object_with_reference(const struct
object_id *oid,
extern struct object *peel_to_type(const char *name, int namelen,
struct object *o, enum object_type);
+enum date_mode_type {
+ DATE_NORMAL = 0,
+ DATE_RELATIVE,
+ DATE_SHORT,
+ DATE_ISO8601,
+ DATE_ISO8601_STRICT,
+ DATE_RFC2822,
+ DATE_STRFTIME,
+ DATE_RAW,
+ DATE_UNIX
+};
+
struct date_mode {
- enum date_mode_type {
- DATE_NORMAL = 0,
- DATE_RELATIVE,
- DATE_SHORT,
- DATE_ISO8601,
- DATE_ISO8601_STRICT,
- DATE_RFC2822,
- DATE_STRFTIME,
- DATE_RAW,
- DATE_UNIX
- } type;
+ enum date_mode_type type;
const char *strftime_fmt;
int local;
};
--
gitgitgadget
[PATCH v2 5/9] vscode: only overwrite C/C++ settings
From: Johannes Schindelin
The C/C++ settings are special, as they are the only generated VS Code
configurations that *will* change over the course of Git's development,
e.g. when a new constant is defined.
Therefore, let's only update the C/C++ settings, also to prevent user
modifications from being overwritten.
Ideally, we would keep user modifications in the C/C++ settings, but
that would require parsing JSON, a task for which a Unix shell script is
distinctly unsuited. So we write out .new files instead, and warn the
user if they may want to reconcile their changes.
Signed-off-by: Johannes Schindelin
---
contrib/vscode/init.sh | 23 ---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/contrib/vscode/init.sh b/contrib/vscode/init.sh
index 494a51ac5..ba9469226 100755
--- a/contrib/vscode/init.sh
+++ b/contrib/vscode/init.sh
@@ -13,7 +13,7 @@ die "Could not create .vscode/"
# General settings
-cat >.vscode/settings.json <<\EOF ||
+cat >.vscode/settings.json.new <<\EOF ||
{
"C_Cpp.intelliSenseEngine": "Default",
"C_Cpp.intelliSenseEngineFallback": "Disabled",
@@ -51,7 +51,7 @@ esac
# Default build task
-cat >.vscode/tasks.json <.vscode/tasks.json.new .vscode/launch.json <.vscode/launch.json.new <
[PATCH v2 2/9] vscode: hard-code a couple defines
From: Johannes Schindelin
Sadly, we do not get all of the definitions via ALL_CFLAGS. Some defines
are passed to GCC *only* when compiling specific files, such as git.o.
Let's just hard-code them into the script for the time being.
Signed-off-by: Johannes Schindelin
---
contrib/vscode/init.sh | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/contrib/vscode/init.sh b/contrib/vscode/init.sh
index 3cc93243f..494a51ac5 100755
--- a/contrib/vscode/init.sh
+++ b/contrib/vscode/init.sh
@@ -115,7 +115,19 @@ include Makefile
vscode-init:
@mkdir -p .vscode && \
incs= && defs= && \
- for e in $(ALL_CFLAGS); do \
+ for e in $(ALL_CFLAGS) \
+ '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
+ '-DGIT_LOCALE_PATH="$(localedir_relative_SQ)"' \
+ '-DBINDIR="$(bindir_relative_SQ)"' \
+ '-DFALLBACK_RUNTIME_PREFIX="$(prefix_SQ)"' \
+ '-DDEFAULT_GIT_TEMPLATE_DIR="$(template_dir_SQ)"' \
+ '-DETC_GITCONFIG="$(ETC_GITCONFIG_SQ)"' \
+ '-DETC_GITATTRIBUTES="$(ETC_GITATTRIBUTES_SQ)"' \
+ '-DGIT_LOCALE_PATH="$(localedir_relative_SQ)"' \
+ '-DCURL_DISABLE_TYPECHECK', \
+ '-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
+ '-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \
+ '-DGIT_INFO_PATH="$(infodir_relative_SQ)"'; do \
case "$$e" in \
-I.) \
incs="$$(printf '% 16s"$${workspaceRoot}",\n%s' \
--
gitgitgadget
[PATCH v2 0/9] Add support to develop Git in Visual Studio Code
[Visual Studio Code](https://code.visualstudio.com/) (nickname "VS Code") is a light-weight, cross-platform, Open Source development environment, with an increasingly powerful extension to support C/C++ development. In particular the intellisense support as well as all the other niceties developers might have come to expect from Integrated Development Environments will help accelerate development. Due to the way Git's source code and build process is structured, it can be quite challenging to use VS Code effectively for developing Git. Which is a shame, as developing with vim and the command-line causes unnecessary churn, and it is quite understandable that most Git developers simply do not want to fight with a modern development environment just to try whether they like developing Git with it. This topic branch makes it easy to get started using VS Code to develop Git. Simply run the script `./contrib/vscode/init.sh`. This will initialize the `.vscode/` directory and some files in that directory. After that, just open Git's top-level directory as "folder" in VS Code. The files have to be generated because of the curious way Git determines what flags to pass to the C compiler, in particular which constants are defined, because they change the compile flow in rather dramatic ways (determining, e.g. which SHA-1 backend to use). Changes since v1: - Clarified commit message of the first commit. Johannes Schindelin (9): contrib: add a script to initialize VS Code configuration vscode: hard-code a couple defines cache.h: extract enum declaration from inside a struct declaration mingw: define WIN32 explicitly vscode: only overwrite C/C++ settings vscode: wrap commit messages at column 72 by default vscode: use 8-space tabs, no trailing ws, etc for Git's source code vscode: add a dictionary for cSpell vscode: let cSpell work on commit messages, too .gitignore| 1 + cache.h | 24 ++- config.mak.uname | 2 +- contrib/vscode/.gitattributes | 1 + contrib/vscode/README.md | 14 ++ contrib/vscode/init.sh| 375 ++ 6 files changed, 405 insertions(+), 12 deletions(-) create mode 100644 contrib/vscode/.gitattributes create mode 100644 contrib/vscode/README.md create mode 100755 contrib/vscode/init.sh base-commit: 53f9a3e157dbbc901a02ac2c73346d375e24978c Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-2%2Fdscho%2Fvscode-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2/dscho/vscode-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/2 Range-diff vs v1: 1: e2e449a00 ! 1: bbf13e40a contrib: add a script to initialize VS Code configuration @@ -4,11 +4,14 @@ VS Code is a lightweight but powerful source code editor which runs on your desktop and is available for Windows, macOS and Linux. Among other -languages, it has support for C/C++ via an extension. +languages, it has support for C/C++ via an extension, which offers to +not only build and debug the code, but also Intellisense, i.e. +code-aware completion and similar niceties. -To start developing Git with VS Code, simply run the Unix shell script -contrib/vscode/init.sh, which creates the configuration files in -.vscode/ that VS Code consumes. +This patch adds a script that helps set up the environment to work +effectively with VS Code: simply run the Unix shell script +contrib/vscode/init.sh, which creates the relevant files, and open the +top level folder of Git's source code in VS Code. Signed-off-by: Johannes Schindelin 2: 3770bd855 = 2: 6c8b5a4f2 vscode: hard-code a couple defines 3: de49c4bf2 = 3: 105b50c62 cache.h: extract enum declaration from inside a struct declaration 4: b3ce2ccf4 = 4: 4b95b1e2a mingw: define WIN32 explicitly 5: 06dcf6d97 = 5: 3dd53c04c vscode: only overwrite C/C++ settings 6: 08212c57e = 6: 384b08836 vscode: wrap commit messages at column 72 by default 7: 2e880b6f1 = 7: ba78af756 vscode: use 8-space tabs, no trailing ws, etc for Git's source code 8: ce216cf43 = 8: 358f38d3a vscode: add a dictionary for cSpell 9: 4c2aa015a = 9: b9c628b88 vscode: let cSpell work on commit messages, too -- gitgitgadget
[PATCH v2 6/9] vscode: wrap commit messages at column 72 by default
From: Johannes Schindelin
When configuring VS Code as core.editor (via `code --wait`), we really
want to adhere to the Git conventions of wrapping commit messages.
Signed-off-by: Johannes Schindelin
---
contrib/vscode/init.sh | 4
1 file changed, 4 insertions(+)
diff --git a/contrib/vscode/init.sh b/contrib/vscode/init.sh
index ba9469226..face115e8 100755
--- a/contrib/vscode/init.sh
+++ b/contrib/vscode/init.sh
@@ -17,6 +17,10 @@ cat >.vscode/settings.json.new <<\EOF ||
{
"C_Cpp.intelliSenseEngine": "Default",
"C_Cpp.intelliSenseEngineFallback": "Disabled",
+"[git-commit]": {
+"editor.wordWrap": "wordWrapColumn",
+"editor.wordWrapColumn": 72
+},
"files.associations": {
"*.h": "c",
"*.c": "c"
--
gitgitgadget
[PATCH v2 9/9] vscode: let cSpell work on commit messages, too
From: Johannes Schindelin By default, the cSpell extension ignores all files under .git/. That includes, unfortunately, COMMIT_EDITMSG, i.e. commit messages. However, spell checking is *quite* useful when writing commit messages... And since the user hardly ever opens any file inside .git (apart from commit messages, the config, and sometimes interactive rebase's todo lists), there is really not much harm in *not* ignoring .git/. The default also ignores `node_modules/`, but that does not apply to Git, so let's skip ignoring that, too. Signed-off-by: Johannes Schindelin --- contrib/vscode/init.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contrib/vscode/init.sh b/contrib/vscode/init.sh index a134cb4c5..27de94994 100755 --- a/contrib/vscode/init.sh +++ b/contrib/vscode/init.sh @@ -33,6 +33,8 @@ cat >.vscode/settings.json.new <<\EOF || "*.h": "c", "*.c": "c" }, +"cSpell.ignorePaths": [ +], "cSpell.words": [ "DATAW", "DBCACHED", -- gitgitgadget
[PATCH 0/1] Support git pull --rebase=i
The patch [https://github.com/git-for-windows/git/commit/4aa8b8c82] that introduced support for pull --rebase= into the Git for Windows project still allowed the very convenient abbreviation git pull --rebase=i which was later lost when it was ported to the builtin git pull, and it was not introduced before the patch eventually made it into Git as f5eb87b98dd (pull: allow interactive rebase with --rebase=interactive, 2016-01-13). However, it is really a useful short hand for the occasional rebasing pull on branches that do not usually want to be rebased. So let's reintroduce this convenience, at long last. Johannes Schindelin (1): pull --rebase=: allow single-letter abbreviations for the type builtin/pull.c | 6 +++--- t/t5520-pull.sh | 12 2 files changed, 15 insertions(+), 3 deletions(-) base-commit: 1d89318c48d233d52f1db230cf622935ac3c69fa Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-14%2Fdscho%2Fpull-rebase-i-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-14/dscho/pull-rebase-i-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/14 -- gitgitgadget
[PATCH 1/1] pull --rebase=: allow single-letter abbreviations for the type
From: Johannes Schindelin Git for Windows' original 4aa8b8c8283 (Teach 'git pull' to handle --rebase=interactive, 2011-10-21) had support for the very convenient abbreviation git pull --rebase=i which was later lost when it was ported to the builtin `git pull`, and it was not introduced before the patch eventually made it into Git as f5eb87b98dd (pull: allow interactive rebase with --rebase=interactive, 2016-01-13). However, it is *really* a useful short hand for the occasional rebasing pull on branches that do not usually want to be rebased. So let's reintroduce this convenience, at long last. Signed-off-by: Johannes Schindelin --- builtin/pull.c | 6 +++--- t/t5520-pull.sh | 12 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/builtin/pull.c b/builtin/pull.c index 4e7893539..53bc5facf 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -48,11 +48,11 @@ static enum rebase_type parse_config_rebase(const char *key, const char *value, return REBASE_FALSE; else if (v > 0) return REBASE_TRUE; - else if (!strcmp(value, "preserve")) + else if (!strcmp(value, "preserve") || !strcmp(value, "p")) return REBASE_PRESERVE; - else if (!strcmp(value, "merges")) + else if (!strcmp(value, "merges") || !strcmp(value, "m")) return REBASE_MERGES; - else if (!strcmp(value, "interactive")) + else if (!strcmp(value, "interactive") || !strcmp(value, "i")) return REBASE_INTERACTIVE; if (fatal) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 68aa5f034..5e501c8b0 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -475,10 +475,22 @@ test_expect_success 'pull.rebase=interactive' ' false EOF test_set_editor "$TRASH_DIRECTORY/fake-editor" && + test_when_finished "test_might_fail git rebase --abort" && test_must_fail git pull --rebase=interactive . copy && test "I was here" = "$(cat fake.out)" ' +test_expect_success 'pull --rebase=i' ' + write_script "$TRASH_DIRECTORY/fake-editor" <<-\EOF && + echo I was here, too >fake.out && + false + EOF + test_set_editor "$TRASH_DIRECTORY/fake-editor" && + test_when_finished "test_might_fail git rebase --abort" && + test_must_fail git pull --rebase=i . copy && + test "I was here, too" = "$(cat fake.out)" +' + test_expect_success 'pull.rebase=invalid fails' ' git reset --hard before-preserve-rebase && test_config pull.rebase invalid && -- gitgitgadget
[PATCH 0/4] line-log: be more careful when adjusting multiple line ranges
I am a heavy user of git log -L In fact, I use the feature where
multiple ranges can be specified extensively, via a not-exactly-trivial
shell script function that takes the currently staged changes (or if none
are staged, the current unstanged changes) and turns them into the
corresponding commit history:
staged_log () { #
diff="$(git diff --cached -U1)"
test -n "$diff" ||
diff="$(git diff -U1)"
test -n "$diff" ||
die "No changes"
eval "git log $(echo "$diff" |
sed -ne '/^--- a\//{s/^-* a\/\(.*\)/'\''\1'\''/;x}' -e \
'/^@@ -/{s/^@@ -\([^, ]*\),\([^ ]*\).*/-L
\1,+\2/;G;s/\n/:/g;p}' |
tr '\n' ' ')"
}
This is an extremely useful way to look at the history, especially when
trying to fix up a commit deep in a long branch (or a thicket of branches).
Today, however, this method failed me, by greeting me with an assertion.
When I tried to paper over that assertion by joining line ranges that became
adjacent (or overlapping), it still produced a segmentation fault when the
line-log tried to print lines past the file contents.
So I had no choice but to fix this properly.
I still wanted to keep the optimization where multiple line ranges are
joined into a single one (I am convinced that this also affects the output,
where previously multiple hunks would have been displayed, but I ran out of
time to investigate this). This is the 3rd patch. It is not purely an
optimization, as the assertion would still trigger when line ranges could be
joined.
Now, I am fairly certain that the changes are correct, but given my track
record with off-by-one bugs (and once even an off-by-two bug), I would
really appreciate some thorough review of this code, in particular the
second one that is the actual bug fix. I am specifically interested in
reviews from people who know line-log.c pretty well and can tell me whether
the src[i].end > target[j].end is correct, or whether it should actually
have been a >= (I tried to wrap my head around this, but I would feel more
comfortable if a domain expert would analyze this, whistling, and looking
Eric's way).
Cc: Eric Sunshine sunsh...@sunshineco.com [sunsh...@sunshineco.com]
Johannes Schindelin (4):
line-log: demonstrate a bug with nearly-overlapping ranges
line-log: adjust start/end of ranges individually
line-log: optimize ranges by joining them when possible
line-log: convert an assertion to a full BUG() call
line-log.c | 18 +++---
t/t4211-line-log.sh | 17 +
2 files changed, 32 insertions(+), 3 deletions(-)
base-commit: 1d89318c48d233d52f1db230cf622935ac3c69fa
Published-As:
https://github.com/gitgitgadget/git/releases/tags/pr-15%2Fdscho%2Fline-log-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-15/dscho/line-log-fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/15
--
gitgitgadget
[PATCH 4/4] line-log: convert an assertion to a full BUG() call
From: Johannes Schindelin
The assertion in question really indicates a bug, when triggered, so we
might just as well use the sanctioned method to report it.
Signed-off-by: Johannes Schindelin
---
line-log.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/line-log.c b/line-log.c
index bc7ef69d6..0e09df9db 100644
--- a/line-log.c
+++ b/line-log.c
@@ -72,7 +72,9 @@ void range_set_append(struct range_set *rs, long a, long b)
rs->ranges[rs->nr-1].end = b;
return;
}
- assert(rs->nr == 0 || rs->ranges[rs->nr-1].end <= a);
+ if (rs->nr > 0 && rs->ranges[rs->nr-1].end > a)
+ BUG("append %ld-%ld, after %ld-%ld?!?", a, b,
+ rs->ranges[rs->nr-1].start, rs->ranges[rs->nr-1].end);
range_set_append_unsafe(rs, a, b);
}
--
gitgitgadget
[PATCH 3/4] line-log: optimize ranges by joining them when possible
From: Johannes Schindelin
Technically, it is okay to have line ranges that touch (i.e. the end of
the first range ends just before the next range begins). However, it is
inefficient, and when the user provides such touching ranges via
multiple `-L` options, we already join them.
When we traverse the history, though, we never join ranges, even they
become "touchy-feely" due to added lines (which are "removed" from
line-log's point of view because it traverses the commit history into
the past).
Let's optimize also this case.
Signed-off-by: Johannes Schindelin
---
line-log.c | 4
t/t4211-line-log.sh | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/line-log.c b/line-log.c
index d8d09b5ee..bc7ef69d6 100644
--- a/line-log.c
+++ b/line-log.c
@@ -68,6 +68,10 @@ void range_set_append_unsafe(struct range_set *rs, long a,
long b)
void range_set_append(struct range_set *rs, long a, long b)
{
+ if (rs->nr > 0 && rs->ranges[rs->nr-1].end + 1 == a) {
+ rs->ranges[rs->nr-1].end = b;
+ return;
+ }
assert(rs->nr == 0 || rs->ranges[rs->nr-1].end <= a);
range_set_append_unsafe(rs, a, b);
}
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 61ff37430..ebaf5ea86 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -119,7 +119,7 @@ q_to_lf () {
tr Q '\012'
}
-test_expect_failure 'close to overlapping ranges' '
+test_expect_success 'close to overlapping ranges' '
test_seq 5 >a1.c &&
git add a1.c &&
git commit -m "5 lines" a1.c &&
--
gitgitgadget
[PATCH 1/4] line-log: demonstrate a bug with nearly-overlapping ranges
From: Johannes Schindelin
Currently, this test case throws an assertion:
Assertion failed!
Program: git.exe
File: line-log.c, Line 71
Signed-off-by: Johannes Schindelin
---
t/t4211-line-log.sh | 17 +
1 file changed, 17 insertions(+)
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 436b13ad2..61ff37430 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -115,4 +115,21 @@ test_expect_success 'range_set_union' '
git log $(for x in $(test_seq 200); do echo -L $((2*x)),+1:c.c; done)
'
+q_to_lf () {
+ tr Q '\012'
+}
+
+test_expect_failure 'close to overlapping ranges' '
+ test_seq 5 >a1.c &&
+ git add a1.c &&
+ git commit -m "5 lines" a1.c &&
+ sed s/3/3QaQb/ tmp &&
+ mv tmp a1.c &&
+ git commit -m "2 more lines" a1.c &&
+ sed s/4/cQ4/ tmp &&
+ mv tmp a1.c &&
+ git commit -m "1 more line" a1.c &&
+ git --no-pager log -L 1,3:a1.c -L 5,8:a1.c
+'
+
test_done
--
gitgitgadget
[PATCH 2/4] line-log: adjust start/end of ranges individually
From: Johannes Schindelin
When traversing commits and adjusting the ranges, things can get really
tricky. For example, when the line range of interest encloses several
hunks of a commit, the line range can actually shrink.
Currently, range_set_shift_diff() does not anticipate that scenario and
blindly adjusts start and end by the same offset ("shift" the range).
This can lead to a couple of surprising issues, such as assertions in
range_set_append() (when the end of a given range is not adjusted
properly, it can point after the start of the next range) or even
segmentation faults (when t_end in the loop of dump_diff_hacky_one()
points outside the valid line range).
Let's fix this by adjusting the start and the end offsets individually.
Signed-off-by: Johannes Schindelin
---
line-log.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/line-log.c b/line-log.c
index 72a5fed66..d8d09b5ee 100644
--- a/line-log.c
+++ b/line-log.c
@@ -427,7 +427,7 @@ static void range_set_shift_diff(struct range_set *out,
struct diff_ranges *diff)
{
unsigned int i, j = 0;
- long offset = 0;
+ long offset = 0, start_offset;
struct range *src = rs->ranges;
struct range *target = diff->target.ranges;
struct range *parent = diff->parent.ranges;
@@ -438,7 +438,13 @@ static void range_set_shift_diff(struct range_set *out,
- (target[j].end-target[j].start);
j++;
}
- range_set_append(out, src[i].start+offset, src[i].end+offset);
+ start_offset = offset;
+ while (j < diff->target.nr && src[i].end > target[j].end) {
+ offset += (parent[j].end-parent[j].start)
+ - (target[j].end-target[j].start);
+ j++;
+ }
+ range_set_append(out, src[i].start+start_offset,
src[i].end+offset);
}
}
--
gitgitgadget
[PATCH v2 1/4] Introduce a function to lock/unlock file descriptors when appending
From: Johannes Schindelin
This function will be used to make write accesses in trace_write() a bit
safer.
Note: this patch takes a very different approach for cross-platform
support than Git is historically taking: the original approach is to
first implement everything on Linux, using the functions available on
Linux, and then trying to emulate these functions on platforms that do
not have those functions such as Windows.
This leads to all kinds of undesirable quirks in Git's source code (and
performance characteristics) because of the lack of a proper abstraction
layer: instead of declaring functions for the functionality we
*actually* need, we abuse POSIX functions to say what we need, even if
those functions serve much broader purposes (and do not make at all
clear what the caller wants of them).
For example, when Git wants to determine whether a path refers to a
symbolic link, it calls lstat(). That POSIX function has to be emulated
on Windows, painstakingly filling all the information lstat() would,
only for the caller to throw away everything except that one bit of
information, and all of the time figuring out the mtime/atime/ctime and
file size and whatnot was spent in vain.
If we were to follow that approach, we would extend the fcntl()
emulation in compat/mingw.c after this commit, to support the function
added in this commit.
But fcntl() allows a lot more versatile file region locking that we
actually need, so by necessity the fcntl() emulation would be quite
complex: To support the `l_whence = SEEK_CUR` (which we would use, if it
did not require so much book-keeping due to our writing between locking
and unlocking the file), we would have to call `SetFilePointerEx()`
(after obtaining the `HANDLE` on which all Win32 API functions work
instead of the integer file descriptors used by all POSIX functions).
Multiplying the number of Win32 API round-trips. Of course, we could
implement an incomplete emulation of fcntl()'s F_WRLCK, but that would
be unsatisfying.
An alternative approach would be to use the `flock()` function, whose
semantics are much closer to existing Win32 API. But `flock()` is not
even a POSIX standard, so we would have to implement emulation of
`flock()` for *other* platforms. And it would again be the wrong thing
to do, as we would once again fail to have an abstraction that clearly
says what *exact*functionality we want to use.
This commit expressly tries to set a precedent for a better approach:
Let's introduce a proper abstraction: a function that says in its name
precisely what Git wants it to do (as opposed to *how* it does it on
Linux): lock_or_unlock_fd_for_appending().
The next commit will provide a Windows-specific implementation of this
function/functionality.
Signed-off-by: Johannes Schindelin
---
git-compat-util.h | 2 ++
wrapper.c | 16
2 files changed, 18 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 9a64998b2..13b83bade 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1202,6 +1202,8 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
#define getc_unlocked(fh) getc(fh)
#endif
+extern int lock_or_unlock_fd_for_appending(int fd, int lock_it);
+
/*
* Our code often opens a path to an optional file, to work on its
* contents when we can successfully open it. We can ignore a failure
diff --git a/wrapper.c b/wrapper.c
index e4fa9d84c..5aecbda34 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -690,3 +690,19 @@ int xgethostname(char *buf, size_t len)
buf[len - 1] = 0;
return ret;
}
+
+#ifndef GIT_WINDOWS_NATIVE
+int lock_or_unlock_fd_for_appending(int fd, int lock_it)
+{
+ struct flock flock;
+
+ flock.l_type = lock_it ? F_WRLCK : F_UNLCK;
+
+ /* (un-)lock the whole file */
+ flock.l_whence = SEEK_SET;
+ flock.l_start = 0;
+ flock.l_len = 0;
+
+ return fcntl(fd, F_SETLKW, );
+}
+#endif
--
gitgitgadget
[PATCH v2 0/4] t5552: fix flakiness by introducing proper locking for GIT_TRACE
I reported a couple of times that t5552 is not passing reliably. It has now reached next, and will no doubt infect master soon. Turns out that it is not a Windows-specific issue, even if it occurs a lot more often on Windows than elsewhere. (And even if it is apparently impossible to trigger on Linux.) The culprit is that two processes try to write simultaneously to the same file specified via GIT_TRACE_PACKET, and it is not well defined how that should work, even when only looking at the POSIX specification (the documentation of write() [http://pubs.opengroup.org/onlinepubs/9699919799/functions/write.html] says "Applications should use some form of concurrency control."). This patch series addresses that by locking the trace fd. I chose to use fcntl() for the Unix(-y) platforms we support instead of flock() (even if the latter has much simpler semantics) because fcntl() is in POSIX while flock() is not. On Windows, I use the Win32 API function LockFileEx() [https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-lockfileex] . Of course, I have to admit that I am not super solid on the fcntl() semantics. Junio was nice enough to educate me on l_len = 0 meaning the entire file. If anybody has more experience with locking file descriptors referring not to files, but, say, to pipes or to an interactive terminal, I would be very thankful for help (while the POSIX documentation states that the errno should be EINVAL on a file descriptor that cannot be locked, apparently macOS sets errno to EBADF when trying to lock a redirected stdout )? Changes since v1: * Touched up the cover letter and the first commit message (in particular, removed the bogus squash! line giving away just how much I rely on the --autosquash feature these days). * Now we're locking the entire file via l_len = 0 (except on Windows). * To cover all bases, EINVAL is now also treated as "cannot lock this fd" (in addition to EBADF). * Removed some superfluous flock()-related left-overs from a previous attempt (that caused a lot of me fighting with Linux). Johannes Schindelin (4): Introduce a function to lock/unlock file descriptors when appending mingw: implement lock_or_unlock_fd_for_appending() trace: lock the trace file to avoid racy trace_write() calls trace: verify that locking works Makefile | 1 + compat/mingw.c | 19 ++ compat/mingw.h | 3 + git-compat-util.h | 2 + t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + t/helper/test-trace.c | 130 + t/t0070-fundamental.sh | 6 ++ trace.c| 11 +++- wrapper.c | 16 + 10 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 t/helper/test-trace.c base-commit: 42cc7485a2ec49ecc440c921d2eb0cae4da80549 Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-17%2Fdscho%2Ffetch-negotiator-skipping-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-17/dscho/fetch-negotiator-skipping-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/17 Range-diff vs v1: 1: e449ed75f ! 1: a19904682 Introduce a function to lock/unlock file descriptors when appending @@ -30,7 +30,7 @@ added in this commit. But fcntl() allows a lot more versatile file region locking that we -actually need, to by necessity the fcntl() emulation would be quite +actually need, so by necessity the fcntl() emulation would be quite complex: To support the `l_whence = SEEK_CUR` (which we would use, if it did not require so much book-keeping due to our writing between locking and unlocking the file), we would have to call `SetFilePointerEx()` @@ -47,18 +47,16 @@ to do, as we would once again fail to have an abstraction that clearly says what *exact*functionality we want to use. -To set a precedent for a better approach, let's introduce a proper -abstraction: a function that says in its name precisely what Git -wants it to do (as opposed to *how* it does it on Linux): -lock_or_unlock_fd_for_appending(). +This commit expressly tries to set a precedent for a better approach: +Let's introduce a proper abstraction: a function that says in its name +precisely what Git wants it to do (as opposed to *how* it does it on +Linux): lock_or_unlock_fd_for_appending(). The next commit will provide a Windows-specific implementation of this function/functionality. Signed-off-by: Johannes Schindelin -squash! Introduce a function to lock/unlock file descriptors when appending - diff --git a/git-compat-util.h b/git-compat-util.h --- a/git-compat-util.h +++ b/git-compat-util.h @@ -86,9 +84,11 @@ + struct flock flock; + + flock.l_type
[PATCH v2 4/4] trace: verify that locking works
From: Johannes Schindelin
Recently, t5552 introduced a pattern where two processes try to write to
the same GIT_TRACE file in parallel. This is not safe, as the two
processes fighting over who gets to append to the file can cause garbled
lines and may even result in data loss on Windows (where buffers are
written to before they are flushed).
To remedy this, we introduced the lock_or_unlock_fd_for_appending()
function. And to make sure that this works, this commit introduces a
regression test.
Signed-off-by: Johannes Schindelin
---
Makefile | 1 +
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/helper/test-trace.c | 130 +
t/t0070-fundamental.sh | 6 ++
5 files changed, 139 insertions(+)
create mode 100644 t/helper/test-trace.c
diff --git a/Makefile b/Makefile
index 617475622..2e3fb5b8d 100644
--- a/Makefile
+++ b/Makefile
@@ -729,6 +729,7 @@ TEST_BUILTINS_OBJS += test-strcmp-offset.o
TEST_BUILTINS_OBJS += test-string-list.o
TEST_BUILTINS_OBJS += test-submodule-config.o
TEST_BUILTINS_OBJS += test-subprocess.o
+TEST_BUILTINS_OBJS += test-trace.o
TEST_BUILTINS_OBJS += test-urlmatch-normalization.o
TEST_BUILTINS_OBJS += test-wildmatch.o
TEST_BUILTINS_OBJS += test-write-cache.o
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 805a45de9..7adce872b 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {
{ "string-list", cmd__string_list },
{ "submodule-config", cmd__submodule_config },
{ "subprocess", cmd__subprocess },
+ { "trace", cmd__trace },
{ "urlmatch-normalization", cmd__urlmatch_normalization },
{ "wildmatch", cmd__wildmatch },
{ "write-cache", cmd__write_cache },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 7116ddfb9..c462ac924 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -33,6 +33,7 @@ int cmd__strcmp_offset(int argc, const char **argv);
int cmd__string_list(int argc, const char **argv);
int cmd__submodule_config(int argc, const char **argv);
int cmd__subprocess(int argc, const char **argv);
+int cmd__trace(int argc, const char **argv);
int cmd__urlmatch_normalization(int argc, const char **argv);
int cmd__wildmatch(int argc, const char **argv);
int cmd__write_cache(int argc, const char **argv);
diff --git a/t/helper/test-trace.c b/t/helper/test-trace.c
new file mode 100644
index 0..04159c77a
--- /dev/null
+++ b/t/helper/test-trace.c
@@ -0,0 +1,130 @@
+#include "test-tool.h"
+#include "cache.h"
+#include "run-command.h"
+
+static struct child_process children[2] = {
+ CHILD_PROCESS_INIT,
+ CHILD_PROCESS_INIT,
+};
+
+#define SAY(child, what) \
+ if (write_in_full(children[child].in, \
+ what "\n", strlen(what) + 1) < 0) \
+ die("Failed to tell child process #%d to %s", child, what)
+
+#define LISTEN(child, what) \
+ if (strbuf_getwholeline_fd(, children[child].out, '\n') < 0) \
+ die("Child process #%d failed to acknowledge %s", child, what)
+
+#define ACK(what) \
+ if (write_in_full(1, what ": ACK\n", strlen(what) + 6) < 0) \
+ die_errno("'%s': %s ACK", child_name, what)
+
+static void contention_orchestrator(const char *argv0)
+{
+ struct strbuf buf = STRBUF_INIT;
+ int i;
+
+ /* Spawn two children and simulate write contention */
+ trace_printf("start");
+
+ for (i = 0; i < 2; i++) {
+ strbuf_reset();
+ strbuf_addf(, "child #%d", i);
+ argv_array_pushl([i].args,
+ argv0, "trace", "lock", buf.buf, NULL);
+ children[i].in = children[i].out = -1;
+ if (start_command([i]) < 0)
+ die("Could not spawn child process #%d", i);
+ }
+
+ SAY(1, "lock");
+ LISTEN(1, "lock");
+
+ SAY(0, "trace delayed");
+ SAY(1, "trace while-locked");
+ LISTEN(1, "trace");
+
+ SAY(1, "unlock");
+ LISTEN(1, "unlock");
+ LISTEN(0, "trace");
+
+ SAY(0, "quit");
+ SAY(1, "quit");
+
+ if (finish_command([0]) < 0 ||
+ finish_command([1]) < 0)
+ die("Child process failed to finish");
+
+ strbuf_release();
+}
+
+static void child(const char *child_name)
+{
+ struct strbuf buf = STRBUF_INIT;
+ int fd, locked = 0;
+ const char *p;
+
+ /* This is the child process */
+ trace_printf("child start: '%s'", child_name);
+ fd = trace_default_key.fd;
+ if (fd <= 0)
+ die("child process: not tracing...");
+ while (!strbuf_getwholeline_fd(, 0, '\n')) {
+ strbuf_rtrim();
+ if (!strcmp("lock", buf.buf)) {
+ if (lock_or_unlock_fd_for_appending(fd, 1) < 0 &&
+ errno != EBADF && errno != EINVAL)
+
[PATCH v2 2/4] mingw: implement lock_or_unlock_fd_for_appending()
From: Johannes Schindelin
For some reason, t/t5552-skipping-fetch-negotiator.sh fails particularly
often on Windows due to racy tracing where the `git upload-pack` and the
`git fetch` processes compete for the same file.
We just introduced a remedy that uses fcntl(), but Windows does not have
fcntl(). So let's implement an alternative.
Signed-off-by: Johannes Schindelin
---
compat/mingw.c | 19 +++
compat/mingw.h | 3 +++
2 files changed, 22 insertions(+)
diff --git a/compat/mingw.c b/compat/mingw.c
index 6ded1c859..6da9ce861 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -514,6 +514,25 @@ int mingw_chmod(const char *filename, int mode)
return _wchmod(wfilename, mode);
}
+int mingw_lock_or_unlock_fd_for_appending(int fd, int lock_it)
+{
+ HANDLE handle = (HANDLE)_get_osfhandle(fd);
+ OVERLAPPED overlapped = { 0 };
+ DWORD err;
+
+ if (!lock_it && UnlockFileEx(handle, 0, -1, 0, ))
+ return 0;
+ if (lock_it &&
+ LockFileEx(handle, LOCKFILE_EXCLUSIVE_LOCK, 0, -1, 0, ))
+ return 0;
+
+ err = GetLastError();
+ /* LockFileEx() cannot lock pipes */
+ errno = err == ERROR_INVALID_FUNCTION ?
+ EBADF : err_win_to_posix(GetLastError());
+ return -1;
+}
+
/*
* The unit of FILETIME is 100-nanoseconds since January 1, 1601, UTC.
* Returns the 100-nanoseconds ("hekto nanoseconds") since the epoch.
diff --git a/compat/mingw.h b/compat/mingw.h
index 571019d0b..0f76d89a9 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -397,6 +397,9 @@ HANDLE winansi_get_osfhandle(int fd);
* git specific compatibility
*/
+int mingw_lock_or_unlock_fd_for_appending(int fd, int lock_it);
+#define lock_or_unlock_fd_for_appending mingw_lock_or_unlock_fd_for_appending
+
#define has_dos_drive_prefix(path) \
(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
int mingw_skip_dos_drive_prefix(char **path);
--
gitgitgadget
[PATCH v2 3/4] trace: lock the trace file to avoid racy trace_write() calls
From: Johannes Schindelin
When multiple processes try to write to the same file, it is not
guaranteed that everything works as expected: those writes can overlap,
and in the worst case even lose messages.
This happens in t/t5552-skipping-fetch-negotiator.sh, where we abuse the
`GIT_TRACE` facility to write traces of two concurrent processes (`git
fetch` and `git upload-pack`) to the same file, and then verify that the
trace contains certain expected breadcrumbs.
To remedy this, let's lock the file descriptors for exclusive writing,
using the function we just introduced in the previous commit.
Note: while the POSIX documentation of fcntl() at
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html
suggests that the `errno` is set to `EINVAL` when being asked to
lock a file descriptor that cannot be locked, on macOS it results in
`EBADF` when trying to lock a redirected `stdout` (which the
documentation claims should indicate that the file descriptor is not
valid for writing).
To cover all our bases, we simply treat both `EINVAL` and `EBADF` as
indicators that we cannot lock/unlock this file descriptor.
Signed-off-by: Johannes Schindelin
---
trace.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/trace.c b/trace.c
index fc623e91f..16abdb816 100644
--- a/trace.c
+++ b/trace.c
@@ -114,11 +114,20 @@ static int prepare_trace_line(const char *file, int line,
static void trace_write(struct trace_key *key, const void *buf, unsigned len)
{
- if (write_in_full(get_trace_fd(key), buf, len) < 0) {
+ int fd = get_trace_fd(key), locked;
+
+ locked = !lock_or_unlock_fd_for_appending(fd, 1);
+ if (!locked && errno != EBADF && errno != EINVAL)
+ warning("unable to lock file descriptor for %s: %s",
+ key->key, strerror(errno));
+ if (write_in_full(fd, buf, len) < 0) {
warning("unable to write trace for %s: %s",
key->key, strerror(errno));
trace_disable(key);
}
+ if (locked && lock_or_unlock_fd_for_appending(fd, 0) < 0)
+ warning("failed to remove lock on fd for %s: %s",
+ key->key, strerror(errno));
}
void trace_verbatim(struct trace_key *key, const void *buf, unsigned len)
--
gitgitgadget
[PATCH v5 20/21] range-diff: make --dual-color the default mode
From: Johannes Schindelin
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 bebb47d42..82c71c682 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 da3ad3eba..ef3ba22e2 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", _factor,
N_("Percentage by which creation is weighted")),
- OPT_BOOL(0, "dual-color", _color,
+ OPT_BOOL(0, "no-dual-color", _color,
N_("color both diff and diff-between-diffs")),
OPT_END()
};
@@ -61,8 +61,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 3d4ec3432..d63d2dffd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1981,7 +1981,7 @@ _git_range_diff ()
case "$cur" in
--*)
__gitcomp "
-
[PATCH v5 18/21] completion: support `git range-diff`
From: Johannes Schindelin
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 94c95516e..3d4ec3432 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
--
gitgitgadget
[PATCH v5 17/21] range-diff: populate the man page
From: Johannes Schindelin
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 49f717db8..bebb47d42 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
[PATCH v5 14/21] diff: add an internal option to dual-color diffs of diffs
From: Johannes Schindelin
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 9c4bd9fa1..e6c857abf 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, '\\',
+
[PATCH v5 12/21] range-diff: use color for the commit pairs
From: Johannes Schindelin
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 6d75563f4..b1663da7c 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 ? _util->oid : _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(_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(, , a_util, NULL);
+ output_pair_header(diffopt,
+ , , a_util, NULL);
i++;
continue;
}
/* Show unmatched RHS commits. */
while (j < b->nr && b_util->matching < 0) {
- output_pair_header(, , NULL, b_util);
+ output_pair_header(diffopt,
+ , , 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(, , a_util, b_util);
+ output_pair_header(diffopt,
+ , , a_util, b_util);
if (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT))
patch_diff(a->items[b_util->matching].string,
[PATCH v5 15/21] range-diff: offer to dual-color the diffs
From: Johannes Schindelin
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 bc7a2fb76..da3ad3eba 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", _factor,
N_("Percentage by which creation is weighted")),
+ OPT_BOOL(0, "dual-color", _color,
+ N_("color both diff and diff-between-diffs")),
OPT_END()
};
int i, j, res = 0;
@@ -58,6 +61,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]);
--
gitgitgadget
[PATCH v5 00/21] Add range-diff, a tbdiff lookalike
The incredibly useful git-tbdiff [https://github.com/trast/tbdiff] tool to
compare patch series (say, to see what changed between two iterations sent
to the Git mailing list) is slightly less useful for this developer due to
the fact that it requires the hungarian and numpy Python packages which are
for some reason really hard to build in MSYS2. So hard that I even had to
give up, because it was simply easier to re-implement the whole shebang as a
builtin command.
The project at https://github.com/trast/tbdiff seems to be dormant, anyway.
Funny (and true) story: I looked at the open Pull Requests to see how active
that project is, only to find to my surprise that I had submitted one in
August 2015, and that it was still unanswered let alone merged.
While at it, I forward-ported AEvar's patch to force --decorate=no because
git -p tbdiff would fail otherwise.
Side note: I work on implementing range-diff not only to make life easier
for reviewers who have to suffer through v2, v3, ... of my patch series, but
also to verify my changes before submitting a new iteration. And also, maybe
even more importantly, I plan to use it to verify my merging-rebases of Git
for Windows (for which I previously used to redirect the
pre-rebase/post-rebase diffs vs upstream and then compare them using git
diff --no-index). And of course any interested person can see what changes
were necessary e.g. in the merging-rebase of Git for Windows onto v2.17.0 by
running a command like:
base=^{/Start.the.merging-rebase}
tag=v2.17.0.windows.1
pre=$tag$base^2
git range-diff $pre$base..$pre $tag$base..$tag
The command uses what it calls the "dual color mode" (can be disabled via
--no-dual-color) which helps identifying what actually changed: it prefixes
lines with a - (and red background) that correspond to the first commit
range, and with a + (and green background) that correspond to the second
range. The rest of the lines will be colored according to the original
diffs.
Changes since v4:
* Fixed a typo in the commit message of "range-diff: add tests" that was
introduced in v4.
* White-space fixes.
* Fixed the length of the first header underline in the man page.
* Changed the preprocessor guard in linear-assignment.h to reflect the new
name (instead of the old name, which was hungarian.h).
* Likewise, changed the preprocessor guards in range-diff.h to hide the
history of the thrice-renamed command.
* Fixed indentation in the completion.
* Instead of trying to paper over white-space error handling that does not
apply to "diffs of diffs", dual color mode now simply disables all
white-space warnings.
* When showing the "single arg must be symmetric range" error message, git
range-diff now also shows the usage.
* Adjusted the commit message of "range-diff: adjust the output of the
commit pairs" to avoid the surprise of the reviewer when onelines are
printed all of a sudden, too.
* "range-diff: adjust the output of the commit pairs" is now using a
simpler way to print onelines.
* We are now sandwiching the diff_opt_parse() loop between two
parse_options(), to make sure that we caught all options, and that the --
separator is handled.
* Adjusted the lookup_commit_reference() call to the newest master (it now
takes a the_repository parameter).
Changes since v3:
* The cover letter was adjusted to reflect the new reality (the command is
called range-diff now, not branch-diff, and --dual-color is the default).
* The documentation was adjusted a bit more in the patch that makes
--dual-color the default.
* Clarified the calculation of the cost matrix, as per Stefan Beller's
request.
* The man page now spells out that merge commits are ignored in the commit
ranges (not merges per se).
* The code in linear-assignment.c was adjusted to use the SWAP() macro.
* The commit message of the patch introducing the first rudimentary
implementation no longer talks about the "Hungarian" algorithm, but about
the "linear assignment algorithm" instead.
* A bogus indentation change was backed out from the patch introducing the
first rudimentary implementation.
* Instead of merely warning about missing .. in the 2-parameter invocation,
we now exit with the error message.
* The diff_opt_parse() function is allowed to return a value larger than 1,
indicating that more than just one command-line parameter was parsed. We
now advance by the indicated value instead of always advancing exactly 1
(which is still correct much of the time).
* A lengthy if...else if...else if...else was simplified (from a logical
point of view) by reordering it.
* The unnecessarily static variable dashes was turned into a local variable
of the caller.
* The commit message talking about the new man page still referred to git
branch --diff, which has been fixed.
* A forgotten t7910 reference was changed to t3206.
* An unbalanced double-tick was fixed
[PATCH v5 10/21] range-diff: do not show "function names" in hunk headers
From: Johannes Schindelin
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 23aa61af5..6d75563f4 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 = _func_name;
return spec;
}
--
gitgitgadget
[PATCH v5 07/21] range-diff: indent the diffs just like tbdiff
From: Johannes Schindelin
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 19192ab31..f6df3f19a 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.output_format = DIFF_FORMAT_PATCH;
+ diffopt.output_prefix = output_prefix_cb;
+ strbuf_addstr(_spaces, "");
+ diffopt.output_prefix_data = _spaces;
argc = parse_options(argc, argv, NULL, options,
builtin_range_diff_usage, PARSE_OPT_KEEP_UNKNOWN |
@@ -88,6 +97,7 @@ int cmd_range_diff(int argc, const char **argv, const char
*prefix)
strbuf_release();
strbuf_release();
+ strbuf_release(_spaces);
return res;
}
--
gitgitgadget
[PATCH v5 09/21] range-diff: adjust the output of the commit pairs
From: Johannes Schindelin
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 1ecee2c09..23aa61af5 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(>oid, DEFAULT_ABBREV);
+ struct object_id *oid = a_util ? _util->oid : _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(_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(_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(, , 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(, , 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(, , 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();
+ strbuf_release();
}
int show_range_diff(const char *range1, const char *range2,
--
gitgitgadget
[PATCH v5 08/21] range-diff: suppress the diff headers
From: Johannes Schindelin
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 f6df3f19a..bc7a2fb76 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.output_format = DIFF_FORMAT_PATCH;
+ diffopt.flags.suppress_diff_headers = 1;
diffopt.output_prefix = output_prefix_cb;
strbuf_addstr(_spaces, "");
diffopt.output_prefix_data = _spaces;
diff --git a/diff.c b/diff.c
index 04d044bbb..9c4bd9fa1 100644
--- a/diff.c
+++ b/diff.c
@@ -3395,13 +3395,16 @@ static void builtin_diff(const char *name_a,
memset(, 0, sizeof(xpp));
memset(, 0, sizeof(xecfg));
memset(, 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(, , );
ecbdata.opt = o;
- ecbdata.header = header.len ? : NULL;
+ if (header.len && !o->flags.suppress_diff_headers)
+ ecbdata.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 a14895bb8..d88ceb357 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,
--
gitgitgadget
[PATCH v5 05/21] range-diff: also show the diff between patches
From: Johannes Schindelin
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 | 29 +++--
range-diff.c | 34 +++---
range-diff.h | 4 +++-
3 files changed, 61 insertions(+), 6 deletions(-)
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 94c1f362c..19192ab31 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,38 @@ 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", _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.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(, argv + i, argc - i, prefix);
+
+ if (!c)
+ argv[j++] = argv[i++];
+ else
+ i += c;
+ }
+ argc = j;
+ diff_setup_done();
+
+ /* 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 +83,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,
+ );
strbuf_release();
strbuf_release();
diff --git a/range-diff.c b/range-diff.c
index 2d94200d3..71883a4b7 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(>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, _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(_queued_diff,
+ get_filespec("a", a), get_filespec("b", b));
+
+ diffcore_std(diffopt);
+
[PATCH v5 02/21] Introduce `range-diff` to compare iterations of a topic branch
From: Johannes Schindelin
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 3284a1e9b..cc0ad74b4 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 0..49f717db8
--- /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 1af719b44..7ff7eba42 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 0362f1ce2..99206df4b 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 0..36788ea4f
--- /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", _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 e1c26c1bb..a9dda3b8a 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -139,6 +139,7 @@ git-prune-packedplumbingmanipulators
git-pullmainporcelain remote
git-pushmainporcelain remote
git-quiltimport foreignscminterface
+git-range-diff mainporcelain
git-read-tree plumbingmanipulators
git-rebase mainporcelain history
git-receive-packsynchelpers
diff --git a/git.c b/git.c
index fc7d15d54..5b48cac3a 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 },
--
gitgitgadget
[PATCH v5 19/21] range-diff: left-pad patch numbers
From: Johannes Schindelin
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 b1663da7c..b6b9abac2 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(_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(_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,
, , 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,
, , 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,
, , a_util, b_util);
if (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT))
patch_diff(a->items[b_util->matching].string,
--
gitgitgadget
[PATCH v5 13/21] color: add the meta color GIT_COLOR_REVERSE
From: Johannes Schindelin 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 5b744e1bc..33e786342 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" -- gitgitgadget
[PATCH v5 04/21] range-diff: improve the order of the shown commits
From: Johannes Schindelin
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 15d418afa..2d94200d3 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++;
+ }
}
}
--
gitgitgadget
[PATCH v5 16/21] range-diff --dual-color: skip white-space warnings
From: Johannes Schindelin 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 e6c857abf..ea8ecae04 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, -- gitgitgadget
[PATCH v5 21/21] range-diff: use dim/bold cues to improve dual color mode
From: Johannes Schindelin 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 63365dcf3..90241ed77 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 82c71c682..f693930fd 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 33e786342..98894d6a1 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"
[PATCH v5 03/21] range-diff: first rudimentary implementation
From: Johannes Schindelin
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 7ff7eba42..72f16882e 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 36788ea4f..94c1f362c 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(, argv[0]);
+
+ if (!strstr(argv[1], ".."))
+ die(_("no .. in range: '%s'"), argv[1]);
+ strbuf_addstr(, argv[1]);
+ } else if (argc == 3) {
+ strbuf_addf(, "%s..%s", argv[0], argv[1]);
+ strbuf_addf(, "%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(, "%s..%.*s", b, a_len, a);
+ strbuf_addf(, "%.*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();
+ strbuf_release();
+
+ return res;
}
diff --git a/range-diff.c b/range-diff.c
new file mode 100644
index 0..15d418afa
--- /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(, "log", "--no-color", "-p", "--no-merges",
+ "--reverse", "--date-order", "--decorate=no",
+
[PATCH v5 06/21] range-diff: right-trim commit messages
From: Johannes Schindelin
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 71883a4b7..1ecee2c09 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(, );
strbuf_addstr(, "\n\n");
} else if (starts_with(line.buf, "")) {
+ strbuf_rtrim();
strbuf_addbuf(, );
strbuf_addch(, '\n');
}
--
gitgitgadget
[PATCH v5 01/21] linear-assignment: a function to solve least-cost assignment problems
From: Johannes Schindelin
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 bc4fc8eea..1af719b44 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 0..9b3e56e28
--- /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;
+
[PATCH v3 0/2] Make git rebase work with --rebase-merges and --exec
It was reported via IRC that the exec lines are inserted in the wrong spots when using --rebase-merges. The reason is that we used a simple, incorrect implementation that happened to work as long as the generated todo list only contains pick, fixup and squash commands. Which is not the case with--rebase-merges. Fix this issue by using a correct implementation instead, that even takes into account merge commands in the --rebase-merges mode. Changes since v1: * Replaced the "look-ahead" design by a "keep looking" one: instead of having a nested loop that looks for the end of the fixup/squash chain, we continue the loop, delaying the insertion until we know where the fixup/squash chain ends, if any. Johannes Schindelin (2): t3430: demonstrate what -r, --autosquash & --exec should do rebase --exec: make it work with --rebase-merges sequencer.c | 42 +--- t/t3430-rebase-merges.sh | 17 2 files changed, 48 insertions(+), 11 deletions(-) base-commit: 1d89318c48d233d52f1db230cf622935ac3c69fa Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-13%2Fdscho%2Frebase-merges-and-exec-commands-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-13/dscho/rebase-merges-and-exec-commands-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/13 Range-diff vs v2: 1: 1d82eb450 = 1: 1d82eb450 t3430: demonstrate what -r, --autosquash & --exec should do 2: 7ca441a89 ! 2: b436f67ba rebase --exec: make it work with --rebase-merges @@ -22,6 +22,11 @@ `pick` lines, skip any fixup/squash chains, and then insert the `exec` line. Lather, rinse, repeat. +Note: we take pains to insert *before* comment lines whenever possible, +as empty commits are represented by commented-out pick lines (and we +want to insert a preceding pick's exec line *before* such a line, not +afterward). + While at it, also add `exec` lines after `merge` commands, because they are similar in spirit to `pick` commands: they add new commits. @@ -81,9 +86,13 @@ + insert = i + 1; } - /* append final */ +- /* append final */ - strbuf_add(buf, commands, commands_len); -+ if (insert >= 0 || !offset) ++ /* insert or append final */ ++ if (insert >= 0 && insert < todo_list.nr) ++ strbuf_insert(buf, todo_list.items[insert].offset_in_buf + ++ offset, commands, commands_len); ++ else if (insert >= 0 || !offset) + strbuf_add(buf, commands, commands_len); i = write_message(buf->buf, buf->len, todo_file, 0); -- gitgitgadget
[PATCH v3 1/2] t3430: demonstrate what -r, --autosquash & --exec should do
From: Johannes Schindelin The --exec option's implementation is not really well-prepared for --rebase-merges. Demonstrate this. Signed-off-by: Johannes Schindelin --- t/t3430-rebase-merges.sh | 17 + 1 file changed, 17 insertions(+) diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 9e6229727..0bf5eaa37 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -363,4 +363,21 @@ test_expect_success 'octopus merges' ' EOF ' +test_expect_failure 'with --autosquash and --exec' ' + git checkout -b with-exec H && + echo Booh >B.t && + test_tick && + git commit --fixup B B.t && + write_script show.sh <<-\EOF && + subject="$(git show -s --format=%s HEAD)" + content="$(git diff HEAD^! | tail -n 1)" + echo "$subject: $content" + EOF + test_tick && + git rebase -ir --autosquash --exec ./show.sh A >actual && + grep "B: +Booh" actual && + grep "E: +Booh" actual && + grep "G: +G" actual +' + test_done -- gitgitgadget
[PATCH v3 2/2] rebase --exec: make it work with --rebase-merges
From: Johannes Schindelin
The idea of `--exec` is to append an `exec` call after each `pick`.
Since the introduction of fixup!/squash! commits, this idea was extended
to apply to "pick, possibly followed by a fixup/squash chain", i.e. an
exec would not be inserted between a `pick` and any of its corresponding
`fixup` or `squash` lines.
The current implementation uses a dirty trick to achieve that: it
assumes that there are only pick/fixup/squash commands, and then
*inserts* the `exec` lines before any `pick` but the first, and appends
a final one.
With the todo lists generated by `git rebase --rebase-merges`, this
simple implementation shows its problems: it produces the exact wrong
thing when there are `label`, `reset` and `merge` commands.
Let's change the implementation to do exactly what we want: look for
`pick` lines, skip any fixup/squash chains, and then insert the `exec`
line. Lather, rinse, repeat.
Note: we take pains to insert *before* comment lines whenever possible,
as empty commits are represented by commented-out pick lines (and we
want to insert a preceding pick's exec line *before* such a line, not
afterward).
While at it, also add `exec` lines after `merge` commands, because they
are similar in spirit to `pick` commands: they add new commits.
Signed-off-by: Johannes Schindelin
---
sequencer.c | 42 +---
t/t3430-rebase-merges.sh | 2 +-
2 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 31038472f..278d34ce9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4244,10 +4244,9 @@ int sequencer_add_exec_commands(const char *commands)
{
const char *todo_file = rebase_path_todo();
struct todo_list todo_list = TODO_LIST_INIT;
- struct todo_item *item;
struct strbuf *buf = _list.buf;
size_t offset = 0, commands_len = strlen(commands);
- int i, first;
+ int i, insert;
if (strbuf_read_file(_list.buf, todo_file, 0) < 0)
return error(_("could not read '%s'."), todo_file);
@@ -4257,19 +4256,40 @@ int sequencer_add_exec_commands(const char *commands)
return error(_("unusable todo list: '%s'"), todo_file);
}
- first = 1;
- /* insert before every pick except the first one */
- for (item = todo_list.items, i = 0; i < todo_list.nr; i++, item++) {
- if (item->command == TODO_PICK && !first) {
- strbuf_insert(buf, item->offset_in_buf + offset,
- commands, commands_len);
+ /*
+* Insert after every pick. Here, fixup/squash chains
+* are considered part of the pick, so we insert the commands *after*
+* those chains if there are any.
+*/
+ insert = -1;
+ for (i = 0; i < todo_list.nr; i++) {
+ enum todo_command command = todo_list.items[i].command;
+
+ if (insert >= 0) {
+ /* skip fixup/squash chains */
+ if (command == TODO_COMMENT)
+ continue;
+ else if (is_fixup(command)) {
+ insert = i + 1;
+ continue;
+ }
+ strbuf_insert(buf,
+ todo_list.items[insert].offset_in_buf +
+ offset, commands, commands_len);
offset += commands_len;
+ insert = -1;
}
- first = 0;
+
+ if (command == TODO_PICK || command == TODO_MERGE)
+ insert = i + 1;
}
- /* append final */
- strbuf_add(buf, commands, commands_len);
+ /* insert or append final */
+ if (insert >= 0 && insert < todo_list.nr)
+ strbuf_insert(buf, todo_list.items[insert].offset_in_buf +
+ offset, commands, commands_len);
+ else if (insert >= 0 || !offset)
+ strbuf_add(buf, commands, commands_len);
i = write_message(buf->buf, buf->len, todo_file, 0);
todo_list_release(_list);
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 0bf5eaa37..90ae613e2 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -363,7 +363,7 @@ test_expect_success 'octopus merges' '
EOF
'
-test_expect_failure 'with --autosquash and --exec' '
+test_expect_success 'with --autosquash and --exec' '
git checkout -b with-exec H &&
echo Booh >B.t &&
test_tick &&
--
gitgitgadget
[PATCH 0/1] Fix make -C t chainlint with DOS line endings
Historically, nobody paid attention to our own source code having correct Git attributes [https://www.edwardthomson.com/blog/git_for_windows_line_endings.html] when it comes to line endings. Because historically, we had no good way to specify that ;-) But now we do, and so we need to use it. Especially when it would break the build otherwise. Johannes Schindelin (1): chainlint: fix for core.autocrlf=true t/.gitattributes | 1 + 1 file changed, 1 insertion(+) base-commit: 1d89318c48d233d52f1db230cf622935ac3c69fa Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-19%2Fdscho%2Ffix-chainlint-on-windows-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-19/dscho/fix-chainlint-on-windows-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/19 -- gitgitgadget
[PATCH 1/1] chainlint: fix for core.autocrlf=true
From: Johannes Schindelin The `chainlint` target compares actual output to expected output, where the actual output is generated from files that are specifically checked out with LF-only line endings. So the expected output needs to be checked out with LF-only line endings, too. Signed-off-by: Johannes Schindelin --- t/.gitattributes | 1 + 1 file changed, 1 insertion(+) diff --git a/t/.gitattributes b/t/.gitattributes index 3bd959ae5..9d09df5a6 100644 --- a/t/.gitattributes +++ b/t/.gitattributes @@ -1,4 +1,5 @@ t[0-9][0-9][0-9][0-9]/* -whitespace +/chainlint/*.expect eol=lf /diff-lib/* eol=lf /t0110/url-* binary /t3900/*.txt eol=lf -- gitgitgadget
[PATCH 1/1] mark_colliding_entries(): fix incorrect #if...#endif guard
From: Johannes Schindelin
The way the guard was put, the code was declaring an unused variable on
Windows. No need to do that, so let's fix it.
Signed-off-by: Johannes Schindelin
---
entry.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/entry.c b/entry.c
index c70340df8..2bce13352 100644
--- a/entry.c
+++ b/entry.c
@@ -402,11 +402,9 @@ static int check_path(const char *path, int len, struct
stat *st, int skiplen)
static void mark_colliding_entries(const struct checkout *state,
struct cache_entry *ce, struct stat *st)
{
+#if !defined(GIT_WINDOWS_NATIVE) /* inode is always zero on Windows */
int i;
- ce->ce_flags |= CE_MATCHED;
-
-#if !defined(GIT_WINDOWS_NATIVE) /* inode is always zero on Windows */
for (i = 0; i < state->istate->cache_nr; i++) {
struct cache_entry *dup = state->istate->cache[i];
@@ -422,6 +420,8 @@ static void mark_colliding_entries(const struct checkout
*state,
}
}
#endif
+
+ ce->ce_flags |= CE_MATCHED;
}
/*
--
gitgitgadget
[PATCH 0/1] Fix a recently-introduced compile warning
With the relatively frequent breakages of pu recently, I had trouble staying on top of the compile errors/test failures, sorry. This one exists since Sunday, and it is a compile error only with DEVELOPER=1, which is, however, the recommended way to build in Git for Windows' SDK. Note: it is based on nd/clone-case-smashing-warning. Johannes Schindelin (1): mark_colliding_entries(): fix incorrect #if...#endif guard entry.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) base-commit: f80218bf4e65ccc06cc9173c0ac5a5520d380f36 Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-18%2Fdscho%2Fclone-case-smashing-warning-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-18/dscho/clone-case-smashing-warning-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/18 -- gitgitgadget
[PATCH v2 0/2] Make git rebase work with --rebase-merges and --exec
It was reported via IRC that the exec lines are inserted in the wrong spots
when using --rebase-merges.
The reason is that we used a simple, incorrect implementation that happened
to work as long as the generated todo list only contains pick, fixup and
squash commands. Which is not the case with--rebase-merges.
Fix this issue by using a correct, if longer and slightly more complex
implementation instead.
Johannes Schindelin (2):
t3430: demonstrate what -r, --autosquash & --exec should do
rebase --exec: make it work with --rebase-merges
sequencer.c | 37 +++--
t/t3430-rebase-merges.sh | 17 +
2 files changed, 44 insertions(+), 10 deletions(-)
base-commit: 1d89318c48d233d52f1db230cf622935ac3c69fa
Published-As:
https://github.com/gitgitgadget/git/releases/tags/pr-13%2Fdscho%2Frebase-merges-and-exec-commands-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-13/dscho/rebase-merges-and-exec-commands-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/13
Range-diff vs v1:
1: 1d82eb450 = 1: 1d82eb450 t3430: demonstrate what -r, --autosquash &
--exec should do
2: b29c4d979 ! 2: 7ca441a89 rebase --exec: make it work with --rebase-merges
@@ -38,7 +38,7 @@
struct strbuf *buf = _list.buf;
size_t offset = 0, commands_len = strlen(commands);
- int i, first;
-+ int i, insert_final_commands;
++ int i, insert;
if (strbuf_read_file(_list.buf, todo_file, 0) < 0)
return error(_("could not read '%s'."), todo_file);
@@ -52,59 +52,38 @@
- if (item->command == TODO_PICK && !first) {
- strbuf_insert(buf, item->offset_in_buf + offset,
- commands, commands_len);
-- offset += commands_len;
+ /*
+ * Insert after every pick. Here, fixup/squash chains
+ * are considered part of the pick, so we insert the commands *after*
+ * those chains if there are any.
+ */
-+ insert_final_commands = 1;
-+ for (i = 0; i < todo_list.nr; ) {
++ insert = -1;
++ for (i = 0; i < todo_list.nr; i++) {
+ enum todo_command command = todo_list.items[i].command;
-+ int j = 0;
+
-+ if (command != TODO_PICK && command != TODO_MERGE) {
-+ i++;
-+ continue;
-+ }
-+
-+ /* skip fixup/squash chain, if any */
-+ for (i++; i < todo_list.nr; i++, j = 0) {
-+ command = todo_list.items[i].command;
-+
-+ if (is_fixup(command))
++ if (insert >= 0) {
++ /* skip fixup/squash chains */
++ if (command == TODO_COMMENT)
+ continue;
-+
-+ if (command != TODO_COMMENT)
-+ break;
-+
-+ /* skip comment if followed by any fixup/squash */
-+ for (j = i + 1; j < todo_list.nr; j++)
-+ if (todo_list.items[j].command != TODO_COMMENT)
-+ break;
-+ if (j < todo_list.nr &&
-+ is_fixup(todo_list.items[j].command)) {
-+ i = j;
++ else if (is_fixup(command)) {
++ insert = i + 1;
+ continue;
+ }
-+ break;
++ strbuf_insert(buf,
++ todo_list.items[insert].offset_in_buf +
++ offset, commands, commands_len);
+ offset += commands_len;
++ insert = -1;
}
- first = 0;
+
-+ if (i >= todo_list.nr) {
-+ insert_final_commands = 1;
-+ break;
-+ }
-+
-+ strbuf_insert(buf, todo_list.items[i].offset_in_buf + offset,
-+ commands, commands_len);
-+ offset += commands_len;
-+ insert_final_commands = 0;
++ if (command == TODO_PICK || command == TODO_MERGE)
++ insert = i + 1;
}
/* append final */
- strbuf_add(buf, commands, commands_len);
-+ if (insert_final_commands)
++ if (insert >= 0 || !offset)
+ strbuf_add(buf, commands, commands_len);
i = write_message(buf->buf, buf->len, todo_file, 0);
--
gitgitgadget
[PATCH v2 1/2] t3430: demonstrate what -r, --autosquash & --exec should do
From: Johannes Schindelin The --exec option's implementation is not really well-prepared for --rebase-merges. Demonstrate this. Signed-off-by: Johannes Schindelin --- t/t3430-rebase-merges.sh | 17 + 1 file changed, 17 insertions(+) diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 9e6229727..0bf5eaa37 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -363,4 +363,21 @@ test_expect_success 'octopus merges' ' EOF ' +test_expect_failure 'with --autosquash and --exec' ' + git checkout -b with-exec H && + echo Booh >B.t && + test_tick && + git commit --fixup B B.t && + write_script show.sh <<-\EOF && + subject="$(git show -s --format=%s HEAD)" + content="$(git diff HEAD^! | tail -n 1)" + echo "$subject: $content" + EOF + test_tick && + git rebase -ir --autosquash --exec ./show.sh A >actual && + grep "B: +Booh" actual && + grep "E: +Booh" actual && + grep "G: +G" actual +' + test_done -- gitgitgadget
[PATCH v2 2/2] rebase --exec: make it work with --rebase-merges
From: Johannes Schindelin
The idea of `--exec` is to append an `exec` call after each `pick`.
Since the introduction of fixup!/squash! commits, this idea was extended
to apply to "pick, possibly followed by a fixup/squash chain", i.e. an
exec would not be inserted between a `pick` and any of its corresponding
`fixup` or `squash` lines.
The current implementation uses a dirty trick to achieve that: it
assumes that there are only pick/fixup/squash commands, and then
*inserts* the `exec` lines before any `pick` but the first, and appends
a final one.
With the todo lists generated by `git rebase --rebase-merges`, this
simple implementation shows its problems: it produces the exact wrong
thing when there are `label`, `reset` and `merge` commands.
Let's change the implementation to do exactly what we want: look for
`pick` lines, skip any fixup/squash chains, and then insert the `exec`
line. Lather, rinse, repeat.
While at it, also add `exec` lines after `merge` commands, because they
are similar in spirit to `pick` commands: they add new commits.
Signed-off-by: Johannes Schindelin
---
sequencer.c | 37 +++--
t/t3430-rebase-merges.sh | 2 +-
2 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 31038472f..ed2e694ff 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4244,10 +4244,9 @@ int sequencer_add_exec_commands(const char *commands)
{
const char *todo_file = rebase_path_todo();
struct todo_list todo_list = TODO_LIST_INIT;
- struct todo_item *item;
struct strbuf *buf = _list.buf;
size_t offset = 0, commands_len = strlen(commands);
- int i, first;
+ int i, insert;
if (strbuf_read_file(_list.buf, todo_file, 0) < 0)
return error(_("could not read '%s'."), todo_file);
@@ -4257,19 +4256,37 @@ int sequencer_add_exec_commands(const char *commands)
return error(_("unusable todo list: '%s'"), todo_file);
}
- first = 1;
- /* insert before every pick except the first one */
- for (item = todo_list.items, i = 0; i < todo_list.nr; i++, item++) {
- if (item->command == TODO_PICK && !first) {
- strbuf_insert(buf, item->offset_in_buf + offset,
- commands, commands_len);
+ /*
+* Insert after every pick. Here, fixup/squash chains
+* are considered part of the pick, so we insert the commands *after*
+* those chains if there are any.
+*/
+ insert = -1;
+ for (i = 0; i < todo_list.nr; i++) {
+ enum todo_command command = todo_list.items[i].command;
+
+ if (insert >= 0) {
+ /* skip fixup/squash chains */
+ if (command == TODO_COMMENT)
+ continue;
+ else if (is_fixup(command)) {
+ insert = i + 1;
+ continue;
+ }
+ strbuf_insert(buf,
+ todo_list.items[insert].offset_in_buf +
+ offset, commands, commands_len);
offset += commands_len;
+ insert = -1;
}
- first = 0;
+
+ if (command == TODO_PICK || command == TODO_MERGE)
+ insert = i + 1;
}
/* append final */
- strbuf_add(buf, commands, commands_len);
+ if (insert >= 0 || !offset)
+ strbuf_add(buf, commands, commands_len);
i = write_message(buf->buf, buf->len, todo_file, 0);
todo_list_release(_list);
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 0bf5eaa37..90ae613e2 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -363,7 +363,7 @@ test_expect_success 'octopus merges' '
EOF
'
-test_expect_failure 'with --autosquash and --exec' '
+test_expect_success 'with --autosquash and --exec' '
git checkout -b with-exec H &&
echo Booh >B.t &&
test_tick &&
--
gitgitgadget
[PATCH v6 03/21] range-diff: first rudimentary implementation
From: Johannes Schindelin
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 7ff7eba42..72f16882e 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 36788ea4f..94c1f362c 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(, argv[0]);
+
+ if (!strstr(argv[1], ".."))
+ die(_("no .. in range: '%s'"), argv[1]);
+ strbuf_addstr(, argv[1]);
+ } else if (argc == 3) {
+ strbuf_addf(, "%s..%s", argv[0], argv[1]);
+ strbuf_addf(, "%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(, "%s..%.*s", b, a_len, a);
+ strbuf_addf(, "%.*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();
+ strbuf_release();
+
+ return res;
}
diff --git a/range-diff.c b/range-diff.c
new file mode 100644
index 0..15d418afa
--- /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(, "log", "--no-color", "-p", "--no-merges",
+ "--reverse", "--date-order", "--decorate=no",
+
[PATCH v6 10/21] range-diff: do not show "function names" in hunk headers
From: Johannes Schindelin
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 23aa61af5..6d75563f4 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 = _func_name;
return spec;
}
--
gitgitgadget
[PATCH v6 06/21] range-diff: right-trim commit messages
From: Johannes Schindelin
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 71883a4b7..1ecee2c09 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(, );
strbuf_addstr(, "\n\n");
} else if (starts_with(line.buf, "")) {
+ strbuf_rtrim();
strbuf_addbuf(, );
strbuf_addch(, '\n');
}
--
gitgitgadget
[PATCH v6 13/21] color: add the meta color GIT_COLOR_REVERSE
From: Johannes Schindelin 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 5b744e1bc..33e786342 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" -- gitgitgadget
[PATCH v6 09/21] range-diff: adjust the output of the commit pairs
From: Johannes Schindelin
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 1ecee2c09..23aa61af5 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(>oid, DEFAULT_ABBREV);
+ struct object_id *oid = a_util ? _util->oid : _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(_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(_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(, , 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(, , 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(, , 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();
+ strbuf_release();
}
int show_range_diff(const char *range1, const char *range2,
--
gitgitgadget
[PATCH v6 08/21] range-diff: suppress the diff headers
From: Johannes Schindelin
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 f0598005a..76659d0b3 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.output_format = DIFF_FORMAT_PATCH;
+ diffopt.flags.suppress_diff_headers = 1;
diffopt.output_prefix = output_prefix_cb;
strbuf_addstr(_spaces, "");
diffopt.output_prefix_data = _spaces;
diff --git a/diff.c b/diff.c
index 04d044bbb..9c4bd9fa1 100644
--- a/diff.c
+++ b/diff.c
@@ -3395,13 +3395,16 @@ static void builtin_diff(const char *name_a,
memset(, 0, sizeof(xpp));
memset(, 0, sizeof(xecfg));
memset(, 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(, , );
ecbdata.opt = o;
- ecbdata.header = header.len ? : NULL;
+ if (header.len && !o->flags.suppress_diff_headers)
+ ecbdata.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 a14895bb8..d88ceb357 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,
--
gitgitgadget
[PATCH v6 05/21] range-diff: also show the diff between patches
From: Johannes Schindelin
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 94c1f362c..3b06ed944 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", _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.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(, 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();
+
+ /* 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,
+ );
strbuf_release();
strbuf_release();
diff --git a/range-diff.c b/range-diff.c
index 2d94200d3..71883a4b7 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(>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, _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(_queued_diff,
+ get_filespec("a", a),
[PATCH v6 12/21] range-diff: use color for the commit pairs
From: Johannes Schindelin
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 6d75563f4..b1663da7c 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 ? _util->oid : _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(_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(, , a_util, NULL);
+ output_pair_header(diffopt,
+ , , a_util, NULL);
i++;
continue;
}
/* Show unmatched RHS commits. */
while (j < b->nr && b_util->matching < 0) {
- output_pair_header(, , NULL, b_util);
+ output_pair_header(diffopt,
+ , , 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(, , a_util, b_util);
+ output_pair_header(diffopt,
+ , , a_util, b_util);
if (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT))
patch_diff(a->items[b_util->matching].string,
[PATCH v6 17/21] range-diff: populate the man page
From: Johannes Schindelin
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 49f717db8..bebb47d42 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
[PATCH v6 20/21] range-diff: make --dual-color the default mode
From: Johannes Schindelin
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 bebb47d42..82c71c682 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 5a9ad82fb..f52d45d9d 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", _factor,
N_("Percentage by which creation is weighted")),
- OPT_BOOL(0, "dual-color", _color,
+ OPT_BOOL(0, "no-dual-color", _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 3d4ec3432..d63d2dffd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1981,7 +1981,7 @@ _git_range_diff ()
case "$cur" in
--*)
__gitcomp "
-
[PATCH v6 19/21] range-diff: left-pad patch numbers
From: Johannes Schindelin
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 b1663da7c..b6b9abac2 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(_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(_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,
, , 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,
, , 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,
, , a_util, b_util);
if (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT))
patch_diff(a->items[b_util->matching].string,
--
gitgitgadget
[PATCH v6 14/21] diff: add an internal option to dual-color diffs of diffs
From: Johannes Schindelin
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 9c4bd9fa1..e6c857abf 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, '\\',
+
[PATCH v6 15/21] range-diff: offer to dual-color the diffs
From: Johannes Schindelin
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 76659d0b3..5a9ad82fb 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", _factor,
N_("Percentage by which creation is weighted")),
+ OPT_BOOL(0, "dual-color", _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]);
--
gitgitgadget
[PATCH v6 01/21] linear-assignment: a function to solve least-cost assignment problems
From: Johannes Schindelin
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 bc4fc8eea..1af719b44 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 0..9b3e56e28
--- /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;
+
[PATCH v6 02/21] Introduce `range-diff` to compare iterations of a topic branch
From: Johannes Schindelin
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 3284a1e9b..cc0ad74b4 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 0..49f717db8
--- /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 1af719b44..7ff7eba42 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 0362f1ce2..99206df4b 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 0..36788ea4f
--- /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", _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 e1c26c1bb..a9dda3b8a 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -139,6 +139,7 @@ git-prune-packedplumbingmanipulators
git-pullmainporcelain remote
git-pushmainporcelain remote
git-quiltimport foreignscminterface
+git-range-diff mainporcelain
git-read-tree plumbingmanipulators
git-rebase mainporcelain history
git-receive-packsynchelpers
diff --git a/git.c b/git.c
index fc7d15d54..5b48cac3a 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 },
--
gitgitgadget
[PATCH v6 00/21] Add range-diff, a tbdiff lookalike
The incredibly useful git-tbdiff [https://github.com/trast/tbdiff] tool to
compare patch series (say, to see what changed between two iterations sent
to the Git mailing list) is slightly less useful for this developer due to
the fact that it requires the hungarian and numpy Python packages which are
for some reason really hard to build in MSYS2. So hard that I even had to
give up, because it was simply easier to re-implement the whole shebang as a
builtin command.
The project at https://github.com/trast/tbdiff seems to be dormant, anyway.
Funny (and true) story: I looked at the open Pull Requests to see how active
that project is, only to find to my surprise that I had submitted one in
August 2015, and that it was still unanswered let alone merged.
While at it, I forward-ported AEvar's patch to force --decorate=no because
git -p tbdiff would fail otherwise.
Side note: I work on implementing range-diff not only to make life easier
for reviewers who have to suffer through v2, v3, ... of my patch series, but
also to verify my changes before submitting a new iteration. And also, maybe
even more importantly, I plan to use it to verify my merging-rebases of Git
for Windows (for which I previously used to redirect the
pre-rebase/post-rebase diffs vs upstream and then compare them using git
diff --no-index). And of course any interested person can see what changes
were necessary e.g. in the merging-rebase of Git for Windows onto v2.17.0 by
running a command like:
base=^{/Start.the.merging-rebase}
tag=v2.17.0.windows.1
pre=$tag$base^2
git range-diff $pre$base..$pre $tag$base..$tag
The command uses what it calls the "dual color mode" (can be disabled via
--no-dual-color) which helps identifying what actually changed: it prefixes
lines with a - (and red background) that correspond to the first commit
range, and with a + (and green background) that correspond to the second
range. The rest of the lines will be colored according to the original
diffs.
Changes since v4:
* Fixed a typo in the commit message of "range-diff: add tests" that was
introduced in v4.
* White-space fixes.
* Fixed the length of the first header underline in the man page.
* Changed the preprocessor guard in linear-assignment.h to reflect the new
name (instead of the old name, which was hungarian.h).
* Likewise, changed the preprocessor guards in range-diff.h to hide the
history of the thrice-renamed command.
* Fixed indentation in the completion.
* Instead of trying to paper over white-space error handling that does not
apply to "diffs of diffs", dual color mode now simply disables all
white-space warnings.
* When showing the "single arg must be symmetric range" error message, git
range-diff now also shows the usage.
* Adjusted the commit message of "range-diff: adjust the output of the
commit pairs" to avoid the surprise of the reviewer when onelines are
printed all of a sudden, too.
* "range-diff: adjust the output of the commit pairs" is now using a
simpler way to print onelines.
* We are now sandwiching the diff_opt_parse() loop between two
parse_options(), to make sure that we caught all options, and that the --
separator is handled.
* Adjusted the lookup_commit_reference() call to the newest master (it now
takes a the_repository parameter).
Changes since v3:
* The cover letter was adjusted to reflect the new reality (the command is
called range-diff now, not branch-diff, and --dual-color is the default).
* The documentation was adjusted a bit more in the patch that makes
--dual-color the default.
* Clarified the calculation of the cost matrix, as per Stefan Beller's
request.
* The man page now spells out that merge commits are ignored in the commit
ranges (not merges per se).
* The code in linear-assignment.c was adjusted to use the SWAP() macro.
* The commit message of the patch introducing the first rudimentary
implementation no longer talks about the "Hungarian" algorithm, but about
the "linear assignment algorithm" instead.
* A bogus indentation change was backed out from the patch introducing the
first rudimentary implementation.
* Instead of merely warning about missing .. in the 2-parameter invocation,
we now exit with the error message.
* The diff_opt_parse() function is allowed to return a value larger than 1,
indicating that more than just one command-line parameter was parsed. We
now advance by the indicated value instead of always advancing exactly 1
(which is still correct much of the time).
* A lengthy if...else if...else if...else was simplified (from a logical
point of view) by reordering it.
* The unnecessarily static variable dashes was turned into a local variable
of the caller.
* The commit message talking about the new man page still referred to git
branch --diff, which has been fixed.
* A forgotten t7910 reference was changed to t3206.
* An unbalanced double-tick was fixed
[PATCH v6 04/21] range-diff: improve the order of the shown commits
From: Johannes Schindelin
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 15d418afa..2d94200d3 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++;
+ }
}
}
--
gitgitgadget
[PATCH v6 07/21] range-diff: indent the diffs just like tbdiff
From: Johannes Schindelin
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 3b06ed944..f0598005a 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.output_format = DIFF_FORMAT_PATCH;
+ diffopt.output_prefix = output_prefix_cb;
+ strbuf_addstr(_spaces, "");
+ diffopt.output_prefix_data = _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();
strbuf_release();
+ strbuf_release(_spaces);
return res;
}
--
gitgitgadget
[PATCH v6 18/21] completion: support `git range-diff`
From: Johannes Schindelin
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 94c95516e..3d4ec3432 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
--
gitgitgadget
[PATCH v6 16/21] range-diff --dual-color: skip white-space warnings
From: Johannes Schindelin 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 e6c857abf..ea8ecae04 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, -- gitgitgadget
[PATCH v6 21/21] range-diff: use dim/bold cues to improve dual color mode
From: Johannes Schindelin 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 63365dcf3..90241ed77 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 82c71c682..f693930fd 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 33e786342..98894d6a1 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"
