stash/reset/checkout -p: optionally use the add --patch backend written in pure C

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

From: Johannes Schindelin <[email protected]>

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.

---
 add-interactive.c |  2 +-
 add-interactive.h |  8 ++++-
 add-patch.c       | 88 +++++++++++++++++++++++++++++++++--------------
 builtin/add.c     | 10 ++++--
 4 files changed, 79 insertions(+), 29 deletions(-)

index 6a5048c83e..0e753d2acc 100644
@@ -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);
 	}

index 062dc3646c..3defa2ff3d 100644
@@ -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

index 1deb59a642..05b727a426 100644
@@ -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

From: Johannes Schindelin <[email protected]>

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).

---
 builtin/add.c       | 14 ++++++++++++++
 git-legacy-stash.sh |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

index 583d8aab0d..f238e8b623 100644
@@ -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));

index 07ad4a5459..ed039dfcbb 100755
@@ -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

From: Johannes Schindelin <[email protected]>

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.

---
 add-interactive.h |  2 ++
 add-patch.c       | 85 ++++++++++++++++++++++++++++++++++++++++++++---
 builtin/add.c     |  4 +++
 3 files changed, 87 insertions(+), 4 deletions(-)

index 3defa2ff3d..c278f3e26f 100644
@@ -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,

index 05b727a426..583d8aab0d 100644
@@ -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

From: Johannes Schindelin <[email protected]>

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.

---
 builtin/stash.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

index 4e806176b0..2dafd97766 100644
@@ -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

From: Johannes Schindelin <[email protected]>

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.

---
 add-interactive.h |   1 +
 add-patch.c       | 139 ++++++++++++++++++++++++++++++++++++++++++++--
 builtin/add.c     |   5 +-
 3 files changed, 138 insertions(+), 7 deletions(-)

index c278f3e26f..f865f1e8ca 100644
@@ -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,

index f238e8b623..f4d6eb9e06 100644
@@ -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

From: Johannes Schindelin <[email protected]>

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.

---
 builtin/commit.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

index e588bc6ad3..32ffc7beee 100644
@@ -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

From: Johannes Schindelin <[email protected]>

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

---
 add-interactive.h |  1 +
 add-patch.c       | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 builtin/add.c     |  2 ++
 3 files changed, 54 insertions(+)

index f865f1e8ca..4895ed1df5 100644
@@ -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,

index f4d6eb9e06..6fa2b2dd17 100644
@@ -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

"Johannes Schindelin via GitGitGadget" <[email protected]>

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

"Johannes Schindelin via GitGitGadget" <[email protected]>

"Johannes Schindelin via GitGitGadget" <[email protected]>

	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);

"Johannes Schindelin via GitGitGadget" <[email protected]>

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.

Hi Junio,

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.

Good point. I added a code comment.

I agree, and introduced PROMPT_MODE_MAX for that purpose.

I renamed all three.

Yep, I dropped those unnecessary initializations.

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

Thanks,
Dscho

Hi Junio,

I made the suggested change.

Indeed. It matches the intuition, but looks incongruent.

Ciao,
Dscho

Hi Junio,

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

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

From: Johannes Schindelin <[email protected]>

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.

---
 add-interactive.c |  2 +-
 add-interactive.h |  8 +++-
 add-patch.c       | 95 ++++++++++++++++++++++++++++++++++-------------
 builtin/add.c     | 10 ++++-
 4 files changed, 85 insertions(+), 30 deletions(-)

index 6a5048c83e..a5bb14f2f4 100644
@@ -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);
 	}

index 062dc3646c..e29a769aba 100644
@@ -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

index 1deb59a642..006016267e 100644
@@ -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

From: Johannes Schindelin <[email protected]>

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).

---
 builtin/add.c       | 14 ++++++++++++++
 git-legacy-stash.sh |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

index b0d6891479..fa8bf6b10a 100644
@@ -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));

index 07ad4a5459..ed039dfcbb 100755
@@ -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

From: Johannes Schindelin <[email protected]>

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.

---
 add-interactive.h |  2 ++
 add-patch.c       | 83 ++++++++++++++++++++++++++++++++++++++++++++---
 builtin/add.c     |  4 +++
 3 files changed, 85 insertions(+), 4 deletions(-)

index e29a769aba..1f6a61326e 100644
@@ -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,

index 006016267e..b0d6891479 100644
@@ -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

From: Johannes Schindelin <[email protected]>

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.

---
 builtin/commit.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

index e588bc6ad3..32ffc7beee 100644
@@ -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

From: Johannes Schindelin <[email protected]>

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.

---
 add-interactive.h |   1 +
 add-patch.c       | 138 ++++++++++++++++++++++++++++++++++++++++++++--
 builtin/add.c     |   5 +-
 3 files changed, 137 insertions(+), 7 deletions(-)

index 1f6a61326e..77907f6e21 100644
@@ -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,

index fa8bf6b10a..191856b036 100644
@@ -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

From: Johannes Schindelin <[email protected]>

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.

---
 builtin/stash.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

index 4e806176b0..2dafd97766 100644
@@ -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

From: Johannes Schindelin <[email protected]>

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

---
 add-interactive.h |  1 +
 add-patch.c       | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 builtin/add.c     |  2 ++
 3 files changed, 53 insertions(+)

index 77907f6e21..b2f23479c5 100644
@@ -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,

index 191856b036..b5927105aa 100644
@@ -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