either the section header or the inner diff hunk header using a
userdiff pattern.
In terms of code changes the biggest change is that we're now re-using
the 'parse_git_header' function from the apply code to parse the diff
headers, instead of trying to parse them with some hacky parsing code
in range-diff.c. This way we are sure that the diff headers are
properly parsed.
I had also considered just outputting the section headers directly
from 'git log', but then decided against that. Parsing the headers
allows a future enhancement of range-diff, where we would allow
parsing an mbox file [2].
I split the "only pass required data" commits up, in the hopes of
making them easier to review, but I'm also happy to squash them if
people feel like that makes it easier to review them.
An added advantage of this is that we're also getting rid of things
like the similarity index, which are not important in the range-diff,
and are thus not represented in the "section header".
One thing that did not change is that the new/deleted strings are not
translated in this version either. This is similar to the regular
consider translating them in the future though.
[1]: https://public-inbox.org/git/20190414210933.20875-1-t.gummerer@gmail.com/
[2]: https://github.com/gitgitgadget/git/issues/207
I'm including the range-diff between this version of the series and
the RFC, and a diff between the range diff and the range-diff without
these changes below. Probably not useful in reviewing the code, but
good to show off the changes made in this series.
range-diff:
-: ---------- > 1: ef2245edda apply: replace marc.info link with public-inbox
-: ---------- > 2: 94578fa45c apply: only pass required data to skip_tree_prefix
-: ---------- > 3: 988269a68e apply: only pass required data to git_header_name
-: ---------- > 4: a2c1ef3f5f apply: only pass required data to check_header_line
-: ---------- > 5: 0f4cfe21cb apply: only pass required data to find_name_*
-: ---------- > 6: 7f1d7a4569 apply: only pass required data to gitdiff_* functions
-: ---------- > 7: a5af8b0845 apply: make parse_git_header public
1: 0e678d222c = 8: 1f25bb1002 range-diff: fix function parameter indentation
-: ---------- > 9: 01ed0f2a6a range-diff: split lines manually
2: 48716230fc ! 10: 044a79868b range-diff: don't remove funcname from inner diff
@@ range-diff.c: static int read_patches(const char *range, struct string_list *lis
strbuf_addch(&buf, '\n');
}
continue;
-- } else if (starts_with(line.buf, "@@ "))
+- } else if (starts_with(line, "@@ "))
- strbuf_addstr(&buf, "@@");
-- else if (!line.buf[0] || starts_with(line.buf, "index "))
-+ } else if (starts_with(line.buf, "@@ ")) {
-+ char *skip_lineno = strstr(line.buf + 3, "@@");
-+ strbuf_remove(&line, 0, skip_lineno - line.buf);
-+ strbuf_addch(&buf, ' ');
-+ strbuf_addbuf(&buf, &line);
-+ } else if (!line.buf[0] || starts_with(line.buf, "index "))
+- else if (!line[0] || starts_with(line, "index "))
++ } else if (skip_prefix(line, "@@ ", &p)) {
++ if (!(p = strstr(p, "@@")))
++ die(_("invalid hunk header in inner diff"));
++ strbuf_addstr(&buf, p);
++ } else if (!line[0] || starts_with(line, "index "))
/*
* A completely blank (not ' \n', which is context)
* line is not valid in a diff. We skip it
+
+ ## t/t3206-range-diff.sh ##
+@@ t/t3206-range-diff.sh: test_expect_success 'changed commit' '
+ 14
+ 4: a63e992 ! 4: d966c5c s/12/B/
+ @@ -8,7 +8,7 @@
+- @@
++ @@ A
+ 9
+ 10
+ - B
+@@ t/t3206-range-diff.sh: test_expect_success 'changed commit with sm config' '
+ 14
+ 4: a63e992 ! 4: d966c5c s/12/B/
+ @@ -8,7 +8,7 @@
+- @@
++ @@ A
+ 9
+ 10
+ - B
+@@ t/t3206-range-diff.sh: test_expect_success 'dual-coloring' '
+ : 14<RESET>
+ :<RED>4: d966c5c <RESET><YELLOW>!<RESET><GREEN> 4: 8add5f1<RESET><YELLOW> s/12/B/<RESET>
+ : <REVERSE><CYAN>@@ -8,7 +8,7 @@<RESET>
+- : <CYAN> @@<RESET>
++ : <CYAN> @@ A<RESET>
+ : 9<RESET>
+ : 10<RESET>
+ : <REVERSE><RED>-<RESET><FAINT> BB<RESET>
3: 71679cb747 < -: ---------- range-diff: add section header instead of diff header
-: ---------- > 11: 69654fe76d range-diff: suppress line count in outer diff
-: ---------- > 12: c38f929b9a range-diff: add section header instead of diff header
-: ---------- > 13: 6df03ecdcf range-diff: add filename to inner diff
4: 8f45b6a995 ! 14: 5ceef49035 range-diff: add section headers to the outer hunk header
@@ Metadata
Author: Thomas Gummerer <t.gummerer@gmail.com>
## Commit message ##
- range-diff: add section headers to the outer hunk header
+ range-diff: add headers to the outer hunk header
- Add the section headers we introduced in the previous commit to the
- outer diff's hunk headers. This makes it easier to understand which
- change we are actually looking at. For example an outer hunk header
- might now look like:
+ Add the section headers/hunk headers we introduced in the previous
+ commits to the outer diff's hunk headers. This makes it easier to
+ understand which change we are actually looking at. For example an
+ outer hunk header might now look like:
- @@ -77,15 +78,43 @@ modified file Documentation/config/interactive.txt
+ @@ Documentation/config/interactive.txt
while previously it would have only been
- @@ -77,15 +78,43 @@
+ @@
which doesn't give a lot of context for the change that follows.
@@ Commit message
## range-diff.c ##
@@ range-diff.c: static int read_patches(const char *range, struct string_list *list)
- strbuf_addstr(&buf, " ##\n");
+ strbuf_addstr(&buf, " ##");
} else if (in_header) {
- if (starts_with(line.buf, "Author: ")) {
+ if (starts_with(line, "Author: ")) {
+ strbuf_addstr(&buf, " ## Metadata ##\n");
- strbuf_addbuf(&buf, &line);
+ strbuf_addstr(&buf, line);
strbuf_addstr(&buf, "\n\n");
+ strbuf_addstr(&buf, " ## Commit message ##\n");
- } else if (starts_with(line.buf, " ")) {
- strbuf_rtrim(&line);
- strbuf_addbuf(&buf, &line);
+ } else if (starts_with(line, " ")) {
+ p = line + len - 2;
+ while (isspace(*p) && p >= line)
@@ range-diff.c: static void output_pair_header(struct diff_options *diffopt,
fwrite(buf->buf, buf->len, 1, diffopt->file);
}
@@ range-diff.c: static void output_pair_header(struct diff_options *diffopt,
-static struct userdiff_driver no_func_name = {
- .funcname = { "$^", 0 }
+static struct userdiff_driver section_headers = {
-+ .funcname = { "^ ## (.*) ##$", REG_EXTENDED }
++ .funcname = { "^ ## (.*) ##$\n"
++ "^.?@@ (.*)$", REG_EXTENDED }
};
static struct diff_filespec *get_filespec(const char *name, const char *p)
@@ range-diff.c: static struct diff_filespec *get_filespec(const char *name, const
return spec;
}
+
+ ## t/t3206-range-diff.sh ##
+@@ t/t3206-range-diff.sh: test_expect_success 'changed commit' '
+ 1: 4de457d = 1: a4b3333 s/5/A/
+ 2: fccce22 = 2: f51d370 s/4/A/
+ 3: 147e64e ! 3: 0559556 s/11/B/
+- @@
++ @@ file: A
+ 9
+ 10
+ -11
+@@ t/t3206-range-diff.sh: test_expect_success 'changed commit' '
+ 13
+ 14
+ 4: a63e992 ! 4: d966c5c s/12/B/
+- @@
++ @@ file
+ @@ file: A
+ 9
+ 10
+@@ t/t3206-range-diff.sh: test_expect_success 'changed commit with sm config' '
+ 1: 4de457d = 1: a4b3333 s/5/A/
+ 2: fccce22 = 2: f51d370 s/4/A/
+ 3: 147e64e ! 3: 0559556 s/11/B/
+- @@
++ @@ file: A
+ 9
+ 10
+ -11
+@@ t/t3206-range-diff.sh: test_expect_success 'changed commit with sm config' '
+ 13
+ 14
+ 4: a63e992 ! 4: d966c5c s/12/B/
+- @@
++ @@ file
+ @@ file: A
+ 9
+ 10
+@@ t/t3206-range-diff.sh: test_expect_success 'renamed file' '
+ sed s/Z/\ /g >expected <<-EOF &&
+ 1: 4de457d = 1: f258d75 s/5/A/
+ 2: fccce22 ! 2: 017b62d s/4/A/
+- @@
++ @@ Metadata
+ ZAuthor: Thomas Rast <trast@inf.ethz.ch>
+ Z
++ Z ## Commit message ##
+ - s/4/A/
+ + s/4/A/ + rename file
+ Z
+@@ t/t3206-range-diff.sh: test_expect_success 'renamed file' '
+ Z 1
+ Z 2
+ 3: 147e64e ! 3: 3ce7af6 s/11/B/
+- @@
+- Z
++ @@ Metadata
++ Z ## Commit message ##
+ Z s/11/B/
+ Z
+ - ## file ##
+@@ t/t3206-range-diff.sh: test_expect_success 'renamed file' '
+ Z 9
+ Z 10
+ 4: a63e992 ! 4: 1e6226b s/12/B/
+- @@
+- Z
++ @@ Metadata
++ Z ## Commit message ##
+ Z s/12/B/
+ Z
+ - ## file ##
+@@ t/t3206-range-diff.sh: test_expect_success 'file added and later removed' '
+ sed s/Z/\ /g >expected <<-EOF &&
+ 1: 4de457d = 1: 096b1ba s/5/A/
+ 2: fccce22 ! 2: d92e698 s/4/A/
+- @@
++ @@ Metadata
+ ZAuthor: Thomas Rast <trast@inf.ethz.ch>
+ Z
++ Z ## Commit message ##
+ - s/4/A/
+ + s/4/A/ + new-file
+ Z
+ Z ## file ##
+ Z@@
+- @@
++ @@ file
+ Z A
+ Z 6
+ Z 7
+ +
+ + ## new-file (new) ##
+ 3: 147e64e ! 3: 9a1db4d s/11/B/
+- @@
++ @@ Metadata
+ ZAuthor: Thomas Rast <trast@inf.ethz.ch>
+ Z
++ Z ## Commit message ##
+ - s/11/B/
+ + s/11/B/ + remove file
+ Z
+ Z ## file ##
+ Z@@ file: A
+- @@
++ @@ file: A
+ Z 12
+ Z 13
+ Z 14
+@@ t/t3206-range-diff.sh: test_expect_success 'changed message' '
+ sed s/Z/\ /g >expected <<-EOF &&
+ 1: 4de457d = 1: f686024 s/5/A/
+ 2: fccce22 ! 2: 4ab067d s/4/A/
+- @@
+- Z
++ @@ Metadata
++ Z ## Commit message ##
+ Z s/4/A/
+ Z
+ + Also a silly comment here!
+@@ t/t3206-range-diff.sh: test_expect_success 'dual-coloring' '
+ sed -e "s|^:||" >expect <<-\EOF &&
+ :<YELLOW>1: a4b3333 = 1: f686024 s/5/A/<RESET>
+ :<RED>2: f51d370 <RESET><YELLOW>!<RESET><GREEN> 2: 4ab067d<RESET><YELLOW> s/4/A/<RESET>
+- : <REVERSE><CYAN>@@<RESET>
+- : <RESET>
++ : <REVERSE><CYAN>@@<RESET> <RESET>Metadata<RESET>
++ : ## Commit message ##<RESET>
+ : s/4/A/<RESET>
+ : <RESET>
+ : <REVERSE><GREEN>+<RESET><BOLD> Also a silly comment here!<RESET>
+@@ t/t3206-range-diff.sh: test_expect_success 'dual-coloring' '
+ : <CYAN> @@<RESET>
+ : 1<RESET>
+ :<RED>3: 0559556 <RESET><YELLOW>!<RESET><GREEN> 3: b9cb956<RESET><YELLOW> s/11/B/<RESET>
+- : <REVERSE><CYAN>@@<RESET>
++ : <REVERSE><CYAN>@@<RESET> <RESET>file: A<RESET>
+ : 9<RESET>
+ : 10<RESET>
+ : <RED> -11<RESET>
+@@ t/t3206-range-diff.sh: test_expect_success 'dual-coloring' '
+ : 13<RESET>
+ : 14<RESET>
+ :<RED>4: d966c5c <RESET><YELLOW>!<RESET><GREEN> 4: 8add5f1<RESET><YELLOW> s/12/B/<RESET>
+- : <REVERSE><CYAN>@@<RESET>
++ : <REVERSE><CYAN>@@<RESET> <RESET>file<RESET>
+ : <CYAN> @@ file: A<RESET>
+ : 9<RESET>
+ : 10<RESET>
insane, though with some coloring I find it actually somewhat
readable):
@@ -8,7 +8,7 @@
1: 0e678d222c = 8: 1f25bb1002 range-diff: fix function parameter indentation
-: ---------- > 9: 01ed0f2a6a range-diff: split lines manually
2: 48716230fc ! 10: 044a79868b range-diff: don't remove funcname from inner diff
- @@ -27,15 +27,45 @@
+ @@ range-diff.c: static int read_patches(const char *range, struct string_list *lis
strbuf_addch(&buf, '\n');
}
continue;
@@ -32,10 +32,8 @@
* A completely blank (not ' \n', which is context)
* line is not valid in a diff. We skip it
+
- + diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
- + --- a/t/t3206-range-diff.sh
- + +++ b/t/t3206-range-diff.sh
- +@@
+ + ## t/t3206-range-diff.sh ##
+ +@@ t/t3206-range-diff.sh: test_expect_success 'changed commit' '
+ 14
+ 4: a63e992 ! 4: d966c5c s/12/B/
+ @@ -8,7 +8,7 @@
@@ -44,7 +42,7 @@
+ 9
+ 10
+ - B
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'changed commit with sm config' '
+ 14
+ 4: a63e992 ! 4: d966c5c s/12/B/
+ @@ -8,7 +8,7 @@
@@ -53,7 +51,7 @@
+ 9
+ 10
+ - B
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'dual-coloring' '
+ : 14<RESET>
+ :<RED>4: d966c5c <RESET><YELLOW>!<RESET><GREEN> 4: 8add5f1<RESET><YELLOW> s/12/B/<RESET>
+ : <REVERSE><CYAN>@@ -8,7 +8,7 @@<RESET>
@@ -67,9 +65,10 @@
-: ---------- > 12: c38f929b9a range-diff: add section header instead of diff header
-: ---------- > 13: 6df03ecdcf range-diff: add filename to inner diff
4: 8f45b6a995 ! 14: 5ceef49035 range-diff: add section headers to the outer hunk header
- @@ -1,17 +1,17 @@
+ @@ Metadata
Author: Thomas Gummerer <t.gummerer@gmail.com>
+ ## Commit message ##
- range-diff: add section headers to the outer hunk header
+ range-diff: add headers to the outer hunk header
@@ -92,10 +91,10 @@
which doesn't give a lot of context for the change that follows.
- @@ -24,16 +24,16 @@
- --- a/range-diff.c
- +++ b/range-diff.c
- @@
+ @@ Commit message
+
+ ## range-diff.c ##
+ @@ range-diff.c: static int read_patches(const char *range, struct string_list *list)
- strbuf_addstr(&buf, " ##\n");
+ strbuf_addstr(&buf, " ##");
} else if (in_header) {
@@ -112,10 +111,10 @@
+ } else if (starts_with(line, " ")) {
+ p = line + len - 2;
+ while (isspace(*p) && p >= line)
- @@
+ @@ range-diff.c: static void output_pair_header(struct diff_options *diffopt,
fwrite(buf->buf, buf->len, 1, diffopt->file);
}
- @@ -41,7 +41,8 @@
+ @@ range-diff.c: static void output_pair_header(struct diff_options *diffopt,
-static struct userdiff_driver no_func_name = {
- .funcname = { "$^", 0 }
+static struct userdiff_driver section_headers = {
@@ -125,15 +124,13 @@
};
static struct diff_filespec *get_filespec(const char *name, const char *p)
- @@ -54,3 +55,154 @@ $^
+ @@ range-diff.c: static struct diff_filespec *get_filespec(const char *name, const
return spec;
}
+
- + diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
- + --- a/t/t3206-range-diff.sh
- + +++ b/t/t3206-range-diff.sh
- +@@
+ + ## t/t3206-range-diff.sh ##
+ +@@ t/t3206-range-diff.sh: test_expect_success 'changed commit' '
+ 1: 4de457d = 1: a4b3333 s/5/A/
+ 2: fccce22 = 2: f51d370 s/4/A/
+ 3: 147e64e ! 3: 0559556 s/11/B/
@@ -142,7 +139,7 @@
+ 9
+ 10
+ -11
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'changed commit' '
+ 13
+ 14
+ 4: a63e992 ! 4: d966c5c s/12/B/
@@ -151,7 +148,7 @@
+ @@ file: A
+ 9
+ 10
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'changed commit with sm config' '
+ 1: 4de457d = 1: a4b3333 s/5/A/
+ 2: fccce22 = 2: f51d370 s/4/A/
+ 3: 147e64e ! 3: 0559556 s/11/B/
@@ -160,7 +157,7 @@
+ 9
+ 10
+ -11
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'changed commit with sm config' '
+ 13
+ 14
+ 4: a63e992 ! 4: d966c5c s/12/B/
@@ -169,7 +166,7 @@
+ @@ file: A
+ 9
+ 10
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'renamed file' '
+ sed s/Z/\ /g >expected <<-EOF &&
+ 1: 4de457d = 1: f258d75 s/5/A/
+ 2: fccce22 ! 2: 017b62d s/4/A/
@@ -181,7 +178,7 @@
+ - s/4/A/
+ + s/4/A/ + rename file
+ Z
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'renamed file' '
+ Z 1
+ Z 2
+ 3: 147e64e ! 3: 3ce7af6 s/11/B/
@@ -192,7 +189,7 @@
+ Z s/11/B/
+ Z
+ - ## file ##
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'renamed file' '
+ Z 9
+ Z 10
+ 4: a63e992 ! 4: 1e6226b s/12/B/
@@ -203,7 +200,7 @@
+ Z s/12/B/
+ Z
+ - ## file ##
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'file added and later removed' '
+ sed s/Z/\ /g >expected <<-EOF &&
+ 1: 4de457d = 1: 096b1ba s/5/A/
+ 2: fccce22 ! 2: d92e698 s/4/A/
@@ -240,7 +237,7 @@
+ Z 12
+ Z 13
+ Z 14
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'changed message' '
+ sed s/Z/\ /g >expected <<-EOF &&
+ 1: 4de457d = 1: f686024 s/5/A/
+ 2: fccce22 ! 2: 4ab067d s/4/A/
@@ -251,7 +248,7 @@
+ Z s/4/A/
+ Z
+ + Also a silly comment here!
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'dual-coloring' '
+ sed -e "s|^:||" >expect <<-\EOF &&
+ :<YELLOW>1: a4b3333 = 1: f686024 s/5/A/<RESET>
+ :<RED>2: f51d370 <RESET><YELLOW>!<RESET><GREEN> 2: 4ab067d<RESET><YELLOW> s/4/A/<RESET>
@@ -262,7 +259,7 @@
+ : s/4/A/<RESET>
+ : <RESET>
+ : <REVERSE><GREEN>+<RESET><BOLD> Also a silly comment here!<RESET>
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'dual-coloring' '
+ : <CYAN> @@<RESET>
+ : 1<RESET>
+ :<RED>3: 0559556 <RESET><YELLOW>!<RESET><GREEN> 3: b9cb956<RESET><YELLOW> s/11/B/<RESET>
@@ -271,7 +268,7 @@
+ : 9<RESET>
+ : 10<RESET>
+ : <RED> -11<RESET>
- +@@
+ +@@ t/t3206-range-diff.sh: test_expect_success 'dual-coloring' '
+ : 13<RESET>
+ : 14<RESET>
+ :<RED>4: d966c5c <RESET><YELLOW>!<RESET><GREEN> 4: 8add5f1<RESET><YELLOW> s/12/B/<RESET>