From b33acef5674f9fa80186d830c1932daf64ab80d9 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 11 Mar 2019 22:01:22 +0100 Subject: [PATCH 01/18] built-in add -i: start implementing the `patch` functionality in C In the previous steps, we re-implemented the main loop of `git add -i` in C, and most of the commands. Notably, we left out the actual functionality of `patch`, as the relevant code makes up more than half of `git-add--interactive.perl`, and is actually pretty independent of the rest of the commands. With this commit, we start to tackle that `patch` part. For better separation of concerns, we keep the code in a separate file, `add-patch.c`. The new code is still guarded behind the `add.interactive.useBuiltin` config setting, and for the moment, it can only be called via `git add -p`. The actual functionality follows the original implementation of 5cde71d64aff (git-add --interactive, 2006-12-10), but not too closely (for example, we use string offsets rather than copying strings around, and we also remember which previous/next hunk was undecided, rather than looking again when the user asked to jump there). As a further deviation from that commit, We also use a comma instead of a slash to separate the available commands in the prompt, as the current version of the Perl script does this, and we also add a line about the question mark ("print help") to the help text. Signed-off-by: Johannes Schindelin --- Makefile | 1 + add-interactive.h | 1 + add-patch.c | 260 ++++++++++++++++++++++++++++++++++++++++++++++ builtin/add.c | 10 +- 4 files changed, 270 insertions(+), 2 deletions(-) create mode 100644 add-patch.c diff --git a/Makefile b/Makefile index 2e375aa135..7f9b65a259 100644 --- a/Makefile +++ b/Makefile @@ -828,6 +828,7 @@ LIB_H := $(sort $(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null LIB_OBJS += abspath.o LIB_OBJS += add-interactive.o +LIB_OBJS += add-patch.o LIB_OBJS += advice.o LIB_OBJS += alias.o LIB_OBJS += alloc.o diff --git a/add-interactive.h b/add-interactive.h index 7043b8741d..0e3d93acc9 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -4,5 +4,6 @@ struct repository; struct pathspec; int run_add_i(struct repository *r, const struct pathspec *ps); +int run_add_p(struct repository *r, const struct pathspec *ps); #endif diff --git a/add-patch.c b/add-patch.c new file mode 100644 index 0000000000..7ff5d9e764 --- /dev/null +++ b/add-patch.c @@ -0,0 +1,260 @@ +#include "cache.h" +#include "add-interactive.h" +#include "strbuf.h" +#include "run-command.h" +#include "argv-array.h" +#include "pathspec.h" + +struct hunk { + size_t start, end; + enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use; +}; + +struct add_p_state { + struct repository *r; + struct strbuf answer, buf; + + /* parsed diff */ + struct strbuf plain; + struct hunk head; + struct hunk *hunk; + size_t hunk_nr, hunk_alloc; +}; + +static void setup_child_process(struct child_process *cp, + struct add_p_state *s, ...) +{ + va_list ap; + const char *arg; + + va_start(ap, s); + while((arg = va_arg(ap, const char *))) + argv_array_push(&cp->args, arg); + va_end(ap); + + cp->git_cmd = 1; + argv_array_pushf(&cp->env_array, + INDEX_ENVIRONMENT "=%s", s->r->index_file); +} + +static int parse_diff(struct add_p_state *s, const struct pathspec *ps) +{ + struct strbuf *plain = &s->plain; + struct child_process cp = CHILD_PROCESS_INIT; + char *p, *pend; + size_t i; + struct hunk *hunk = NULL; + int res; + + /* Use `--no-color` explicitly, just in case `diff.color = always`. */ + setup_child_process(&cp, s, + "diff-files", "-p", "--no-color", "--", NULL); + for (i = 0; i < ps->nr; i++) + argv_array_push(&cp.args, ps->items[i].original); + + res = capture_command(&cp, plain, 0); + if (res) + return error(_("could not parse diff")); + if (!plain->len) + return 0; + strbuf_complete_line(plain); + + /* parse hunks */ + p = plain->buf; + pend = p + plain->len; + while (p != pend) { + char *eol = memchr(p, '\n', pend - p); + if (!eol) + eol = pend; + + if (starts_with(p, "diff ")) { + if (p != plain->buf) + BUG("multi-file diff not yet handled"); + hunk = &s->head; + } else if (p == plain->buf) + BUG("diff starts with unexpected line:\n" + "%.*s\n", (int)(eol - p), p); + else if (starts_with(p, "@@ ")) { + s->hunk_nr++; + ALLOC_GROW(s->hunk, s->hunk_nr, + s->hunk_alloc); + hunk = s->hunk + s->hunk_nr - 1; + memset(hunk, 0, sizeof(*hunk)); + + hunk->start = p - plain->buf; + } + + p = eol == pend ? pend : eol + 1; + hunk->end = p - plain->buf; + } + + return 0; +} + +static void render_hunk(struct add_p_state *s, struct hunk *hunk, + struct strbuf *out) +{ + strbuf_add(out, s->plain.buf + hunk->start, + hunk->end - hunk->start); +} + +static void reassemble_patch(struct add_p_state *s, struct strbuf *out) +{ + struct hunk *hunk; + size_t i; + + render_hunk(s, &s->head, out); + + for (i = 0; i < s->hunk_nr; i++) { + hunk = s->hunk + i; + if (hunk->use == USE_HUNK) + render_hunk(s, hunk, out); + } +} + +static const char help_patch_text[] = +N_("y - stage this hunk\n" + "n - do not stage this hunk\n" + "a - stage this and all the remaining hunks\n" + "d - do not stage this hunk nor any of the remaining hunks\n" + "j - leave this hunk undecided, see next undecided hunk\n" + "J - leave this hunk undecided, see next hunk\n" + "k - leave this hunk undecided, see previous undecided hunk\n" + "K - leave this hunk undecided, see previous hunk\n" + "? - print help\n"); + +static int patch_update_file(struct add_p_state *s) +{ + size_t hunk_index = 0; + ssize_t i, undecided_previous, undecided_next; + struct hunk *hunk; + char ch; + struct child_process cp = CHILD_PROCESS_INIT; + + if (!s->hunk_nr) + return 0; + + strbuf_reset(&s->buf); + render_hunk(s, &s->head, &s->buf); + fputs(s->buf.buf, stdout); + for (;;) { + if (hunk_index >= s->hunk_nr) + hunk_index = 0; + hunk = s->hunk + hunk_index; + + undecided_previous = -1; + for (i = hunk_index - 1; i >= 0; i--) + if (s->hunk[i].use == UNDECIDED_HUNK) { + undecided_previous = i; + break; + } + + undecided_next = -1; + for (i = hunk_index + 1; i < s->hunk_nr; i++) + if (s->hunk[i].use == UNDECIDED_HUNK) { + undecided_next = i; + break; + } + + /* Everything decided? */ + if (undecided_previous < 0 && undecided_next < 0 && + hunk->use != UNDECIDED_HUNK) + break; + + strbuf_reset(&s->buf); + render_hunk(s, hunk, &s->buf); + fputs(s->buf.buf, stdout); + + strbuf_reset(&s->buf); + if (undecided_previous >= 0) + strbuf_addstr(&s->buf, ",k"); + if (hunk_index) + strbuf_addstr(&s->buf, ",K"); + if (undecided_next >= 0) + strbuf_addstr(&s->buf, ",j"); + if (hunk_index + 1 < s->hunk_nr) + strbuf_addstr(&s->buf, ",J"); + printf(_("Stage this hunk [y,n,a,d%s,?]? "), s->buf.buf); + fflush(stdout); + if (strbuf_getline(&s->answer, stdin) == EOF) + break; + strbuf_trim_trailing_newline(&s->answer); + + if (!s->answer.len) + continue; + ch = tolower(s->answer.buf[0]); + if (ch == 'y') { + hunk->use = USE_HUNK; +soft_increment: + while (++hunk_index < s->hunk_nr && + s->hunk[hunk_index].use + != UNDECIDED_HUNK) + ; /* continue looking */ + } else if (ch == 'n') { + hunk->use = SKIP_HUNK; + goto soft_increment; + } else if (ch == 'a') { + for (; hunk_index < s->hunk_nr; hunk_index++) { + hunk = s->hunk + hunk_index; + if (hunk->use == UNDECIDED_HUNK) + hunk->use = USE_HUNK; + } + } else if (ch == 'd') { + for (; hunk_index < s->hunk_nr; hunk_index++) { + hunk = s->hunk + hunk_index; + if (hunk->use == UNDECIDED_HUNK) + hunk->use = SKIP_HUNK; + } + } else if (hunk_index && s->answer.buf[0] == 'K') + hunk_index--; + else if (hunk_index + 1 < s->hunk_nr && + s->answer.buf[0] == 'J') + hunk_index++; + else if (undecided_previous >= 0 && + s->answer.buf[0] == 'k') + hunk_index = undecided_previous; + else if (undecided_next >= 0 && s->answer.buf[0] == 'j') + hunk_index = undecided_next; + else + puts(_(help_patch_text)); + } + + /* Any hunk to be used? */ + for (i = 0; i < s->hunk_nr; i++) + if (s->hunk[i].use == USE_HUNK) + break; + + if (i < s->hunk_nr) { + /* At least one hunk selected: apply */ + strbuf_reset(&s->buf); + reassemble_patch(s, &s->buf); + + setup_child_process(&cp, s, "apply", "--cached", NULL); + if (pipe_command(&cp, s->buf.buf, s->buf.len, + NULL, 0, NULL, 0)) + error(_("'git apply --cached' failed")); + repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0); + } + + putchar('\n'); + return 0; +} + +int run_add_p(struct repository *r, const struct pathspec *ps) +{ + struct add_p_state s = { r, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; + + if (repo_refresh_and_write_index(r, REFRESH_QUIET, 0) < 0 || + parse_diff(&s, ps) < 0) { + strbuf_release(&s.plain); + return -1; + } + + if (s.hunk_nr) + patch_update_file(&s); + + strbuf_release(&s.answer); + strbuf_release(&s.buf); + strbuf_release(&s.plain); + return 0; +} diff --git a/builtin/add.c b/builtin/add.c index 4f625691b5..cf02e63ae0 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -192,8 +192,14 @@ int run_add_interactive(const char *revision, const char *patch_mode, git_config_get_bool("add.interactive.usebuiltin", &use_builtin_add_i); - if (use_builtin_add_i == 1 && !patch_mode) - return !!run_add_i(the_repository, pathspec); + if (use_builtin_add_i == 1) { + if (!patch_mode) + return !!run_add_i(the_repository, pathspec); + if (strcmp(patch_mode, "--patch")) + die("'%s' not yet supported in the built-in add -p", + patch_mode); + return !!run_add_p(the_repository, pathspec); + } argv_array_push(&argv, "add--interactive"); if (patch_mode) From cb7d0a05693dc71317001a569f6c514b902c9630 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 12 Mar 2019 12:52:51 +0100 Subject: [PATCH 02/18] built-in add -i: wire up the new C code for the `patch` command The code in `git-add--interactive.perl` that takes care of the `patch` command can look quite intimidating. There are so many modes in which it can be called, for example. But for the `patch` command in `git add -i`, only one mode is relevant: the `stage` mode. And we just implemented the beginnings of that mode in C so far. So let's use it when `add.interactive.useBuiltin=true`. Now, while the code in `add-patch.c` is far from reaching feature parity with the code in `git-add--interactive.perl` (color is not implemented, the diff algorithm cannot be configured, the colored diff cannot be post-processed via `interactive.diffFilter`, many commands are unimplemented yet, etc), hooking it all up with the part of `git add -i` that is already converted to C makes it easier to test and develop it. Note: at this stage, both the `add.interactive.useBuiltin` config setting is still safely opt-in, and will probably be fore quite some time, to allow for thorough testing "in the wild" without adversely affecting existing users. Signed-off-by: Johannes Schindelin --- add-interactive.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 7bc999e929..051581fca5 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -838,14 +838,17 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, count = list_and_choose(items, selected, files->nr, s, opts); if (count >= 0) { struct argv_array args = ARGV_ARRAY_INIT; + struct pathspec ps_selected = { 0 }; - argv_array_pushl(&args, "git", "add--interactive", "--patch", - "--", NULL); for (i = 0; i < files->nr; i++) if (selected[i]) argv_array_push(&args, items[i]->name); - res = run_command_v_opt(args.argv, 0); + parse_pathspec(&ps_selected, + PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL, + PATHSPEC_LITERAL_PATH, "", args.argv); + res = run_add_p(s->r, &ps_selected); argv_array_clear(&args); + clear_pathspec(&ps_selected); } free(selected); From e4ec704727d26457fb4febc10aa49f4f86bf1942 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 12 Mar 2019 15:42:10 +0100 Subject: [PATCH 03/18] built-in add -p: show colored hunks by default Just like the Perl version, we now generate two diffs if `color.diff` is set: one with and one without color. Then we parse them in parallel and record which hunks start at which offsets in both. Note that this is a (slight) deviation from the way the Perl version did it: we are no longer reading the output of `diff-files` line by line (which is more natural for Perl than for C), but in one go, and parse everything later, so we might just as well do it in synchrony. Signed-off-by: Johannes Schindelin --- add-patch.c | 79 +++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 62 insertions(+), 17 deletions(-) diff --git a/add-patch.c b/add-patch.c index 7ff5d9e764..a137036a7e 100644 --- a/add-patch.c +++ b/add-patch.c @@ -4,9 +4,10 @@ #include "run-command.h" #include "argv-array.h" #include "pathspec.h" +#include "color.h" struct hunk { - size_t start, end; + size_t start, end, colored_start, colored_end; enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use; }; @@ -15,7 +16,7 @@ struct add_p_state { struct strbuf answer, buf; /* parsed diff */ - struct strbuf plain; + struct strbuf plain, colored; struct hunk head; struct hunk *hunk; size_t hunk_nr, hunk_alloc; @@ -39,26 +40,50 @@ static void setup_child_process(struct child_process *cp, static int parse_diff(struct add_p_state *s, const struct pathspec *ps) { - struct strbuf *plain = &s->plain; + struct argv_array args = ARGV_ARRAY_INIT; + struct strbuf *plain = &s->plain, *colored = NULL; struct child_process cp = CHILD_PROCESS_INIT; - char *p, *pend; - size_t i; + char *p, *pend, *colored_p = NULL, *colored_pend = NULL; + size_t i, color_arg_index; struct hunk *hunk = NULL; int res; /* Use `--no-color` explicitly, just in case `diff.color = always`. */ - setup_child_process(&cp, s, - "diff-files", "-p", "--no-color", "--", NULL); + argv_array_pushl(&args, "diff-files", "-p", "--no-color", "--", NULL); + color_arg_index = args.argc - 2; for (i = 0; i < ps->nr; i++) - argv_array_push(&cp.args, ps->items[i].original); + argv_array_push(&args, ps->items[i].original); + setup_child_process(&cp, s, NULL); + cp.argv = args.argv; res = capture_command(&cp, plain, 0); - if (res) + if (res) { + argv_array_clear(&args); return error(_("could not parse diff")); - if (!plain->len) + } + if (!plain->len) { + argv_array_clear(&args); return 0; + } strbuf_complete_line(plain); + if (want_color_fd(1, -1)) { + struct child_process colored_cp = CHILD_PROCESS_INIT; + + setup_child_process(&colored_cp, s, NULL); + xsnprintf((char *)args.argv[color_arg_index], 8, "--color"); + colored_cp.argv = args.argv; + colored = &s->colored; + res = capture_command(&colored_cp, colored, 0); + argv_array_clear(&args); + if (res) + return error(_("could not parse colored diff")); + strbuf_complete_line(colored); + colored_p = colored->buf; + colored_pend = colored_p + colored->len; + } + argv_array_clear(&args); + /* parse hunks */ p = plain->buf; pend = p + plain->len; @@ -82,20 +107,37 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) memset(hunk, 0, sizeof(*hunk)); hunk->start = p - plain->buf; + if (colored) + hunk->colored_start = colored_p - colored->buf; } p = eol == pend ? pend : eol + 1; hunk->end = p - plain->buf; + + if (colored) { + char *colored_eol = memchr(colored_p, '\n', + colored_pend - colored_p); + if (colored_eol) + colored_p = colored_eol + 1; + else + colored_p = colored_pend; + + hunk->colored_end = colored_p - colored->buf; + } } return 0; } static void render_hunk(struct add_p_state *s, struct hunk *hunk, - struct strbuf *out) + int colored, struct strbuf *out) { - strbuf_add(out, s->plain.buf + hunk->start, - hunk->end - hunk->start); + if (colored) + strbuf_add(out, s->colored.buf + hunk->colored_start, + hunk->colored_end - hunk->colored_start); + else + strbuf_add(out, s->plain.buf + hunk->start, + hunk->end - hunk->start); } static void reassemble_patch(struct add_p_state *s, struct strbuf *out) @@ -103,12 +145,12 @@ static void reassemble_patch(struct add_p_state *s, struct strbuf *out) struct hunk *hunk; size_t i; - render_hunk(s, &s->head, out); + render_hunk(s, &s->head, 0, out); for (i = 0; i < s->hunk_nr; i++) { hunk = s->hunk + i; if (hunk->use == USE_HUNK) - render_hunk(s, hunk, out); + render_hunk(s, hunk, 0, out); } } @@ -130,12 +172,13 @@ static int patch_update_file(struct add_p_state *s) struct hunk *hunk; char ch; struct child_process cp = CHILD_PROCESS_INIT; + int colored = !!s->colored.len; if (!s->hunk_nr) return 0; strbuf_reset(&s->buf); - render_hunk(s, &s->head, &s->buf); + render_hunk(s, &s->head, colored, &s->buf); fputs(s->buf.buf, stdout); for (;;) { if (hunk_index >= s->hunk_nr) @@ -162,7 +205,7 @@ static int patch_update_file(struct add_p_state *s) break; strbuf_reset(&s->buf); - render_hunk(s, hunk, &s->buf); + render_hunk(s, hunk, colored, &s->buf); fputs(s->buf.buf, stdout); strbuf_reset(&s->buf); @@ -247,6 +290,7 @@ int run_add_p(struct repository *r, const struct pathspec *ps) if (repo_refresh_and_write_index(r, REFRESH_QUIET, 0) < 0 || parse_diff(&s, ps) < 0) { strbuf_release(&s.plain); + strbuf_release(&s.colored); return -1; } @@ -256,5 +300,6 @@ int run_add_p(struct repository *r, const struct pathspec *ps) strbuf_release(&s.answer); strbuf_release(&s.buf); strbuf_release(&s.plain); + strbuf_release(&s.colored); return 0; } From 7d6c0df169af3e17577ce8bb39857b85ef38dde0 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 21 Mar 2019 09:40:20 +0100 Subject: [PATCH 04/18] 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 --- add-interactive.c | 15 ++--- add-interactive.h | 15 +++++ add-patch.c | 145 ++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 154 insertions(+), 21 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 051581fca5..5020d09732 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -12,16 +12,6 @@ #include "argv-array.h" #include "run-command.h" -struct add_i_state { - struct repository *r; - int use_color; - char header_color[COLOR_MAXLEN]; - char help_color[COLOR_MAXLEN]; - char prompt_color[COLOR_MAXLEN]; - char error_color[COLOR_MAXLEN]; - char reset_color[COLOR_MAXLEN]; -}; - static void init_color(struct repository *r, struct add_i_state *s, const char *slot_name, char *dst, const char *default_color) @@ -38,7 +28,7 @@ static void init_color(struct repository *r, struct add_i_state *s, free(key); } -static int init_add_i_state(struct repository *r, struct add_i_state *s) +int init_add_i_state(struct repository *r, struct add_i_state *s) { const char *value; @@ -57,6 +47,9 @@ static int init_add_i_state(struct repository *r, struct add_i_state *s) init_color(r, s, "error", s->error_color, GIT_COLOR_BOLD_RED); init_color(r, s, "reset", s->reset_color, GIT_COLOR_RESET); + strlcpy(s->fraginfo_color, + diff_get_color(s->use_color, DIFF_FRAGINFO), COLOR_MAXLEN); + return 0; } diff --git a/add-interactive.h b/add-interactive.h index 0e3d93acc9..e3c6a8bff0 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -1,6 +1,21 @@ #ifndef ADD_INTERACTIVE_H #define ADD_INTERACTIVE_H +#include "color.h" + +struct add_i_state { + struct repository *r; + int use_color; + char header_color[COLOR_MAXLEN]; + char help_color[COLOR_MAXLEN]; + char prompt_color[COLOR_MAXLEN]; + char error_color[COLOR_MAXLEN]; + char reset_color[COLOR_MAXLEN]; + char fraginfo_color[COLOR_MAXLEN]; +}; + +int init_add_i_state(struct repository *r, struct add_i_state *s); + struct repository; struct pathspec; int run_add_i(struct repository *r, const struct pathspec *ps); diff --git a/add-patch.c b/add-patch.c index a137036a7e..f8ca2a5e2c 100644 --- a/add-patch.c +++ b/add-patch.c @@ -5,14 +5,26 @@ #include "argv-array.h" #include "pathspec.h" #include "color.h" +#include "diff.h" + +struct hunk_header { + unsigned long old_offset, old_count, new_offset, new_count; + /* + * Start/end offsets to the extra text after the second `@@` in the + * hunk header, e.g. the function signature. This is expected to + * include the newline. + */ + size_t extra_start, extra_end, colored_extra_start, colored_extra_end; +}; struct hunk { size_t start, end, colored_start, colored_end; enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use; + struct hunk_header header; }; struct add_p_state { - struct repository *r; + struct add_i_state s; struct strbuf answer, buf; /* parsed diff */ @@ -35,7 +47,70 @@ static void setup_child_process(struct child_process *cp, cp->git_cmd = 1; argv_array_pushf(&cp->env_array, - INDEX_ENVIRONMENT "=%s", s->r->index_file); + INDEX_ENVIRONMENT "=%s", s->s.r->index_file); +} + +static int parse_range(const char **p, + unsigned long *offset, unsigned long *count) +{ + char *pend; + + *offset = strtoul(*p, &pend, 10); + if (pend == *p) + return -1; + if (*pend != ',') { + *count = 1; + *p = pend; + return 0; + } + *count = strtoul(pend + 1, (char **)p, 10); + return *p == pend + 1 ? -1 : 0; +} + +static int parse_hunk_header(struct add_p_state *s, struct hunk *hunk) +{ + struct hunk_header *header = &hunk->header; + const char *line = s->plain.buf + hunk->start, *p = line; + char *eol = memchr(p, '\n', s->plain.len - hunk->start); + + if (!eol) + eol = s->plain.buf + s->plain.len; + + if (!skip_prefix(p, "@@ -", &p) || + parse_range(&p, &header->old_offset, &header->old_count) < 0 || + !skip_prefix(p, " +", &p) || + parse_range(&p, &header->new_offset, &header->new_count) < 0 || + !skip_prefix(p, " @@", &p)) + return error(_("could not parse hunk header '%.*s'"), + (int)(eol - line), line); + + hunk->start = eol - s->plain.buf + (*eol == '\n'); + header->extra_start = p - s->plain.buf; + header->extra_end = hunk->start; + + if (!s->colored.len) { + header->colored_extra_start = header->colored_extra_end = 0; + return 0; + } + + /* Now find the extra text in the colored diff */ + line = s->colored.buf + hunk->colored_start; + eol = memchr(line, '\n', s->colored.len - hunk->colored_start); + if (!eol) + eol = s->colored.buf + s->colored.len; + p = memmem(line, eol - line, "@@ -", 4); + if (!p) + return error(_("could not parse colored hunk header '%.*s'"), + (int)(eol - line), line); + p = memmem(p + 4, eol - p - 4, " @@", 3); + if (!p) + return error(_("could not parse colored hunk header '%.*s'"), + (int)(eol - line), line); + hunk->colored_start = eol - s->colored.buf + (*eol == '\n'); + header->colored_extra_start = p + 3 - s->colored.buf; + header->colored_extra_end = hunk->colored_start; + + return 0; } 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) hunk->start = p - plain->buf; if (colored) hunk->colored_start = colored_p - colored->buf; + + if (parse_hunk_header(s, hunk) < 0) + return -1; } p = eol == pend ? pend : eol + 1; @@ -130,8 +208,46 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } static void render_hunk(struct add_p_state *s, struct hunk *hunk, - int colored, struct strbuf *out) + ssize_t delta, int colored, struct strbuf *out) { + struct hunk_header *header = &hunk->header; + + if (hunk->header.old_offset != 0 || hunk->header.new_offset != 0) { + /* + * Generate the hunk header dynamically, except for special + * hunks (such as the diff header). + */ + const char *p; + size_t len; + unsigned long old_offset = header->old_offset; + unsigned long new_offset = header->new_offset; + + if (!colored) { + p = s->plain.buf + header->extra_start; + len = header->extra_end - header->extra_start; + } else { + strbuf_addstr(out, s->s.fraginfo_color); + p = s->colored.buf + header->colored_extra_start; + len = header->colored_extra_end + - header->colored_extra_start; + } + + if (s->mode->is_reverse) + old_offset -= delta; + else + new_offset += delta; + + strbuf_addf(out, "@@ -%lu,%lu +%lu,%lu @@", + old_offset, header->old_count, + new_offset, header->new_count); + if (len) + strbuf_add(out, p, len); + else if (colored) + strbuf_addf(out, "%s\n", GIT_COLOR_RESET); + else + strbuf_addch(out, '\n'); + } + if (colored) strbuf_add(out, s->colored.buf + hunk->colored_start, hunk->colored_end - hunk->colored_start); @@ -144,13 +260,17 @@ static void reassemble_patch(struct add_p_state *s, struct strbuf *out) { struct hunk *hunk; size_t i; + ssize_t delta = 0; - render_hunk(s, &s->head, 0, out); + render_hunk(s, &s->head, 0, 0, out); for (i = 0; i < s->hunk_nr; i++) { hunk = s->hunk + i; - if (hunk->use == USE_HUNK) - render_hunk(s, hunk, 0, out); + if (hunk->use != USE_HUNK) + delta += hunk->header.old_count + - hunk->header.new_count; + else + render_hunk(s, hunk, delta, 0, out); } } @@ -178,7 +298,7 @@ static int patch_update_file(struct add_p_state *s) return 0; strbuf_reset(&s->buf); - render_hunk(s, &s->head, colored, &s->buf); + render_hunk(s, &s->head, 0, colored, &s->buf); fputs(s->buf.buf, stdout); for (;;) { if (hunk_index >= s->hunk_nr) @@ -205,7 +325,7 @@ static int patch_update_file(struct add_p_state *s) break; strbuf_reset(&s->buf); - render_hunk(s, hunk, colored, &s->buf); + render_hunk(s, hunk, 0, colored, &s->buf); fputs(s->buf.buf, stdout); strbuf_reset(&s->buf); @@ -276,7 +396,7 @@ soft_increment: if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) error(_("'git apply --cached' failed")); - repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0); + repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0); } putchar('\n'); @@ -285,7 +405,12 @@ soft_increment: int run_add_p(struct repository *r, const struct pathspec *ps) { - struct add_p_state s = { r, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; + struct add_p_state s = { + { r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT + }; + + if (init_add_i_state(r, &s.s)) + return error("Could not read `add -i` config"); if (repo_refresh_and_write_index(r, REFRESH_QUIET, 0) < 0 || parse_diff(&s, ps) < 0) { From 1059aef286c47daf366d6458c97b2033eda3e37a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 12 Mar 2019 16:40:46 +0100 Subject: [PATCH 05/18] built-in add -p: color the prompt and the help text ... just like the Perl version ;-) Note that this requires the `get_add_i_color()` function being defined globally, which is the entire reason why we gave it such a descriptive name in the first place. Signed-off-by: Johannes Schindelin --- add-interactive.h | 9 +++++++++ add-patch.c | 7 +++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/add-interactive.h b/add-interactive.h index e3c6a8bff0..eb024066ba 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -16,6 +16,15 @@ struct add_i_state { int init_add_i_state(struct repository *r, struct add_i_state *s); +enum color_add_i { + COLOR_HEADER = 0, + COLOR_HELP, + COLOR_PROMPT, + COLOR_ERROR, + COLOR_RESET, +}; +const char *get_add_i_color(enum color_add_i ix); + struct repository; struct pathspec; int run_add_i(struct repository *r, const struct pathspec *ps); diff --git a/add-patch.c b/add-patch.c index f8ca2a5e2c..3861848dc7 100644 --- a/add-patch.c +++ b/add-patch.c @@ -337,7 +337,9 @@ static int patch_update_file(struct add_p_state *s) strbuf_addstr(&s->buf, ",j"); if (hunk_index + 1 < s->hunk_nr) strbuf_addstr(&s->buf, ",J"); - printf(_("Stage this hunk [y,n,a,d%s,?]? "), s->buf.buf); + color_fprintf(stdout, s->s.prompt_color, + _("Stage this hunk [y,n,a,d%s,?]? "), + s->buf.buf); fflush(stdout); if (strbuf_getline(&s->answer, stdin) == EOF) break; @@ -379,7 +381,8 @@ soft_increment: else if (undecided_next >= 0 && s->answer.buf[0] == 'j') hunk_index = undecided_next; else - puts(_(help_patch_text)); + color_fprintf(stdout, s->s.help_color, + _(help_patch_text)); } /* Any hunk to be used? */ From 0e2f00126392745b0d851f63356a163108fbe06c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 17 Mar 2019 21:10:47 +0100 Subject: [PATCH 06/18] built-in add -p: offer a helpful error message when hunk navigation failed ... just like the Perl version currently does... Signed-off-by: Johannes Schindelin --- add-patch.c | 44 +++++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/add-patch.c b/add-patch.c index 3861848dc7..e30bbc39fc 100644 --- a/add-patch.c +++ b/add-patch.c @@ -34,6 +34,18 @@ struct add_p_state { size_t hunk_nr, hunk_alloc; }; +static void err(struct add_p_state *s, const char *fmt, ...) +{ + va_list args; + + va_start(args, fmt); + fputs(s->s.error_color, stderr); + vfprintf(stderr, fmt, args); + fputs(s->s.reset_color, stderr); + fputc('\n', stderr); + va_end(args); +} + static void setup_child_process(struct child_process *cp, struct add_p_state *s, ...) { @@ -370,17 +382,27 @@ soft_increment: if (hunk->use == UNDECIDED_HUNK) hunk->use = SKIP_HUNK; } - } else if (hunk_index && s->answer.buf[0] == 'K') - hunk_index--; - else if (hunk_index + 1 < s->hunk_nr && - s->answer.buf[0] == 'J') - hunk_index++; - else if (undecided_previous >= 0 && - s->answer.buf[0] == 'k') - hunk_index = undecided_previous; - else if (undecided_next >= 0 && s->answer.buf[0] == 'j') - hunk_index = undecided_next; - else + } else if (s->answer.buf[0] == 'K') { + if (hunk_index) + hunk_index--; + else + err(s, _("No previous hunk")); + } else if (s->answer.buf[0] == 'J') { + if (hunk_index + 1 < s->hunk_nr) + hunk_index++; + else + err(s, _("No next hunk")); + } else if (s->answer.buf[0] == 'k') { + if (undecided_previous >= 0) + hunk_index = undecided_previous; + else + err(s, _("No previous hunk")); + } else if (s->answer.buf[0] == 'j') { + if (undecided_next >= 0) + hunk_index = undecided_next; + else + err(s, _("No next hunk")); + } else color_fprintf(stdout, s->s.help_color, _(help_patch_text)); } From e8b8506670cc757cc69d6fb6624dcef98f277ebd Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 15 Mar 2019 11:11:43 +0100 Subject: [PATCH 07/18] built-in add -p: support multi-file diffs For simplicity, the initial implementation in C handled only a single modified file. Now it handles an arbitrary number of files. Signed-off-by: Johannes Schindelin --- add-patch.c | 90 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 38 deletions(-) diff --git a/add-patch.c b/add-patch.c index e30bbc39fc..b50419fac3 100644 --- a/add-patch.c +++ b/add-patch.c @@ -29,9 +29,12 @@ struct add_p_state { /* parsed diff */ struct strbuf plain, colored; - struct hunk head; - struct hunk *hunk; - size_t hunk_nr, hunk_alloc; + struct file_diff { + struct hunk head; + struct hunk *hunk; + size_t hunk_nr, hunk_alloc; + } *file_diff; + size_t file_diff_nr; }; static void err(struct add_p_state *s, const char *fmt, ...) @@ -131,7 +134,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) struct strbuf *plain = &s->plain, *colored = NULL; struct child_process cp = CHILD_PROCESS_INIT; char *p, *pend, *colored_p = NULL, *colored_pend = NULL; - size_t i, color_arg_index; + size_t file_diff_alloc = 0, i, color_arg_index; + struct file_diff *file_diff = NULL; struct hunk *hunk = NULL; int res; @@ -171,7 +175,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } argv_array_clear(&args); - /* parse hunks */ + /* parse files and hunks */ p = plain->buf; pend = p + plain->len; while (p != pend) { @@ -180,17 +184,23 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) eol = pend; if (starts_with(p, "diff ")) { - if (p != plain->buf) - BUG("multi-file diff not yet handled"); - hunk = &s->head; + s->file_diff_nr++; + ALLOC_GROW(s->file_diff, s->file_diff_nr, + file_diff_alloc); + file_diff = s->file_diff + s->file_diff_nr - 1; + memset(file_diff, 0, sizeof(*file_diff)); + hunk = &file_diff->head; + hunk->start = p - plain->buf; + if (colored_p) + hunk->colored_start = colored_p - colored->buf; } else if (p == plain->buf) BUG("diff starts with unexpected line:\n" "%.*s\n", (int)(eol - p), p); else if (starts_with(p, "@@ ")) { - s->hunk_nr++; - ALLOC_GROW(s->hunk, s->hunk_nr, - s->hunk_alloc); - hunk = s->hunk + s->hunk_nr - 1; + file_diff->hunk_nr++; + ALLOC_GROW(file_diff->hunk, file_diff->hunk_nr, + file_diff->hunk_alloc); + hunk = file_diff->hunk + file_diff->hunk_nr - 1; memset(hunk, 0, sizeof(*hunk)); hunk->start = p - plain->buf; @@ -268,16 +278,17 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, hunk->end - hunk->start); } -static void reassemble_patch(struct add_p_state *s, struct strbuf *out) +static void reassemble_patch(struct add_p_state *s, + struct file_diff *file_diff, struct strbuf *out) { struct hunk *hunk; size_t i; ssize_t delta = 0; - render_hunk(s, &s->head, 0, 0, out); + render_hunk(s, &file_diff->head, 0, 0, out); - for (i = 0; i < s->hunk_nr; i++) { - hunk = s->hunk + i; + for (i = 0; i < file_diff->hunk_nr; i++) { + hunk = file_diff->hunk + i; if (hunk->use != USE_HUNK) delta += hunk->header.old_count - hunk->header.new_count; @@ -297,7 +308,8 @@ N_("y - stage this hunk\n" "K - leave this hunk undecided, see previous hunk\n" "? - print help\n"); -static int patch_update_file(struct add_p_state *s) +static int patch_update_file(struct add_p_state *s, + struct file_diff *file_diff) { size_t hunk_index = 0; ssize_t i, undecided_previous, undecided_next; @@ -306,27 +318,27 @@ static int patch_update_file(struct add_p_state *s) struct child_process cp = CHILD_PROCESS_INIT; int colored = !!s->colored.len; - if (!s->hunk_nr) + if (!file_diff->hunk_nr) return 0; strbuf_reset(&s->buf); - render_hunk(s, &s->head, 0, colored, &s->buf); + render_hunk(s, &file_diff->head, 0, colored, &s->buf); fputs(s->buf.buf, stdout); for (;;) { - if (hunk_index >= s->hunk_nr) + if (hunk_index >= file_diff->hunk_nr) hunk_index = 0; - hunk = s->hunk + hunk_index; + hunk = file_diff->hunk + hunk_index; undecided_previous = -1; for (i = hunk_index - 1; i >= 0; i--) - if (s->hunk[i].use == UNDECIDED_HUNK) { + if (file_diff->hunk[i].use == UNDECIDED_HUNK) { undecided_previous = i; break; } undecided_next = -1; - for (i = hunk_index + 1; i < s->hunk_nr; i++) - if (s->hunk[i].use == UNDECIDED_HUNK) { + for (i = hunk_index + 1; i < file_diff->hunk_nr; i++) + if (file_diff->hunk[i].use == UNDECIDED_HUNK) { undecided_next = i; break; } @@ -347,7 +359,7 @@ static int patch_update_file(struct add_p_state *s) strbuf_addstr(&s->buf, ",K"); if (undecided_next >= 0) strbuf_addstr(&s->buf, ",j"); - if (hunk_index + 1 < s->hunk_nr) + if (hunk_index + 1 < file_diff->hunk_nr) strbuf_addstr(&s->buf, ",J"); color_fprintf(stdout, s->s.prompt_color, _("Stage this hunk [y,n,a,d%s,?]? "), @@ -363,22 +375,22 @@ static int patch_update_file(struct add_p_state *s) if (ch == 'y') { hunk->use = USE_HUNK; soft_increment: - while (++hunk_index < s->hunk_nr && - s->hunk[hunk_index].use + while (++hunk_index < file_diff->hunk_nr && + file_diff->hunk[hunk_index].use != UNDECIDED_HUNK) ; /* continue looking */ } else if (ch == 'n') { hunk->use = SKIP_HUNK; goto soft_increment; } else if (ch == 'a') { - for (; hunk_index < s->hunk_nr; hunk_index++) { - hunk = s->hunk + hunk_index; + for (; hunk_index < file_diff->hunk_nr; hunk_index++) { + hunk = file_diff->hunk + hunk_index; if (hunk->use == UNDECIDED_HUNK) hunk->use = USE_HUNK; } } else if (ch == 'd') { - for (; hunk_index < s->hunk_nr; hunk_index++) { - hunk = s->hunk + hunk_index; + for (; hunk_index < file_diff->hunk_nr; hunk_index++) { + hunk = file_diff->hunk + hunk_index; if (hunk->use == UNDECIDED_HUNK) hunk->use = SKIP_HUNK; } @@ -388,7 +400,7 @@ soft_increment: else err(s, _("No previous hunk")); } else if (s->answer.buf[0] == 'J') { - if (hunk_index + 1 < s->hunk_nr) + if (hunk_index + 1 < file_diff->hunk_nr) hunk_index++; else err(s, _("No next hunk")); @@ -408,14 +420,14 @@ soft_increment: } /* Any hunk to be used? */ - for (i = 0; i < s->hunk_nr; i++) - if (s->hunk[i].use == USE_HUNK) + for (i = 0; i < file_diff->hunk_nr; i++) + if (file_diff->hunk[i].use == USE_HUNK) break; - if (i < s->hunk_nr) { + if (i < file_diff->hunk_nr) { /* At least one hunk selected: apply */ strbuf_reset(&s->buf); - reassemble_patch(s, &s->buf); + reassemble_patch(s, file_diff, &s->buf); setup_child_process(&cp, s, "apply", "--cached", NULL); if (pipe_command(&cp, s->buf.buf, s->buf.len, @@ -433,6 +445,7 @@ int run_add_p(struct repository *r, const struct pathspec *ps) struct add_p_state s = { { r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; + size_t i; if (init_add_i_state(r, &s.s)) return error("Could not read `add -i` config"); @@ -444,8 +457,9 @@ int run_add_p(struct repository *r, const struct pathspec *ps) return -1; } - if (s.hunk_nr) - patch_update_file(&s); + for (i = 0; i < s.file_diff_nr; i++) + if (patch_update_file(&s, s.file_diff + i)) + break; strbuf_release(&s.answer); strbuf_release(&s.buf); From 1d6fc90747d1ee7ac9f0198e938ec69512f86826 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 15 Mar 2019 17:32:44 +0100 Subject: [PATCH 08/18] built-in add -p: handle deleted empty files This addresses the same problem as 24ab81ae4d (add-interactive: handle deletion of empty files, 2009-10-27), although in a different way: we not only stick the "deleted file" line into its own pseudo hunk, but also the entire remainder (if any) of the same diff. That way, we do not have to play any funny games with regards to coalescing the diff after the user selected what (possibly pseudo-)hunks to stage. Signed-off-by: Johannes Schindelin --- add-patch.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/add-patch.c b/add-patch.c index b50419fac3..83ff2612de 100644 --- a/add-patch.c +++ b/add-patch.c @@ -33,6 +33,7 @@ struct add_p_state { struct hunk head; struct hunk *hunk; size_t hunk_nr, hunk_alloc; + unsigned deleted:1; } *file_diff; size_t file_diff_nr; }; @@ -180,6 +181,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) pend = p + plain->len; while (p != pend) { char *eol = memchr(p, '\n', pend - p); + const char *deleted = NULL; + if (!eol) eol = pend; @@ -196,7 +199,11 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } else if (p == plain->buf) BUG("diff starts with unexpected line:\n" "%.*s\n", (int)(eol - p), p); - else if (starts_with(p, "@@ ")) { + else if (file_diff->deleted) + ; /* keep the rest of the file in a single "hunk" */ + else if (starts_with(p, "@@ ") || + (hunk == &file_diff->head && + skip_prefix(p, "deleted file", &deleted))) { file_diff->hunk_nr++; ALLOC_GROW(file_diff->hunk, file_diff->hunk_nr, file_diff->hunk_alloc); @@ -207,7 +214,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) if (colored) hunk->colored_start = colored_p - colored->buf; - if (parse_hunk_header(s, hunk) < 0) + if (deleted) + file_diff->deleted = 1; + else if (parse_hunk_header(s, hunk) < 0) return -1; } From ff5c4b0c47191bede6b1192d43c5297e8625b4f0 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 15 Mar 2019 17:59:11 +0100 Subject: [PATCH 09/18] built-in app -p: allow selecting a mode change as a "hunk" This imitates the way the Perl version treats mode changes: it offers the mode change up for the user to decide, as if it was a diff hunk. In contrast to the Perl version, we make use of the fact that the mode line is the first hunk, and explicitly strip out that line from the diff header if that "hunk" was not selected to be applied, and skipping that hunk while coalescing the diff. The Perl version plays some kind of diff line lego instead. Signed-off-by: Johannes Schindelin --- add-patch.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 75 insertions(+), 5 deletions(-) diff --git a/add-patch.c b/add-patch.c index 83ff2612de..b85c713823 100644 --- a/add-patch.c +++ b/add-patch.c @@ -33,7 +33,7 @@ struct add_p_state { struct hunk head; struct hunk *hunk; size_t hunk_nr, hunk_alloc; - unsigned deleted:1; + unsigned deleted:1, mode_change:1; } *file_diff; size_t file_diff_nr; }; @@ -129,6 +129,14 @@ static int parse_hunk_header(struct add_p_state *s, struct hunk *hunk) return 0; } +static int is_octal(const char *p, size_t len) +{ + while (len--) + if (*p < '0' || *(p++) > '7') + return 0; + return 1; +} + static int parse_diff(struct add_p_state *s, const struct pathspec *ps) { struct argv_array args = ARGV_ARRAY_INIT; @@ -181,7 +189,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) pend = p + plain->len; while (p != pend) { char *eol = memchr(p, '\n', pend - p); - const char *deleted = NULL; + const char *deleted = NULL, *mode_change = NULL; if (!eol) eol = pend; @@ -218,8 +226,30 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) file_diff->deleted = 1; else if (parse_hunk_header(s, hunk) < 0) return -1; + } else if (hunk == &file_diff->head && + ((skip_prefix(p, "old mode ", &mode_change) || + skip_prefix(p, "new mode ", &mode_change)) && + is_octal(mode_change, eol - mode_change))) { + if (!file_diff->mode_change) { + if (file_diff->hunk_nr++) + BUG("mode change before first hunk"); + ALLOC_GROW(file_diff->hunk, file_diff->hunk_nr, + file_diff->hunk_alloc); + memset(file_diff->hunk, 0, sizeof(struct hunk)); + file_diff->hunk->start = p - plain->buf; + if (colored_p) + file_diff->hunk->colored_start = + colored_p - colored->buf; + file_diff->mode_change = 1; + } else if (file_diff->hunk_nr != 1) + BUG("mode change after first hunk?"); } + if (file_diff->deleted && file_diff->mode_change) + BUG("diff contains delete *and* a mode change?!?\n%.*s", + (int)(eol - (plain->buf + file_diff->head.start)), + plain->buf + file_diff->head.start); + p = eol == pend ? pend : eol + 1; hunk->end = p - plain->buf; @@ -233,6 +263,13 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) hunk->colored_end = colored_p - colored->buf; } + + if (mode_change) { + file_diff->hunk->end = hunk->end; + if (colored_p) + file_diff->hunk->colored_end = + hunk->colored_end; + } } return 0; @@ -287,6 +324,39 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, hunk->end - hunk->start); } +static void render_diff_header(struct add_p_state *s, + struct file_diff *file_diff, int colored, + struct strbuf *out) +{ + /* + * If there was a mode change, the first hunk is a pseudo hunk that + * corresponds to the mode line in the header. If the user did not want + * to stage that "hunk", we actually have to cut it out from the header. + */ + int skip_mode_change = + file_diff->mode_change && file_diff->hunk->use != USE_HUNK; + struct hunk *head = &file_diff->head, *first = file_diff->hunk; + + if (!skip_mode_change) { + render_hunk(s, head, 0, colored, out); + return; + } + + if (colored) { + const char *p = s->colored.buf; + + strbuf_add(out, p + head->colored_start, + first->colored_start - head->colored_start); + strbuf_add(out, p + first->colored_end, + head->colored_end - first->colored_end); + } else { + const char *p = s->plain.buf; + + strbuf_add(out, p + head->start, first->start - head->start); + strbuf_add(out, p + first->end, head->end - first->end); + } +} + static void reassemble_patch(struct add_p_state *s, struct file_diff *file_diff, struct strbuf *out) { @@ -294,9 +364,9 @@ static void reassemble_patch(struct add_p_state *s, size_t i; ssize_t delta = 0; - render_hunk(s, &file_diff->head, 0, 0, out); + render_diff_header(s, file_diff, 0, out); - for (i = 0; i < file_diff->hunk_nr; i++) { + for (i = file_diff->mode_change; i < file_diff->hunk_nr; i++) { hunk = file_diff->hunk + i; if (hunk->use != USE_HUNK) delta += hunk->header.old_count @@ -331,7 +401,7 @@ static int patch_update_file(struct add_p_state *s, return 0; strbuf_reset(&s->buf); - render_hunk(s, &file_diff->head, 0, colored, &s->buf); + render_diff_header(s, file_diff, colored, &s->buf); fputs(s->buf.buf, stdout); for (;;) { if (hunk_index >= file_diff->hunk_nr) From 0f95447627ef5d2f32e02d3355c38226ef2e690f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 21:33:55 +0100 Subject: [PATCH 10/18] built-in add -p: show different prompts for mode changes and deletions Just like the Perl version, we now helpfully ask the user whether they want to stage a mode change, or a deletion. Note that we define the prompts in an array, in preparation for a later patch that changes those prompts to yet different versions for `git reset -p`, `git stash -p` and `git checkout -p` (which all call the `git add -p` machinery to do the actual work). Signed-off-by: Johannes Schindelin --- add-patch.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/add-patch.c b/add-patch.c index b85c713823..020aa92c54 100644 --- a/add-patch.c +++ b/add-patch.c @@ -7,6 +7,16 @@ #include "color.h" #include "diff.h" +enum prompt_mode_type { + PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK +}; + +static const char *prompt_mode[] = { + N_("Stage mode change [y,n,a,d%s,?]? "), + N_("Stage deletion [y,n,a,d%s,?]? "), + N_("Stage this hunk [y,n,a,d%s,?]? ") +}; + struct hunk_header { unsigned long old_offset, old_count, new_offset, new_count; /* @@ -396,6 +406,7 @@ static int patch_update_file(struct add_p_state *s, char ch; struct child_process cp = CHILD_PROCESS_INIT; int colored = !!s->colored.len; + enum prompt_mode_type prompt_mode_type; if (!file_diff->hunk_nr) return 0; @@ -440,9 +451,16 @@ static int patch_update_file(struct add_p_state *s, strbuf_addstr(&s->buf, ",j"); if (hunk_index + 1 < file_diff->hunk_nr) strbuf_addstr(&s->buf, ",J"); + + if (file_diff->deleted) + prompt_mode_type = PROMPT_DELETION; + else if (file_diff->mode_change && !hunk_index) + prompt_mode_type = PROMPT_MODE_CHANGE; + else + prompt_mode_type = PROMPT_HUNK; + color_fprintf(stdout, s->s.prompt_color, - _("Stage this hunk [y,n,a,d%s,?]? "), - s->buf.buf); + _(prompt_mode[prompt_mode_type]), s->buf.buf); fflush(stdout); if (strbuf_getline(&s->answer, stdin) == EOF) break; From c071e664ace0380efb3ee874ecb4b94c32129579 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 17 Mar 2019 21:12:41 +0100 Subject: [PATCH 11/18] built-in add -p: implement the hunk splitting feature If this developer's workflow is any indication, then this is *the* most useful feature of Git's interactive `add `command. Note: once again, this is not a verbatim conversion from the Perl code to C: the `hunk_splittable()` function, for example, essentially did all the work of splitting the hunk, just to find out whether more than one hunk would have been the result (and then tossed that result into the trash). In C we instead count the number of resulting hunks (without actually doing the work of splitting, but just counting the transitions from non-context lines to context lines), and store that information with the hunk, and we do that *while* parsing the diff in the first place. Another deviation: the built-in `git add -p` was designed with a single strbuf holding the diff (and another one holding the colored diff, if that one was asked for) in mind, and hunks essentially store just the start and end offsets pointing into that strbuf. As a consequence, when we split hunks, we now use a special mode where the hunk header is generated dynamically, and only the rest of the hunk is stored using such start/end offsets. This way, we also avoid the frequent formatting/re-parsing of the hunk header of the Perl version. Signed-off-by: Johannes Schindelin --- add-patch.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 182 insertions(+), 2 deletions(-) diff --git a/add-patch.c b/add-patch.c index 020aa92c54..97d4ec9278 100644 --- a/add-patch.c +++ b/add-patch.c @@ -28,7 +28,7 @@ struct hunk_header { }; struct hunk { - size_t start, end, colored_start, colored_end; + size_t start, end, colored_start, colored_end, splittable_into; enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use; struct hunk_header header; }; @@ -152,7 +152,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) struct argv_array args = ARGV_ARRAY_INIT; struct strbuf *plain = &s->plain, *colored = NULL; struct child_process cp = CHILD_PROCESS_INIT; - char *p, *pend, *colored_p = NULL, *colored_pend = NULL; + char *p, *pend, *colored_p = NULL, *colored_pend = NULL, marker = '\0'; size_t file_diff_alloc = 0, i, color_arg_index; struct file_diff *file_diff = NULL; struct hunk *hunk = NULL; @@ -222,6 +222,13 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) else if (starts_with(p, "@@ ") || (hunk == &file_diff->head && skip_prefix(p, "deleted file", &deleted))) { + if (marker == '-' || marker == '+') + /* + * Should not happen; previous hunk did not end + * in a context line? Handle it anyway. + */ + hunk->splittable_into++; + file_diff->hunk_nr++; ALLOC_GROW(file_diff->hunk, file_diff->hunk_nr, file_diff->hunk_alloc); @@ -236,6 +243,12 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) file_diff->deleted = 1; else if (parse_hunk_header(s, hunk) < 0) return -1; + + /* + * Start counting into how many hunks this one can be + * split + */ + marker = *p; } else if (hunk == &file_diff->head && ((skip_prefix(p, "old mode ", &mode_change) || skip_prefix(p, "new mode ", &mode_change)) && @@ -260,6 +273,12 @@ 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 == ' ' || *p == '\\')) + hunk->splittable_into++; + if (marker) + marker = *p; + p = eol == pend ? pend : eol + 1; hunk->end = p - plain->buf; @@ -282,9 +301,25 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } } + if (marker == '-' || marker == '+') + /* + * Last hunk ended in non-context line (i.e. it appended lines + * to the file, so there are no trailing context lines). + */ + hunk->splittable_into++; + return 0; } +static size_t find_next_line(struct strbuf *sb, size_t offset) +{ + char *eol = memchr(sb->buf + offset, '\n', sb->len - offset); + + if (!eol) + return sb->len; + return eol - sb->buf + 1; +} + static void render_hunk(struct add_p_state *s, struct hunk *hunk, ssize_t delta, int colored, struct strbuf *out) { @@ -386,6 +421,139 @@ static void reassemble_patch(struct add_p_state *s, } } +static int split_hunk(struct add_p_state *s, struct file_diff *file_diff, + size_t hunk_index) +{ + int colored = !!s->colored.len, first = 1; + struct hunk *hunk = file_diff->hunk + hunk_index; + size_t splittable_into; + size_t end, colored_end, current, colored_current = 0, context_line_count; + struct hunk_header remaining, *header; + char marker, ch; + + if (hunk_index >= file_diff->hunk_nr) + BUG("invalid hunk index: %d (must be >= 0 and < %d)", + (int)hunk_index, (int)file_diff->hunk_nr); + + if (hunk->splittable_into < 2) + return 0; + splittable_into = hunk->splittable_into; + + end = hunk->end; + colored_end = hunk->colored_end; + + memcpy(&remaining, &hunk->header, sizeof(remaining)); + + file_diff->hunk_nr += splittable_into - 1; + ALLOC_GROW(file_diff->hunk, file_diff->hunk_nr, file_diff->hunk_alloc); + if (hunk_index + splittable_into < file_diff->hunk_nr) + memmove(file_diff->hunk + hunk_index + splittable_into, + file_diff->hunk + hunk_index + 1, + (file_diff->hunk_nr - hunk_index - splittable_into) + * sizeof(*hunk)); + hunk = file_diff->hunk + hunk_index; + hunk->splittable_into = 1; + memset(hunk + 1, 0, (splittable_into - 1) * sizeof(*hunk)); + + header = &hunk->header; + header->old_count = header->new_count = 0; + + current = hunk->start; + if (colored) + colored_current = hunk->colored_start; + marker = '\0'; + context_line_count = 0; + + while (splittable_into > 1) { + ch = s->plain.buf[current]; + if ((marker == '-' || marker == '+') && ch == ' ') { + first = 0; + hunk[1].start = current; + if (colored) + hunk[1].colored_start = colored_current; + context_line_count = 0; + } + + if (marker != ' ' || (ch != '-' && ch != '+')) { +next_hunk_line: + /* current hunk not done yet */ + if (ch == ' ') + context_line_count++; + else if (ch == '-') + header->old_count++; + else if (ch == '+') + header->new_count++; + else + BUG("unhandled diff marker: '%c'", ch); + marker = ch; + current = find_next_line(&s->plain, current); + if (colored) + colored_current = + find_next_line(&s->colored, + colored_current); + continue; + } + + if (first) { + if (header->old_count || header->new_count) + BUG("counts are off: %d/%d", + (int)header->old_count, + (int)header->new_count); + + header->old_count = context_line_count; + header->new_count = context_line_count; + context_line_count = 0; + first = 0; + goto next_hunk_line; + } + + remaining.old_offset += header->old_count; + remaining.old_count -= header->old_count; + remaining.new_offset += header->new_count; + remaining.new_count -= header->new_count; + + /* initialize next hunk header's offsets */ + hunk[1].header.old_offset = + header->old_offset + header->old_count; + hunk[1].header.new_offset = + header->new_offset + header->new_count; + + /* add one split hunk */ + header->old_count += context_line_count; + header->new_count += context_line_count; + + hunk->end = current; + if (colored) + hunk->colored_end = colored_current; + + hunk++; + hunk->splittable_into = 1; + hunk->use = hunk[-1].use; + header = &hunk->header; + + header->old_count = header->new_count = context_line_count; + context_line_count = 0; + + splittable_into--; + marker = ch; + } + + /* last hunk simply gets the rest */ + if (header->old_offset != remaining.old_offset) + BUG("miscounted old_offset: %lu != %lu", + header->old_offset, remaining.old_offset); + if (header->new_offset != remaining.new_offset) + BUG("miscounted new_offset: %lu != %lu", + header->new_offset, remaining.new_offset); + header->old_count = remaining.old_count; + header->new_count = remaining.new_count; + hunk->end = end; + if (colored) + hunk->colored_end = colored_end; + + return 0; +} + static const char help_patch_text[] = N_("y - stage this hunk\n" "n - do not stage this hunk\n" @@ -395,6 +563,7 @@ N_("y - stage this hunk\n" "J - leave this hunk undecided, see next hunk\n" "k - leave this hunk undecided, see previous undecided hunk\n" "K - leave this hunk undecided, see previous hunk\n" + "s - split the current hunk into smaller hunks\n" "? - print help\n"); static int patch_update_file(struct add_p_state *s, @@ -451,6 +620,8 @@ static int patch_update_file(struct add_p_state *s, strbuf_addstr(&s->buf, ",j"); if (hunk_index + 1 < file_diff->hunk_nr) strbuf_addstr(&s->buf, ",J"); + if (hunk->splittable_into > 1) + strbuf_addstr(&s->buf, ",s"); if (file_diff->deleted) prompt_mode_type = PROMPT_DELETION; @@ -511,6 +682,15 @@ soft_increment: hunk_index = undecided_next; else err(s, _("No next hunk")); + } else if (s->answer.buf[0] == 's') { + size_t splittable_into = hunk->splittable_into; + if (splittable_into < 2) + err(s, _("Sorry, cannot split this hunk")); + else if (!split_hunk(s, file_diff, + hunk - file_diff->hunk)) + color_fprintf_ln(stdout, s->s.header_color, + _("Split into %d hunks."), + (int)splittable_into); } else color_fprintf(stdout, s->s.help_color, _(help_patch_text)); From 53e0349e20261ec062e37c06cd1f95e701ea83e0 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 19 Mar 2019 20:50:14 +0100 Subject: [PATCH 12/18] built-in add -p: coalesce hunks after splitting them This is considered "the right thing to do", according to 933e44d3a0 ("add -p": work-around an old laziness that does not coalesce hunks, 2011-04-06). Note: we cannot simply modify the hunks while merging them; Once we implement hunk editing, we will call `reassemble_patch()` whenever a hunk is edited, therefore we must not modify the hunks (because the user might e.g. hit `K` and change their mind whether to stage the previous hunk). Signed-off-by: Johannes Schindelin --- add-patch.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/add-patch.c b/add-patch.c index 97d4ec9278..caa1d76f58 100644 --- a/add-patch.c +++ b/add-patch.c @@ -402,6 +402,48 @@ static void render_diff_header(struct add_p_state *s, } } +/* Coalesce hunks again that were split */ +static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff, + size_t *hunk_index, struct hunk *temp) +{ + size_t i = *hunk_index; + struct hunk *hunk = file_diff->hunk + i; + struct hunk_header *header = &temp->header, *next; + + if (hunk->use != USE_HUNK) + return 0; + + memcpy(temp, hunk, sizeof(*temp)); + /* We simply skip the colored part (if any) when merging hunks */ + temp->colored_start = temp->colored_end = 0; + + for (; i + 1 < file_diff->hunk_nr; i++) { + hunk++; + next = &hunk->header; + + if (hunk->use != USE_HUNK || + header->new_offset >= next->new_offset || + header->new_offset + header->new_count < next->new_offset || + temp->start >= hunk->start || + temp->end < hunk->start) + break; + + temp->end = hunk->end; + temp->colored_end = hunk->colored_end; + + header->old_count = next->old_offset + next->old_count + - header->old_offset; + header->new_count = next->new_offset + next->new_count + - header->new_offset; + } + + if (i == *hunk_index) + return 0; + + *hunk_index = i; + return 1; +} + static void reassemble_patch(struct add_p_state *s, struct file_diff *file_diff, struct strbuf *out) { @@ -412,12 +454,19 @@ static void reassemble_patch(struct add_p_state *s, render_diff_header(s, file_diff, 0, out); for (i = file_diff->mode_change; i < file_diff->hunk_nr; i++) { + struct hunk temp = { 0 }; + hunk = file_diff->hunk + i; if (hunk->use != USE_HUNK) delta += hunk->header.old_count - hunk->header.new_count; - else + else { + /* merge overlapping hunks into a temporary hunk */ + if (merge_hunks(s, file_diff, &i, &temp)) + hunk = &temp; + render_hunk(s, hunk, delta, 0, out); + } } } From a9be4d21d9e3e02736fa66ca8a555c8230574951 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 19 Mar 2019 22:27:51 +0100 Subject: [PATCH 13/18] built-in add -p: implement hunk editing Just like `git add --edit` allows the user to edit the diff before it is being applied to the index, this feature allows the user to edit the diff *hunk*. Naturally, it gets a bit more complicated here because the result has to play well with the remaining hunks of the overall diff. Therefore, we have to do a loop in which we let the user edit the hunk, then test whether the result would work, and if not, drop the edits and let the user decide whether to try editing the hunk again. Note: in contrast to the Perl version, we use the same diff "coalescing" (i.e. merging overlapping hunks into a single one) also for the check after editing, and we introduce a new flag for that purpose that asks the `reassemble_patch()` function to pretend that all hunks were selected for use. This allows us to continue to run `git apply` *without* the `--allow-overlap` option (unlike the Perl version), and it also fixes two known breakages in `t3701-add-interactive.sh` (which we cannot mark as resolved so far because the Perl script version is still the default and continues to have those breakages). Signed-off-by: Johannes Schindelin --- add-interactive.c | 7 + add-interactive.h | 3 + add-patch.c | 351 +++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 344 insertions(+), 17 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 5020d09732..2e262a7cba 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -49,6 +49,13 @@ int init_add_i_state(struct repository *r, struct add_i_state *s) strlcpy(s->fraginfo_color, diff_get_color(s->use_color, DIFF_FRAGINFO), COLOR_MAXLEN); + strlcpy(s->context_color, + diff_get_color(s->use_color, DIFF_CONTEXT), COLOR_MAXLEN); + strlcpy(s->file_old_color, + diff_get_color(s->use_color, DIFF_FILE_OLD), COLOR_MAXLEN); + strlcpy(s->file_new_color, + diff_get_color(s->use_color, DIFF_FILE_NEW), COLOR_MAXLEN); + return 0; } diff --git a/add-interactive.h b/add-interactive.h index eb024066ba..cc16b9436b 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -12,6 +12,9 @@ struct add_i_state { char error_color[COLOR_MAXLEN]; char reset_color[COLOR_MAXLEN]; char fraginfo_color[COLOR_MAXLEN]; + char context_color[COLOR_MAXLEN]; + char file_old_color[COLOR_MAXLEN]; + char file_new_color[COLOR_MAXLEN]; }; int init_add_i_state(struct repository *r, struct add_i_state *s); diff --git a/add-patch.c b/add-patch.c index caa1d76f58..440a1b3940 100644 --- a/add-patch.c +++ b/add-patch.c @@ -29,6 +29,7 @@ struct hunk_header { struct hunk { size_t start, end, colored_start, colored_end, splittable_into; + ssize_t delta; enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use; struct hunk_header header; }; @@ -404,13 +405,13 @@ static void render_diff_header(struct add_p_state *s, /* Coalesce hunks again that were split */ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff, - size_t *hunk_index, struct hunk *temp) + size_t *hunk_index, int use_all, struct hunk *temp) { - size_t i = *hunk_index; + size_t i = *hunk_index, delta; struct hunk *hunk = file_diff->hunk + i; struct hunk_header *header = &temp->header, *next; - if (hunk->use != USE_HUNK) + if (!use_all && hunk->use != USE_HUNK) return 0; memcpy(temp, hunk, sizeof(*temp)); @@ -421,20 +422,94 @@ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff, hunk++; next = &hunk->header; - if (hunk->use != USE_HUNK || - header->new_offset >= next->new_offset || - header->new_offset + header->new_count < next->new_offset || - temp->start >= hunk->start || - temp->end < hunk->start) + if ((!use_all && hunk->use != USE_HUNK) || + header->new_offset >= next->new_offset + temp->delta || + header->new_offset + header->new_count + < next->new_offset + temp->delta) break; - temp->end = hunk->end; - temp->colored_end = hunk->colored_end; + if (temp->start < hunk->start && temp->end > hunk->start) { + temp->end = hunk->end; + temp->colored_end = hunk->colored_end; + delta = 0; + } else { + const char *plain = s->plain.buf; + size_t overlapping_line_count = header->new_offset + + header->new_count - temp->delta + - next->new_offset; + size_t overlap_end = hunk->start; + size_t overlap_start = overlap_end; + size_t overlap_next, len, i; + + /* + * One of the hunks was edited; let's ensure that at + * least the last context line of the first hunk + * overlaps with the corresponding line of the second + * hunk, and then merge. + */ + + for (i = 0; i < overlapping_line_count; i++) { + overlap_next = find_next_line(&s->plain, + overlap_end); + + if (overlap_next > hunk->end) + BUG("failed to find %d context lines " + "in:\n%.*s", + (int)overlapping_line_count, + (int)(hunk->end - hunk->start), + plain + hunk->start); + + if (plain[overlap_end] != ' ') + return error(_("expected context line " + "#%d in\n%.*s"), + (int)(i + 1), + (int)(hunk->end + - hunk->start), + plain + hunk->start); + + overlap_start = overlap_end; + overlap_end = overlap_next; + } + len = overlap_end - overlap_start; + + if (len > temp->end - temp->start || + memcmp(plain + temp->end - len, + plain + overlap_start, len)) + return error(_("hunks do not overlap:\n%.*s\n" + "\tdoes not end with:\n%.*s"), + (int)(temp->end - temp->start), + plain + temp->start, + (int)len, plain + overlap_start); + + /* + * Since the start-end ranges are not adjacent, we + * cannot simply take the union of the ranges. To + * address that, we temporarily append the union of the + * lines to the `plain` strbuf. + */ + if (temp->end != s->plain.len) { + size_t start = s->plain.len; + + strbuf_add(&s->plain, plain + temp->start, + temp->end - temp->start); + plain = s->plain.buf; + temp->start = start; + temp->end = s->plain.len; + } + + strbuf_add(&s->plain, + plain + overlap_end, + hunk->end - overlap_end); + temp->end = s->plain.len; + temp->splittable_into += hunk->splittable_into; + delta = temp->delta; + temp->delta += hunk->delta; + } header->old_count = next->old_offset + next->old_count - header->old_offset; - header->new_count = next->new_offset + next->new_count - - header->new_offset; + header->new_count = next->new_offset + delta + + next->new_count - header->new_offset; } if (i == *hunk_index) @@ -445,10 +520,11 @@ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff, } static void reassemble_patch(struct add_p_state *s, - struct file_diff *file_diff, struct strbuf *out) + struct file_diff *file_diff, int use_all, + struct strbuf *out) { struct hunk *hunk; - size_t i; + size_t save_len = s->plain.len, i; ssize_t delta = 0; render_diff_header(s, file_diff, 0, out); @@ -457,15 +533,24 @@ static void reassemble_patch(struct add_p_state *s, struct hunk temp = { 0 }; hunk = file_diff->hunk + i; - if (hunk->use != USE_HUNK) + if (!use_all && hunk->use != USE_HUNK) delta += hunk->header.old_count - hunk->header.new_count; else { /* merge overlapping hunks into a temporary hunk */ - if (merge_hunks(s, file_diff, &i, &temp)) + if (merge_hunks(s, file_diff, &i, use_all, &temp)) hunk = &temp; render_hunk(s, hunk, delta, 0, out); + + /* + * In case `merge_hunks()` used `plain` as a scratch + * pad (this happens when an edited hunk had to be + * coalesced with another hunk). + */ + strbuf_setlen(&s->plain, save_len); + + delta += hunk->delta; } } } @@ -603,6 +688,227 @@ next_hunk_line: return 0; } +static void recolor_hunk(struct add_p_state *s, struct hunk *hunk) +{ + const char *plain = s->plain.buf; + size_t current, eol, next; + + if (!s->colored.len) + return; + + hunk->colored_start = s->colored.len; + for (current = hunk->start; current < hunk->end; ) { + for (eol = current; eol < hunk->end; eol++) + if (plain[eol] == '\n') + break; + next = eol + (eol < hunk->end); + if (eol > current && plain[eol - 1] == '\r') + eol--; + + strbuf_addstr(&s->colored, + plain[current] == '-' ? + s->s.file_old_color : + plain[current] == '+' ? + s->s.file_new_color : + s->s.context_color); + strbuf_add(&s->colored, plain + current, eol - current); + strbuf_addstr(&s->colored, GIT_COLOR_RESET); + if (next > eol) + strbuf_add(&s->colored, plain + eol, next - eol); + current = next; + } + hunk->colored_end = s->colored.len; +} + +static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk) +{ + char *path = xstrdup(git_path("addp-hunk-edit.diff")); + int fd = xopen(path, O_WRONLY | O_CREAT | O_TRUNC, 0666); + struct strbuf buf = STRBUF_INIT; + size_t i, j; + int res, copy; + + if (fd < 0) { + res = error_errno(_("could not open '%s' for writing"), path); + goto edit_hunk_manually_finish; + } + + strbuf_commented_addf(&buf, _("Manual hunk edit mode -- see bottom for " + "a quick guide.\n")); + render_hunk(s, hunk, 0, 0, &buf); + strbuf_commented_addf(&buf, + _("---\n" + "To remove '%c' lines, make them ' ' lines " + "(context).\n" + "To remove '%c' lines, delete them.\n" + "Lines starting with %c will be removed.\n"), + '-', '+', comment_line_char); + strbuf_commented_addf(&buf, + _("If the patch applies cleanly, the edited hunk " + "will immediately be\n" + "marked for staging.\n")); + /* + * TRANSLATORS: 'it' refers to the patch mentioned in the previous + * messages. + */ + strbuf_commented_addf(&buf, + _("If it does not apply cleanly, you will be " + "given an opportunity to\n" + "edit again. If all lines of the hunk are " + "removed, then the edit is\n" + "aborted and the hunk is left unchanged.\n")); + if (write_in_full(fd, buf.buf, buf.len) < 0) { + res = error_errno(_("could not write to '%s'"), path); + goto edit_hunk_manually_finish; + } + + res = close(fd); + fd = -1; + if (res < 0) + goto edit_hunk_manually_finish; + + hunk->start = s->plain.len; + if (launch_editor(path, &s->plain, NULL) < 0) { + res = error_errno(_("could not edit '%s'"), path); + goto edit_hunk_manually_finish; + } + unlink(path); + + /* strip out commented lines */ + copy = s->plain.buf[hunk->start] != comment_line_char; + for (i = j = hunk->start; i < s->plain.len; ) { + if (copy) + s->plain.buf[j++] = s->plain.buf[i]; + if (s->plain.buf[i++] == '\n') + copy = s->plain.buf[i] != comment_line_char; + } + + if (j == hunk->start) + /* User aborted by deleting everything */ + goto edit_hunk_manually_finish; + + res = 1; + strbuf_setlen(&s->plain, j); + hunk->end = j; + recolor_hunk(s, hunk); + if (s->plain.buf[hunk->start] == '@' && + /* If the hunk header was deleted, simply use the original one. */ + parse_hunk_header(s, hunk) < 0) + res = -1; + +edit_hunk_manually_finish: + if (fd >= 0) + close(fd); + free(path); + strbuf_release(&buf); + + return res; +} + +static ssize_t recount_edited_hunk(struct add_p_state *s, struct hunk *hunk, + size_t orig_old_count, size_t orig_new_count) +{ + struct hunk_header *header = &hunk->header; + size_t i; + + header->old_count = header->new_count = 0; + for (i = hunk->start; i < hunk->end; ) { + switch (s->plain.buf[i]) { + case '-': + header->old_count++; + break; + case '+': + header->new_count++; + break; + case ' ': case '\r': case '\n': + header->old_count++; + header->new_count++; + break; + } + + i = find_next_line(&s->plain, i); + } + + return orig_old_count - orig_new_count + - header->old_count + header->new_count; +} + +static int run_apply_check(struct add_p_state *s, + struct file_diff *file_diff) +{ + struct child_process cp = CHILD_PROCESS_INIT; + + strbuf_reset(&s->buf); + reassemble_patch(s, file_diff, 1, &s->buf); + + setup_child_process(&cp, s, + "apply", "--cached", "--check", NULL); + if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) + return error(_("'git apply --cached' failed")); + + return 0; +} + +static int prompt_yesno(struct add_p_state *s, const char *prompt) +{ + for (;;) { + color_fprintf(stdout, s->s.prompt_color, "%s", _(prompt)); + fflush(stdout); + if (strbuf_getline(&s->answer, stdin) == EOF) + return -1; + strbuf_trim_trailing_newline(&s->answer); + switch (tolower(s->answer.buf[0])) { + case 'n': return 0; + case 'y': return 1; + } + } +} + +static int edit_hunk_loop(struct add_p_state *s, + struct file_diff *file_diff, struct hunk *hunk) +{ + size_t plain_len = s->plain.len, colored_len = s->colored.len; + struct hunk backup; + + memcpy(&backup, hunk, sizeof(backup)); + + for (;;) { + int res = edit_hunk_manually(s, hunk); + if (res == 0) { + /* abandonded */ + memcpy(hunk, &backup, sizeof(backup)); + return -1; + } + + if (res > 0) { + hunk->delta += + recount_edited_hunk(s, hunk, + backup.header.old_count, + backup.header.new_count); + if (!run_apply_check(s, file_diff)) + return 0; + } + + /* Drop edits (they were appended to s->plain) */ + strbuf_setlen(&s->plain, plain_len); + strbuf_setlen(&s->colored, colored_len); + memcpy(hunk, &backup, sizeof(backup)); + + /* + * TRANSLATORS: do not translate [y/n] + * The program will only accept that input at this point. + * Consider translating (saying "no" discards!) as + * (saying "n" for "no" discards!) if the translation + * of the word "no" does not start with n. + */ + res = prompt_yesno(s, _("Your edited hunk does not apply. " + "Edit again (saying \"no\" discards!) " + "[y/n]? ")); + if (res < 1) + return -1; + } +} + static const char help_patch_text[] = N_("y - stage this hunk\n" "n - do not stage this hunk\n" @@ -613,6 +919,7 @@ N_("y - stage this hunk\n" "k - leave this hunk undecided, see previous undecided hunk\n" "K - leave this hunk undecided, see previous hunk\n" "s - split the current hunk into smaller hunks\n" + "e - manually edit the current hunk\n" "? - print help\n"); static int patch_update_file(struct add_p_state *s, @@ -671,6 +978,9 @@ static int patch_update_file(struct add_p_state *s, strbuf_addstr(&s->buf, ",J"); if (hunk->splittable_into > 1) strbuf_addstr(&s->buf, ",s"); + if (hunk_index + 1 > file_diff->mode_change && + !file_diff->deleted) + strbuf_addstr(&s->buf, ",e"); if (file_diff->deleted) prompt_mode_type = PROMPT_DELETION; @@ -740,6 +1050,13 @@ soft_increment: color_fprintf_ln(stdout, s->s.header_color, _("Split into %d hunks."), (int)splittable_into); + } else if (s->answer.buf[0] == 'e') { + if (hunk_index + 1 == file_diff->mode_change) + err(s, _("Sorry, cannot edit this hunk")); + else if (edit_hunk_loop(s, file_diff, hunk) >= 0) { + hunk->use = USE_HUNK; + goto soft_increment; + } } else color_fprintf(stdout, s->s.help_color, _(help_patch_text)); @@ -753,7 +1070,7 @@ soft_increment: if (i < file_diff->hunk_nr) { /* At least one hunk selected: apply */ strbuf_reset(&s->buf); - reassemble_patch(s, file_diff, &s->buf); + reassemble_patch(s, file_diff, 0, &s->buf); setup_child_process(&cp, s, "apply", "--cached", NULL); if (pipe_command(&cp, s->buf.buf, s->buf.len, From 8d886ad90f5508798be25468fdaa66562cb28118 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 21 Mar 2019 23:50:53 +0100 Subject: [PATCH 14/18] built-in add -p: implement the 'g' ("goto") command With this patch, it is now possible to see a summary of the available hunks and to navigate between them (by number). A test is added to verify that this behavior matches the one of the Perl version of `git add -p`. Signed-off-by: Johannes Schindelin --- add-patch.c | 86 ++++++++++++++++++++++++++++++++++++++ t/t3701-add-interactive.sh | 16 +++++++ 2 files changed, 102 insertions(+) diff --git a/add-patch.c b/add-patch.c index 440a1b3940..d523f8cb17 100644 --- a/add-patch.c +++ b/add-patch.c @@ -909,6 +909,54 @@ static int edit_hunk_loop(struct add_p_state *s, } } +#define SUMMARY_HEADER_WIDTH 20 +#define SUMMARY_LINE_WIDTH 80 +static void summarize_hunk(struct add_p_state *s, struct hunk *hunk, + struct strbuf *out) +{ + struct hunk_header *header = &hunk->header; + struct strbuf *plain = &s->plain; + size_t len = out->len, i; + + strbuf_addf(out, " -%lu,%lu +%lu,%lu ", + header->old_offset, header->old_count, + header->new_offset, header->new_count); + if (out->len - len < SUMMARY_HEADER_WIDTH) + strbuf_addchars(out, ' ', + SUMMARY_HEADER_WIDTH + len - out->len); + for (i = hunk->start; i < hunk->end; i = find_next_line(plain, i)) + if (plain->buf[i] != ' ') + break; + if (i < hunk->end) + strbuf_add(out, plain->buf + i, find_next_line(plain, i) - i); + if (out->len - len > SUMMARY_LINE_WIDTH) + strbuf_setlen(out, len + SUMMARY_LINE_WIDTH); + strbuf_complete_line(out); +} + +#define DISPLAY_HUNKS_LINES 20 +static size_t display_hunks(struct add_p_state *s, + struct file_diff *file_diff, size_t start_index) +{ + size_t end_index = start_index + DISPLAY_HUNKS_LINES; + + if (end_index > file_diff->hunk_nr) + end_index = file_diff->hunk_nr; + + while (start_index < end_index) { + struct hunk *hunk = file_diff->hunk + start_index++; + + strbuf_reset(&s->buf); + strbuf_addf(&s->buf, "%c%2d: ", hunk->use == USE_HUNK ? '+' + : hunk->use == SKIP_HUNK ? '-' : ' ', + (int)start_index); + summarize_hunk(s, hunk, &s->buf); + fputs(s->buf.buf, stdout); + } + + return end_index; +} + static const char help_patch_text[] = N_("y - stage this hunk\n" "n - do not stage this hunk\n" @@ -918,6 +966,7 @@ N_("y - stage this hunk\n" "J - leave this hunk undecided, see next hunk\n" "k - leave this hunk undecided, see previous undecided hunk\n" "K - leave this hunk undecided, see previous hunk\n" + "g - select a hunk to go to\n" "s - split the current hunk into smaller hunks\n" "e - manually edit the current hunk\n" "? - print help\n"); @@ -976,6 +1025,8 @@ static int patch_update_file(struct add_p_state *s, strbuf_addstr(&s->buf, ",j"); if (hunk_index + 1 < file_diff->hunk_nr) strbuf_addstr(&s->buf, ",J"); + if (file_diff->hunk_nr > 1) + strbuf_addstr(&s->buf, ",g"); if (hunk->splittable_into > 1) strbuf_addstr(&s->buf, ",s"); if (hunk_index + 1 > file_diff->mode_change && @@ -1041,6 +1092,41 @@ soft_increment: hunk_index = undecided_next; else err(s, _("No next hunk")); + } else if (s->answer.buf[0] == 'g') { + char *pend; + unsigned long response; + + if (file_diff->hunk_nr < 2) { + err(s, _("No other hunks to goto")); + continue; + } + strbuf_remove(&s->answer, 0, 1); + strbuf_trim(&s->answer); + i = hunk_index > 10 ? hunk_index - 10 : 0; + while (s->answer.len == 0) { + i = display_hunks(s, file_diff, i); + printf("%s", i < file_diff->hunk_nr ? + _("go to which hunk ( to see " + "more)? ") : _("go to which hunk? ")); + fflush(stdout); + if (strbuf_getline(&s->answer, + stdin) == EOF) + break; + strbuf_trim_trailing_newline(&s->answer); + } + + strbuf_trim(&s->answer); + response = strtoul(s->answer.buf, &pend, 10); + if (*pend || pend == s->answer.buf) + err(s, _("Invalid number: '%s'"), + s->answer.buf); + else if (0 < response && response <= file_diff->hunk_nr) + hunk_index = response - 1; + else + err(s, Q_("Sorry, only %d hunk available.", + "Sorry, only %d hunks available.", + file_diff->hunk_nr), + (int)file_diff->hunk_nr); } else if (s->answer.buf[0] == 's') { size_t splittable_into = hunk->splittable_into; if (splittable_into < 2) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index ab218b71e5..dcdde19343 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -413,6 +413,22 @@ test_expect_success 'split hunk setup' ' test_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test ' +test_expect_success 'goto hunk' ' + test_when_finished "git reset" && + tr _ " " >expect <<-EOF && + Stage this hunk [y,n,q,a,d,K,g,/,e,?]? + 1: -1,2 +1,3 +15 + _ 2: -2,4 +3,8 +21 + go to which hunk? @@ -1,2 +1,3 @@ + _10 + +15 + _20 + Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?_ + EOF + test_write_lines s y g 1 | git add -p >actual && + tail -n 7 actual.trimmed && + test_cmp expect actual.trimmed +' + test_expect_success 'split hunk "add -p (edit)"' ' # Split, say Edit and do nothing. Then: # From 1b3486ddaff9584e645d6259c603a27101337834 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 21 Mar 2019 23:50:53 +0100 Subject: [PATCH 15/18] built-in add -p: implement the '/' ("search regex") command This patch implements the hunk searching feature in the C version of `git add -p`. A test is added to verify that this behavior matches the one of the Perl version of `git add -p`. Note that this involves a change of behavior: the Perl version uses (of course) the Perl flavor of regular expressions, while this patch uses the regcomp()/regexec(), i.e. POSIX extended regular expressions. In practice, this behavior change is unlikely to matter. Signed-off-by: Johannes Schindelin --- add-patch.c | 50 +++++++++++++++++++++++++++++++++++++- t/t3701-add-interactive.sh | 14 +++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/add-patch.c b/add-patch.c index d523f8cb17..f1ab97dc9c 100644 --- a/add-patch.c +++ b/add-patch.c @@ -967,6 +967,7 @@ N_("y - stage this hunk\n" "k - leave this hunk undecided, see previous undecided hunk\n" "K - leave this hunk undecided, see previous hunk\n" "g - select a hunk to go to\n" + "/ - search for a hunk matching the given regex\n" "s - split the current hunk into smaller hunks\n" "e - manually edit the current hunk\n" "? - print help\n"); @@ -1026,7 +1027,7 @@ static int patch_update_file(struct add_p_state *s, if (hunk_index + 1 < file_diff->hunk_nr) strbuf_addstr(&s->buf, ",J"); if (file_diff->hunk_nr > 1) - strbuf_addstr(&s->buf, ",g"); + strbuf_addstr(&s->buf, ",g,/"); if (hunk->splittable_into > 1) strbuf_addstr(&s->buf, ",s"); if (hunk_index + 1 > file_diff->mode_change && @@ -1127,6 +1128,53 @@ soft_increment: "Sorry, only %d hunks available.", file_diff->hunk_nr), (int)file_diff->hunk_nr); + } else if (s->answer.buf[0] == '/') { + regex_t regex; + int ret; + + if (file_diff->hunk_nr < 2) { + err(s, _("No other hunks to search")); + continue; + } + strbuf_remove(&s->answer, 0, 1); + strbuf_trim_trailing_newline(&s->answer); + if (s->answer.len == 0) { + printf("%s", _("search for regex? ")); + fflush(stdout); + if (strbuf_getline(&s->answer, + stdin) == EOF) + break; + strbuf_trim_trailing_newline(&s->answer); + if (s->answer.len == 0) + continue; + } + ret = regcomp(®ex, s->answer.buf, + REG_EXTENDED | REG_NOSUB | REG_NEWLINE); + if (ret) { + char errbuf[1024]; + + regerror(ret, ®ex, errbuf, sizeof(errbuf)); + err(s, _("Malformed search regexp %s: %s"), + s->answer.buf, errbuf); + continue; + } + i = hunk_index; + for (;;) { + /* render the hunk into a scratch buffer */ + render_hunk(s, file_diff->hunk + i, 0, 0, + &s->buf); + if (regexec(®ex, s->buf.buf, 0, NULL, 0) + != REG_NOMATCH) + break; + i++; + if (i == file_diff->hunk_nr) + i = 0; + if (i != hunk_index) + continue; + err(s, _("No hunk matches the given pattern")); + break; + } + hunk_index = i; } else if (s->answer.buf[0] == 's') { size_t splittable_into = hunk->splittable_into; if (splittable_into < 2) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index dcdde19343..7497f10959 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -429,6 +429,20 @@ test_expect_success 'goto hunk' ' test_cmp expect actual.trimmed ' +test_expect_success 'navigate to hunk via regex' ' + test_when_finished "git reset" && + tr _ " " >expect <<-EOF && + Stage this hunk [y,n,q,a,d,K,g,/,e,?]? @@ -1,2 +1,3 @@ + _10 + +15 + _20 + Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?_ + EOF + test_write_lines s y /1,2 | git add -p >actual && + tail -n 5 actual.trimmed && + test_cmp expect actual.trimmed +' + test_expect_success 'split hunk "add -p (edit)"' ' # Split, say Edit and do nothing. Then: # From e3ef06c0db1cfbdcd478546b0d5534dd96800771 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 22 Mar 2019 01:24:47 +0100 Subject: [PATCH 16/18] built-in add -p: implement the 'q' ("quit") command This command is actually very similar to the 'd' ("do not stage this hunk or any of the later hunks in the file") command: it just does something on top, namely leave the loop and return a value indicating that we're quittin'. Signed-off-by: Johannes Schindelin --- add-patch.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/add-patch.c b/add-patch.c index f1ab97dc9c..cb62d01f5f 100644 --- a/add-patch.c +++ b/add-patch.c @@ -12,9 +12,9 @@ enum prompt_mode_type { }; static const char *prompt_mode[] = { - N_("Stage mode change [y,n,a,d%s,?]? "), - N_("Stage deletion [y,n,a,d%s,?]? "), - N_("Stage this hunk [y,n,a,d%s,?]? ") + N_("Stage mode change [y,n,a,q,d%s,?]? "), + N_("Stage deletion [y,n,a,q,d%s,?]? "), + N_("Stage this hunk [y,n,a,q,d%s,?]? ") }; struct hunk_header { @@ -960,6 +960,7 @@ static size_t display_hunks(struct add_p_state *s, static const char help_patch_text[] = N_("y - stage this hunk\n" "n - do not stage this hunk\n" + "q - quit; do not stage this hunk or any of the remaining ones\n" "a - stage this and all the remaining hunks\n" "d - do not stage this hunk nor any of the remaining hunks\n" "j - leave this hunk undecided, see next undecided hunk\n" @@ -980,7 +981,7 @@ static int patch_update_file(struct add_p_state *s, struct hunk *hunk; char ch; struct child_process cp = CHILD_PROCESS_INIT; - int colored = !!s->colored.len; + int colored = !!s->colored.len, quit = 0; enum prompt_mode_type prompt_mode_type; if (!file_diff->hunk_nr) @@ -1067,12 +1068,16 @@ soft_increment: if (hunk->use == UNDECIDED_HUNK) hunk->use = USE_HUNK; } - } else if (ch == 'd') { + } else if (ch == 'd' || ch == 'q') { for (; hunk_index < file_diff->hunk_nr; hunk_index++) { hunk = file_diff->hunk + hunk_index; if (hunk->use == UNDECIDED_HUNK) hunk->use = SKIP_HUNK; } + if (ch == 'q') { + quit = 1; + break; + } } else if (s->answer.buf[0] == 'K') { if (hunk_index) hunk_index--; @@ -1214,7 +1219,7 @@ soft_increment: } putchar('\n'); - return 0; + return quit; } int run_add_p(struct repository *r, const struct pathspec *ps) From de20820acd30beb8996a55a2bfefdf183dcfb1bb Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 24 Mar 2019 23:26:58 +0100 Subject: [PATCH 17/18] built-in add -p: only show the applicable parts of the help text When displaying the only hunk in a file's diff, the prompt already excludes the commands to navigate to the previous/next hunk. Let's also let the `?` command show only the help lines corresponding to the commands that are displayed in the prompt. Signed-off-by: Johannes Schindelin --- add-patch.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/add-patch.c b/add-patch.c index cb62d01f5f..fecef93909 100644 --- a/add-patch.c +++ b/add-patch.c @@ -962,8 +962,10 @@ N_("y - stage this hunk\n" "n - do not stage this hunk\n" "q - quit; do not stage this hunk or any of the remaining ones\n" "a - stage this and all the remaining hunks\n" - "d - do not stage this hunk nor any of the remaining hunks\n" - "j - leave this hunk undecided, see next undecided hunk\n" + "d - do not stage this hunk nor any of the remaining hunks\n"); + +static const char help_patch_remainder[] = +N_("j - leave this hunk undecided, see next undecided hunk\n" "J - leave this hunk undecided, see next hunk\n" "k - leave this hunk undecided, see previous undecided hunk\n" "K - leave this hunk undecided, see previous hunk\n" @@ -1196,9 +1198,31 @@ soft_increment: hunk->use = USE_HUNK; goto soft_increment; } - } else - color_fprintf(stdout, s->s.help_color, + } else { + const char *p = _(help_patch_remainder), *eol = p; + + color_fprintf(stdout, s->s.help_color, "%s", _(help_patch_text)); + + /* + * Show only those lines of the remainder that are + * actually applicable with the current hunk. + */ + for (; *p; p = eol + (*eol == '\n')) { + eol = strchrnul(p, '\n'); + + /* + * `s->buf` still contains the part of the + * commands shown in the prompt that are not + * always available. + */ + if (*p != '?' && !strchr(s->buf.buf, *p)) + continue; + + color_fprintf_ln(stdout, s->s.help_color, + "%.*s", (int)(eol - p), p); + } + } } /* Any hunk to be used? */ From 015e09dcfa308f2232c8141ba8f7b522ec324468 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 24 Mar 2019 22:54:01 +0100 Subject: [PATCH 18/18] built-in add -p: show helpful hint when nothing can be staged This patch will make `git add -p` show "No changes." or "Only binary files changed." in that case. Signed-off-by: Johannes Schindelin --- add-patch.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/add-patch.c b/add-patch.c index fecef93909..5dad203f08 100644 --- a/add-patch.c +++ b/add-patch.c @@ -44,7 +44,7 @@ struct add_p_state { struct hunk head; struct hunk *hunk; size_t hunk_nr, hunk_alloc; - unsigned deleted:1, mode_change:1; + unsigned deleted:1, mode_change:1,binary:1; } *file_diff; size_t file_diff_nr; }; @@ -267,7 +267,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) file_diff->mode_change = 1; } else if (file_diff->hunk_nr != 1) BUG("mode change after first hunk?"); - } + } else if (hunk == &file_diff->head && + starts_with(p, "Binary files ")) + file_diff->binary = 1; if (file_diff->deleted && file_diff->mode_change) BUG("diff contains delete *and* a mode change?!?\n%.*s", @@ -1251,7 +1253,7 @@ int run_add_p(struct repository *r, const struct pathspec *ps) struct add_p_state s = { { r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; - size_t i; + size_t i, binary_count = 0; if (init_add_i_state(r, &s.s)) return error("Could not read `add -i` config"); @@ -1264,9 +1266,16 @@ int run_add_p(struct repository *r, const struct pathspec *ps) } for (i = 0; i < s.file_diff_nr; i++) - if (patch_update_file(&s, s.file_diff + i)) + if (s.file_diff[i].binary && !s.file_diff[i].hunk_nr) + binary_count++; + else if (patch_update_file(&s, s.file_diff + i)) break; + if (s.file_diff_nr == 0) + fprintf(stderr, _("No changes.\n")); + else if (binary_count == s.file_diff_nr) + fprintf(stderr, _("Only binary files changed.\n")); + strbuf_release(&s.answer); strbuf_release(&s.buf); strbuf_release(&s.plain);