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

From: "Johannes Schindelin via GitGitGadget" <[email-masked]>
Date: Sat, 21 Dec 2019 21:57:09 +0000

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