From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
Junio C Hamano <gitster@pobox.com>,
Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH 01/19] built-in add -i: start implementing the `patch` functionality in C
Date: Fri, 13 Dec 2019 08:07:48 +0000 [thread overview]
Message-ID: <63b571f351beb95828fb754bc94dac36347e1083.1576224486.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.173.git.1576224486.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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 after seeing whether the `k` and `j` commands are applicable, in the
C version we remember which previous/next hunk was undecided, and use it
rather than looking again when the user asked to jump).
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.
While it is tempting to use this conversion of `git add -p` as an excuse
to work on `apply_all_patches()` so that it does _not_ want to read a
file from `stdin` or from a file, but accepts, say, an `strbuf` instead,
we will refrain from this particular rabbit hole at this stage.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Makefile | 1 +
add-interactive.h | 1 +
add-patch.c | 265 ++++++++++++++++++++++++++++++++++++++++++++++
builtin/add.c | 15 ++-
4 files changed, 277 insertions(+), 5 deletions(-)
create mode 100644 add-patch.c
diff --git a/Makefile b/Makefile
index 6c4a1e0ee5..0345d7408b 100644
--- a/Makefile
+++ b/Makefile
@@ -824,6 +824,7 @@ LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentat
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..d1b1a080e4
--- /dev/null
+++ b/add-patch.c
@@ -0,0 +1,265 @@
+#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 add_p_state *s,
+ struct child_process *cp, ...)
+{
+ va_list ap;
+ const char *arg;
+
+ va_start(ap, cp);
+ 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(s, &cp,
+ "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("(%"PRIuMAX"/%"PRIuMAX") ",
+ (uintmax_t)hunk_index + 1, (uintmax_t)s->hunk_nr);
+ 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:
+ hunk_index = undecided_next < 0 ?
+ s->hunk_nr : undecided_next;
+ } 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);
+
+ discard_index(s->r->index);
+ setup_child_process(s, &cp, "apply", "--cached", NULL);
+ if (pipe_command(&cp, s->buf.buf, s->buf.len,
+ NULL, 0, NULL, 0))
+ error(_("'git apply --cached' failed"));
+ if (!repo_read_index(s->r))
+ repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0,
+ 1, NULL, NULL, NULL);
+ }
+
+ 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 (discard_index(r->index) < 0 || repo_read_index(r) < 0 ||
+ repo_refresh_and_write_index(r, REFRESH_QUIET, 0, 1,
+ NULL, NULL, NULL) < 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 d4686d5218..1deb59a642 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -189,12 +189,17 @@ int run_add_interactive(const char *revision, const char *patch_mode,
int use_builtin_add_i =
git_env_bool("GIT_TEST_ADD_I_USE_BUILTIN", -1);
- if (!patch_mode) {
- if (use_builtin_add_i < 0)
- git_config_get_bool("add.interactive.usebuiltin",
- &use_builtin_add_i);
- if (use_builtin_add_i == 1)
+ if (use_builtin_add_i < 0)
+ git_config_get_bool("add.interactive.usebuiltin",
+ &use_builtin_add_i);
+
+ 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");
--
gitgitgadget
next prev parent reply other threads:[~2019-12-13 8:08 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-13 8:07 [PATCH 00/19] Implement the git add --patch backend in C Johannes Schindelin via GitGitGadget
2019-12-13 8:07 ` Johannes Schindelin via GitGitGadget [this message]
2019-12-13 8:07 ` [PATCH 02/19] built-in add -i: wire up the new C code for the `patch` command Johannes Schindelin via GitGitGadget
2019-12-13 8:07 ` [PATCH 03/19] built-in add -p: show colored hunks by default Johannes Schindelin via GitGitGadget
2019-12-13 8:07 ` [PATCH 04/19] built-in add -p: adjust hunk headers as needed Johannes Schindelin via GitGitGadget
2019-12-13 8:07 ` [PATCH 05/19] built-in add -p: color the prompt and the help text Johannes Schindelin via GitGitGadget
2019-12-13 8:07 ` [PATCH 06/19] built-in add -p: offer a helpful error message when hunk navigation failed Johannes Schindelin via GitGitGadget
2019-12-13 8:07 ` [PATCH 07/19] built-in add -p: support multi-file diffs Johannes Schindelin via GitGitGadget
2019-12-13 8:07 ` [PATCH 08/19] built-in add -p: handle deleted empty files Johannes Schindelin via GitGitGadget
2019-12-13 8:07 ` [PATCH 09/19] built-in app -p: allow selecting a mode change as a "hunk" Johannes Schindelin via GitGitGadget
2019-12-13 8:07 ` [PATCH 10/19] built-in add -p: show different prompts for mode changes and deletions Johannes Schindelin via GitGitGadget
2019-12-13 8:07 ` [PATCH 11/19] built-in add -p: implement the hunk splitting feature Johannes Schindelin via GitGitGadget
2019-12-13 8:07 ` [PATCH 12/19] built-in add -p: coalesce hunks after splitting them Johannes Schindelin via GitGitGadget
2019-12-13 8:08 ` [PATCH 13/19] strbuf: add a helper function to call the editor "on an strbuf" Johannes Schindelin via GitGitGadget
2019-12-13 8:08 ` [PATCH 14/19] built-in add -p: implement hunk editing Johannes Schindelin via GitGitGadget
2019-12-13 8:08 ` [PATCH 15/19] built-in add -p: implement the 'g' ("goto") command Johannes Schindelin via GitGitGadget
2019-12-13 8:08 ` [PATCH 16/19] built-in add -p: implement the '/' ("search regex") command Johannes Schindelin via GitGitGadget
2019-12-13 8:08 ` [PATCH 17/19] built-in add -p: implement the 'q' ("quit") command Johannes Schindelin via GitGitGadget
2019-12-13 8:08 ` [PATCH 18/19] built-in add -p: only show the applicable parts of the help text Johannes Schindelin via GitGitGadget
2019-12-13 8:08 ` [PATCH 19/19] built-in add -p: show helpful hint when nothing can be staged Johannes Schindelin via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=63b571f351beb95828fb754bc94dac36347e1083.1576224486.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johannes.schindelin@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.