From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
When "add -p" parses diffs, it looks for context lines starting with a
single space. But when diff.suppressBlankEmpty is in effect, an empty
context line will omit the space, giving us a true empty line. This
confuses the parser, which is unable to split based on such a line.
It's tempting to say that we should just make sure that we generate a
diff without that option. However, although we do not parse hunks that
the user has manually edited with parse_diff() we do allow the user
to split such hunks. As POSIX calls the decision of whether to print the
space here "implementation-defined" we need to handle edited hunks where
empty context lines omit the space.
So let's handle both cases: a context line either starts with a space or
consists of a totally empty line by normalizing the first character to a
space when we parse them. Normalizing the first character rather than
changing the code to check for a space or newline will hopefully future
proof against introducing similar bugs if the code is changed.
Reported-by: Ilya Tumaykin <itumaykin@xxxxxxxxx>
Helped-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
---
add-patch: handle splitting hunks with diff.suppressBlankEmpty
This is an alternative to jk/add-patch-with-suppress-blank-empty which
was recently discarded from next. I hope that normalizing the context
marker will simplify any future changes to the code.
While I was writing this I realized that we should be recalculating
hunk->splittable_into when we re-count the hunk header of it is edited
but I've left to for a future series.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1763%2Fphillipwood%2Fadd-p-suppress-blank-empty-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1763/phillipwood/add-p-suppress-blank-empty-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1763
add-patch.c | 17 ++++++++++++-----
t/t3701-add-interactive.sh | 11 +++++++++++
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/add-patch.c b/add-patch.c
index d8ea05ff108..13b2607f544 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -400,6 +400,12 @@ static void complete_file(char marker, struct hunk *hunk)
hunk->splittable_into++;
}
+/* Empty context lines may omit the leading ' ' */
+static int normalize_marker(char *p)
+{
+ return p[0] == '\n' || (p[0] == '\r' && p[1] == '\n') ? ' ' : p[0];
+}
+
static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
{
struct strvec args = STRVEC_INIT;
@@ -485,6 +491,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
while (p != pend) {
char *eol = memchr(p, '\n', pend - p);
const char *deleted = NULL, *mode_change = NULL;
+ char ch = normalize_marker(p);
if (!eol)
eol = pend;
@@ -532,7 +539,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
* Start counting into how many hunks this one can be
* split
*/
- marker = *p;
+ marker = ch;
} else if (hunk == &file_diff->head &&
starts_with(p, "new file")) {
file_diff->added = 1;
@@ -586,10 +593,10 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
(int)(eol - (plain->buf + file_diff->head.start)),
plain->buf + file_diff->head.start);
- if ((marker == '-' || marker == '+') && *p == ' ')
+ if ((marker == '-' || marker == '+') && ch == ' ')
hunk->splittable_into++;
- if (marker && *p != '\\')
- marker = *p;
+ if (marker && ch != '\\')
+ marker = ch;
p = eol == pend ? pend : eol + 1;
hunk->end = p - plain->buf;
@@ -953,7 +960,7 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,
context_line_count = 0;
while (splittable_into > 1) {
- ch = s->plain.buf[current];
+ ch = normalize_marker(s->plain.buf + current);
if (!ch)
BUG("buffer overrun while splitting hunks");
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 5d78868ac16..351dd2b4332 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -1164,4 +1164,15 @@ test_expect_success 'reset -p with unmerged files' '
test_must_be_empty staged
'
+test_expect_success 'hunk splitting works with diff.suppressBlankEmpty' '
+ test_config diff.suppressBlankEmpty true &&
+ test_write_lines a b c "" d e f >file &&
+ git add file &&
+ test_write_lines p q r "" s t u >file &&
+ test_write_lines s n y q | git add -p &&
+ git cat-file blob :file >actual &&
+ test_write_lines a b c "" s t u >expect &&
+ test_cmp expect actual
+'
+
test_done
base-commit: 790a17fb19d6eadd16c52e5d284a5c6921744766
--
gitgitgadget
