git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
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


  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

  List information: http://vger.kernel.org/majordomo-info.html

* 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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).