[PATCH 0/7] stash/reset/checkout -p: optionally use the add --patch backend written in pure C

[PATCH 0/7] stash/reset/checkout -p: optionally use the add --patch backend written in pure C

[Johannes Schindelin via GitGitGadget]

At this stage on the journey to a fully built-in git add, we already have
everything we need, including the --interactive and --patch options, as long
as the add.interactive.useBuiltin setting is set to true. This config
setting is kind of a feature flag that is currently turned off by default,
and will be for a while, until we get confident enough that the built-in
version does the job, and retire the Perl script.

However, the internal add--interactive helper is also used to back the 
--patch option of git stash, git reset, git checkout and git worktree.

This patch series (based on js/add-p-in-c) brings them "online".

Johannes Schindelin (7):
  built-in add -p: prepare for patch modes other than "stage"
  built-in add -p: implement the "stash" and "reset" patch modes
  legacy stash -p: respect the add.interactive.usebuiltin setting
  built-in stash: use the built-in `git add -p` if so configured
  built-in add -p: implement the "checkout" patch modes
  built-in add -p: implement the "worktree" patch modes
  commit --interactive: make it work with the built-in `add -i`

 add-interactive.c   |   2 +-
 add-interactive.h   |  12 +-
 add-patch.c         | 353 ++++++++++++++++++++++++++++++++++++++++----
 builtin/add.c       |  35 ++++-
 builtin/commit.c    |   8 +-
 builtin/stash.c     |  25 ++--
 git-legacy-stash.sh |   2 +-
 7 files changed, 388 insertions(+), 49 deletions(-)


base-commit: 2e4083198d1508206488af4c82093ceb6cf20f4e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-174%2Fdscho%2Fother-command-p-in-c-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-174/dscho/other-command-p-in-c-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/174
-- 
gitgitgadget

[PATCH 1/7] built-in add -p: prepare for patch modes other than "stage"

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The Perl script backing `git add -p` is used not only for that command,
but also for `git stash -p`, `git reset -p` and `git checkout -p`.

In preparation for teaching the C version of `git add -p` to support
also the latter commands, let's abstract away what is "stage" specific
into a dedicated data structure describing the differences between the
patch modes.

Finally, please note that the Perl version tries to make sure that the
diffs are only generated for the modified files. This is not actually
necessary, as the calls to Git's diff machinery already perform that
work, and perform it well. This makes it unnecessary to port the
`FILTER` field of the `%patch_modes` struct, as well as the
`get_diff_reference()` function.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c |  2 +-
 add-interactive.h |  8 ++++-
 add-patch.c       | 88 +++++++++++++++++++++++++++++++++--------------
 builtin/add.c     | 10 ++++--
 4 files changed, 79 insertions(+), 29 deletions(-)

diff --git a/add-interactive.c b/add-interactive.c
index 6a5048c83e..0e753d2acc 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -924,7 +924,7 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps,
 		parse_pathspec(&ps_selected,
 			       PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL,
 			       PATHSPEC_LITERAL_PATH, "", args.argv);
-		res = run_add_p(s->r, &ps_selected);
+		res = run_add_p(s->r, ADD_P_STAGE, NULL, &ps_selected);
 		argv_array_clear(&args);
 		clear_pathspec(&ps_selected);
 	}
diff --git a/add-interactive.h b/add-interactive.h
index 062dc3646c..3defa2ff3d 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -22,6 +22,12 @@ void init_add_i_state(struct add_i_state *s, struct repository *r);
 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);
+
+enum add_p_mode {
+	ADD_P_STAGE,
+};
+
+int run_add_p(struct repository *r, enum add_p_mode mode,
+	      const char *revision, const struct pathspec *ps);
 
 #endif
diff --git a/add-patch.c b/add-patch.c
index 2c46fe5b33..8a691f07da 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -11,10 +11,33 @@ enum prompt_mode_type {
 	PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK
 };
 
-static const char *prompt_mode[] = {
-	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 patch_mode {
+	const char *diff[4], *apply[4], *apply_check[4];
+	unsigned is_reverse:1, apply_for_checkout:1;
+	const char *prompt_mode[PROMPT_HUNK + 1];
+	const char *edit_hunk_hint, *help_patch_text;
+};
+
+static struct patch_mode patch_mode_stage = {
+	.diff = { "diff-files", NULL },
+	.apply = { "--cached", NULL },
+	.apply_check = { "--cached", NULL },
+	.is_reverse = 0,
+	.prompt_mode = {
+		N_("Stage mode change [y,n,q,a,d%s,?]? "),
+		N_("Stage deletion [y,n,q,a,d%s,?]? "),
+		N_("Stage this hunk [y,n,q,a,d%s,?]? ")
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for staging."),
+	.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 hunk and all later hunks in the file\n"
+		   "d - do not stage this hunk or any of the later hunks in "
+			"the file\n")
 };
 
 struct hunk_header {
@@ -47,6 +70,10 @@ struct add_p_state {
 		unsigned deleted:1, mode_change:1,binary:1;
 	} *file_diff;
 	size_t file_diff_nr;
+
+	/* patch mode */
+	struct patch_mode *mode;
+	const char *revision;
 };
 
 static void err(struct add_p_state *s, const char *fmt, ...)
@@ -162,9 +189,18 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 	struct hunk *hunk = NULL;
 	int res;
 
+	argv_array_pushv(&args, s->mode->diff);
+	if (s->revision) {
+		struct object_id oid;
+		argv_array_push(&args,
+				/* could be on an unborn branch */
+				!strcmp("HEAD", s->revision) &&
+				get_oid("HEAD", &oid) ?
+				empty_tree_oid_hex() : s->revision);
+	}
+	color_arg_index = args.argc;
 	/* Use `--no-color` explicitly, just in case `diff.color = always`. */
-	argv_array_pushl(&args, "diff-files", "-p", "--no-color", "--", NULL);
-	color_arg_index = args.argc - 2;
+	argv_array_pushl(&args, "--no-color", "-p", "--", NULL);
 	for (i = 0; i < ps->nr; i++)
 		argv_array_push(&args, ps->items[i].original);
 
@@ -382,7 +418,10 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,
 				- header->colored_extra_start;
 		}
 
-		new_offset += delta;
+		if (s->mode->is_reverse)
+			old_offset -= delta;
+		else
+			new_offset += delta;
 
 		strbuf_addf(out, "@@ -%lu,%lu +%lu,%lu @@",
 			    old_offset, header->old_count,
@@ -805,11 +844,10 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
 				"(context).\n"
 				"To remove '%c' lines, delete them.\n"
 				"Lines starting with %c will be removed.\n"),
-			      '-', '+', comment_line_char);
-	strbuf_commented_addf(&s->buf,
-			      _("If the patch applies cleanly, the edited hunk "
-				"will immediately be\n"
-				"marked for staging.\n"));
+			      s->mode->is_reverse ? '+' : '-',
+			      s->mode->is_reverse ? '-' : '+',
+			      comment_line_char);
+	strbuf_commented_addf(&s->buf, "%s", _(s->mode->edit_hunk_hint));
 	/*
 	 * TRANSLATORS: 'it' refers to the patch mentioned in the previous
 	 * messages.
@@ -890,7 +928,8 @@ static int run_apply_check(struct add_p_state *s,
 	reassemble_patch(s, file_diff, 1, &s->buf);
 
 	setup_child_process(s, &cp,
-			    "apply", "--cached", "--check", NULL);
+			    "apply", "--check", NULL);
+	argv_array_pushv(&cp.args, s->mode->apply_check);
 	if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0))
 		return error(_("'git apply --cached' failed"));
 
@@ -1005,13 +1044,6 @@ static size_t display_hunks(struct add_p_state *s,
 	return end_index;
 }
 
-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");
-
 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"
@@ -1097,7 +1129,8 @@ static int patch_update_file(struct add_p_state *s,
 			      (uintmax_t)hunk_index + 1,
 			      (uintmax_t)file_diff->hunk_nr);
 		color_fprintf(stdout, s->s.prompt_color,
-			      _(prompt_mode[prompt_mode_type]), s->buf.buf);
+			      _(s->mode->prompt_mode[prompt_mode_type]),
+			      s->buf.buf);
 		fflush(stdout);
 		if (strbuf_getline(&s->answer, stdin) == EOF)
 			break;
@@ -1254,7 +1287,7 @@ static int patch_update_file(struct add_p_state *s,
 			const char *p = _(help_patch_remainder), *eol = p;
 
 			color_fprintf(stdout, s->s.help_color, "%s",
-				      _(help_patch_text));
+				      _(s->mode->help_patch_text));
 
 			/*
 			 * Show only those lines of the remainder that are
@@ -1288,10 +1321,11 @@ static int patch_update_file(struct add_p_state *s,
 		reassemble_patch(s, file_diff, 0, &s->buf);
 
 		discard_index(s->s.r->index);
-		setup_child_process(s, &cp, "apply", "--cached", NULL);
+		setup_child_process(s, &cp, "apply", NULL);
+		argv_array_pushv(&cp.args, s->mode->apply);
 		if (pipe_command(&cp, s->buf.buf, s->buf.len,
 				 NULL, 0, NULL, 0))
-			error(_("'git apply --cached' failed"));
+			error(_("'git apply' failed"));
 		if (!repo_read_index(s->s.r))
 			repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0,
 						     1, NULL, NULL, NULL);
@@ -1301,7 +1335,8 @@ static int patch_update_file(struct add_p_state *s,
 	return quit;
 }
 
-int run_add_p(struct repository *r, const struct pathspec *ps)
+int run_add_p(struct repository *r, enum add_p_mode mode,
+	      const char *revision, const struct pathspec *ps)
 {
 	struct add_p_state s = {
 		{ r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
@@ -1310,6 +1345,9 @@ int run_add_p(struct repository *r, const struct pathspec *ps)
 
 	init_add_i_state(&s.s, r);
 
+	s.mode = &patch_mode_stage;
+	s.revision = revision;
+
 	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 ||
diff --git a/builtin/add.c b/builtin/add.c
index 1deb59a642..05b727a426 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -194,12 +194,18 @@ int run_add_interactive(const char *revision, const char *patch_mode,
 				    &use_builtin_add_i);
 
 	if (use_builtin_add_i == 1) {
+		enum add_p_mode mode;
+
 		if (!patch_mode)
 			return !!run_add_i(the_repository, pathspec);
-		if (strcmp(patch_mode, "--patch"))
+
+		if (!strcmp(patch_mode, "--patch"))
+			mode = ADD_P_STAGE;
+		else
 			die("'%s' not yet supported in the built-in add -p",
 			    patch_mode);
-		return !!run_add_p(the_repository, pathspec);
+
+		return !!run_add_p(the_repository, mode, revision, pathspec);
 	}
 
 	argv_array_push(&argv, "add--interactive");
-- 
gitgitgadget

[PATCH 2/7] built-in add -p: implement the "stash" and "reset" patch modes

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The `git stash` and `git reset` commands support a `--patch` option, and
both simply hand off to `git add -p` to perform that work. Let's teach
the built-in version of that command to be able to perform that work, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.h |  2 ++
 add-patch.c       | 85 ++++++++++++++++++++++++++++++++++++++++++++---
 builtin/add.c     |  4 +++
 3 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/add-interactive.h b/add-interactive.h
index 3defa2ff3d..c278f3e26f 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -25,6 +25,8 @@ int run_add_i(struct repository *r, const struct pathspec *ps);
 
 enum add_p_mode {
 	ADD_P_STAGE,
+	ADD_P_STASH,
+	ADD_P_RESET,
 };
 
 int run_add_p(struct repository *r, enum add_p_mode mode,
diff --git a/add-patch.c b/add-patch.c
index 8a691f07da..694e8e6624 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -13,7 +13,7 @@ enum prompt_mode_type {
 
 struct patch_mode {
 	const char *diff[4], *apply[4], *apply_check[4];
-	unsigned is_reverse:1, apply_for_checkout:1;
+	unsigned is_reverse:1, index_only:1, apply_for_checkout:1;
 	const char *prompt_mode[PROMPT_HUNK + 1];
 	const char *edit_hunk_hint, *help_patch_text;
 };
@@ -40,6 +40,74 @@ static struct patch_mode patch_mode_stage = {
 			"the file\n")
 };
 
+static struct patch_mode patch_mode_stash = {
+	.diff = { "diff-index", "HEAD", NULL },
+	.apply = { "--cached", NULL },
+	.apply_check = { "--cached", NULL },
+	.is_reverse = 0,
+	.prompt_mode = {
+		N_("Stash mode change [y,n,q,a,d%s,?]? "),
+		N_("Stash deletion [y,n,q,a,d%s,?]? "),
+		N_("Stash this hunk [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for stashing."),
+	.help_patch_text =
+		N_("y - stash this hunk\n"
+		   "n - do not stash this hunk\n"
+		   "q - quit; do not stash this hunk or any of the remaining "
+			"ones\n"
+		   "a - stash this hunk and all later hunks in the file\n"
+		   "d - do not stash this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
+static struct patch_mode patch_mode_reset_head = {
+	.diff = { "diff-index", "--cached", NULL },
+	.apply = { "-R", "--cached", NULL },
+	.apply_check = { "-R", "--cached", NULL },
+	.is_reverse = 1,
+	.index_only = 1,
+	.prompt_mode = {
+		N_("Unstage mode change [y,n,q,a,d%s,?]? "),
+		N_("Unstage deletion [y,n,q,a,d%s,?]? "),
+		N_("Unstage this hunk [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for unstaging."),
+	.help_patch_text =
+		N_("y - unstage this hunk\n"
+		   "n - do not unstage this hunk\n"
+		   "q - quit; do not unstage this hunk or any of the remaining "
+			"ones\n"
+		   "a - unstage this hunk and all later hunks in the file\n"
+		   "d - do not unstage this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
+static struct patch_mode patch_mode_reset_nothead = {
+	.diff = { "diff-index", "-R", "--cached", NULL },
+	.apply = { "--cached", NULL },
+	.apply_check = { "--cached", NULL },
+	.is_reverse = 0,
+	.index_only = 1,
+	.prompt_mode = {
+		N_("Apply mode change to index [y,n,q,a,d%s,?]? "),
+		N_("Apply deletion to index [y,n,q,a,d%s,?]? "),
+		N_("Apply this hunk to index [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for applying."),
+	.help_patch_text =
+		N_("y - apply this hunk to index\n"
+		   "n - do not apply this hunk to index\n"
+		   "q - quit; do not apply this hunk or any of the remaining "
+			"ones\n"
+		   "a - apply this hunk and all later hunks in the file\n"
+		   "d - do not apply this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
 struct hunk_header {
 	unsigned long old_offset, old_count, new_offset, new_count;
 	/*
@@ -1345,12 +1413,21 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 
 	init_add_i_state(&s.s, r);
 
-	s.mode = &patch_mode_stage;
+	if (mode == ADD_P_STASH)
+		s.mode = &patch_mode_stash;
+	else if (mode == ADD_P_RESET) {
+		if (!revision || !strcmp(revision, "HEAD"))
+			s.mode = &patch_mode_reset_head;
+		else
+			s.mode = &patch_mode_reset_nothead;
+	} else
+		s.mode = &patch_mode_stage;
 	s.revision = revision;
 
 	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 ||
+	    (!s.mode->index_only &&
+	     repo_refresh_and_write_index(r, REFRESH_QUIET, 0, 1,
+					  NULL, NULL, NULL) < 0) ||
 	    parse_diff(&s, ps) < 0) {
 		strbuf_release(&s.plain);
 		strbuf_release(&s.colored);
diff --git a/builtin/add.c b/builtin/add.c
index 05b727a426..583d8aab0d 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -201,6 +201,10 @@ int run_add_interactive(const char *revision, const char *patch_mode,
 
 		if (!strcmp(patch_mode, "--patch"))
 			mode = ADD_P_STAGE;
+		else if (!strcmp(patch_mode, "--patch=stash"))
+			mode = ADD_P_STASH;
+		else if (!strcmp(patch_mode, "--patch=reset"))
+			mode = ADD_P_RESET;
 		else
 			die("'%s' not yet supported in the built-in add -p",
 			    patch_mode);
-- 
gitgitgadget

[PATCH 3/7] legacy stash -p: respect the add.interactive.usebuiltin setting

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

As `git add` traditionally did not expose the `--patch=<mode>` modes via
command-line options, the scripted version of `git stash` had to call
`git add--interactive` directly.

But this prevents the built-in `add -p` from kicking in, as
`add--interactive` is the scripted version (which does not have a
"fall-back" to the built-in version).

So let's introduce support for internal switch for `git add` that the
scripted `git stash` can use to call the appropriate backend (scripted
or built-in, depending on `add.interactive.useBuiltin`).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/add.c       | 14 ++++++++++++++
 git-legacy-stash.sh |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/builtin/add.c b/builtin/add.c
index 583d8aab0d..f238e8b623 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -29,6 +29,7 @@ static const char * const builtin_add_usage[] = {
 static int patch_interactive, add_interactive, edit_interactive;
 static int take_worktree_changes;
 static int add_renormalize;
+static int legacy_stash_p; /* support for the scripted `git stash` */
 
 struct update_callback_data {
 	int flags;
@@ -335,6 +336,8 @@ static struct option builtin_add_options[] = {
 		   N_("override the executable bit of the listed files")),
 	OPT_HIDDEN_BOOL(0, "warn-embedded-repo", &warn_on_embedded_repo,
 			N_("warn when adding an embedded repository")),
+	OPT_HIDDEN_BOOL(0, "legacy-stash-p", &legacy_stash_p,
+			N_("backend for `git stash -p`")),
 	OPT_END(),
 };
 
@@ -431,6 +434,17 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		add_interactive = 1;
 	if (add_interactive)
 		exit(interactive_add(argc - 1, argv + 1, prefix, patch_interactive));
+	if (legacy_stash_p) {
+		struct pathspec pathspec;
+
+		parse_pathspec(&pathspec, 0,
+			PATHSPEC_PREFER_FULL |
+			PATHSPEC_SYMLINK_LEADING_PATH |
+			PATHSPEC_PREFIX_ORIGIN,
+			prefix, argv);
+
+		return run_add_interactive(NULL, "--patch=stash", &pathspec);
+	}
 
 	if (edit_interactive)
 		return(edit_patch(argc, argv, prefix));
diff --git a/git-legacy-stash.sh b/git-legacy-stash.sh
index 07ad4a5459..ed039dfcbb 100755
--- a/git-legacy-stash.sh
+++ b/git-legacy-stash.sh
@@ -206,7 +206,7 @@ create_stash () {
 
 		# find out what the user wants
 		GIT_INDEX_FILE="$TMP-index" \
-			git add--interactive --patch=stash -- "$@" &&
+			git add --legacy-stash-p -- "$@" &&
 
 		# state of the working tree
 		w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) ||
-- 
gitgitgadget

[PATCH 4/7] built-in stash: use the built-in `git add -p` if so configured

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The scripted version of `git stash` called directly into the Perl script
`git-add--interactive.perl`, and this was faithfully converted to C.

However, we have a much better way to do this now: call the internal API
directly, which will now incidentally also respect the
`add.interactive.useBuiltin` setting. Let's just do this.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/stash.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index 4e806176b0..2dafd97766 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -999,9 +999,9 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,
 {
 	int ret = 0;
 	struct child_process cp_read_tree = CHILD_PROCESS_INIT;
-	struct child_process cp_add_i = CHILD_PROCESS_INIT;
 	struct child_process cp_diff_tree = CHILD_PROCESS_INIT;
 	struct index_state istate = { NULL };
+	char *old_index_env = NULL, *old_repo_index_file;
 
 	remove_path(stash_index_path.buf);
 
@@ -1015,16 +1015,19 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,
 	}
 
 	/* Find out what the user wants. */
-	cp_add_i.git_cmd = 1;
-	argv_array_pushl(&cp_add_i.args, "add--interactive", "--patch=stash",
-			 "--", NULL);
-	add_pathspecs(&cp_add_i.args, ps);
-	argv_array_pushf(&cp_add_i.env_array, "GIT_INDEX_FILE=%s",
-			 stash_index_path.buf);
-	if (run_command(&cp_add_i)) {
-		ret = -1;
-		goto done;
-	}
+	old_repo_index_file = the_repository->index_file;
+	the_repository->index_file = stash_index_path.buf;
+	old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT));
+	setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1);
+
+	ret = run_add_interactive(NULL, "--patch=stash", ps);
+
+	the_repository->index_file = old_repo_index_file;
+	if (old_index_env && *old_index_env)
+		setenv(INDEX_ENVIRONMENT, old_index_env, 1);
+	else
+		unsetenv(INDEX_ENVIRONMENT);
+	FREE_AND_NULL(old_index_env);
 
 	/* State of the working tree. */
 	if (write_index_as_tree(&info->w_tree, &istate, stash_index_path.buf, 0,
-- 
gitgitgadget

[PATCH 5/7] built-in add -p: implement the "checkout" patch modes

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

This patch teaches the built-in `git add -p` machinery all the tricks it
needs to know in order to act as the work horse for `git checkout -p`.

Apart from the minor changes (slightly reworded messages, different
`diff` and `apply --check` invocations), it requires a new function to
actually apply the changes, as `git checkout -p` is a bit special in
that respect: when the desired changes do not apply to the index, but
apply to the work tree, Git does not fail straight away, but asks the
user whether to apply the changes to the worktree at least.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.h |   1 +
 add-patch.c       | 139 ++++++++++++++++++++++++++++++++++++++++++++--
 builtin/add.c     |   5 +-
 3 files changed, 138 insertions(+), 7 deletions(-)

diff --git a/add-interactive.h b/add-interactive.h
index c278f3e26f..f865f1e8ca 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -27,6 +27,7 @@ enum add_p_mode {
 	ADD_P_STAGE,
 	ADD_P_STASH,
 	ADD_P_RESET,
+	ADD_P_CHECKOUT,
 };
 
 int run_add_p(struct repository *r, enum add_p_mode mode,
diff --git a/add-patch.c b/add-patch.c
index 694e8e6624..dea99a79a4 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -108,6 +108,72 @@ static struct patch_mode patch_mode_reset_nothead = {
 			"the file\n"),
 };
 
+static struct patch_mode patch_mode_checkout_index = {
+	.diff = { "diff-files", NULL },
+	.apply = { "-R", NULL },
+	.apply_check = { "-R", NULL },
+	.is_reverse = 1,
+	.prompt_mode = {
+		N_("Discard mode change from worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard deletion from worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard this hunk from worktree [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for discarding."),
+	.help_patch_text =
+		N_("y - discard this hunk from worktree\n"
+		   "n - do not discard this hunk from worktree\n"
+		   "q - quit; do not discard this hunk or any of the remaining "
+			"ones\n"
+		   "a - discard this hunk and all later hunks in the file\n"
+		   "d - do not discard this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
+static struct patch_mode patch_mode_checkout_head = {
+	.diff = { "diff-index", NULL },
+	.apply_for_checkout = 1,
+	.apply_check = { "-R", NULL },
+	.is_reverse = 1,
+	.prompt_mode = {
+		N_("Discard mode change from index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard deletion from index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard this hunk from index and worktree [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for discarding."),
+	.help_patch_text =
+		N_("y - discard this hunk from index and worktree\n"
+		   "n - do not discard this hunk from index and worktree\n"
+		   "q - quit; do not discard this hunk or any of the remaining "
+			"ones\n"
+		   "a - discard this hunk and all later hunks in the file\n"
+		   "d - do not discard this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
+static struct patch_mode patch_mode_checkout_nothead = {
+	.diff = { "diff-index", "-R", NULL },
+	.apply_for_checkout = 1,
+	.apply_check = { NULL },
+	.is_reverse = 0,
+	.prompt_mode = {
+		N_("Apply mode change to index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Apply deletion to index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Apply this hunk to index and worktree [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for applying."),
+	.help_patch_text =
+		N_("y - apply this hunk to index and worktree\n"
+		   "n - do not apply this hunk to index and worktree\n"
+		   "q - quit; do not apply this hunk or any of the remaining "
+			"ones\n"
+		   "a - apply this hunk and all later hunks in the file\n"
+		   "d - do not apply this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
 struct hunk_header {
 	unsigned long old_offset, old_count, new_offset, new_count;
 	/*
@@ -1064,6 +1130,57 @@ static int edit_hunk_loop(struct add_p_state *s,
 	}
 }
 
+static int apply_for_checkout(struct add_p_state *s, struct strbuf *diff,
+			      int is_reverse)
+{
+	const char *reverse = is_reverse ? "-R" : NULL;
+	struct child_process check_index = CHILD_PROCESS_INIT;
+	struct child_process check_worktree = CHILD_PROCESS_INIT;
+	struct child_process apply_index = CHILD_PROCESS_INIT;
+	struct child_process apply_worktree = CHILD_PROCESS_INIT;
+	int applies_index, applies_worktree;
+
+	setup_child_process(s, &check_index,
+			    "apply", "--cached", "--check", reverse, NULL);
+	applies_index = !pipe_command(&check_index, diff->buf, diff->len,
+				      NULL, 0, NULL, 0);
+
+	setup_child_process(s, &check_worktree,
+			    "apply", "--check", reverse, NULL);
+	applies_worktree = !pipe_command(&check_worktree, diff->buf, diff->len,
+					 NULL, 0, NULL, 0);
+
+	if (applies_worktree && applies_index) {
+		setup_child_process(s, &apply_index,
+				    "apply", "--cached", reverse, NULL);
+		pipe_command(&apply_index, diff->buf, diff->len,
+			     NULL, 0, NULL, 0);
+
+		setup_child_process(s, &apply_worktree,
+				    "apply", reverse, NULL);
+		pipe_command(&apply_worktree, diff->buf, diff->len,
+			     NULL, 0, NULL, 0);
+
+		return 1;
+	}
+
+	if (!applies_index) {
+		err(s, _("The selected hunks do not apply to the index!"));
+		if (prompt_yesno(s, _("Apply them to the worktree "
+					  "anyway? ")) > 0) {
+			setup_child_process(s, &apply_worktree,
+					    "apply", reverse, NULL);
+			return pipe_command(&apply_worktree, diff->buf,
+					    diff->len, NULL, 0, NULL, 0);
+		}
+		err(s, _("Nothing was applied.\n"));
+	} else
+		/* As a last resort, show the diff to the user */
+		fwrite(diff->buf, diff->len, 1, stderr);
+
+	return 0;
+}
+
 #define SUMMARY_HEADER_WIDTH 20
 #define SUMMARY_LINE_WIDTH 80
 static void summarize_hunk(struct add_p_state *s, struct hunk *hunk,
@@ -1389,11 +1506,16 @@ static int patch_update_file(struct add_p_state *s,
 		reassemble_patch(s, file_diff, 0, &s->buf);
 
 		discard_index(s->s.r->index);
-		setup_child_process(s, &cp, "apply", NULL);
-		argv_array_pushv(&cp.args, s->mode->apply);
-		if (pipe_command(&cp, s->buf.buf, s->buf.len,
-				 NULL, 0, NULL, 0))
-			error(_("'git apply' failed"));
+		if (s->mode->apply_for_checkout)
+			apply_for_checkout(s, &s->buf,
+					   s->mode->is_reverse);
+		else {
+			setup_child_process(s, &cp, "apply", NULL);
+			argv_array_pushv(&cp.args, s->mode->apply);
+			if (pipe_command(&cp, s->buf.buf, s->buf.len,
+					 NULL, 0, NULL, 0))
+				error(_("'git apply' failed"));
+		}
 		if (!repo_read_index(s->s.r))
 			repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0,
 						     1, NULL, NULL, NULL);
@@ -1420,6 +1542,13 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 			s.mode = &patch_mode_reset_head;
 		else
 			s.mode = &patch_mode_reset_nothead;
+	} else if (mode == ADD_P_CHECKOUT) {
+		if (!revision)
+			s.mode = &patch_mode_checkout_index;
+		else if (!strcmp(revision, "HEAD"))
+			s.mode = &patch_mode_checkout_head;
+		else
+			s.mode = &patch_mode_checkout_nothead;
 	} else
 		s.mode = &patch_mode_stage;
 	s.revision = revision;
diff --git a/builtin/add.c b/builtin/add.c
index f238e8b623..f4d6eb9e06 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -206,9 +206,10 @@ int run_add_interactive(const char *revision, const char *patch_mode,
 			mode = ADD_P_STASH;
 		else if (!strcmp(patch_mode, "--patch=reset"))
 			mode = ADD_P_RESET;
+		else if (!strcmp(patch_mode, "--patch=checkout"))
+			mode = ADD_P_CHECKOUT;
 		else
-			die("'%s' not yet supported in the built-in add -p",
-			    patch_mode);
+			die("'%s' not supported", patch_mode);
 
 		return !!run_add_p(the_repository, mode, revision, pathspec);
 	}
-- 
gitgitgadget

[PATCH 6/7] built-in add -p: implement the "worktree" patch modes

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

This is a straight-forward port of 2f0896ec3ad4 (restore: support
--patch, 2019-04-25) which added support for `git restore -p`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.h |  1 +
 add-patch.c       | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 builtin/add.c     |  2 ++
 3 files changed, 54 insertions(+)

diff --git a/add-interactive.h b/add-interactive.h
index f865f1e8ca..4895ed1df5 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -28,6 +28,7 @@ enum add_p_mode {
 	ADD_P_STASH,
 	ADD_P_RESET,
 	ADD_P_CHECKOUT,
+	ADD_P_WORKTREE,
 };
 
 int run_add_p(struct repository *r, enum add_p_mode mode,
diff --git a/add-patch.c b/add-patch.c
index dea99a79a4..2ad18dc3cb 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -174,6 +174,50 @@ static struct patch_mode patch_mode_checkout_nothead = {
 			"the file\n"),
 };
 
+static struct patch_mode patch_mode_worktree_head = {
+	.diff = { "diff-index", NULL },
+	.apply = { "-R", NULL },
+	.apply_check = { "-R", NULL },
+	.is_reverse = 1,
+	.prompt_mode = {
+		N_("Discard mode change from index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard deletion from index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard this hunk from index and worktree [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for discarding."),
+	.help_patch_text =
+		N_("y - discard this hunk from worktree\n"
+		   "n - do not discard this hunk from worktree\n"
+		   "q - quit; do not discard this hunk or any of the remaining "
+			"ones\n"
+		   "a - discard this hunk and all later hunks in the file\n"
+		   "d - do not discard this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
+static struct patch_mode patch_mode_worktree_nothead = {
+	.diff = { "diff-index", "-R", NULL },
+	.apply = { NULL },
+	.apply_check = { NULL },
+	.is_reverse = 0,
+	.prompt_mode = {
+		N_("Apply mode change to index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Apply deletion to index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Apply this hunk to index and worktree [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for applying."),
+	.help_patch_text =
+		N_("y - apply this hunk to worktree\n"
+		   "n - do not apply this hunk to worktree\n"
+		   "q - quit; do not apply this hunk or any of the remaining "
+			"ones\n"
+		   "a - apply this hunk and all later hunks in the file\n"
+		   "d - do not apply this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
 struct hunk_header {
 	unsigned long old_offset, old_count, new_offset, new_count;
 	/*
@@ -1549,6 +1593,13 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 			s.mode = &patch_mode_checkout_head;
 		else
 			s.mode = &patch_mode_checkout_nothead;
+	} else if (mode == ADD_P_WORKTREE) {
+		if (!revision)
+			s.mode = &patch_mode_checkout_index;
+		else if (!strcmp(revision, "HEAD"))
+			s.mode = &patch_mode_worktree_head;
+		else
+			s.mode = &patch_mode_worktree_nothead;
 	} else
 		s.mode = &patch_mode_stage;
 	s.revision = revision;
diff --git a/builtin/add.c b/builtin/add.c
index f4d6eb9e06..6fa2b2dd17 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -208,6 +208,8 @@ int run_add_interactive(const char *revision, const char *patch_mode,
 			mode = ADD_P_RESET;
 		else if (!strcmp(patch_mode, "--patch=checkout"))
 			mode = ADD_P_CHECKOUT;
+		else if (!strcmp(patch_mode, "--patch=worktree"))
+			mode = ADD_P_WORKTREE;
 		else
 			die("'%s' not supported", patch_mode);
 
-- 
gitgitgadget

[PATCH 7/7] commit --interactive: make it work with the built-in `add -i`

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The built-in `git add -i` machinery obviously has its `the_repository`
structure initialized at the point where `cmd_commit()` calls it, and
therefore does not look at the environment variable `GIT_INDEX_FILE`.

But when being called from `commit --interactive`, it has to, because
the index was already locked in that case, and we want to ask the
interactive add machinery to work on the `index.lock` file instead of
the `index` file.

Technically, we could teach `run_add_i()`, or for that matter
`run_add_p()`, to look specifically at that environment variable, but
the entire idea of passing in a parameter of type `struct repository *`
is to allow working on multiple repositories (and their index files)
independently.

So let's instead override the `index_file` field of that structure
temporarily.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/commit.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index e588bc6ad3..32ffc7beee 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -347,7 +347,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 		die(_("index file corrupt"));
 
 	if (interactive) {
-		char *old_index_env = NULL;
+		char *old_index_env = NULL, *old_repo_index_file;
 		hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
 
 		refresh_cache_or_die(refresh_flags);
@@ -355,12 +355,16 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 		if (write_locked_index(&the_index, &index_lock, 0))
 			die(_("unable to create temporary index"));
 
+		old_repo_index_file = the_repository->index_file;
+		the_repository->index_file =
+			(char *)get_lock_file_path(&index_lock);
 		old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT));
-		setenv(INDEX_ENVIRONMENT, get_lock_file_path(&index_lock), 1);
+		setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1);
 
 		if (interactive_add(argc, argv, prefix, patch_interactive) != 0)
 			die(_("interactive add failed"));
 
+		the_repository->index_file = old_repo_index_file;
 		if (old_index_env && *old_index_env)
 			setenv(INDEX_ENVIRONMENT, old_index_env, 1);
 		else
-- 
gitgitgadget

Re: [PATCH 1/7] built-in add -p: prepare for patch modes other than "stage"

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> +
> +enum add_p_mode {
> +	ADD_P_STAGE,
> +};

Nice to see a trailing comma here ;-)

> +int run_add_p(struct repository *r, enum add_p_mode mode,
> +	      const char *revision, const struct pathspec *ps);

This makes readers wonder if "const struct object_id *" is more
appropriate; "const char *revision" that holds human-readable name
is better when the internal machinery uses it for reporting, so that
may be what is going on here (so is the new field in add_p_state
structure).

>  #endif
> diff --git a/add-patch.c b/add-patch.c
> index 2c46fe5b33..8a691f07da 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -11,10 +11,33 @@ enum prompt_mode_type {
>  	PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK
>  };
>  
> -static const char *prompt_mode[] = {
> -	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 patch_mode {
> +	const char *diff[4], *apply[4], *apply_check[4];

Hardcoded "4" and not-quite descriptive names puzzle readers at the
first glance.  Let's read on to see if they need any further
improvement.

> +	unsigned is_reverse:1, apply_for_checkout:1;
> +	const char *prompt_mode[PROMPT_HUNK + 1];

This relies on the enum value assignment (or listing) order to
ensure that PROMPT_HUNK always comes at the end.  Perhaps that
deserves a comment before "enum prompt_mode_type", e.g.

	+/* Keep PROMPT_HUNK at the end */
	 enum prompt_mode_type {
	 	PROMPT_MODE_CHANGE = 0, ...
	 };

> +	const char *edit_hunk_hint, *help_patch_text;
> +};
> +
> +static struct patch_mode patch_mode_stage = {
> +	.diff = { "diff-files", NULL },

Nice to see designated initializers used ;-)

Mental note: the "diff" field is (probably) for "the command line
to be used to generate the patch"

> +	.apply = { "--cached", NULL },
> +	.apply_check = { "--cached", NULL },

Mental note: these "apply" and "apply_check" fields are (probably)
not for the command line; unlike the "diff" field, these only have
arguments.

Mental note: if the three field names become confusing, perhaps we
can clarify them by either (1) calling diff as diff_cmd[], or (2)
calling the other as apply_args[] and apply_check_args[], or (3)
rename both.

> +	.is_reverse = 0,

Wouldn't it be sufficient to apply the default initialization, just
like it is done for apply_for_checkout bitfield?

> @@ -1310,6 +1345,9 @@ int run_add_p(struct repository *r, const struct pathspec *ps)
>  
>  	init_add_i_state(&s.s, r);
>  
> +	s.mode = &patch_mode_stage;
> +	s.revision = revision;

The phrase "mode_stage" may become problematic, as other modes that
will be introduced, like "reset", "checkout" all will stage
different contents to the index.  The only mode the machinery knows
at this point in the series is how "add" stages contents to the
index, so "patch_mode_add" might turn out to be a better choice of
the phrase as we read the series along.  We'll see.

> +		if (!strcmp(patch_mode, "--patch"))
> +			mode = ADD_P_STAGE;

The same comment applies to this enum token.

Thanks.

Re: [PATCH 2/7] built-in add -p: implement the "stash" and "reset" patch modes

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The `git stash` and `git reset` commands support a `--patch` option, and
> both simply hand off to `git add -p` to perform that work. Let's teach
> the built-in version of that command to be able to perform that work, too.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  add-interactive.h |  2 ++
>  add-patch.c       | 85 ++++++++++++++++++++++++++++++++++++++++++++---
>  builtin/add.c     |  4 +++
>  3 files changed, 87 insertions(+), 4 deletions(-)
>
> diff --git a/add-interactive.h b/add-interactive.h
> index 3defa2ff3d..c278f3e26f 100644
> --- a/add-interactive.h
> +++ b/add-interactive.h
> @@ -25,6 +25,8 @@ int run_add_i(struct repository *r, const struct pathspec *ps);
>  
>  enum add_p_mode {
>  	ADD_P_STAGE,
> +	ADD_P_STASH,
> +	ADD_P_RESET,

As I mentioned in my review on the previous step, ADD_P_ADD would be
more descriptive of what is going on when listed together with STASH
and RESET here.

> +static struct patch_mode patch_mode_reset_head = {
> +	.diff = { "diff-index", "--cached", NULL },
> +	.apply = { "-R", "--cached", NULL },
> +	.apply_check = { "-R", "--cached", NULL },
> +	.is_reverse = 1,
> +	.index_only = 1,
> +	.prompt_mode = {
> +		N_("Unstage mode change [y,n,q,a,d%s,?]? "),
> +		N_("Unstage deletion [y,n,q,a,d%s,?]? "),
> +		N_("Unstage this hunk [y,n,q,a,d%s,?]? "),
> +	},
> +	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
> +			     "will immediately be marked for unstaging."),
> +	.help_patch_text =
> +		N_("y - unstage this hunk\n"
> +		   "n - do not unstage this hunk\n"
> +		   "q - quit; do not unstage this hunk or any of the remaining "
> +			"ones\n"
> +		   "a - unstage this hunk and all later hunks in the file\n"
> +		   "d - do not unstage this hunk or any of the later hunks in "
> +			"the file\n"),
> +};
> +
> +static struct patch_mode patch_mode_reset_nothead = {
> +	.diff = { "diff-index", "-R", "--cached", NULL },
> +	.apply = { "--cached", NULL },
> +	.apply_check = { "--cached", NULL },
> +	.is_reverse = 0,
> +	.index_only = 1,
> +	.prompt_mode = {
> +		N_("Apply mode change to index [y,n,q,a,d%s,?]? "),
> +		N_("Apply deletion to index [y,n,q,a,d%s,?]? "),
> +		N_("Apply this hunk to index [y,n,q,a,d%s,?]? "),
> +	},
> +	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
> +			     "will immediately be marked for applying."),
> +	.help_patch_text =
> +		N_("y - apply this hunk to index\n"
> +		   "n - do not apply this hunk to index\n"
> +		   "q - quit; do not apply this hunk or any of the remaining "
> +			"ones\n"
> +		   "a - apply this hunk and all later hunks in the file\n"
> +		   "d - do not apply this hunk or any of the later hunks in "
> +			"the file\n"),
> +};

Interesting that "reset to HEAD" and "reset to non-HEAD" would have
to swap the direction to make it feel more natural to the users.
This is nothing new---just re-discovering that it is/was interesting.

Re: [PATCH 4/7] built-in stash: use the built-in `git add -p` if so configured

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> diff --git a/builtin/stash.c b/builtin/stash.c
> index 4e806176b0..2dafd97766 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -999,9 +999,9 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,
>  {
>  	int ret = 0;
>  	struct child_process cp_read_tree = CHILD_PROCESS_INIT;
> -	struct child_process cp_add_i = CHILD_PROCESS_INIT;
>  	struct child_process cp_diff_tree = CHILD_PROCESS_INIT;
>  	struct index_state istate = { NULL };
> +	char *old_index_env = NULL, *old_repo_index_file;
>  
>  	remove_path(stash_index_path.buf);
>  

Together with the previous step, it makes sense.  Earlier we always
spawned the scripted version.  Now we first give control to the C
code and allow it to punt to the scripted version unless it is told
to take control with USE_BUILTIN.

> @@ -1015,16 +1015,19 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,
>  	}
>  
>  	/* Find out what the user wants. */
> -	cp_add_i.git_cmd = 1;
> -	argv_array_pushl(&cp_add_i.args, "add--interactive", "--patch=stash",
> -			 "--", NULL);
> -	add_pathspecs(&cp_add_i.args, ps);
> -	argv_array_pushf(&cp_add_i.env_array, "GIT_INDEX_FILE=%s",
> -			 stash_index_path.buf);
> -	if (run_command(&cp_add_i)) {
> -		ret = -1;
> -		goto done;
> -	}
> +	old_repo_index_file = the_repository->index_file;
> +	the_repository->index_file = stash_index_path.buf;
> +	old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT));
> +	setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1);
> +
> +	ret = run_add_interactive(NULL, "--patch=stash", ps);
> +
> +	the_repository->index_file = old_repo_index_file;
> +	if (old_index_env && *old_index_env)
> +		setenv(INDEX_ENVIRONMENT, old_index_env, 1);
> +	else
> +		unsetenv(INDEX_ENVIRONMENT);
> +	FREE_AND_NULL(old_index_env);

OK.  I suspect that, as we move more stuff that used to be an
external call with one-shot environment assignments like this to an
internall call, we'd see patterns of "save away the state of this
and that environment variables, then replace them with these values"
paired with "now restore the state of those environment variables".

We might eventually want to add a helper for doing so easily, which
may make the caller look like

	extern void save_environment(struct saved_env *, ...);

	struct saved_env env;
	save_environment(&env, 
			INDEX_ENVIRONMENT, the_repository->index_file,
			NULL /* end of "VAR, val" tuples */);

	ret = run_add_interactive(NULL, "--patch=stash", ps);

        restore_environment(&env);

It might (or might not) be premature to introduce such a helper at
this stage in the series, though.

Thanks.

Re: [PATCH 0/7] stash/reset/checkout -p: optionally use the add --patch backend written in pure C

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> At this stage on the journey to a fully built-in git add, we already have
> everything we need, including the --interactive and --patch options, as long
> as the add.interactive.useBuiltin setting is set to true. This config
> setting is kind of a feature flag that is currently turned off by default,
> and will be for a while, until we get confident enough that the built-in
> version does the job, and retire the Perl script.

Except for a few things that are all minor I already mentioned, and
that I'd need to think a bit more to form an opinion on 6/7 and 7/7,
they all looked quite straight-forward.

Will queue.

Re: [PATCH 1/7] built-in add -p: prepare for patch modes other than "stage"

[Johannes Schindelin]

Hi Junio,

On Tue, 17 Dec 2019, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > +int run_add_p(struct repository *r, enum add_p_mode mode,
> > +	      const char *revision, const struct pathspec *ps);
>
> This makes readers wonder if "const struct object_id *" is more
> appropriate; "const char *revision" that holds human-readable name
> is better when the internal machinery uses it for reporting, so that
> may be what is going on here (so is the new field in add_p_state
> structure).

The main reason why this is a string instead of an object ID is to be able
to discern between the `HEAD` vs `non-HEAD` versions.

A secondary consideration was that we will want to keep the scripted
version of `add -p` as the default for now, and that needs the value as
the original string, not as the parsed object ID.

> >  #endif
> > diff --git a/add-patch.c b/add-patch.c
> > index 2c46fe5b33..8a691f07da 100644
> > --- a/add-patch.c
> > +++ b/add-patch.c
> > @@ -11,10 +11,33 @@ enum prompt_mode_type {
> >  	PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK
> >  };
> >
> > -static const char *prompt_mode[] = {
> > -	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 patch_mode {
> > +	const char *diff[4], *apply[4], *apply_check[4];
>
> Hardcoded "4" and not-quite descriptive names puzzle readers at the
> first glance.  Let's read on to see if they need any further
> improvement.

Good point. I added a code comment.

> > +	unsigned is_reverse:1, apply_for_checkout:1;
> > +	const char *prompt_mode[PROMPT_HUNK + 1];
>
> This relies on the enum value assignment (or listing) order to
> ensure that PROMPT_HUNK always comes at the end.  Perhaps that
> deserves a comment before "enum prompt_mode_type", e.g.
>
> 	+/* Keep PROMPT_HUNK at the end */
> 	 enum prompt_mode_type {
> 	 	PROMPT_MODE_CHANGE = 0, ...
> 	 };

I agree, and introduced `PROMPT_MODE_MAX` for that purpose.

>
> > +	const char *edit_hunk_hint, *help_patch_text;
> > +};
> > +
> > +static struct patch_mode patch_mode_stage = {
> > +	.diff = { "diff-files", NULL },
>
> Nice to see designated initializers used ;-)
>
> Mental note: the "diff" field is (probably) for "the command line
> to be used to generate the patch"
>
> > +	.apply = { "--cached", NULL },
> > +	.apply_check = { "--cached", NULL },
>
> Mental note: these "apply" and "apply_check" fields are (probably)
> not for the command line; unlike the "diff" field, these only have
> arguments.
>
> Mental note: if the three field names become confusing, perhaps we
> can clarify them by either (1) calling diff as diff_cmd[], or (2)
> calling the other as apply_args[] and apply_check_args[], or (3)
> rename both.

I renamed all three.

> > +	.is_reverse = 0,
>
> Wouldn't it be sufficient to apply the default initialization, just
> like it is done for apply_for_checkout bitfield?

Yep, I dropped those unnecessary initializations.

> > @@ -1310,6 +1345,9 @@ int run_add_p(struct repository *r, const struct pathspec *ps)
> >
> >  	init_add_i_state(&s.s, r);
> >
> > +	s.mode = &patch_mode_stage;
> > +	s.revision = revision;
>
> The phrase "mode_stage" may become problematic, as other modes that
> will be introduced, like "reset", "checkout" all will stage
> different contents to the index.  The only mode the machinery knows
> at this point in the series is how "add" stages contents to the
> index, so "patch_mode_add" might turn out to be a better choice of
> the phrase as we read the series along.  We'll see.
>
> > +		if (!strcmp(patch_mode, "--patch"))
> > +			mode = ADD_P_STAGE;
>
> The same comment applies to this enum token.

I renamed both to the `*_add` and `*_ADD` version, respectively.

Thanks,
Dscho

Re: [PATCH 2/7] built-in add -p: implement the "stash" and "reset" patch modes

[Johannes Schindelin]

Hi Junio,

On Tue, 17 Dec 2019, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > diff --git a/add-interactive.h b/add-interactive.h
> > index 3defa2ff3d..c278f3e26f 100644
> > --- a/add-interactive.h
> > +++ b/add-interactive.h
> > @@ -25,6 +25,8 @@ int run_add_i(struct repository *r, const struct pathspec *ps);
> >
> >  enum add_p_mode {
> >  	ADD_P_STAGE,
> > +	ADD_P_STASH,
> > +	ADD_P_RESET,
>
> As I mentioned in my review on the previous step, ADD_P_ADD would be
> more descriptive of what is going on when listed together with STASH
> and RESET here.

I made the suggested change.

> > +static struct patch_mode patch_mode_reset_head = {
> > +	.diff = { "diff-index", "--cached", NULL },
> > +	.apply = { "-R", "--cached", NULL },
> > +	.apply_check = { "-R", "--cached", NULL },
> > +	.is_reverse = 1,
> > +	.index_only = 1,
> > +	.prompt_mode = {
> > +		N_("Unstage mode change [y,n,q,a,d%s,?]? "),
> > +		N_("Unstage deletion [y,n,q,a,d%s,?]? "),
> > +		N_("Unstage this hunk [y,n,q,a,d%s,?]? "),
> > +	},
> > +	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
> > +			     "will immediately be marked for unstaging."),
> > +	.help_patch_text =
> > +		N_("y - unstage this hunk\n"
> > +		   "n - do not unstage this hunk\n"
> > +		   "q - quit; do not unstage this hunk or any of the remaining "
> > +			"ones\n"
> > +		   "a - unstage this hunk and all later hunks in the file\n"
> > +		   "d - do not unstage this hunk or any of the later hunks in "
> > +			"the file\n"),
> > +};
> > +
> > +static struct patch_mode patch_mode_reset_nothead = {
> > +	.diff = { "diff-index", "-R", "--cached", NULL },
> > +	.apply = { "--cached", NULL },
> > +	.apply_check = { "--cached", NULL },
> > +	.is_reverse = 0,
> > +	.index_only = 1,
> > +	.prompt_mode = {
> > +		N_("Apply mode change to index [y,n,q,a,d%s,?]? "),
> > +		N_("Apply deletion to index [y,n,q,a,d%s,?]? "),
> > +		N_("Apply this hunk to index [y,n,q,a,d%s,?]? "),
> > +	},
> > +	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
> > +			     "will immediately be marked for applying."),
> > +	.help_patch_text =
> > +		N_("y - apply this hunk to index\n"
> > +		   "n - do not apply this hunk to index\n"
> > +		   "q - quit; do not apply this hunk or any of the remaining "
> > +			"ones\n"
> > +		   "a - apply this hunk and all later hunks in the file\n"
> > +		   "d - do not apply this hunk or any of the later hunks in "
> > +			"the file\n"),
> > +};
>
> Interesting that "reset to HEAD" and "reset to non-HEAD" would have
> to swap the direction to make it feel more natural to the users.
> This is nothing new---just re-discovering that it is/was interesting.

Indeed. It matches the intuition, but looks incongruent.

Ciao,
Dscho

Re: [PATCH 4/7] built-in stash: use the built-in `git add -p` if so configured

[Johannes Schindelin]

Hi Junio,

On Tue, 17 Dec 2019, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > diff --git a/builtin/stash.c b/builtin/stash.c
> > index 4e806176b0..2dafd97766 100644
> > --- a/builtin/stash.c
> > +++ b/builtin/stash.c
> > @@ -999,9 +999,9 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,
> >  {
> >  	int ret = 0;
> >  	struct child_process cp_read_tree = CHILD_PROCESS_INIT;
> > -	struct child_process cp_add_i = CHILD_PROCESS_INIT;
> >  	struct child_process cp_diff_tree = CHILD_PROCESS_INIT;
> >  	struct index_state istate = { NULL };
> > +	char *old_index_env = NULL, *old_repo_index_file;
> >
> >  	remove_path(stash_index_path.buf);
> >
>
> Together with the previous step, it makes sense.  Earlier we always
> spawned the scripted version.  Now we first give control to the C
> code and allow it to punt to the scripted version unless it is told
> to take control with USE_BUILTIN.
>
> > @@ -1015,16 +1015,19 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,
> >  	}
> >
> >  	/* Find out what the user wants. */
> > -	cp_add_i.git_cmd = 1;
> > -	argv_array_pushl(&cp_add_i.args, "add--interactive", "--patch=stash",
> > -			 "--", NULL);
> > -	add_pathspecs(&cp_add_i.args, ps);
> > -	argv_array_pushf(&cp_add_i.env_array, "GIT_INDEX_FILE=%s",
> > -			 stash_index_path.buf);
> > -	if (run_command(&cp_add_i)) {
> > -		ret = -1;
> > -		goto done;
> > -	}
> > +	old_repo_index_file = the_repository->index_file;
> > +	the_repository->index_file = stash_index_path.buf;
> > +	old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT));
> > +	setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1);
> > +
> > +	ret = run_add_interactive(NULL, "--patch=stash", ps);
> > +
> > +	the_repository->index_file = old_repo_index_file;
> > +	if (old_index_env && *old_index_env)
> > +		setenv(INDEX_ENVIRONMENT, old_index_env, 1);
> > +	else
> > +		unsetenv(INDEX_ENVIRONMENT);
> > +	FREE_AND_NULL(old_index_env);
>
> OK.  I suspect that, as we move more stuff that used to be an
> external call with one-shot environment assignments like this to an
> internall call, we'd see patterns of "save away the state of this
> and that environment variables, then replace them with these values"
> paired with "now restore the state of those environment variables".
>
> We might eventually want to add a helper for doing so easily, which
> may make the caller look like
>
> 	extern void save_environment(struct saved_env *, ...);
>
> 	struct saved_env env;
> 	save_environment(&env,
> 			INDEX_ENVIRONMENT, the_repository->index_file,
> 			NULL /* end of "VAR, val" tuples */);
>
> 	ret = run_add_interactive(NULL, "--patch=stash", ps);
>
>         restore_environment(&env);
>
> It might (or might not) be premature to introduce such a helper at
> this stage in the series, though.

To be honest, I would rather have this at a different layer: I'd like
`run_add_interactive()` to take an optional path to the index, to be able
to override the default `<gitdir>/index`.

But that requires either quite a lot of changes to the Perl script, or it
needs to wait until the built-in version of `git add -p` stabilized enough
to allow retiring the Perl script.

I kinda aimed for the latter solution.

Ciao,
Dscho

[PATCH v2 0/7] stash/reset/checkout -p: optionally use the add --patch backend written in pure C

[Johannes Schindelin via GitGitGadget]

At this stage on the journey to a fully built-in git add, we already have
everything we need, including the --interactive and --patch options, as long
as the add.interactive.useBuiltin setting is set to true. This config
setting is kind of a feature flag that is currently turned off by default,
and will be for a while, until we get confident enough that the built-in
version does the job, and retire the Perl script.

However, the internal add--interactive helper is also used to back the 
--patch option of git stash, git reset, git checkout and git worktree.

This patch series (based on js/add-p-in-c) brings them "online".

Changes since v1:

 * Renamed patch_mode_stage to patch_mode_add (and ADD_P_STAGE to ADD_P_ADD
   ).
 * Renamed the fields diff, apply and apply_check (that were inherited from
   the Perl script) to diff_cmd, apply_args and apply_check_args,
   respectively.
 * Clarified the magic array size 4 of diff_cmd and friends.
 * Introduced pseudo value PROMPT_MODE_MAX to make the array size of 
   prompt_mode more obvious.
 * Got rid of the unneeded = 0 initializers for is_reverse.

Johannes Schindelin (7):
  built-in add -p: prepare for patch modes other than "stage"
  built-in add -p: implement the "stash" and "reset" patch modes
  legacy stash -p: respect the add.interactive.usebuiltin setting
  built-in stash: use the built-in `git add -p` if so configured
  built-in add -p: implement the "checkout" patch modes
  built-in add -p: implement the "worktree" patch modes
  commit --interactive: make it work with the built-in `add -i`

 add-interactive.c   |   2 +-
 add-interactive.h   |  12 +-
 add-patch.c         | 356 ++++++++++++++++++++++++++++++++++++++++----
 builtin/add.c       |  35 ++++-
 builtin/commit.c    |   8 +-
 builtin/stash.c     |  25 ++--
 git-legacy-stash.sh |   2 +-
 7 files changed, 390 insertions(+), 50 deletions(-)


base-commit: 2e4083198d1508206488af4c82093ceb6cf20f4e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-174%2Fdscho%2Fother-command-p-in-c-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-174/dscho/other-command-p-in-c-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/174

Range-diff vs v1:

 1:  8f6139f94d ! 1:  d894f8f427 built-in add -p: prepare for patch modes other than "stage"
     @@ -27,7 +27,7 @@
       			       PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL,
       			       PATHSPEC_LITERAL_PATH, "", args.argv);
      -		res = run_add_p(s->r, &ps_selected);
     -+		res = run_add_p(s->r, ADD_P_STAGE, NULL, &ps_selected);
     ++		res = run_add_p(s->r, ADD_P_ADD, NULL, &ps_selected);
       		argv_array_clear(&args);
       		clear_pathspec(&ps_selected);
       	}
     @@ -42,7 +42,7 @@
      -int run_add_p(struct repository *r, const struct pathspec *ps);
      +
      +enum add_p_mode {
     -+	ADD_P_STAGE,
     ++	ADD_P_ADD,
      +};
      +
      +int run_add_p(struct repository *r, enum add_p_mode mode,
     @@ -54,7 +54,12 @@
       --- a/add-patch.c
       +++ b/add-patch.c
      @@
     - 	PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK
     + #include "diff.h"
     + 
     + enum prompt_mode_type {
     +-	PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK
     ++	PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK,
     ++	PROMPT_MODE_MAX, /* must be last */
       };
       
      -static const char *prompt_mode[] = {
     @@ -62,17 +67,21 @@
      -	N_("Stage deletion [y,n,a,q,d%s,?]? "),
      -	N_("Stage this hunk [y,n,a,q,d%s,?]? ")
      +struct patch_mode {
     -+	const char *diff[4], *apply[4], *apply_check[4];
     ++	/*
     ++	 * The magic constant 4 is chosen such that all patch modes
     ++	 * provide enough space for three command-line arguments followed by a
     ++	 * trailing `NULL`.
     ++	 */
     ++	const char *diff_cmd[4], *apply_args[4], *apply_check_args[4];
      +	unsigned is_reverse:1, apply_for_checkout:1;
     -+	const char *prompt_mode[PROMPT_HUNK + 1];
     ++	const char *prompt_mode[PROMPT_MODE_MAX];
      +	const char *edit_hunk_hint, *help_patch_text;
      +};
      +
     -+static struct patch_mode patch_mode_stage = {
     -+	.diff = { "diff-files", NULL },
     -+	.apply = { "--cached", NULL },
     -+	.apply_check = { "--cached", NULL },
     -+	.is_reverse = 0,
     ++static struct patch_mode patch_mode_add = {
     ++	.diff_cmd = { "diff-files", NULL },
     ++	.apply_args = { "--cached", NULL },
     ++	.apply_check_args = { "--cached", NULL },
      +	.prompt_mode = {
      +		N_("Stage mode change [y,n,q,a,d%s,?]? "),
      +		N_("Stage deletion [y,n,q,a,d%s,?]? "),
     @@ -106,7 +115,7 @@
       	struct hunk *hunk = NULL;
       	int res;
       
     -+	argv_array_pushv(&args, s->mode->diff);
     ++	argv_array_pushv(&args, s->mode->diff_cmd);
      +	if (s->revision) {
      +		struct object_id oid;
      +		argv_array_push(&args,
     @@ -157,7 +166,7 @@
       	setup_child_process(s, &cp,
      -			    "apply", "--cached", "--check", NULL);
      +			    "apply", "--check", NULL);
     -+	argv_array_pushv(&cp.args, s->mode->apply_check);
     ++	argv_array_pushv(&cp.args, s->mode->apply_check_args);
       	if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0))
       		return error(_("'git apply --cached' failed"));
       
     @@ -200,7 +209,7 @@
       		discard_index(s->s.r->index);
      -		setup_child_process(s, &cp, "apply", "--cached", NULL);
      +		setup_child_process(s, &cp, "apply", NULL);
     -+		argv_array_pushv(&cp.args, s->mode->apply);
     ++		argv_array_pushv(&cp.args, s->mode->apply_args);
       		if (pipe_command(&cp, s->buf.buf, s->buf.len,
       				 NULL, 0, NULL, 0))
      -			error(_("'git apply --cached' failed"));
     @@ -222,7 +231,7 @@
       
       	init_add_i_state(&s.s, r);
       
     -+	s.mode = &patch_mode_stage;
     ++	s.mode = &patch_mode_add;
      +	s.revision = revision;
      +
       	if (discard_index(r->index) < 0 || repo_read_index(r) < 0 ||
     @@ -243,7 +252,7 @@
      -		if (strcmp(patch_mode, "--patch"))
      +
      +		if (!strcmp(patch_mode, "--patch"))
     -+			mode = ADD_P_STAGE;
     ++			mode = ADD_P_ADD;
      +		else
       			die("'%s' not yet supported in the built-in add -p",
       			    patch_mode);
 2:  846cf16e77 ! 2:  6977bede86 built-in add -p: implement the "stash" and "reset" patch modes
     @@ -14,7 +14,7 @@
      @@
       
       enum add_p_mode {
     - 	ADD_P_STAGE,
     + 	ADD_P_ADD,
      +	ADD_P_STASH,
      +	ADD_P_RESET,
       };
     @@ -25,12 +25,12 @@
       --- a/add-patch.c
       +++ b/add-patch.c
      @@
     - 
     - struct patch_mode {
     - 	const char *diff[4], *apply[4], *apply_check[4];
     + 	 * trailing `NULL`.
     + 	 */
     + 	const char *diff_cmd[4], *apply_args[4], *apply_check_args[4];
      -	unsigned is_reverse:1, apply_for_checkout:1;
      +	unsigned is_reverse:1, index_only:1, apply_for_checkout:1;
     - 	const char *prompt_mode[PROMPT_HUNK + 1];
     + 	const char *prompt_mode[PROMPT_MODE_MAX];
       	const char *edit_hunk_hint, *help_patch_text;
       };
      @@
     @@ -38,10 +38,9 @@
       };
       
      +static struct patch_mode patch_mode_stash = {
     -+	.diff = { "diff-index", "HEAD", NULL },
     -+	.apply = { "--cached", NULL },
     -+	.apply_check = { "--cached", NULL },
     -+	.is_reverse = 0,
     ++	.diff_cmd = { "diff-index", "HEAD", NULL },
     ++	.apply_args = { "--cached", NULL },
     ++	.apply_check_args = { "--cached", NULL },
      +	.prompt_mode = {
      +		N_("Stash mode change [y,n,q,a,d%s,?]? "),
      +		N_("Stash deletion [y,n,q,a,d%s,?]? "),
     @@ -60,9 +59,9 @@
      +};
      +
      +static struct patch_mode patch_mode_reset_head = {
     -+	.diff = { "diff-index", "--cached", NULL },
     -+	.apply = { "-R", "--cached", NULL },
     -+	.apply_check = { "-R", "--cached", NULL },
     ++	.diff_cmd = { "diff-index", "--cached", NULL },
     ++	.apply_args = { "-R", "--cached", NULL },
     ++	.apply_check_args = { "-R", "--cached", NULL },
      +	.is_reverse = 1,
      +	.index_only = 1,
      +	.prompt_mode = {
     @@ -83,10 +82,9 @@
      +};
      +
      +static struct patch_mode patch_mode_reset_nothead = {
     -+	.diff = { "diff-index", "-R", "--cached", NULL },
     -+	.apply = { "--cached", NULL },
     -+	.apply_check = { "--cached", NULL },
     -+	.is_reverse = 0,
     ++	.diff_cmd = { "diff-index", "-R", "--cached", NULL },
     ++	.apply_args = { "--cached", NULL },
     ++	.apply_check_args = { "--cached", NULL },
      +	.index_only = 1,
      +	.prompt_mode = {
      +		N_("Apply mode change to index [y,n,q,a,d%s,?]? "),
     @@ -112,7 +110,7 @@
       
       	init_add_i_state(&s.s, r);
       
     --	s.mode = &patch_mode_stage;
     +-	s.mode = &patch_mode_add;
      +	if (mode == ADD_P_STASH)
      +		s.mode = &patch_mode_stash;
      +	else if (mode == ADD_P_RESET) {
     @@ -121,7 +119,7 @@
      +		else
      +			s.mode = &patch_mode_reset_nothead;
      +	} else
     -+		s.mode = &patch_mode_stage;
     ++		s.mode = &patch_mode_add;
       	s.revision = revision;
       
       	if (discard_index(r->index) < 0 || repo_read_index(r) < 0 ||
     @@ -140,7 +138,7 @@
      @@
       
       		if (!strcmp(patch_mode, "--patch"))
     - 			mode = ADD_P_STAGE;
     + 			mode = ADD_P_ADD;
      +		else if (!strcmp(patch_mode, "--patch=stash"))
      +			mode = ADD_P_STASH;
      +		else if (!strcmp(patch_mode, "--patch=reset"))
 3:  cddc2cb9de = 3:  bf9b7f897a legacy stash -p: respect the add.interactive.usebuiltin setting
 4:  40c00302f6 = 4:  5051f37559 built-in stash: use the built-in `git add -p` if so configured
 5:  3c55f106c7 ! 5:  b3672b35e6 built-in add -p: implement the "checkout" patch modes
     @@ -18,7 +18,7 @@
       --- a/add-interactive.h
       +++ b/add-interactive.h
      @@
     - 	ADD_P_STAGE,
     + 	ADD_P_ADD,
       	ADD_P_STASH,
       	ADD_P_RESET,
      +	ADD_P_CHECKOUT,
     @@ -34,9 +34,9 @@
       };
       
      +static struct patch_mode patch_mode_checkout_index = {
     -+	.diff = { "diff-files", NULL },
     -+	.apply = { "-R", NULL },
     -+	.apply_check = { "-R", NULL },
     ++	.diff_cmd = { "diff-files", NULL },
     ++	.apply_args = { "-R", NULL },
     ++	.apply_check_args = { "-R", NULL },
      +	.is_reverse = 1,
      +	.prompt_mode = {
      +		N_("Discard mode change from worktree [y,n,q,a,d%s,?]? "),
     @@ -56,9 +56,9 @@
      +};
      +
      +static struct patch_mode patch_mode_checkout_head = {
     -+	.diff = { "diff-index", NULL },
     ++	.diff_cmd = { "diff-index", NULL },
      +	.apply_for_checkout = 1,
     -+	.apply_check = { "-R", NULL },
     ++	.apply_check_args = { "-R", NULL },
      +	.is_reverse = 1,
      +	.prompt_mode = {
      +		N_("Discard mode change from index and worktree [y,n,q,a,d%s,?]? "),
     @@ -78,10 +78,9 @@
      +};
      +
      +static struct patch_mode patch_mode_checkout_nothead = {
     -+	.diff = { "diff-index", "-R", NULL },
     ++	.diff_cmd = { "diff-index", "-R", NULL },
      +	.apply_for_checkout = 1,
     -+	.apply_check = { NULL },
     -+	.is_reverse = 0,
     ++	.apply_check_args = { NULL },
      +	.prompt_mode = {
      +		N_("Apply mode change to index and worktree [y,n,q,a,d%s,?]? "),
      +		N_("Apply deletion to index and worktree [y,n,q,a,d%s,?]? "),
     @@ -165,7 +164,7 @@
       
       		discard_index(s->s.r->index);
      -		setup_child_process(s, &cp, "apply", NULL);
     --		argv_array_pushv(&cp.args, s->mode->apply);
     +-		argv_array_pushv(&cp.args, s->mode->apply_args);
      -		if (pipe_command(&cp, s->buf.buf, s->buf.len,
      -				 NULL, 0, NULL, 0))
      -			error(_("'git apply' failed"));
     @@ -174,7 +173,7 @@
      +					   s->mode->is_reverse);
      +		else {
      +			setup_child_process(s, &cp, "apply", NULL);
     -+			argv_array_pushv(&cp.args, s->mode->apply);
     ++			argv_array_pushv(&cp.args, s->mode->apply_args);
      +			if (pipe_command(&cp, s->buf.buf, s->buf.len,
      +					 NULL, 0, NULL, 0))
      +				error(_("'git apply' failed"));
     @@ -194,7 +193,7 @@
      +		else
      +			s.mode = &patch_mode_checkout_nothead;
       	} else
     - 		s.mode = &patch_mode_stage;
     + 		s.mode = &patch_mode_add;
       	s.revision = revision;
      
       diff --git a/builtin/add.c b/builtin/add.c
 6:  b63fca6dab ! 6:  83e2ac6e67 built-in add -p: implement the "worktree" patch modes
     @@ -27,9 +27,9 @@
       };
       
      +static struct patch_mode patch_mode_worktree_head = {
     -+	.diff = { "diff-index", NULL },
     -+	.apply = { "-R", NULL },
     -+	.apply_check = { "-R", NULL },
     ++	.diff_cmd = { "diff-index", NULL },
     ++	.apply_args = { "-R", NULL },
     ++	.apply_check_args = { "-R", NULL },
      +	.is_reverse = 1,
      +	.prompt_mode = {
      +		N_("Discard mode change from index and worktree [y,n,q,a,d%s,?]? "),
     @@ -49,10 +49,9 @@
      +};
      +
      +static struct patch_mode patch_mode_worktree_nothead = {
     -+	.diff = { "diff-index", "-R", NULL },
     -+	.apply = { NULL },
     -+	.apply_check = { NULL },
     -+	.is_reverse = 0,
     ++	.diff_cmd = { "diff-index", "-R", NULL },
     ++	.apply_args = { NULL },
     ++	.apply_check_args = { NULL },
      +	.prompt_mode = {
      +		N_("Apply mode change to index and worktree [y,n,q,a,d%s,?]? "),
      +		N_("Apply deletion to index and worktree [y,n,q,a,d%s,?]? "),
     @@ -85,7 +84,7 @@
      +		else
      +			s.mode = &patch_mode_worktree_nothead;
       	} else
     - 		s.mode = &patch_mode_stage;
     + 		s.mode = &patch_mode_add;
       	s.revision = revision;
      
       diff --git a/builtin/add.c b/builtin/add.c
 7:  7a4c330d03 = 7:  ecb22159d8 commit --interactive: make it work with the built-in `add -i`

-- 
gitgitgadget

[PATCH v2 1/7] built-in add -p: prepare for patch modes other than "stage"

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The Perl script backing `git add -p` is used not only for that command,
but also for `git stash -p`, `git reset -p` and `git checkout -p`.

In preparation for teaching the C version of `git add -p` to support
also the latter commands, let's abstract away what is "stage" specific
into a dedicated data structure describing the differences between the
patch modes.

Finally, please note that the Perl version tries to make sure that the
diffs are only generated for the modified files. This is not actually
necessary, as the calls to Git's diff machinery already perform that
work, and perform it well. This makes it unnecessary to port the
`FILTER` field of the `%patch_modes` struct, as well as the
`get_diff_reference()` function.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c |  2 +-
 add-interactive.h |  8 +++-
 add-patch.c       | 95 ++++++++++++++++++++++++++++++++++-------------
 builtin/add.c     | 10 ++++-
 4 files changed, 85 insertions(+), 30 deletions(-)

diff --git a/add-interactive.c b/add-interactive.c
index 6a5048c83e..a5bb14f2f4 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -924,7 +924,7 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps,
 		parse_pathspec(&ps_selected,
 			       PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL,
 			       PATHSPEC_LITERAL_PATH, "", args.argv);
-		res = run_add_p(s->r, &ps_selected);
+		res = run_add_p(s->r, ADD_P_ADD, NULL, &ps_selected);
 		argv_array_clear(&args);
 		clear_pathspec(&ps_selected);
 	}
diff --git a/add-interactive.h b/add-interactive.h
index 062dc3646c..e29a769aba 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -22,6 +22,12 @@ void init_add_i_state(struct add_i_state *s, struct repository *r);
 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);
+
+enum add_p_mode {
+	ADD_P_ADD,
+};
+
+int run_add_p(struct repository *r, enum add_p_mode mode,
+	      const char *revision, const struct pathspec *ps);
 
 #endif
diff --git a/add-patch.c b/add-patch.c
index 2c46fe5b33..71356fbd9a 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -8,13 +8,41 @@
 #include "diff.h"
 
 enum prompt_mode_type {
-	PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK
+	PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK,
+	PROMPT_MODE_MAX, /* must be last */
 };
 
-static const char *prompt_mode[] = {
-	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 patch_mode {
+	/*
+	 * The magic constant 4 is chosen such that all patch modes
+	 * provide enough space for three command-line arguments followed by a
+	 * trailing `NULL`.
+	 */
+	const char *diff_cmd[4], *apply_args[4], *apply_check_args[4];
+	unsigned is_reverse:1, apply_for_checkout:1;
+	const char *prompt_mode[PROMPT_MODE_MAX];
+	const char *edit_hunk_hint, *help_patch_text;
+};
+
+static struct patch_mode patch_mode_add = {
+	.diff_cmd = { "diff-files", NULL },
+	.apply_args = { "--cached", NULL },
+	.apply_check_args = { "--cached", NULL },
+	.prompt_mode = {
+		N_("Stage mode change [y,n,q,a,d%s,?]? "),
+		N_("Stage deletion [y,n,q,a,d%s,?]? "),
+		N_("Stage this hunk [y,n,q,a,d%s,?]? ")
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for staging."),
+	.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 hunk and all later hunks in the file\n"
+		   "d - do not stage this hunk or any of the later hunks in "
+			"the file\n")
 };
 
 struct hunk_header {
@@ -47,6 +75,10 @@ struct add_p_state {
 		unsigned deleted:1, mode_change:1,binary:1;
 	} *file_diff;
 	size_t file_diff_nr;
+
+	/* patch mode */
+	struct patch_mode *mode;
+	const char *revision;
 };
 
 static void err(struct add_p_state *s, const char *fmt, ...)
@@ -162,9 +194,18 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 	struct hunk *hunk = NULL;
 	int res;
 
+	argv_array_pushv(&args, s->mode->diff_cmd);
+	if (s->revision) {
+		struct object_id oid;
+		argv_array_push(&args,
+				/* could be on an unborn branch */
+				!strcmp("HEAD", s->revision) &&
+				get_oid("HEAD", &oid) ?
+				empty_tree_oid_hex() : s->revision);
+	}
+	color_arg_index = args.argc;
 	/* Use `--no-color` explicitly, just in case `diff.color = always`. */
-	argv_array_pushl(&args, "diff-files", "-p", "--no-color", "--", NULL);
-	color_arg_index = args.argc - 2;
+	argv_array_pushl(&args, "--no-color", "-p", "--", NULL);
 	for (i = 0; i < ps->nr; i++)
 		argv_array_push(&args, ps->items[i].original);
 
@@ -382,7 +423,10 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,
 				- header->colored_extra_start;
 		}
 
-		new_offset += delta;
+		if (s->mode->is_reverse)
+			old_offset -= delta;
+		else
+			new_offset += delta;
 
 		strbuf_addf(out, "@@ -%lu,%lu +%lu,%lu @@",
 			    old_offset, header->old_count,
@@ -805,11 +849,10 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
 				"(context).\n"
 				"To remove '%c' lines, delete them.\n"
 				"Lines starting with %c will be removed.\n"),
-			      '-', '+', comment_line_char);
-	strbuf_commented_addf(&s->buf,
-			      _("If the patch applies cleanly, the edited hunk "
-				"will immediately be\n"
-				"marked for staging.\n"));
+			      s->mode->is_reverse ? '+' : '-',
+			      s->mode->is_reverse ? '-' : '+',
+			      comment_line_char);
+	strbuf_commented_addf(&s->buf, "%s", _(s->mode->edit_hunk_hint));
 	/*
 	 * TRANSLATORS: 'it' refers to the patch mentioned in the previous
 	 * messages.
@@ -890,7 +933,8 @@ static int run_apply_check(struct add_p_state *s,
 	reassemble_patch(s, file_diff, 1, &s->buf);
 
 	setup_child_process(s, &cp,
-			    "apply", "--cached", "--check", NULL);
+			    "apply", "--check", NULL);
+	argv_array_pushv(&cp.args, s->mode->apply_check_args);
 	if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0))
 		return error(_("'git apply --cached' failed"));
 
@@ -1005,13 +1049,6 @@ static size_t display_hunks(struct add_p_state *s,
 	return end_index;
 }
 
-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");
-
 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"
@@ -1097,7 +1134,8 @@ static int patch_update_file(struct add_p_state *s,
 			      (uintmax_t)hunk_index + 1,
 			      (uintmax_t)file_diff->hunk_nr);
 		color_fprintf(stdout, s->s.prompt_color,
-			      _(prompt_mode[prompt_mode_type]), s->buf.buf);
+			      _(s->mode->prompt_mode[prompt_mode_type]),
+			      s->buf.buf);
 		fflush(stdout);
 		if (strbuf_getline(&s->answer, stdin) == EOF)
 			break;
@@ -1254,7 +1292,7 @@ static int patch_update_file(struct add_p_state *s,
 			const char *p = _(help_patch_remainder), *eol = p;
 
 			color_fprintf(stdout, s->s.help_color, "%s",
-				      _(help_patch_text));
+				      _(s->mode->help_patch_text));
 
 			/*
 			 * Show only those lines of the remainder that are
@@ -1288,10 +1326,11 @@ static int patch_update_file(struct add_p_state *s,
 		reassemble_patch(s, file_diff, 0, &s->buf);
 
 		discard_index(s->s.r->index);
-		setup_child_process(s, &cp, "apply", "--cached", NULL);
+		setup_child_process(s, &cp, "apply", NULL);
+		argv_array_pushv(&cp.args, s->mode->apply_args);
 		if (pipe_command(&cp, s->buf.buf, s->buf.len,
 				 NULL, 0, NULL, 0))
-			error(_("'git apply --cached' failed"));
+			error(_("'git apply' failed"));
 		if (!repo_read_index(s->s.r))
 			repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0,
 						     1, NULL, NULL, NULL);
@@ -1301,7 +1340,8 @@ static int patch_update_file(struct add_p_state *s,
 	return quit;
 }
 
-int run_add_p(struct repository *r, const struct pathspec *ps)
+int run_add_p(struct repository *r, enum add_p_mode mode,
+	      const char *revision, const struct pathspec *ps)
 {
 	struct add_p_state s = {
 		{ r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
@@ -1310,6 +1350,9 @@ int run_add_p(struct repository *r, const struct pathspec *ps)
 
 	init_add_i_state(&s.s, r);
 
+	s.mode = &patch_mode_add;
+	s.revision = revision;
+
 	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 ||
diff --git a/builtin/add.c b/builtin/add.c
index 1deb59a642..006016267e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -194,12 +194,18 @@ int run_add_interactive(const char *revision, const char *patch_mode,
 				    &use_builtin_add_i);
 
 	if (use_builtin_add_i == 1) {
+		enum add_p_mode mode;
+
 		if (!patch_mode)
 			return !!run_add_i(the_repository, pathspec);
-		if (strcmp(patch_mode, "--patch"))
+
+		if (!strcmp(patch_mode, "--patch"))
+			mode = ADD_P_ADD;
+		else
 			die("'%s' not yet supported in the built-in add -p",
 			    patch_mode);
-		return !!run_add_p(the_repository, pathspec);
+
+		return !!run_add_p(the_repository, mode, revision, pathspec);
 	}
 
 	argv_array_push(&argv, "add--interactive");
-- 
gitgitgadget

[PATCH v2 2/7] built-in add -p: implement the "stash" and "reset" patch modes

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The `git stash` and `git reset` commands support a `--patch` option, and
both simply hand off to `git add -p` to perform that work. Let's teach
the built-in version of that command to be able to perform that work, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.h |  2 ++
 add-patch.c       | 83 ++++++++++++++++++++++++++++++++++++++++++++---
 builtin/add.c     |  4 +++
 3 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/add-interactive.h b/add-interactive.h
index e29a769aba..1f6a61326e 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -25,6 +25,8 @@ int run_add_i(struct repository *r, const struct pathspec *ps);
 
 enum add_p_mode {
 	ADD_P_ADD,
+	ADD_P_STASH,
+	ADD_P_RESET,
 };
 
 int run_add_p(struct repository *r, enum add_p_mode mode,
diff --git a/add-patch.c b/add-patch.c
index 71356fbd9a..af0a86f0f7 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -19,7 +19,7 @@ struct patch_mode {
 	 * trailing `NULL`.
 	 */
 	const char *diff_cmd[4], *apply_args[4], *apply_check_args[4];
-	unsigned is_reverse:1, apply_for_checkout:1;
+	unsigned is_reverse:1, index_only:1, apply_for_checkout:1;
 	const char *prompt_mode[PROMPT_MODE_MAX];
 	const char *edit_hunk_hint, *help_patch_text;
 };
@@ -45,6 +45,72 @@ static struct patch_mode patch_mode_add = {
 			"the file\n")
 };
 
+static struct patch_mode patch_mode_stash = {
+	.diff_cmd = { "diff-index", "HEAD", NULL },
+	.apply_args = { "--cached", NULL },
+	.apply_check_args = { "--cached", NULL },
+	.prompt_mode = {
+		N_("Stash mode change [y,n,q,a,d%s,?]? "),
+		N_("Stash deletion [y,n,q,a,d%s,?]? "),
+		N_("Stash this hunk [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for stashing."),
+	.help_patch_text =
+		N_("y - stash this hunk\n"
+		   "n - do not stash this hunk\n"
+		   "q - quit; do not stash this hunk or any of the remaining "
+			"ones\n"
+		   "a - stash this hunk and all later hunks in the file\n"
+		   "d - do not stash this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
+static struct patch_mode patch_mode_reset_head = {
+	.diff_cmd = { "diff-index", "--cached", NULL },
+	.apply_args = { "-R", "--cached", NULL },
+	.apply_check_args = { "-R", "--cached", NULL },
+	.is_reverse = 1,
+	.index_only = 1,
+	.prompt_mode = {
+		N_("Unstage mode change [y,n,q,a,d%s,?]? "),
+		N_("Unstage deletion [y,n,q,a,d%s,?]? "),
+		N_("Unstage this hunk [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for unstaging."),
+	.help_patch_text =
+		N_("y - unstage this hunk\n"
+		   "n - do not unstage this hunk\n"
+		   "q - quit; do not unstage this hunk or any of the remaining "
+			"ones\n"
+		   "a - unstage this hunk and all later hunks in the file\n"
+		   "d - do not unstage this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
+static struct patch_mode patch_mode_reset_nothead = {
+	.diff_cmd = { "diff-index", "-R", "--cached", NULL },
+	.apply_args = { "--cached", NULL },
+	.apply_check_args = { "--cached", NULL },
+	.index_only = 1,
+	.prompt_mode = {
+		N_("Apply mode change to index [y,n,q,a,d%s,?]? "),
+		N_("Apply deletion to index [y,n,q,a,d%s,?]? "),
+		N_("Apply this hunk to index [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for applying."),
+	.help_patch_text =
+		N_("y - apply this hunk to index\n"
+		   "n - do not apply this hunk to index\n"
+		   "q - quit; do not apply this hunk or any of the remaining "
+			"ones\n"
+		   "a - apply this hunk and all later hunks in the file\n"
+		   "d - do not apply this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
 struct hunk_header {
 	unsigned long old_offset, old_count, new_offset, new_count;
 	/*
@@ -1350,12 +1416,21 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 
 	init_add_i_state(&s.s, r);
 
-	s.mode = &patch_mode_add;
+	if (mode == ADD_P_STASH)
+		s.mode = &patch_mode_stash;
+	else if (mode == ADD_P_RESET) {
+		if (!revision || !strcmp(revision, "HEAD"))
+			s.mode = &patch_mode_reset_head;
+		else
+			s.mode = &patch_mode_reset_nothead;
+	} else
+		s.mode = &patch_mode_add;
 	s.revision = revision;
 
 	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 ||
+	    (!s.mode->index_only &&
+	     repo_refresh_and_write_index(r, REFRESH_QUIET, 0, 1,
+					  NULL, NULL, NULL) < 0) ||
 	    parse_diff(&s, ps) < 0) {
 		strbuf_release(&s.plain);
 		strbuf_release(&s.colored);
diff --git a/builtin/add.c b/builtin/add.c
index 006016267e..b0d6891479 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -201,6 +201,10 @@ int run_add_interactive(const char *revision, const char *patch_mode,
 
 		if (!strcmp(patch_mode, "--patch"))
 			mode = ADD_P_ADD;
+		else if (!strcmp(patch_mode, "--patch=stash"))
+			mode = ADD_P_STASH;
+		else if (!strcmp(patch_mode, "--patch=reset"))
+			mode = ADD_P_RESET;
 		else
 			die("'%s' not yet supported in the built-in add -p",
 			    patch_mode);
-- 
gitgitgadget

[PATCH v2 3/7] legacy stash -p: respect the add.interactive.usebuiltin setting

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

As `git add` traditionally did not expose the `--patch=<mode>` modes via
command-line options, the scripted version of `git stash` had to call
`git add--interactive` directly.

But this prevents the built-in `add -p` from kicking in, as
`add--interactive` is the scripted version (which does not have a
"fall-back" to the built-in version).

So let's introduce support for internal switch for `git add` that the
scripted `git stash` can use to call the appropriate backend (scripted
or built-in, depending on `add.interactive.useBuiltin`).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/add.c       | 14 ++++++++++++++
 git-legacy-stash.sh |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/builtin/add.c b/builtin/add.c
index b0d6891479..fa8bf6b10a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -29,6 +29,7 @@ static const char * const builtin_add_usage[] = {
 static int patch_interactive, add_interactive, edit_interactive;
 static int take_worktree_changes;
 static int add_renormalize;
+static int legacy_stash_p; /* support for the scripted `git stash` */
 
 struct update_callback_data {
 	int flags;
@@ -335,6 +336,8 @@ static struct option builtin_add_options[] = {
 		   N_("override the executable bit of the listed files")),
 	OPT_HIDDEN_BOOL(0, "warn-embedded-repo", &warn_on_embedded_repo,
 			N_("warn when adding an embedded repository")),
+	OPT_HIDDEN_BOOL(0, "legacy-stash-p", &legacy_stash_p,
+			N_("backend for `git stash -p`")),
 	OPT_END(),
 };
 
@@ -431,6 +434,17 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		add_interactive = 1;
 	if (add_interactive)
 		exit(interactive_add(argc - 1, argv + 1, prefix, patch_interactive));
+	if (legacy_stash_p) {
+		struct pathspec pathspec;
+
+		parse_pathspec(&pathspec, 0,
+			PATHSPEC_PREFER_FULL |
+			PATHSPEC_SYMLINK_LEADING_PATH |
+			PATHSPEC_PREFIX_ORIGIN,
+			prefix, argv);
+
+		return run_add_interactive(NULL, "--patch=stash", &pathspec);
+	}
 
 	if (edit_interactive)
 		return(edit_patch(argc, argv, prefix));
diff --git a/git-legacy-stash.sh b/git-legacy-stash.sh
index 07ad4a5459..ed039dfcbb 100755
--- a/git-legacy-stash.sh
+++ b/git-legacy-stash.sh
@@ -206,7 +206,7 @@ create_stash () {
 
 		# find out what the user wants
 		GIT_INDEX_FILE="$TMP-index" \
-			git add--interactive --patch=stash -- "$@" &&
+			git add --legacy-stash-p -- "$@" &&
 
 		# state of the working tree
 		w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) ||
-- 
gitgitgadget

[PATCH v2 4/7] built-in stash: use the built-in `git add -p` if so configured

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The scripted version of `git stash` called directly into the Perl script
`git-add--interactive.perl`, and this was faithfully converted to C.

However, we have a much better way to do this now: call the internal API
directly, which will now incidentally also respect the
`add.interactive.useBuiltin` setting. Let's just do this.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/stash.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index 4e806176b0..2dafd97766 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -999,9 +999,9 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,
 {
 	int ret = 0;
 	struct child_process cp_read_tree = CHILD_PROCESS_INIT;
-	struct child_process cp_add_i = CHILD_PROCESS_INIT;
 	struct child_process cp_diff_tree = CHILD_PROCESS_INIT;
 	struct index_state istate = { NULL };
+	char *old_index_env = NULL, *old_repo_index_file;
 
 	remove_path(stash_index_path.buf);
 
@@ -1015,16 +1015,19 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,
 	}
 
 	/* Find out what the user wants. */
-	cp_add_i.git_cmd = 1;
-	argv_array_pushl(&cp_add_i.args, "add--interactive", "--patch=stash",
-			 "--", NULL);
-	add_pathspecs(&cp_add_i.args, ps);
-	argv_array_pushf(&cp_add_i.env_array, "GIT_INDEX_FILE=%s",
-			 stash_index_path.buf);
-	if (run_command(&cp_add_i)) {
-		ret = -1;
-		goto done;
-	}
+	old_repo_index_file = the_repository->index_file;
+	the_repository->index_file = stash_index_path.buf;
+	old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT));
+	setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1);
+
+	ret = run_add_interactive(NULL, "--patch=stash", ps);
+
+	the_repository->index_file = old_repo_index_file;
+	if (old_index_env && *old_index_env)
+		setenv(INDEX_ENVIRONMENT, old_index_env, 1);
+	else
+		unsetenv(INDEX_ENVIRONMENT);
+	FREE_AND_NULL(old_index_env);
 
 	/* State of the working tree. */
 	if (write_index_as_tree(&info->w_tree, &istate, stash_index_path.buf, 0,
-- 
gitgitgadget

[PATCH v2 5/7] built-in add -p: implement the "checkout" patch modes

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

This patch teaches the built-in `git add -p` machinery all the tricks it
needs to know in order to act as the work horse for `git checkout -p`.

Apart from the minor changes (slightly reworded messages, different
`diff` and `apply --check` invocations), it requires a new function to
actually apply the changes, as `git checkout -p` is a bit special in
that respect: when the desired changes do not apply to the index, but
apply to the work tree, Git does not fail straight away, but asks the
user whether to apply the changes to the worktree at least.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.h |   1 +
 add-patch.c       | 138 ++++++++++++++++++++++++++++++++++++++++++++--
 builtin/add.c     |   5 +-
 3 files changed, 137 insertions(+), 7 deletions(-)

diff --git a/add-interactive.h b/add-interactive.h
index 1f6a61326e..77907f6e21 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -27,6 +27,7 @@ enum add_p_mode {
 	ADD_P_ADD,
 	ADD_P_STASH,
 	ADD_P_RESET,
+	ADD_P_CHECKOUT,
 };
 
 int run_add_p(struct repository *r, enum add_p_mode mode,
diff --git a/add-patch.c b/add-patch.c
index af0a86f0f7..ec5116c187 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -111,6 +111,71 @@ static struct patch_mode patch_mode_reset_nothead = {
 			"the file\n"),
 };
 
+static struct patch_mode patch_mode_checkout_index = {
+	.diff_cmd = { "diff-files", NULL },
+	.apply_args = { "-R", NULL },
+	.apply_check_args = { "-R", NULL },
+	.is_reverse = 1,
+	.prompt_mode = {
+		N_("Discard mode change from worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard deletion from worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard this hunk from worktree [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for discarding."),
+	.help_patch_text =
+		N_("y - discard this hunk from worktree\n"
+		   "n - do not discard this hunk from worktree\n"
+		   "q - quit; do not discard this hunk or any of the remaining "
+			"ones\n"
+		   "a - discard this hunk and all later hunks in the file\n"
+		   "d - do not discard this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
+static struct patch_mode patch_mode_checkout_head = {
+	.diff_cmd = { "diff-index", NULL },
+	.apply_for_checkout = 1,
+	.apply_check_args = { "-R", NULL },
+	.is_reverse = 1,
+	.prompt_mode = {
+		N_("Discard mode change from index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard deletion from index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard this hunk from index and worktree [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for discarding."),
+	.help_patch_text =
+		N_("y - discard this hunk from index and worktree\n"
+		   "n - do not discard this hunk from index and worktree\n"
+		   "q - quit; do not discard this hunk or any of the remaining "
+			"ones\n"
+		   "a - discard this hunk and all later hunks in the file\n"
+		   "d - do not discard this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
+static struct patch_mode patch_mode_checkout_nothead = {
+	.diff_cmd = { "diff-index", "-R", NULL },
+	.apply_for_checkout = 1,
+	.apply_check_args = { NULL },
+	.prompt_mode = {
+		N_("Apply mode change to index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Apply deletion to index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Apply this hunk to index and worktree [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for applying."),
+	.help_patch_text =
+		N_("y - apply this hunk to index and worktree\n"
+		   "n - do not apply this hunk to index and worktree\n"
+		   "q - quit; do not apply this hunk or any of the remaining "
+			"ones\n"
+		   "a - apply this hunk and all later hunks in the file\n"
+		   "d - do not apply this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
 struct hunk_header {
 	unsigned long old_offset, old_count, new_offset, new_count;
 	/*
@@ -1067,6 +1132,57 @@ static int edit_hunk_loop(struct add_p_state *s,
 	}
 }
 
+static int apply_for_checkout(struct add_p_state *s, struct strbuf *diff,
+			      int is_reverse)
+{
+	const char *reverse = is_reverse ? "-R" : NULL;
+	struct child_process check_index = CHILD_PROCESS_INIT;
+	struct child_process check_worktree = CHILD_PROCESS_INIT;
+	struct child_process apply_index = CHILD_PROCESS_INIT;
+	struct child_process apply_worktree = CHILD_PROCESS_INIT;
+	int applies_index, applies_worktree;
+
+	setup_child_process(s, &check_index,
+			    "apply", "--cached", "--check", reverse, NULL);
+	applies_index = !pipe_command(&check_index, diff->buf, diff->len,
+				      NULL, 0, NULL, 0);
+
+	setup_child_process(s, &check_worktree,
+			    "apply", "--check", reverse, NULL);
+	applies_worktree = !pipe_command(&check_worktree, diff->buf, diff->len,
+					 NULL, 0, NULL, 0);
+
+	if (applies_worktree && applies_index) {
+		setup_child_process(s, &apply_index,
+				    "apply", "--cached", reverse, NULL);
+		pipe_command(&apply_index, diff->buf, diff->len,
+			     NULL, 0, NULL, 0);
+
+		setup_child_process(s, &apply_worktree,
+				    "apply", reverse, NULL);
+		pipe_command(&apply_worktree, diff->buf, diff->len,
+			     NULL, 0, NULL, 0);
+
+		return 1;
+	}
+
+	if (!applies_index) {
+		err(s, _("The selected hunks do not apply to the index!"));
+		if (prompt_yesno(s, _("Apply them to the worktree "
+					  "anyway? ")) > 0) {
+			setup_child_process(s, &apply_worktree,
+					    "apply", reverse, NULL);
+			return pipe_command(&apply_worktree, diff->buf,
+					    diff->len, NULL, 0, NULL, 0);
+		}
+		err(s, _("Nothing was applied.\n"));
+	} else
+		/* As a last resort, show the diff to the user */
+		fwrite(diff->buf, diff->len, 1, stderr);
+
+	return 0;
+}
+
 #define SUMMARY_HEADER_WIDTH 20
 #define SUMMARY_LINE_WIDTH 80
 static void summarize_hunk(struct add_p_state *s, struct hunk *hunk,
@@ -1392,11 +1508,16 @@ static int patch_update_file(struct add_p_state *s,
 		reassemble_patch(s, file_diff, 0, &s->buf);
 
 		discard_index(s->s.r->index);
-		setup_child_process(s, &cp, "apply", NULL);
-		argv_array_pushv(&cp.args, s->mode->apply_args);
-		if (pipe_command(&cp, s->buf.buf, s->buf.len,
-				 NULL, 0, NULL, 0))
-			error(_("'git apply' failed"));
+		if (s->mode->apply_for_checkout)
+			apply_for_checkout(s, &s->buf,
+					   s->mode->is_reverse);
+		else {
+			setup_child_process(s, &cp, "apply", NULL);
+			argv_array_pushv(&cp.args, s->mode->apply_args);
+			if (pipe_command(&cp, s->buf.buf, s->buf.len,
+					 NULL, 0, NULL, 0))
+				error(_("'git apply' failed"));
+		}
 		if (!repo_read_index(s->s.r))
 			repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0,
 						     1, NULL, NULL, NULL);
@@ -1423,6 +1544,13 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 			s.mode = &patch_mode_reset_head;
 		else
 			s.mode = &patch_mode_reset_nothead;
+	} else if (mode == ADD_P_CHECKOUT) {
+		if (!revision)
+			s.mode = &patch_mode_checkout_index;
+		else if (!strcmp(revision, "HEAD"))
+			s.mode = &patch_mode_checkout_head;
+		else
+			s.mode = &patch_mode_checkout_nothead;
 	} else
 		s.mode = &patch_mode_add;
 	s.revision = revision;
diff --git a/builtin/add.c b/builtin/add.c
index fa8bf6b10a..191856b036 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -206,9 +206,10 @@ int run_add_interactive(const char *revision, const char *patch_mode,
 			mode = ADD_P_STASH;
 		else if (!strcmp(patch_mode, "--patch=reset"))
 			mode = ADD_P_RESET;
+		else if (!strcmp(patch_mode, "--patch=checkout"))
+			mode = ADD_P_CHECKOUT;
 		else
-			die("'%s' not yet supported in the built-in add -p",
-			    patch_mode);
+			die("'%s' not supported", patch_mode);
 
 		return !!run_add_p(the_repository, mode, revision, pathspec);
 	}
-- 
gitgitgadget

[PATCH v2 6/7] built-in add -p: implement the "worktree" patch modes

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

This is a straight-forward port of 2f0896ec3ad4 (restore: support
--patch, 2019-04-25) which added support for `git restore -p`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.h |  1 +
 add-patch.c       | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 builtin/add.c     |  2 ++
 3 files changed, 53 insertions(+)

diff --git a/add-interactive.h b/add-interactive.h
index 77907f6e21..b2f23479c5 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -28,6 +28,7 @@ enum add_p_mode {
 	ADD_P_STASH,
 	ADD_P_RESET,
 	ADD_P_CHECKOUT,
+	ADD_P_WORKTREE,
 };
 
 int run_add_p(struct repository *r, enum add_p_mode mode,
diff --git a/add-patch.c b/add-patch.c
index ec5116c187..46c6c183d5 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -176,6 +176,49 @@ static struct patch_mode patch_mode_checkout_nothead = {
 			"the file\n"),
 };
 
+static struct patch_mode patch_mode_worktree_head = {
+	.diff_cmd = { "diff-index", NULL },
+	.apply_args = { "-R", NULL },
+	.apply_check_args = { "-R", NULL },
+	.is_reverse = 1,
+	.prompt_mode = {
+		N_("Discard mode change from index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard deletion from index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard this hunk from index and worktree [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for discarding."),
+	.help_patch_text =
+		N_("y - discard this hunk from worktree\n"
+		   "n - do not discard this hunk from worktree\n"
+		   "q - quit; do not discard this hunk or any of the remaining "
+			"ones\n"
+		   "a - discard this hunk and all later hunks in the file\n"
+		   "d - do not discard this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
+static struct patch_mode patch_mode_worktree_nothead = {
+	.diff_cmd = { "diff-index", "-R", NULL },
+	.apply_args = { NULL },
+	.apply_check_args = { NULL },
+	.prompt_mode = {
+		N_("Apply mode change to index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Apply deletion to index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Apply this hunk to index and worktree [y,n,q,a,d%s,?]? "),
+	},
+	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
+			     "will immediately be marked for applying."),
+	.help_patch_text =
+		N_("y - apply this hunk to worktree\n"
+		   "n - do not apply this hunk to worktree\n"
+		   "q - quit; do not apply this hunk or any of the remaining "
+			"ones\n"
+		   "a - apply this hunk and all later hunks in the file\n"
+		   "d - do not apply this hunk or any of the later hunks in "
+			"the file\n"),
+};
+
 struct hunk_header {
 	unsigned long old_offset, old_count, new_offset, new_count;
 	/*
@@ -1551,6 +1594,13 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 			s.mode = &patch_mode_checkout_head;
 		else
 			s.mode = &patch_mode_checkout_nothead;
+	} else if (mode == ADD_P_WORKTREE) {
+		if (!revision)
+			s.mode = &patch_mode_checkout_index;
+		else if (!strcmp(revision, "HEAD"))
+			s.mode = &patch_mode_worktree_head;
+		else
+			s.mode = &patch_mode_worktree_nothead;
 	} else
 		s.mode = &patch_mode_add;
 	s.revision = revision;
diff --git a/builtin/add.c b/builtin/add.c
index 191856b036..b5927105aa 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -208,6 +208,8 @@ int run_add_interactive(const char *revision, const char *patch_mode,
 			mode = ADD_P_RESET;
 		else if (!strcmp(patch_mode, "--patch=checkout"))
 			mode = ADD_P_CHECKOUT;
+		else if (!strcmp(patch_mode, "--patch=worktree"))
+			mode = ADD_P_WORKTREE;
 		else
 			die("'%s' not supported", patch_mode);
 
-- 
gitgitgadget

[PATCH v2 7/7] commit --interactive: make it work with the built-in `add -i`

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The built-in `git add -i` machinery obviously has its `the_repository`
structure initialized at the point where `cmd_commit()` calls it, and
therefore does not look at the environment variable `GIT_INDEX_FILE`.

But when being called from `commit --interactive`, it has to, because
the index was already locked in that case, and we want to ask the
interactive add machinery to work on the `index.lock` file instead of
the `index` file.

Technically, we could teach `run_add_i()`, or for that matter
`run_add_p()`, to look specifically at that environment variable, but
the entire idea of passing in a parameter of type `struct repository *`
is to allow working on multiple repositories (and their index files)
independently.

So let's instead override the `index_file` field of that structure
temporarily.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/commit.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index e588bc6ad3..32ffc7beee 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -347,7 +347,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 		die(_("index file corrupt"));
 
 	if (interactive) {
-		char *old_index_env = NULL;
+		char *old_index_env = NULL, *old_repo_index_file;
 		hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
 
 		refresh_cache_or_die(refresh_flags);
@@ -355,12 +355,16 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 		if (write_locked_index(&the_index, &index_lock, 0))
 			die(_("unable to create temporary index"));
 
+		old_repo_index_file = the_repository->index_file;
+		the_repository->index_file =
+			(char *)get_lock_file_path(&index_lock);
 		old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT));
-		setenv(INDEX_ENVIRONMENT, get_lock_file_path(&index_lock), 1);
+		setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1);
 
 		if (interactive_add(argc, argv, prefix, patch_interactive) != 0)
 			die(_("interactive add failed"));
 
+		the_repository->index_file = old_repo_index_file;
 		if (old_index_env && *old_index_env)
 			setenv(INDEX_ENVIRONMENT, old_index_env, 1);
 		else
-- 
gitgitgadget