built-in add -p: adjust hunk headers as needed
When skipping a hunk that adds a different number of lines than it removes, we need to adjust the subsequent hunk headers of non-skipped hunks: in pathological cases, the context is not enough to determine precisely where the patch should be applied. This problem was identified in 23fea4c240 (t3701: add failing test for pathological context lines, 2018-03-01) and fixed in the Perl version in fecc6f3a68 (add -p: adjust offsets of subsequent hunks when one is skipped, 2018-03-01). And this patch fixes it in the C version of `git add -p`. In contrast to the Perl version, we try to keep the extra text on the hunk header (which typically contains the signature of the function whose code is changed in the hunk) intact. Note: while the C version does not support staging mode changes at this stage, we already prepare for this by simply skipping the hunk header if both old and new offset is 0 (this cannot happen for regular hunks, and we will use this as an indicator that we are looking at a special hunk). Likewise, we already prepare for hunk splitting by handling the absence of extra text in the hunk header gracefully: only the first split hunk will have that text, the others will not (indicated by an empty extra text start/end range). Preparing for hunk splitting already at this stage avoids an indentation change of the entire hunk header-printing block later, and is almost as easy to review as without that handling. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Schindelin committed
Dec 13, 2019 at 08:07 UTC
25ea47af494c5ec086aae102d358c4001c9a1459
3 files changed
+151
-23
add-interactive.c
+3
-11
@@ -10,16 +10,6 @@
10
#include "dir.h"
11
#include "run-command.h"
12
13
-struct add_i_state {
14
- struct repository *r;
15
- int use_color;
16
- char header_color[COLOR_MAXLEN];
17
- char help_color[COLOR_MAXLEN];
18
- char prompt_color[COLOR_MAXLEN];
19
- char error_color[COLOR_MAXLEN];
20
- char reset_color[COLOR_MAXLEN];
21
-};
22
-
13
static void init_color(struct repository *r, struct add_i_state *s,
14
const char *slot_name, char *dst,
15
const char *default_color)
@@ -36,7 +26,7 @@ static void init_color(struct repository *r, struct add_i_state *s,
26
free(key);
27
}
28
39
-static void init_add_i_state(struct add_i_state *s, struct repository *r)
29
+void init_add_i_state(struct add_i_state *s, struct repository *r)
30
{
31
const char *value;
32
@@ -54,6 +44,8 @@ static void init_add_i_state(struct add_i_state *s, struct repository *r)
44
init_color(r, s, "prompt", s->prompt_color, GIT_COLOR_BOLD_BLUE);
45
init_color(r, s, "error", s->error_color, GIT_COLOR_BOLD_RED);
46
init_color(r, s, "reset", s->reset_color, GIT_COLOR_RESET);
47
+ init_color(r, s, "fraginfo", s->fraginfo_color,
48
+ diff_get_color(s->use_color, DIFF_FRAGINFO));
49
}
50
51
/*
add-interactive.h
+15
@@ -1,6 +1,21 @@
1
#ifndef ADD_INTERACTIVE_H
2
#define ADD_INTERACTIVE_H
3
4
+#include "color.h"
5
+
6
+struct add_i_state {
7
+ struct repository *r;
8
+ int use_color;
9
+ char header_color[COLOR_MAXLEN];
10
+ char help_color[COLOR_MAXLEN];
11
+ char prompt_color[COLOR_MAXLEN];
12
+ char error_color[COLOR_MAXLEN];
13
+ char reset_color[COLOR_MAXLEN];
14
+ char fraginfo_color[COLOR_MAXLEN];
15
+};
16
+
17
+void init_add_i_state(struct add_i_state *s, struct repository *r);
18
+
19
struct repository;
20
struct pathspec;
21
int run_add_i(struct repository *r, const struct pathspec *ps);
add-patch.c
+133
-12
@@ -5,14 +5,26 @@
5
#include "argv-array.h"
6
#include "pathspec.h"
7
#include "color.h"
8
+#include "diff.h"
9
+
10
+struct hunk_header {
11
+ unsigned long old_offset, old_count, new_offset, new_count;
12
+ /*
13
+ * Start/end offsets to the extra text after the second `@@` in the
14
+ * hunk header, e.g. the function signature. This is expected to
15
+ * include the newline.
16
+ */
17
+ size_t extra_start, extra_end, colored_extra_start, colored_extra_end;
18
+};
19
20
struct hunk {
21
size_t start, end, colored_start, colored_end;
22
enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use;
23
+ struct hunk_header header;
24
};
25
26
struct add_p_state {
15
- struct repository *r;
27
+ struct add_i_state s;
28
struct strbuf answer, buf;
29
30
/* parsed diff */
@@ -35,7 +47,70 @@ static void setup_child_process(struct add_p_state *s,
47
48
cp->git_cmd = 1;
49
argv_array_pushf(&cp->env_array,
38
- INDEX_ENVIRONMENT "=%s", s->r->index_file);
50
+ INDEX_ENVIRONMENT "=%s", s->s.r->index_file);
51
+}
52
+
53
+static int parse_range(const char **p,
54
+ unsigned long *offset, unsigned long *count)
55
+{
56
+ char *pend;
57
+
58
+ *offset = strtoul(*p, &pend, 10);
59
+ if (pend == *p)
60
+ return -1;
61
+ if (*pend != ',') {
62
+ *count = 1;
63
+ *p = pend;
64
+ return 0;
65
+ }
66
+ *count = strtoul(pend + 1, (char **)p, 10);
67
+ return *p == pend + 1 ? -1 : 0;
68
+}
69
+
70
+static int parse_hunk_header(struct add_p_state *s, struct hunk *hunk)
71
+{
72
+ struct hunk_header *header = &hunk->header;
73
+ const char *line = s->plain.buf + hunk->start, *p = line;
74
+ char *eol = memchr(p, '\n', s->plain.len - hunk->start);
75
+
76
+ if (!eol)
77
+ eol = s->plain.buf + s->plain.len;
78
+
79
+ if (!skip_prefix(p, "@@ -", &p) ||
80
+ parse_range(&p, &header->old_offset, &header->old_count) < 0 ||
81
+ !skip_prefix(p, " +", &p) ||
82
+ parse_range(&p, &header->new_offset, &header->new_count) < 0 ||
83
+ !skip_prefix(p, " @@", &p))
84
+ return error(_("could not parse hunk header '%.*s'"),
85
+ (int)(eol - line), line);
86
+
87
+ hunk->start = eol - s->plain.buf + (*eol == '\n');
88
+ header->extra_start = p - s->plain.buf;
89
+ header->extra_end = hunk->start;
90
+
91
+ if (!s->colored.len) {
92
+ header->colored_extra_start = header->colored_extra_end = 0;
93
+ return 0;
94
+ }
95
+
96
+ /* Now find the extra text in the colored diff */
97
+ line = s->colored.buf + hunk->colored_start;
98
+ eol = memchr(line, '\n', s->colored.len - hunk->colored_start);
99
+ if (!eol)
100
+ eol = s->colored.buf + s->colored.len;
101
+ p = memmem(line, eol - line, "@@ -", 4);
102
+ if (!p)
103
+ return error(_("could not parse colored hunk header '%.*s'"),
104
+ (int)(eol - line), line);
105
+ p = memmem(p + 4, eol - p - 4, " @@", 3);
106
+ if (!p)
107
+ return error(_("could not parse colored hunk header '%.*s'"),
108
+ (int)(eol - line), line);
109
+ hunk->colored_start = eol - s->colored.buf + (*eol == '\n');
110
+ header->colored_extra_start = p + 3 - s->colored.buf;
111
+ header->colored_extra_end = hunk->colored_start;
112
+
113
+ return 0;
114
}
115
116
static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
@@ -109,6 +184,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
184
hunk->start = p - plain->buf;
185
if (colored)
186
hunk->colored_start = colored_p - colored->buf;
187
+
188
+ if (parse_hunk_header(s, hunk) < 0)
189
+ return -1;
190
}
191
192
p = eol == pend ? pend : eol + 1;
@@ -130,8 +208,43 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
208
}
209
210
static void render_hunk(struct add_p_state *s, struct hunk *hunk,
133
- int colored, struct strbuf *out)
211
+ ssize_t delta, int colored, struct strbuf *out)
212
{
213
+ struct hunk_header *header = &hunk->header;
214
+
215
+ if (hunk->header.old_offset != 0 || hunk->header.new_offset != 0) {
216
+ /*
217
+ * Generate the hunk header dynamically, except for special
218
+ * hunks (such as the diff header).
219
+ */
220
+ const char *p;
221
+ size_t len;
222
+ unsigned long old_offset = header->old_offset;
223
+ unsigned long new_offset = header->new_offset;
224
+
225
+ if (!colored) {
226
+ p = s->plain.buf + header->extra_start;
227
+ len = header->extra_end - header->extra_start;
228
+ } else {
229
+ strbuf_addstr(out, s->s.fraginfo_color);
230
+ p = s->colored.buf + header->colored_extra_start;
231
+ len = header->colored_extra_end
232
+ - header->colored_extra_start;
233
+ }
234
+
235
+ new_offset += delta;
236
+
237
+ strbuf_addf(out, "@@ -%lu,%lu +%lu,%lu @@",
238
+ old_offset, header->old_count,
239
+ new_offset, header->new_count);
240
+ if (len)
241
+ strbuf_add(out, p, len);
242
+ else if (colored)
243
+ strbuf_addf(out, "%s\n", GIT_COLOR_RESET);
244
+ else
245
+ strbuf_addch(out, '\n');
246
+ }
247
+
248
if (colored)
249
strbuf_add(out, s->colored.buf + hunk->colored_start,
250
hunk->colored_end - hunk->colored_start);
@@ -144,13 +257,17 @@ static void reassemble_patch(struct add_p_state *s, struct strbuf *out)
257
{
258
struct hunk *hunk;
259
size_t i;
260
+ ssize_t delta = 0;
261
148
- render_hunk(s, &s->head, 0, out);
262
+ render_hunk(s, &s->head, 0, 0, out);
263
264
for (i = 0; i < s->hunk_nr; i++) {
265
hunk = s->hunk + i;
152
- if (hunk->use == USE_HUNK)
153
- render_hunk(s, hunk, 0, out);
266
+ if (hunk->use != USE_HUNK)
267
+ delta += hunk->header.old_count
268
+ - hunk->header.new_count;
269
+ else
270
+ render_hunk(s, hunk, delta, 0, out);
271
}
272
}
273
@@ -178,7 +295,7 @@ static int patch_update_file(struct add_p_state *s)
295
return 0;
296
297
strbuf_reset(&s->buf);
181
- render_hunk(s, &s->head, colored, &s->buf);
298
+ render_hunk(s, &s->head, 0, colored, &s->buf);
299
fputs(s->buf.buf, stdout);
300
for (;;) {
301
if (hunk_index >= s->hunk_nr)
@@ -205,7 +322,7 @@ static int patch_update_file(struct add_p_state *s)
322
break;
323
324
strbuf_reset(&s->buf);
208
- render_hunk(s, hunk, colored, &s->buf);
325
+ render_hunk(s, hunk, 0, colored, &s->buf);
326
fputs(s->buf.buf, stdout);
327
328
strbuf_reset(&s->buf);
@@ -272,13 +389,13 @@ soft_increment:
389
strbuf_reset(&s->buf);
390
reassemble_patch(s, &s->buf);
391
275
- discard_index(s->r->index);
392
+ discard_index(s->s.r->index);
393
setup_child_process(s, &cp, "apply", "--cached", NULL);
394
if (pipe_command(&cp, s->buf.buf, s->buf.len,
395
NULL, 0, NULL, 0))
396
error(_("'git apply --cached' failed"));
280
- if (!repo_read_index(s->r))
281
- repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0,
397
+ if (!repo_read_index(s->s.r))
398
+ repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0,
399
1, NULL, NULL, NULL);
400
}
401
@@ -288,7 +405,11 @@ soft_increment:
405
406
int run_add_p(struct repository *r, const struct pathspec *ps)
407
{
291
- struct add_p_state s = { r, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT };
408
+ struct add_p_state s = {
409
+ { r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
410
+ };
411
+
412
+ init_add_i_state(&s.s, r);
413
414
if (discard_index(r->index) < 0 || repo_read_index(r) < 0 ||
415
repo_refresh_and_write_index(r, REFRESH_QUIET, 0, 1,