threads / patch / 52271

patch, 8 partsbuilt-in add -i: allow filtering the modified files list

Subject: [PATCH 1/8] built-in add -i: allow filtering the modified files list

## tl;dr

29 messages between Nov 15, 2019 and Nov 29, 2019. Diffs are folded; open one to read it.

replies: 28people: 2as markdown or json

Johannes Schindelin via GitGitGadget· Nov 15, 2019, 12:36 UTC · lore

[PATCH 0/8] build-in add -i: implement all commands in the main loop

Based on the js/builtin-add-i branch, this patch series implements the rest of the commands in git add -i's main loop: update, revert, add_untracked, patch, diff, and quit. Apart from quit, these commands are all very similar in that they first build a list of files, display it, let the user choose which ones to act on, and then perform the action.

Note that the patch command is not actually converted to C, not completely at least: the built-in version simply hands off to git add--interactive after letting the user select which files to act on.

The reason for this omission is practicality. Out of the 1,800+ lines of git-add--interactive.perl, over a thousand deal just with the git add -p part. I did convert that functionality already (to be contributed in a separate patch series, see https://github.com/gitgitgadget/git/pull/173), discovering that there is so little overlap between the git add --patch part and the rest of git add --interactive that I could put the former into a totally different file: add-patch.c.

Johannes Schindelin (8):
  built-in add -i: allow filtering the modified files list
  built-in add -i: prepare for multi-selection commands
  built-in add -i: implement the `update` command
  built-in add -i: re-implement `revert` in C
  built-in add -i: re-implement `add-untracked` in C
  built-in add -i: implement the `patch` command
  built-in add -i: re-implement the `diff` command
  built-in add -i: offer the `quit` command
 add-interactive.c | 608 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 553 insertions(+), 55 deletions(-)
base-commit: 2b5d5c1524d62add395da2b0ef50bbbe342362e4
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-171%2Fdscho%2Fadd-i-in-c-all-except-patch-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-171/dscho/add-i-in-c-all-except-patch-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/171
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 15, 2019, 12:36 UTC · re: Johannes Schindelin via GitGitGadget · lore
From: Johannes Schindelin <johannes.schindelin@gmx.de>

In the `update` command of `git add -i`, we are primarily interested in the list of modified files that have worktree (i.e. unstaged) changes.

At the same time, we need to determine _also_ the staged changes, to be able to produce the full added/deleted information.

The Perl script version of `git add -i` has a parameter of the `list_modified()` function for that matter. In C, we can be a lot more precise, using an `enum`.

The C implementation of the filter also has an easier time to avoid unnecessary work, simply by using an adaptive order of the `diff-index` and `diff-files` phases, and then skipping files in the second phase when they have not been seen in the first phase.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)
Show changes to add-interactive.c +22 −3
diff --git a/add-interactive.c b/add-interactive.c
index 5d89863bab..8ec930ac15 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -347,6 +347,7 @@ struct collection_status {
 
 	const char *reference;
 
+	unsigned skip_unseen:1;
 	struct string_list *files;
 	struct hashmap file_map;
 };
@@ -374,6 +375,9 @@ static void collect_changes_cb(struct diff_queue_struct *q,
 		entry = hashmap_get_entry_from_hash(&s->file_map, hash, name,
 						    struct pathname_entry, ent);
 		if (!entry) {
+			if (s->skip_unseen)
+				continue;
+
 			add_file_item(s->files, name);
 
 			entry = xcalloc(sizeof(*entry), 1);
@@ -395,13 +399,22 @@ static void collect_changes_cb(struct diff_queue_struct *q,
 	free_diffstat_info(&stat);
 }
 
-static int get_modified_files(struct repository *r, struct string_list *files,
+enum modified_files_filter {
+	NO_FILTER = 0,
+	WORKTREE_ONLY = 1,
+	INDEX_ONLY = 2,
+};
+
+static int get_modified_files(struct repository *r,
+			      enum modified_files_filter filter,
+			      struct string_list *files,
 			      const struct pathspec *ps)
 {
 	struct object_id head_oid;
 	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
 					     &head_oid, NULL);
 	struct collection_status s = { FROM_WORKTREE };
+	int i;
 
 	if (discard_index(r->index) < 0 ||
 	    repo_read_index_preload(r, ps, 0) < 0)
@@ -411,10 +424,16 @@ static int get_modified_files(struct repository *r, struct string_list *files,
 	s.files = files;
 	hashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0);
 
-	for (s.phase = FROM_WORKTREE; s.phase <= FROM_INDEX; s.phase++) {
+	for (i = 0; i < 2; i++) {
 		struct rev_info rev;
 		struct setup_revision_opt opt = { 0 };
 
+		if (filter == INDEX_ONLY)
+			s.phase = i ? FROM_WORKTREE : FROM_INDEX;
+		else
+			s.phase = i ? FROM_INDEX : FROM_WORKTREE;
+		s.skip_unseen = filter && i;
+
 		opt.def = is_initial ?
 			empty_tree_oid_hex() : oid_to_hex(&head_oid);
 
@@ -498,7 +517,7 @@ static void print_file_item(int i, struct string_list_item *item,
 static int run_status(struct add_i_state *s, const struct pathspec *ps,
 		      struct string_list *files, struct list_options *opts)
 {
-	if (get_modified_files(s->r, files, ps) < 0)
+	if (get_modified_files(s->r, NO_FILTER, files, ps) < 0)
 		return -1;
 
 	list(s, files, opts);
-- 
gitgitgadget
Junio C Hamano· Nov 21, 2019, 08:06 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH 1/8] built-in add -i: allow filtering the modified files list

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

Show 15 quoted lines
> +enum modified_files_filter {
> +	NO_FILTER = 0,
> +	WORKTREE_ONLY = 1,
> +	INDEX_ONLY = 2,
> +};
> +
> +static int get_modified_files(struct repository *r,
> +			      enum modified_files_filter filter,
> +			      struct string_list *files,
>  			      const struct pathspec *ps)
>  {
>  	struct object_id head_oid;
>  	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
>  					     &head_oid, NULL);
>  	struct collection_status s = { FROM_WORKTREE };
Will have a comment on this later.
Show 18 quoted lines
> +	int i;
>  
>  	if (discard_index(r->index) < 0 ||
>  	    repo_read_index_preload(r, ps, 0) < 0)
> @@ -411,10 +424,16 @@ static int get_modified_files(struct repository *r, struct string_list *files,
>  	s.files = files;
>  	hashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0);
>  
> -	for (s.phase = FROM_WORKTREE; s.phase <= FROM_INDEX; s.phase++) {
> +	for (i = 0; i < 2; i++) {
>  		struct rev_info rev;
>  		struct setup_revision_opt opt = { 0 };
>  
> +		if (filter == INDEX_ONLY)
> +			s.phase = i ? FROM_WORKTREE : FROM_INDEX;
> +		else
> +			s.phase = i ? FROM_INDEX : FROM_WORKTREE;
> +		s.skip_unseen = filter && i;
;-)

Looks somewhat crazy but it works---when the caller wants to do 'index-only' for example we are not interested in paths that did not appear in the INDEX side of the diff, so we run FROM_INDEX diff first and then the let next one (i.e. FROM_WORKTREE diff) be skipped for paths that are not in the result of the first one. To me personally, I would find the tertially expession written like this

	s.phase = (i == 0) ? FROM_INDEX : FROM_WORKTREE;

much easier to follow, as it matches the order which ones are done first.

Also I notice two things.
 - It used to make 100% sense to call this field .phase because we
   always processed the first phase and then on to the second phase,
   where the first one was called WORKTREE and the second one was
   called INDEX.  In the new world order after this step, the name
   .phase no longer makes any sense.  Sometimes we run diff-files in
   phase 0 and diff-index in phase 1, but some other times we run
   diff-index in phase 0 and diff-files in phase 0.  The variable
   that has the closest meaning to the 'phase' is the newly
   introduced 'i'.
 - The initialization of the local "struct collection_status s" at
   the beginning of the function still uses .phase = FROM_WORKTREE
   which might be somewhat misleading.  We cannot remove the whole
   initialization, as the original used to nul initialize the other
   fields in this structure and I suspect the code still relies on
   it, but the initialization of .phase in particular no longer has
   any effect; it always is assigned inside the loop before the
   field gets used.
>  		opt.def = is_initial ?
>  			empty_tree_oid_hex() : oid_to_hex(&head_oid);

By the way, this is not a new issue introduced by this patch, but I notice copy_pathspec() is used twice on the same rev.prune_data in this functino. Who is responsible for releasing the resource held in this struct?

Johannes Schindelin· Nov 25, 2019, 15:20 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/8] built-in add -i: allow filtering the modified files list

Hi Junio,
On Thu, 21 Nov 2019, Junio C Hamano wrote:
Show 20 quoted lines
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > +enum modified_files_filter {
> > +	NO_FILTER = 0,
> > +	WORKTREE_ONLY = 1,
> > +	INDEX_ONLY = 2,
> > +};
> > +
> > +static int get_modified_files(struct repository *r,
> > +			      enum modified_files_filter filter,
> > +			      struct string_list *files,
> >  			      const struct pathspec *ps)
> >  {
> >  	struct object_id head_oid;
> >  	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
> >  					     &head_oid, NULL);
> >  	struct collection_status s = { FROM_WORKTREE };
>
> Will have a comment on this later.

Yes, you're right, this initialization does not make sense. I changed it to `{ 0 }` because I still want the struct to be zero'ed out.

Show 32 quoted lines
> > +	int i;
> >
> >  	if (discard_index(r->index) < 0 ||
> >  	    repo_read_index_preload(r, ps, 0) < 0)
> > @@ -411,10 +424,16 @@ static int get_modified_files(struct repository *r, struct string_list *files,
> >  	s.files = files;
> >  	hashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0);
> >
> > -	for (s.phase = FROM_WORKTREE; s.phase <= FROM_INDEX; s.phase++) {
> > +	for (i = 0; i < 2; i++) {
> >  		struct rev_info rev;
> >  		struct setup_revision_opt opt = { 0 };
> >
> > +		if (filter == INDEX_ONLY)
> > +			s.phase = i ? FROM_WORKTREE : FROM_INDEX;
> > +		else
> > +			s.phase = i ? FROM_INDEX : FROM_WORKTREE;
> > +		s.skip_unseen = filter && i;
>
> ;-)
>
> Looks somewhat crazy but it works---when the caller wants to do
> 'index-only' for example we are not interested in paths that did not
> appear in the INDEX side of the diff, so we run FROM_INDEX diff first
> and then the let next one (i.e. FROM_WORKTREE diff) be skipped for
> paths that are not in the result of the first one.  To me personally,
> I would find the tertially expession written like this
>
> 	s.phase = (i == 0) ? FROM_INDEX : FROM_WORKTREE;
>
> much easier to follow, as it matches the order which ones are done
> first.

Sure, that reverses the order, but it makes it more intuitive because `i == 0` happens first. Changed.

Show 11 quoted lines
> Also I notice two things.
>
>  - It used to make 100% sense to call this field .phase because we
>    always processed the first phase and then on to the second phase,
>    where the first one was called WORKTREE and the second one was
>    called INDEX.  In the new world order after this step, the name
>    .phase no longer makes any sense.  Sometimes we run diff-files in
>    phase 0 and diff-index in phase 1, but some other times we run
>    diff-index in phase 0 and diff-files in phase 0.  The variable
>    that has the closest meaning to the 'phase' is the newly
>    introduced 'i'.

I renamed it to `mode` in this commit. While I usually frown on renaming fields in the same commit as introducing changes in behavior, I think in this case it's kind of okay because it does not add many lines.

Show 17 quoted lines
>  - The initialization of the local "struct collection_status s" at
>    the beginning of the function still uses .phase = FROM_WORKTREE
>    which might be somewhat misleading.  We cannot remove the whole
>    initialization, as the original used to nul initialize the other
>    fields in this structure and I suspect the code still relies on
>    it, but the initialization of .phase in particular no longer has
>    any effect; it always is assigned inside the loop before the
>    field gets used.
>
>
> >  		opt.def = is_initial ?
> >  			empty_tree_oid_hex() : oid_to_hex(&head_oid);
>
> By the way, this is not a new issue introduced by this patch, but I
> notice copy_pathspec() is used twice on the same rev.prune_data in
> this functino.  Who is responsible for releasing the resource held
> in this struct?

Good point. I assumed that the diff machinery would take care of this, but it does not. So I introduced another patch to fix up what is already in `next`.

Ciao, Dscho

Johannes Schindelin via GitGitGadget· Nov 15, 2019, 12:36 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 3/8] built-in add -i: implement the `update` command

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

After `status` and `help`, it is now time to port the `update` command to C, the second command that is shown in the main loop menu of `git add -i`.

This `git add -i` command is the first one which lets the user choose a subset of a list of files, and as such, this patch lays the groundwork for the other commands of that category:

- It teaches the `print_file_item()` function to show a unique prefix
  if we found any (the code to find it had been added already in the
  previous patch where we colored the unique prefixes of the main loop
  commands, but that patch uses the `print_command_item()` function to
  display the menu items).
- This patch also adds the help text that is shown when the user input
  to select items from the shown list could not be parsed.
- As `get_modified_files()` clears the list of files, it now has to take
  care of clearing the _full_ `prefix_item_list` lest the `sorted` and
  `selected` fields go stale and inconsistent.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 130 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 110 insertions(+), 20 deletions(-)
Show changes to add-interactive.c +110 −20
diff --git a/add-interactive.c b/add-interactive.c
index 33a751150a..b0bda0cd2d 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -6,6 +6,7 @@
 #include "revision.h"
 #include "refs.h"
 #include "string-list.h"
+#include "lockfile.h"
 
 struct add_i_state {
 	struct repository *r;
@@ -376,6 +377,7 @@ struct adddel {
 };
 
 struct file_item {
+	size_t prefix_length;
 	struct adddel index, worktree;
 };
 
@@ -470,7 +472,7 @@ enum modified_files_filter {
 
 static int get_modified_files(struct repository *r,
 			      enum modified_files_filter filter,
-			      struct string_list *files,
+			      struct prefix_item_list *files,
 			      const struct pathspec *ps)
 {
 	struct object_id head_oid;
@@ -483,8 +485,8 @@ static int get_modified_files(struct repository *r,
 	    repo_read_index_preload(r, ps, 0) < 0)
 		return error(_("could not read index"));
 
-	string_list_clear(files, 1);
-	s.files = files;
+	prefix_item_list_clear(files);
+	s.files = &files->items;
 	hashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0);
 
 	for (i = 0; i < 2; i++) {
@@ -520,7 +522,7 @@ static int get_modified_files(struct repository *r,
 	hashmap_free_entries(&s.file_map, struct pathname_entry, ent);
 
 	/* While the diffs are ordered already, we ran *two* diffs... */
-	string_list_sort(files);
+	string_list_sort(&files->items);
 
 	return 0;
 }
@@ -555,8 +557,8 @@ static int is_valid_prefix(const char *prefix, size_t prefix_len)
 }
 
 struct print_file_item_data {
-	const char *modified_fmt;
-	struct strbuf buf, index, worktree;
+	const char *modified_fmt, *color, *reset;
+	struct strbuf buf, name, index, worktree;
 };
 
 static void print_file_item(int i, int selected, struct string_list_item *item,
@@ -564,34 +566,96 @@ static void print_file_item(int i, int selected, struct string_list_item *item,
 {
 	struct file_item *c = item->util;
 	struct print_file_item_data *d = print_file_item_data;
+	const char *highlighted = NULL;
 
 	strbuf_reset(&d->index);
 	strbuf_reset(&d->worktree);
 	strbuf_reset(&d->buf);
 
+	/* Format the item with the prefix highlighted. */
+	if (c->prefix_length > 0 &&
+	    is_valid_prefix(item->string, c->prefix_length)) {
+		strbuf_reset(&d->name);
+		strbuf_addf(&d->name, "%s%.*s%s%s", d->color,
+			    (int)c->prefix_length, item->string, d->reset,
+			    item->string + c->prefix_length);
+		highlighted = d->name.buf;
+	}
+
 	render_adddel(&d->worktree, &c->worktree, _("nothing"));
 	render_adddel(&d->index, &c->index, _("unchanged"));
-	strbuf_addf(&d->buf, d->modified_fmt,
-		    d->index.buf, d->worktree.buf, item->string);
+
+	strbuf_addf(&d->buf, d->modified_fmt, d->index.buf, d->worktree.buf,
+		    highlighted ? highlighted : item->string);
 
 	printf("%c%2d: %s", selected ? '*' : ' ', i + 1, d->buf.buf);
 }
 
 static int run_status(struct add_i_state *s, const struct pathspec *ps,
-		      struct string_list *files, struct list_options *opts)
+		      struct prefix_item_list *files,
+		      struct list_and_choose_options *opts)
 {
 	if (get_modified_files(s->r, NO_FILTER, files, ps) < 0)
 		return -1;
 
-	list(s, files, NULL, opts);
+	list(s, &files->items, NULL, &opts->list_opts);
 	putchar('\n');
 
 	return 0;
 }
 
+static int run_update(struct add_i_state *s, const struct pathspec *ps,
+		      struct prefix_item_list *files,
+		      struct list_and_choose_options *opts)
+{
+	int res = 0, fd;
+	size_t count, i;
+	struct lock_file index_lock;
+
+	if (get_modified_files(s->r, WORKTREE_ONLY, files, ps) < 0)
+		return -1;
+
+	if (!files->items.nr) {
+		putchar('\n');
+		return 0;
+	}
+
+	opts->prompt = N_("Update");
+	count = list_and_choose(s, files, opts);
+	if (count <= 0) {
+		putchar('\n');
+		return 0;
+	}
+
+	fd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR);
+	if (fd < 0) {
+		putchar('\n');
+		return -1;
+	}
+
+	for (i = 0; i < files->items.nr; i++) {
+		const char *name = files->items.items[i].string;
+		if (files->selected[i] &&
+		    add_file_to_index(s->r->index, name, 0) < 0) {
+			res = error(_("could not stage '%s'"), name);
+			break;
+		}
+	}
+
+	if (!res && write_locked_index(s->r->index, &index_lock, COMMIT_LOCK) < 0)
+		res = error(_("could not write index"));
+
+	if (!res)
+		printf(Q_("updated %d path\n",
+			  "updated %d paths\n", count), (int)count);
+
+	putchar('\n');
+	return res;
+}
+
 static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
-		    struct string_list *unused_files,
-		    struct list_options *unused_opts)
+		    struct prefix_item_list *unused_files,
+		    struct list_and_choose_options *unused_opts)
 {
 	color_fprintf_ln(stdout, s->help_color, "status        - %s",
 			 _("show paths with changes"));
@@ -609,9 +673,29 @@ static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
 	return 0;
 }
 
+static void choose_prompt_help(struct add_i_state *s)
+{
+	color_fprintf_ln(stdout, s->help_color, "%s",
+			 _("Prompt help:"));
+	color_fprintf_ln(stdout, s->help_color, "1          - %s",
+			 _("select a single item"));
+	color_fprintf_ln(stdout, s->help_color, "3-5        - %s",
+			 _("select a range of items"));
+	color_fprintf_ln(stdout, s->help_color, "2-3,6-9    - %s",
+			 _("select multiple ranges"));
+	color_fprintf_ln(stdout, s->help_color, "foo        - %s",
+			 _("select item based on unique prefix"));
+	color_fprintf_ln(stdout, s->help_color, "-...       - %s",
+			 _("unselect specified items"));
+	color_fprintf_ln(stdout, s->help_color, "*          - %s",
+			 _("choose all items"));
+	color_fprintf_ln(stdout, s->help_color, "           - %s",
+			 _("(empty) finish selecting"));
+}
+
 typedef int (*command_t)(struct add_i_state *s, const struct pathspec *ps,
-			 struct string_list *files,
-			 struct list_options *opts);
+			 struct prefix_item_list *files,
+			 struct list_and_choose_options *opts);
 
 struct command_item {
 	size_t prefix_length;
@@ -663,18 +747,21 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		command_t command;
 	} command_list[] = {
 		{ "status", run_status },
+		{ "update", run_update },
 		{ "help", run_help },
 	};
 	struct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;
 
 	struct print_file_item_data print_file_item_data = {
-		"%12s %12s %s", STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
+		"%12s %12s %s", NULL, NULL,
+		STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
 	};
-	struct list_options opts = {
-		0, NULL, print_file_item, &print_file_item_data
+	struct list_and_choose_options opts = {
+		{ 0, NULL, print_file_item, &print_file_item_data },
+		NULL, 0, choose_prompt_help
 	};
 	struct strbuf header = STRBUF_INIT;
-	struct string_list files = STRING_LIST_INIT_DUP;
+	struct prefix_item_list files = PREFIX_ITEM_LIST_INIT;
 	ssize_t i;
 	int res = 0;
 
@@ -695,11 +782,13 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		data.color = s.prompt_color;
 		data.reset = s.reset_color;
 	}
+	print_file_item_data.color = data.color;
+	print_file_item_data.reset = data.reset;
 
 	strbuf_addstr(&header, "      ");
 	strbuf_addf(&header, print_file_item_data.modified_fmt,
 		    _("staged"), _("unstaged"), _("path"));
-	opts.header = header.buf;
+	opts.list_opts.header = header.buf;
 
 	if (discard_index(r->index) < 0 ||
 	    repo_read_index(r) < 0 ||
@@ -723,8 +812,9 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		}
 	}
 
-	string_list_clear(&files, 1);
+	prefix_item_list_clear(&files);
 	strbuf_release(&print_file_item_data.buf);
+	strbuf_release(&print_file_item_data.name);
 	strbuf_release(&print_file_item_data.index);
 	strbuf_release(&print_file_item_data.worktree);
 	strbuf_release(&header);
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 15, 2019, 12:36 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 5/8] built-in add -i: re-implement `add-untracked` in C

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

This is yet another command, ported to C. It builds nicely on the support functions introduced for other commands, with the notable difference that only names are displayed for untracked files, no file type or diff summary.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)
Show changes to add-interactive.c +91 −0
diff --git a/add-interactive.c b/add-interactive.c
index 191a10b97d..9ed4455a86 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -7,6 +7,7 @@
 #include "refs.h"
 #include "string-list.h"
 #include "lockfile.h"
+#include "dir.h"
 
 struct add_i_state {
 	struct repository *r;
@@ -559,6 +560,7 @@ static int is_valid_prefix(const char *prefix, size_t prefix_len)
 struct print_file_item_data {
 	const char *modified_fmt, *color, *reset;
 	struct strbuf buf, name, index, worktree;
+	unsigned only_names:1;
 };
 
 static void print_file_item(int i, int selected, struct string_list_item *item,
@@ -582,6 +584,12 @@ static void print_file_item(int i, int selected, struct string_list_item *item,
 		highlighted = d->name.buf;
 	}
 
+	if (d->only_names) {
+		printf("%c%2d: %s", selected ? '*' : ' ', i + 1,
+		       highlighted ? highlighted : item->string);
+		return;
+	}
+
 	render_adddel(&d->worktree, &c->worktree, _("nothing"));
 	render_adddel(&d->index, &c->index, _("unchanged"));
 
@@ -761,6 +769,88 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps,
 	return res;
 }
 
+static int get_untracked_files(struct repository *r,
+			       struct prefix_item_list *files,
+			       const struct pathspec *ps)
+{
+	struct dir_struct dir = { 0 };
+	size_t i;
+	struct strbuf buf = STRBUF_INIT;
+
+	if (repo_read_index(r) < 0)
+		return error(_("could not read index"));
+
+	prefix_item_list_clear(files);
+	setup_standard_excludes(&dir);
+	add_pattern_list(&dir, EXC_CMDL, "--exclude option");
+	fill_directory(&dir, r->index, ps);
+
+	for (i = 0; i < dir.nr; i++) {
+		struct dir_entry *ent = dir.entries[i];
+
+		if (index_name_is_other(r->index, ent->name, ent->len)) {
+			strbuf_reset(&buf);
+			strbuf_add(&buf, ent->name, ent->len);
+			add_file_item(&files->items, buf.buf);
+		}
+	}
+
+	strbuf_release(&buf);
+	return 0;
+}
+
+static int run_add_untracked(struct add_i_state *s, const struct pathspec *ps,
+		      struct prefix_item_list *files,
+		      struct list_and_choose_options *opts)
+{
+	struct print_file_item_data *d = opts->list_opts.print_item_data;
+	int res = 0, fd;
+	size_t count, i;
+	struct lock_file index_lock;
+
+	if (get_untracked_files(s->r, files, ps) < 0)
+		return -1;
+
+	if (!files->items.nr) {
+		printf(_("No untracked files.\n"));
+		goto finish_add_untracked;
+	}
+
+	opts->prompt = N_("Add untracked");
+	d->only_names = 1;
+	count = list_and_choose(s, files, opts);
+	d->only_names = 0;
+	if (count <= 0)
+		goto finish_add_untracked;
+
+	fd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR);
+	if (fd < 0) {
+		res = -1;
+		goto finish_add_untracked;
+	}
+
+	for (i = 0; i < files->items.nr; i++) {
+		const char *name = files->items.items[i].string;
+		if (files->selected[i] &&
+		    add_file_to_index(s->r->index, name, 0) < 0) {
+			res = error(_("could not stage '%s'"), name);
+			break;
+		}
+	}
+
+	if (!res &&
+	    write_locked_index(s->r->index, &index_lock, COMMIT_LOCK) < 0)
+		res = error(_("could not write index"));
+
+	if (!res)
+		printf(Q_("added %d path\n",
+			  "added %d paths\n", count), (int)count);
+
+finish_add_untracked:
+	putchar('\n');
+	return res;
+}
+
 static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
 		    struct prefix_item_list *unused_files,
 		    struct list_and_choose_options *unused_opts)
@@ -857,6 +947,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		{ "status", run_status },
 		{ "update", run_update },
 		{ "revert", run_revert },
+		{ "add untracked", run_add_untracked },
 		{ "help", run_help },
 	};
 	struct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 15, 2019, 12:36 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 4/8] built-in add -i: re-implement `revert` in C

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

This is a relatively straight-forward port from the Perl version, with the notable exception that we imitate `git reset -- <paths>` in the C version rather than the convoluted `git ls-tree HEAD -- <paths> | git update-index --index-info` followed by `git update-index --force-remove -- <paths>` for the missed ones.

While at it, we fix the pretty obvious bug where the `revert` command offers to unstage files that do not have staged changes.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)
Show changes to add-interactive.c +109 −0
diff --git a/add-interactive.c b/add-interactive.c
index b0bda0cd2d..191a10b97d 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -653,6 +653,114 @@ static int run_update(struct add_i_state *s, const struct pathspec *ps,
 	return res;
 }
 
+static void revert_from_diff(struct diff_queue_struct *q,
+			     struct diff_options *opt, void *data)
+{
+	int i, add_flags = ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE;
+
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filespec *one = q->queue[i]->one;
+		struct cache_entry *ce;
+
+		if (!(one->mode && !is_null_oid(&one->oid))) {
+			remove_file_from_index(opt->repo->index, one->path);
+			printf(_("note: %s is untracked now.\n"), one->path);
+		} else {
+			ce = make_cache_entry(opt->repo->index, one->mode,
+					      &one->oid, one->path, 0, 0);
+			if (!ce)
+				die(_("make_cache_entry failed for path '%s'"),
+				    one->path);
+			add_index_entry(opt->repo->index, ce, add_flags);
+		}
+	}
+}
+
+static int run_revert(struct add_i_state *s, const struct pathspec *ps,
+		      struct prefix_item_list *files,
+		      struct list_and_choose_options *opts)
+{
+	int res = 0, fd;
+	size_t count, i, j;
+
+	struct object_id oid;
+	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &oid,
+					     NULL);
+	struct lock_file index_lock;
+	const char **paths;
+	struct tree *tree;
+	struct diff_options diffopt = { NULL };
+
+	if (get_modified_files(s->r, INDEX_ONLY, files, ps) < 0)
+		return -1;
+
+	if (!files->items.nr) {
+		putchar('\n');
+		return 0;
+	}
+
+	opts->prompt = N_("Revert");
+	count = list_and_choose(s, files, opts);
+	if (count <= 0)
+		goto finish_revert;
+
+	fd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR);
+	if (fd < 0) {
+		res = -1;
+		goto finish_revert;
+	}
+
+	if (is_initial)
+		oidcpy(&oid, s->r->hash_algo->empty_tree);
+	else {
+		tree = parse_tree_indirect(&oid);
+		if (!tree) {
+			res = error(_("Could not parse HEAD^{tree}"));
+			goto finish_revert;
+		}
+		oidcpy(&oid, &tree->object.oid);
+	}
+
+	ALLOC_ARRAY(paths, count + 1);
+	for (i = j = 0; i < files->items.nr; i++)
+		if (files->selected[i])
+			paths[j++] = files->items.items[i].string;
+	paths[j] = NULL;
+
+	parse_pathspec(&diffopt.pathspec, 0,
+		       PATHSPEC_PREFER_FULL | PATHSPEC_LITERAL_PATH,
+		       NULL, paths);
+
+	diffopt.output_format = DIFF_FORMAT_CALLBACK;
+	diffopt.format_callback = revert_from_diff;
+	diffopt.flags.override_submodule_config = 1;
+	diffopt.repo = s->r;
+
+	if (do_diff_cache(&oid, &diffopt))
+		res = -1;
+	else {
+		diffcore_std(&diffopt);
+		diff_flush(&diffopt);
+	}
+	free(paths);
+	clear_pathspec(&diffopt.pathspec);
+
+	if (!res && write_locked_index(s->r->index, &index_lock,
+				       COMMIT_LOCK) < 0)
+		res = -1;
+	else
+		res = repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0, 1,
+						   NULL, NULL, NULL);
+
+	if (!res)
+		printf(Q_("reverted %d path\n",
+			  "reverted %d paths\n", count), (int)count);
+
+finish_revert:
+	putchar('\n');
+	return res;
+}
+
 static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
 		    struct prefix_item_list *unused_files,
 		    struct list_and_choose_options *unused_opts)
@@ -748,6 +856,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 	} command_list[] = {
 		{ "status", run_status },
 		{ "update", run_update },
+		{ "revert", run_revert },
 		{ "help", run_help },
 	};
 	struct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 15, 2019, 12:36 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 6/8] built-in add -i: implement the `patch` command

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

Well, it is not a full implementation yet. In the interest of making this easy to review (and easy to keep bugs out), we still hand off to the Perl script to do the actual work.

The `patch` functionality actually makes up for more than half of the 1,800+ lines of `git-add--interactive.perl`. It will be ported from Perl to C incrementally, later.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 91 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 84 insertions(+), 7 deletions(-)
Show changes to add-interactive.c +84 −7
diff --git a/add-interactive.c b/add-interactive.c
index 9ed4455a86..fb8124fc57 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -8,6 +8,7 @@
 #include "string-list.h"
 #include "lockfile.h"
 #include "dir.h"
+#include "run-command.h"
 
 struct add_i_state {
 	struct repository *r;
@@ -374,7 +375,7 @@ static ssize_t list_and_choose(struct add_i_state *s,
 
 struct adddel {
 	uintmax_t add, del;
-	unsigned seen:1, binary:1;
+	unsigned seen:1, unmerged:1, binary:1;
 };
 
 struct file_item {
@@ -414,6 +415,7 @@ struct collection_status {
 	const char *reference;
 
 	unsigned skip_unseen:1;
+	size_t unmerged_count, binary_count;
 	struct string_list *files;
 	struct hashmap file_map;
 };
@@ -436,7 +438,7 @@ static void collect_changes_cb(struct diff_queue_struct *q,
 		int hash = strhash(name);
 		struct pathname_entry *entry;
 		struct file_item *file_item;
-		struct adddel *adddel;
+		struct adddel *adddel, *other_adddel;
 
 		entry = hashmap_get_entry_from_hash(&s->file_map, hash, name,
 						    struct pathname_entry, ent);
@@ -456,11 +458,21 @@ static void collect_changes_cb(struct diff_queue_struct *q,
 		file_item = entry->item;
 		adddel = s->phase == FROM_INDEX ?
 			&file_item->index : &file_item->worktree;
+		other_adddel = s->phase == FROM_INDEX ?
+			&file_item->worktree : &file_item->index;
 		adddel->seen = 1;
 		adddel->add = stat.files[i]->added;
 		adddel->del = stat.files[i]->deleted;
-		if (stat.files[i]->is_binary)
+		if (stat.files[i]->is_binary) {
+			if (!other_adddel->binary)
+				s->binary_count++;
 			adddel->binary = 1;
+		}
+		if (stat.files[i]->is_unmerged) {
+			if (!other_adddel->unmerged)
+				s->unmerged_count++;
+			adddel->unmerged = 1;
+		}
 	}
 	free_diffstat_info(&stat);
 }
@@ -474,7 +486,9 @@ enum modified_files_filter {
 static int get_modified_files(struct repository *r,
 			      enum modified_files_filter filter,
 			      struct prefix_item_list *files,
-			      const struct pathspec *ps)
+			      const struct pathspec *ps,
+			      size_t *unmerged_count,
+			      size_t *binary_count)
 {
 	struct object_id head_oid;
 	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
@@ -521,6 +535,10 @@ static int get_modified_files(struct repository *r,
 		}
 	}
 	hashmap_free_entries(&s.file_map, struct pathname_entry, ent);
+	if (unmerged_count)
+		*unmerged_count = s.unmerged_count;
+	if (binary_count)
+		*binary_count = s.binary_count;
 
 	/* While the diffs are ordered already, we ran *two* diffs... */
 	string_list_sort(&files->items);
@@ -603,7 +621,7 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps,
 		      struct prefix_item_list *files,
 		      struct list_and_choose_options *opts)
 {
-	if (get_modified_files(s->r, NO_FILTER, files, ps) < 0)
+	if (get_modified_files(s->r, NO_FILTER, files, ps, NULL, NULL) < 0)
 		return -1;
 
 	list(s, &files->items, NULL, &opts->list_opts);
@@ -620,7 +638,7 @@ static int run_update(struct add_i_state *s, const struct pathspec *ps,
 	size_t count, i;
 	struct lock_file index_lock;
 
-	if (get_modified_files(s->r, WORKTREE_ONLY, files, ps) < 0)
+	if (get_modified_files(s->r, WORKTREE_ONLY, files, ps, NULL, NULL) < 0)
 		return -1;
 
 	if (!files->items.nr) {
@@ -699,7 +717,7 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps,
 	struct tree *tree;
 	struct diff_options diffopt = { NULL };
 
-	if (get_modified_files(s->r, INDEX_ONLY, files, ps) < 0)
+	if (get_modified_files(s->r, INDEX_ONLY, files, ps, NULL, NULL) < 0)
 		return -1;
 
 	if (!files->items.nr) {
@@ -851,6 +869,64 @@ static int run_add_untracked(struct add_i_state *s, const struct pathspec *ps,
 	return res;
 }
 
+static int run_patch(struct add_i_state *s, const struct pathspec *ps,
+		     struct prefix_item_list *files,
+		     struct list_and_choose_options *opts)
+{
+	int res = 0;
+	ssize_t count, i, j;
+	size_t unmerged_count = 0, binary_count = 0;
+
+	if (get_modified_files(s->r, WORKTREE_ONLY, files, ps,
+			       &unmerged_count, &binary_count) < 0)
+		return -1;
+
+	if (unmerged_count || binary_count) {
+		for (i = j = 0; i < files->items.nr; i++) {
+			struct file_item *item = files->items.items[i].util;
+
+			if (item->index.binary || item->worktree.binary) {
+				free(item);
+				free(files->items.items[i].string);
+			} else if (item->index.unmerged ||
+				 item->worktree.unmerged) {
+				color_fprintf_ln(stderr, s->error_color,
+						 _("ignoring unmerged: %s"),
+						 files->items.items[i].string);
+				free(item);
+				free(files->items.items[i].string);
+			} else
+				files->items.items[j++] = files->items.items[i];
+		}
+		files->items.nr = j;
+	}
+
+	if (!files->items.nr) {
+		if (binary_count)
+			fprintf(stderr, _("Only binary files changed.\n"));
+		else
+			fprintf(stderr, _("No changes.\n"));
+		return 0;
+	}
+
+	opts->prompt = N_("Patch update");
+	count = list_and_choose(s, files, opts);
+	if (count >= 0) {
+		struct argv_array args = ARGV_ARRAY_INIT;
+
+		argv_array_pushl(&args, "git", "add--interactive", "--patch",
+				 "--", NULL);
+		for (i = 0; i < files->items.nr; i++)
+			if (files->selected[i])
+				argv_array_push(&args,
+						files->items.items[i].string);
+		res = run_command_v_opt(args.argv, 0);
+		argv_array_clear(&args);
+	}
+
+	return res;
+}
+
 static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
 		    struct prefix_item_list *unused_files,
 		    struct list_and_choose_options *unused_opts)
@@ -948,6 +1024,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		{ "update", run_update },
 		{ "revert", run_revert },
 		{ "add untracked", run_add_untracked },
+		{ "patch", run_patch },
 		{ "help", run_help },
 	};
 	struct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 15, 2019, 12:36 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 8/8] built-in add -i: offer the `quit` command

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

We do not really want to `exit()` here, of course, as this is safely libified code.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
Show changes to add-interactive.c +11 −5
diff --git a/add-interactive.c b/add-interactive.c
index f33075b202..40bec9acbe 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -1067,6 +1067,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		{ "add untracked", run_add_untracked },
 		{ "patch", run_patch },
 		{ "diff", run_diff },
+		{ "quit", NULL },
 		{ "help", run_help },
 	};
 	struct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;
@@ -1118,17 +1119,22 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 	res = run_status(&s, ps, &files, &opts);
 
 	for (;;) {
+		struct command_item *util;
+
 		i = list_and_choose(&s, &commands, &main_loop_opts);
-		if (i == LIST_AND_CHOOSE_QUIT) {
+		if (i < 0 || i >= commands.items.nr)
+			util = NULL;
+		else
+			util = commands.items.items[i].util;
+
+		if (i == LIST_AND_CHOOSE_QUIT || (util && !util->command)) {
 			printf(_("Bye.\n"));
 			res = 0;
 			break;
 		}
-		if (i != LIST_AND_CHOOSE_ERROR) {
-			struct command_item *util =
-				commands.items.items[i].util;
+
+		if (util)
 			res = util->command(&s, ps, &files, &opts);
-		}
 	}
 
 	prefix_item_list_clear(&files);
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 15, 2019, 12:36 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 7/8] built-in add -i: re-implement the `diff` command

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

It is not only laziness that we simply spawn `git diff -p --cached` here: this command needs to use the pager, and the pager needs to exit when the diff is done. Currently we do not have any way to make that happen if we run the diff in-process. So let's just spawn.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
Show changes to add-interactive.c +42 −0
diff --git a/add-interactive.c b/add-interactive.c
index fb8124fc57..f33075b202 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -927,6 +927,47 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps,
 	return res;
 }
 
+static int run_diff(struct add_i_state *s, const struct pathspec *ps,
+		    struct prefix_item_list *files,
+		    struct list_and_choose_options *opts)
+{
+	int res = 0;
+	ssize_t count, i;
+
+	struct object_id oid;
+	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &oid,
+					     NULL);
+	if (get_modified_files(s->r, INDEX_ONLY, files, ps, NULL, NULL) < 0)
+		return -1;
+
+	if (!files->items.nr) {
+		putchar('\n');
+		return 0;
+	}
+
+	opts->prompt = N_("Review diff");
+	opts->flags = IMMEDIATE;
+	count = list_and_choose(s, files, opts);
+	opts->flags = 0;
+	if (count >= 0) {
+		struct argv_array args = ARGV_ARRAY_INIT;
+
+		argv_array_pushl(&args, "git", "diff", "-p", "--cached",
+				 oid_to_hex(!is_initial ? &oid :
+					    s->r->hash_algo->empty_tree),
+				 "--", NULL);
+		for (i = 0; i < files->items.nr; i++)
+			if (files->selected[i])
+				argv_array_push(&args,
+						files->items.items[i].string);
+		res = run_command_v_opt(args.argv, 0);
+		argv_array_clear(&args);
+	}
+
+	putchar('\n');
+	return res;
+}
+
 static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
 		    struct prefix_item_list *unused_files,
 		    struct list_and_choose_options *unused_opts)
@@ -1025,6 +1066,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		{ "revert", run_revert },
 		{ "add untracked", run_add_untracked },
 		{ "patch", run_patch },
+		{ "diff", run_diff },
 		{ "help", run_help },
 	};
 	struct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 15, 2019, 12:36 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 2/8] built-in add -i: prepare for multi-selection commands

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

The `upgrade`, `revert` and `add-untracked` commands allow selecting multiple entries. Let's extend the `list_and_choose()` function to accommodate those use cases.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 114 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 89 insertions(+), 25 deletions(-)
Show changes to add-interactive.c +89 −25
diff --git a/add-interactive.c b/add-interactive.c
index 8ec930ac15..33a751150a 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -72,15 +72,17 @@ static void init_add_i_state(struct add_i_state *s, struct repository *r)
 struct prefix_item_list {
 	struct string_list items;
 	struct string_list sorted;
+	int *selected; /* for multi-selections */
 	size_t min_length, max_length;
 };
 #define PREFIX_ITEM_LIST_INIT \
-	{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_NODUP, 1, 4 }
+	{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_NODUP, NULL, 1, 4 }
 
 static void prefix_item_list_clear(struct prefix_item_list *list)
 {
 	string_list_clear(&list->items, 1);
 	string_list_clear(&list->sorted, 0);
+	FREE_AND_NULL(list->selected);
 }
 
 static void extend_prefix_length(struct string_list_item *p,
@@ -182,11 +184,12 @@ static ssize_t find_unique(const char *string, struct prefix_item_list *list)
 struct list_options {
 	int columns;
 	const char *header;
-	void (*print_item)(int i, struct string_list_item *item, void *print_item_data);
+	void (*print_item)(int i, int selected, struct string_list_item *item,
+			   void *print_item_data);
 	void *print_item_data;
 };
 
-static void list(struct add_i_state *s, struct string_list *list,
+static void list(struct add_i_state *s, struct string_list *list, int *selected,
 		 struct list_options *opts)
 {
 	int i, last_lf = 0;
@@ -199,7 +202,8 @@ static void list(struct add_i_state *s, struct string_list *list,
 				 "%s", opts->header);
 
 	for (i = 0; i < list->nr; i++) {
-		opts->print_item(i, list->items + i, opts->print_item_data);
+		opts->print_item(i, selected ? selected[i] : 0, list->items + i,
+				 opts->print_item_data);
 
 		if ((opts->columns) && ((i + 1) % (opts->columns))) {
 			putchar('\t');
@@ -218,6 +222,10 @@ struct list_and_choose_options {
 	struct list_options list_opts;
 
 	const char *prompt;
+	enum {
+		SINGLETON = (1<<0),
+		IMMEDIATE = (1<<1),
+	} flags;
 	void (*print_help)(struct add_i_state *s);
 };
 
@@ -225,7 +233,8 @@ struct list_and_choose_options {
 #define LIST_AND_CHOOSE_QUIT  (-2)
 
 /*
- * Returns the selected index.
+ * Returns the selected index in singleton mode, the number of selected items
+ * otherwise.
  *
  * If an error occurred, returns `LIST_AND_CHOOSE_ERROR`. Upon EOF,
  * `LIST_AND_CHOOSE_QUIT` is returned.
@@ -234,8 +243,19 @@ static ssize_t list_and_choose(struct add_i_state *s,
 			       struct prefix_item_list *items,
 			       struct list_and_choose_options *opts)
 {
+	int singleton = opts->flags & SINGLETON;
+	int immediate = opts->flags & IMMEDIATE;
+
 	struct strbuf input = STRBUF_INIT;
-	ssize_t res = LIST_AND_CHOOSE_ERROR;
+	ssize_t res = singleton ? LIST_AND_CHOOSE_ERROR : 0;
+
+	if (!singleton) {
+		free(items->selected);
+		CALLOC_ARRAY(items->selected, items->items.nr);
+	}
+
+	if (singleton && !immediate)
+		BUG("singleton requires immediate");
 
 	find_unique_prefixes(items);
 
@@ -244,15 +264,16 @@ static ssize_t list_and_choose(struct add_i_state *s,
 
 		strbuf_reset(&input);
 
-		list(s, &items->items, &opts->list_opts);
+		list(s, &items->items, items->selected, &opts->list_opts);
 
 		color_fprintf(stdout, s->prompt_color, "%s", opts->prompt);
-		fputs("> ", stdout);
+		fputs(singleton ? "> " : ">> ", stdout);
 		fflush(stdout);
 
 		if (strbuf_getline(&input, stdin) == EOF) {
 			putchar('\n');
-			res = LIST_AND_CHOOSE_QUIT;
+			if (immediate)
+				res = LIST_AND_CHOOSE_QUIT;
 			break;
 		}
 		strbuf_trim(&input);
@@ -268,7 +289,9 @@ static ssize_t list_and_choose(struct add_i_state *s,
 		p = input.buf;
 		for (;;) {
 			size_t sep = strcspn(p, " \t\r\n,");
-			ssize_t index = -1;
+			int choose = 1;
+			/* `from` is inclusive, `to` is exclusive */
+			ssize_t from = -1, to = -1;
 
 			if (!sep) {
 				if (!*p)
@@ -277,29 +300,69 @@ static ssize_t list_and_choose(struct add_i_state *s,
 				continue;
 			}
 
-			if (isdigit(*p)) {
+			/* Input that begins with '-'; de-select */
+			if (*p == '-') {
+				choose = 0;
+				p++;
+				sep--;
+			}
+
+			if (sep == 1 && *p == '*') {
+				from = 0;
+				to = items->items.nr;
+			} else if (isdigit(*p)) {
 				char *endp;
-				index = strtoul(p, &endp, 10) - 1;
-				if (endp != p + sep)
-					index = -1;
+				/*
+				 * A range can be specified like 5-7 or 5-.
+				 *
+				 * Note: `from` is 0-based while the user input
+				 * is 1-based, hence we have to decrement by
+				 * one. We do not have to decrement `to` even
+				 * if it is 0-based because it is an exclusive
+				 * boundary.
+				 */
+				from = strtoul(p, &endp, 10) - 1;
+				if (endp == p + sep)
+					to = from + 1;
+				else if (*endp == '-') {
+					to = strtoul(++endp, &endp, 10);
+					/* extra characters after the range? */
+					if (endp != p + sep)
+						from = -1;
+				}
 			}
 
 			p[sep] = '\0';
-			if (index < 0)
-				index = find_unique(p, items);
+			if (from < 0) {
+				from = find_unique(p, items);
+				if (from >= 0)
+					to = from + 1;
+			}
 
-			if (index < 0 || index >= items->items.nr)
+			if (from < 0 || from >= items->items.nr ||
+			    (singleton && from + 1 != to)) {
 				color_fprintf_ln(stdout, s->error_color,
 						 _("Huh (%s)?"), p);
-			else {
-				res = index;
+				break;
+			} else if (singleton) {
+				res = from;
 				break;
 			}
 
+			if (to > items->items.nr)
+				to = items->items.nr;
+
+			for (; from < to; from++)
+				if (items->selected[from] != choose) {
+					items->selected[from] = choose;
+					res += choose ? +1 : -1;
+				}
+
 			p += sep + 1;
 		}
 
-		if (res != LIST_AND_CHOOSE_ERROR)
+		if ((immediate && res != LIST_AND_CHOOSE_ERROR) ||
+		    !strcmp(input.buf, "*"))
 			break;
 	}
 
@@ -496,7 +559,7 @@ struct print_file_item_data {
 	struct strbuf buf, index, worktree;
 };
 
-static void print_file_item(int i, struct string_list_item *item,
+static void print_file_item(int i, int selected, struct string_list_item *item,
 			    void *print_file_item_data)
 {
 	struct file_item *c = item->util;
@@ -511,7 +574,7 @@ static void print_file_item(int i, struct string_list_item *item,
 	strbuf_addf(&d->buf, d->modified_fmt,
 		    d->index.buf, d->worktree.buf, item->string);
 
-	printf(" %2d: %s", i + 1, d->buf.buf);
+	printf("%c%2d: %s", selected ? '*' : ' ', i + 1, d->buf.buf);
 }
 
 static int run_status(struct add_i_state *s, const struct pathspec *ps,
@@ -520,7 +583,7 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps,
 	if (get_modified_files(s->r, NO_FILTER, files, ps) < 0)
 		return -1;
 
-	list(s, files, opts);
+	list(s, files, NULL, opts);
 	putchar('\n');
 
 	return 0;
@@ -559,7 +622,8 @@ struct print_command_item_data {
 	const char *color, *reset;
 };
 
-static void print_command_item(int i, struct string_list_item *item,
+static void print_command_item(int i, int selected,
+			       struct string_list_item *item,
 			       void *print_command_item_data)
 {
 	struct print_command_item_data *d = print_command_item_data;
@@ -592,7 +656,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 	struct print_command_item_data data = { "[", "]" };
 	struct list_and_choose_options main_loop_opts = {
 		{ 4, N_("*** Commands ***"), print_command_item, &data },
-		N_("What now"), command_prompt_help
+		N_("What now"), SINGLETON | IMMEDIATE, command_prompt_help
 	};
 	struct {
 		const char *string;
-- 
gitgitgadget
Junio C Hamano· Nov 21, 2019, 08:12 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH 2/8] built-in add -i: prepare for multi-selection commands

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

Show 5 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The `upgrade`, `revert` and `add-untracked` commands allow selecting
> multiple entries. Let's extend the `list_and_choose()` function to
> accommodate those use cases.
upgrade???
Show 17 quoted lines
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  add-interactive.c | 114 ++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 89 insertions(+), 25 deletions(-)
>
> diff --git a/add-interactive.c b/add-interactive.c
> index 8ec930ac15..33a751150a 100644
> --- a/add-interactive.c
> +++ b/add-interactive.c
> @@ -72,15 +72,17 @@ static void init_add_i_state(struct add_i_state *s, struct repository *r)
>  struct prefix_item_list {
>  	struct string_list items;
>  	struct string_list sorted;
> +	int *selected; /* for multi-selections */
>  	size_t min_length, max_length;
>  };

OK. The shape of the data is enough to tell us what you did in this patch and I think it makes sense ;-)

Johannes Schindelin· Nov 25, 2019, 15:21 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/8] built-in add -i: prepare for multi-selection commands

Hi Junio,
On Thu, 21 Nov 2019, Junio C Hamano wrote:
Show 10 quoted lines
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > The `upgrade`, `revert` and `add-untracked` commands allow selecting
> > multiple entries. Let's extend the `list_and_choose()` function to
> > accommodate those use cases.
>
> upgrade???
Whoooooops. "update", of course.
Show 20 quoted lines
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  add-interactive.c | 114 ++++++++++++++++++++++++++++++++++++----------
> >  1 file changed, 89 insertions(+), 25 deletions(-)
> >
> > diff --git a/add-interactive.c b/add-interactive.c
> > index 8ec930ac15..33a751150a 100644
> > --- a/add-interactive.c
> > +++ b/add-interactive.c
> > @@ -72,15 +72,17 @@ static void init_add_i_state(struct add_i_state *s, struct repository *r)
> >  struct prefix_item_list {
> >  	struct string_list items;
> >  	struct string_list sorted;
> > +	int *selected; /* for multi-selections */
> >  	size_t min_length, max_length;
> >  };
>
> OK.  The shape of the data is enough to tell us what you did in this
> patch and I think it makes sense ;-)
:-)

Thanks, Dscho

Junio C Hamano· Nov 18, 2019, 02:17 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH 0/8] build-in add -i: implement all commands in the main loop

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

Show 5 quoted lines
> Based on the js/builtin-add-i branch, this patch series implements the rest
> of the commands in git add -i's main loop: update, revert, add_untracked, 
> patch, diff, and quit. Apart from quit, these commands are all very similar
> in that they first build a list of files, display it, let the user choose
> which ones to act on, and then perform the action.

So, this obviously depends on the previous "let's start the main loop" series, in which I recall you found at least one remaining bug after sending the latest round.

I'd prefer to queue this on top of the iteration of that series, which would hopefully become the final one, which would let me avoid one rebase of this series ;-)

Thanks.
Junio C Hamano· Nov 18, 2019, 02:22 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/8] build-in add -i: implement all commands in the main loop

Junio C Hamano <gitster@pobox.com> writes:
Show 18 quoted lines
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
>> Based on the js/builtin-add-i branch, this patch series implements the rest
>> of the commands in git add -i's main loop: update, revert, add_untracked, 
>> patch, diff, and quit. Apart from quit, these commands are all very similar
>> in that they first build a list of files, display it, let the user choose
>> which ones to act on, and then perform the action.
>
> So, this obviously depends on the previous "let's start the main
> loop" series, in which I recall you found at least one remaining bug
> after sending the latest round.
>
> I'd prefer to queue this on top of the iteration of that series,
> which would hopefully become the final one, which would let me avoid
> one rebase of this series ;-)
>
> Thanks.

I found the v7 on the list, which is what I wanted to queue this on top of. So my wait is over ;-)

Thanks.
Johannes Schindelin· Nov 18, 2019, 19:22 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/8] build-in add -i: implement all commands in the main loop

Hi Junio,
On Mon, 18 Nov 2019, Junio C Hamano wrote:
Show 23 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
> > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> > writes:
> >
> >> Based on the js/builtin-add-i branch, this patch series implements the rest
> >> of the commands in git add -i's main loop: update, revert, add_untracked,
> >> patch, diff, and quit. Apart from quit, these commands are all very similar
> >> in that they first build a list of files, display it, let the user choose
> >> which ones to act on, and then perform the action.
> >
> > So, this obviously depends on the previous "let's start the main
> > loop" series, in which I recall you found at least one remaining bug
> > after sending the latest round.
> >
> > I'd prefer to queue this on top of the iteration of that series,
> > which would hopefully become the final one, which would let me avoid
> > one rebase of this series ;-)
> >
> > Thanks.
>
> I found the v7 on the list, which is what I wanted to queue this on
> top of.  So my wait is over ;-)
Yes, sorry, I did not think that I was opaque but I was.

So to clarify: I will always rebase the built-in add -i/-p patch series on top of what you put on gitster/git. That is, this here patch series will always be rebased on top of js/builtin-add-i, the next patch series will always be rebased on top of js/builtin-add-i-cmds, etc

Ciao, Dscho

Junio C Hamano· Nov 18, 2019, 02:27 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH 0/8] build-in add -i: implement all commands in the main loop

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

> ... there is so little overlap between the git add --patch part
> and the rest of git add --interactive that I could put the former
> into a totally different file: add-patch.c.

We would want to call, after "add -i in C" stabilizes, directly into "add -p" machinery from different subcommands such as "reset", "checkout", etc., and bypass the rest of "add -i" when we do so. We can credit the lack of overlap to the design that supports such usage while it is still in a scripted Porcelain.

Preparing the "add -p" machinery in such a way that "add -i" merely is one of the users would make a lot of sense from organizational point of view.

Thanks.
Johannes Schindelin· Nov 18, 2019, 18:53 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/8] build-in add -i: implement all commands in the main loop

Hi Junio,
On Mon, 18 Nov 2019, Junio C Hamano wrote:
Show 16 quoted lines
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > ... there is so little overlap between the git add --patch part
> > and the rest of git add --interactive that I could put the former
> > into a totally different file: add-patch.c.
>
> We would want to call, after "add -i in C" stabilizes, directly into
> "add -p" machinery from different subcommands such as "reset",
> "checkout", etc., and bypass the rest of "add -i" when we do so.  We
> can credit the lack of overlap to the design that supports such
> usage while it is still in a scripted Porcelain.
>
> Preparing the "add -p" machinery in such a way that "add -i" merely
> is one of the users would make a lot of sense from organizational
> point of view.

If I understand you correctly, you mean that e.g. `git checkout -p` will call the code from `add-patch.c` directly? That is indeed the case. Already now, `git checkout -p` calls `run_add_interactive()` (see https://github.com/git/git/blob/v2.24.0/builtin/checkout.c#L463):

	if (opts->patch_mode) {
		const char *patch_mode;
		if (opts->checkout_index && opts->checkout_worktree)
			patch_mode = "--patch=checkout";
		else if (opts->checkout_index && !opts->checkout_worktree)
			patch_mode = "--patch=reset";
		else if (!opts->checkout_index && opts->checkout_worktree)
			patch_mode = "--patch=worktree";
		else
			BUG("either flag must have been set, worktree=%d, index=%d",
			    opts->checkout_worktree, opts->checkout_index);
		return run_add_interactive(revision, patch_mode, &opts->pathspec);
	}

And with the changes in PR #174 (i.e. three patch series later, see https://github.com/gitgitgadget/git/pull/174/files#diff-1f1eb09e4703764f9f9b9f1f1e67eb97R205-R218 for details), this will call `run_add_p()`, i.e. the newly-added code in `add-patch.c` directly:

		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 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);
		return !!run_add_p(the_repository, mode, revision, pathspec);

Now that I see it, it does not look the most elegant that the `patch_mode` is a string instead of that enum, but I think that will be relatively easy to fix once the legacy `git-add--interactive.perl` will have been decomissioned.

Ciao, Dscho

Junio C Hamano· Nov 19, 2019, 01:29 UTC · re: Johannes Schindelin · lore

Re: [PATCH 0/8] build-in add -i: implement all commands in the main loop

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 6 quoted lines
>> Preparing the "add -p" machinery in such a way that "add -i" merely
>> is one of the users would make a lot of sense from organizational
>> point of view.
>
> If I understand you correctly, you mean that e.g. `git checkout -p` will
> call the code from `add-patch.c` directly?...

Yup, and it is a good organization that the "add -p" machinery is held not too intimate with the rest of "add -i" but treats "add -i" as just one of its users from the beginning, which is what you did here.

In case it wasn't clear, I am saying that I like the resulting organization of these 8 patches.

Johannes Schindelin via GitGitGadget· Nov 29, 2019, 21:11 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 0/9] built-in add -i: implement all commands in the main loop

Based on the js/builtin-add-i branch, this patch series implements the rest of the commands in git add -i's main loop: update, revert, add_untracked, patch, diff, and quit. Apart from quit, these commands are all very similar in that they first build a list of files, display it, let the user choose which ones to act on, and then perform the action.

Note that the patch command is not actually converted to C, not completely at least: the built-in version simply hands off to git add--interactive after letting the user select which files to act on.

The reason for this omission is practicality. Out of the 1,800+ lines of git-add--interactive.perl, over a thousand deal just with the git add -p part. I did convert that functionality already (to be contributed in a separate patch series, see https://github.com/gitgitgadget/git/pull/173), discovering that there is so little overlap between the git add --patch part and the rest of git add --interactive that I could put the former into a totally different file: add-patch.c.

Changes since v1:
 * As an introductory commit, we now release the pathspecs that we passed to
   the diff machinery.
 * The ternary in 1/8 (now 1/9) was adjusted from i ? to (i == 0) ? to make
   the flow more intuitive.
 * The commit message of 2/8 (now 3/9) no longer talks about an upgrade 
   command (which never existed)but about the update command (that I
   actually meant).
Johannes Schindelin (9):
  add-interactive: make sure to release `rev.prune_data`
  built-in add -i: allow filtering the modified files list
  built-in add -i: prepare for multi-selection commands
  built-in add -i: implement the `update` command
  built-in add -i: re-implement `revert` in C
  built-in add -i: re-implement `add-untracked` in C
  built-in add -i: implement the `patch` command
  built-in add -i: re-implement the `diff` command
  built-in add -i: offer the `quit` command
 add-interactive.c | 619 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 560 insertions(+), 59 deletions(-)
base-commit: 8c159044625e46de67cd8467f07424f38eb8301e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-171%2Fdscho%2Fadd-i-in-c-all-except-patch-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-171/dscho/add-i-in-c-all-except-patch-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/171
Range-diff vs v1:
  -:  ---------- >  1:  1e02bf2d63 add-interactive: make sure to release `rev.prune_data`
  1:  1844c4d55e !  2:  fab098d86e built-in add -i: allow filtering the modified files list
     @@ -17,12 +17,21 @@
          and `diff-files` phases, and then skipping files in the second phase
          when they have not been seen in the first phase.
      
     +    Seeing as we change the meaning of the `phase` field, we rename it to
     +    `mode` to reflect that the order depends on the exact invocation of the
     +    `git add -i` command.
     +
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
       diff --git a/add-interactive.c b/add-interactive.c
       --- a/add-interactive.c
       +++ b/add-interactive.c
      @@
     + }
     + 
     + struct collection_status {
     +-	enum { FROM_WORKTREE = 0, FROM_INDEX = 1 } phase;
     ++	enum { FROM_WORKTREE = 0, FROM_INDEX = 1 } mode;
       
       	const char *reference;
       
     @@ -40,6 +49,15 @@
       			add_file_item(s->files, name);
       
       			entry = xcalloc(sizeof(*entry), 1);
     +@@
     + 		}
     + 
     + 		file_item = entry->item;
     +-		adddel = s->phase == FROM_INDEX ?
     ++		adddel = s->mode == FROM_INDEX ?
     + 			&file_item->index : &file_item->worktree;
     + 		adddel->seen = 1;
     + 		adddel->add = stat.files[i]->added;
      @@
       	free_diffstat_info(&stat);
       }
     @@ -59,7 +77,8 @@
       	struct object_id head_oid;
       	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
       					     &head_oid, NULL);
     - 	struct collection_status s = { FROM_WORKTREE };
     +-	struct collection_status s = { FROM_WORKTREE };
     ++	struct collection_status s = { 0 };
      +	int i;
       
       	if (discard_index(r->index) < 0 ||
     @@ -74,14 +93,23 @@
       		struct setup_revision_opt opt = { 0 };
       
      +		if (filter == INDEX_ONLY)
     -+			s.phase = i ? FROM_WORKTREE : FROM_INDEX;
     ++			s.mode = (i == 0) ? FROM_INDEX : FROM_WORKTREE;
      +		else
     -+			s.phase = i ? FROM_INDEX : FROM_WORKTREE;
     ++			s.mode = (i == 0) ? FROM_WORKTREE : FROM_INDEX;
      +		s.skip_unseen = filter && i;
      +
       		opt.def = is_initial ?
       			empty_tree_oid_hex() : oid_to_hex(&head_oid);
       
     +@@
     + 		if (ps)
     + 			copy_pathspec(&rev.prune_data, ps);
     + 
     +-		if (s.phase == FROM_INDEX)
     ++		if (s.mode == FROM_INDEX)
     + 			run_diff_index(&rev, 1);
     + 		else {
     + 			rev.diffopt.flags.ignore_dirty_submodules = 1;
      @@
       static int run_status(struct add_i_state *s, const struct pathspec *ps,
       		      struct string_list *files, struct list_options *opts)
  2:  c324b47592 !  3:  58a581f4ee built-in add -i: prepare for multi-selection commands
     @@ -2,7 +2,7 @@
      
          built-in add -i: prepare for multi-selection commands
      
     -    The `upgrade`, `revert` and `add-untracked` commands allow selecting
     +    The `update`, `revert` and `add-untracked` commands allow selecting
          multiple entries. Let's extend the `list_and_choose()` function to
          accommodate those use cases.
      
     @@ -168,7 +168,8 @@
      +				}
       			}
       
     - 			p[sep] = '\0';
     + 			if (p[sep])
     + 				p[sep++] = '\0';
      -			if (index < 0)
      -				index = find_unique(p, items);
      +			if (from < 0) {
     @@ -199,7 +200,7 @@
      +					res += choose ? +1 : -1;
      +				}
      +
     - 			p += sep + 1;
     + 			p += sep;
       		}
       
      -		if (res != LIST_AND_CHOOSE_ERROR)
  3:  cba5f13152 =  4:  c311a29c77 built-in add -i: implement the `update` command
  4:  5c31bbd24a =  5:  f70723a160 built-in add -i: re-implement `revert` in C
  5:  5c498795b3 =  6:  952fbc8f79 built-in add -i: re-implement `add-untracked` in C
  6:  0c8a71e2e8 !  7:  cbd10da523 built-in add -i: implement the `patch` command
     @@ -51,9 +51,9 @@
       						    struct pathname_entry, ent);
      @@
       		file_item = entry->item;
     - 		adddel = s->phase == FROM_INDEX ?
     + 		adddel = s->mode == FROM_INDEX ?
       			&file_item->index : &file_item->worktree;
     -+		other_adddel = s->phase == FROM_INDEX ?
     ++		other_adddel = s->mode == FROM_INDEX ?
      +			&file_item->worktree : &file_item->index;
       		adddel->seen = 1;
       		adddel->add = stat.files[i]->added;
     @@ -84,7 +84,7 @@
       	struct object_id head_oid;
       	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
      @@
     - 		}
     + 			clear_pathspec(&rev.prune_data);
       	}
       	hashmap_free_entries(&s.file_map, struct pathname_entry, ent);
      +	if (unmerged_count)
  7:  499f3f19e0 =  8:  e4907112e4 built-in add -i: re-implement the `diff` command
  8:  dbcb340ede =  9:  5ba6cd31ab built-in add -i: offer the `quit` command
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 29, 2019, 21:11 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 1/9] add-interactive: make sure to release `rev.prune_data`

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

During a review, Junio Hamano pointed out that the `rev.prune_data` was copied from another pathspec but never cleaned up.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 3 +++
 1 file changed, 3 insertions(+)
Show changes to add-interactive.c +3 −0
diff --git a/add-interactive.c b/add-interactive.c
index d6cb98cd40..de2fccb0ef 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -435,6 +435,9 @@ static int get_modified_files(struct repository *r, struct string_list *files,
 			rev.diffopt.flags.ignore_dirty_submodules = 1;
 			run_diff_files(&rev, 0);
 		}
+
+		if (ps)
+			clear_pathspec(&rev.prune_data);
 	}
 	hashmap_free_entries(&s.file_map, struct pathname_entry, ent);
 
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 29, 2019, 21:11 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 2/9] built-in add -i: allow filtering the modified files list

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

In the `update` command of `git add -i`, we are primarily interested in the list of modified files that have worktree (i.e. unstaged) changes.

At the same time, we need to determine _also_ the staged changes, to be able to produce the full added/deleted information.

The Perl script version of `git add -i` has a parameter of the `list_modified()` function for that matter. In C, we can be a lot more precise, using an `enum`.

The C implementation of the filter also has an easier time to avoid unnecessary work, simply by using an adaptive order of the `diff-index` and `diff-files` phases, and then skipping files in the second phase when they have not been seen in the first phase.

Seeing as we change the meaning of the `phase` field, we rename it to `mode` to reflect that the order depends on the exact invocation of the `git add -i` command.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)
Show changes to add-interactive.c +26 −7
diff --git a/add-interactive.c b/add-interactive.c
index de2fccb0ef..c62d63e35b 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -344,10 +344,11 @@ static int pathname_entry_cmp(const void *unused_cmp_data,
 }
 
 struct collection_status {
-	enum { FROM_WORKTREE = 0, FROM_INDEX = 1 } phase;
+	enum { FROM_WORKTREE = 0, FROM_INDEX = 1 } mode;
 
 	const char *reference;
 
+	unsigned skip_unseen:1;
 	struct string_list *files;
 	struct hashmap file_map;
 };
@@ -375,6 +376,9 @@ static void collect_changes_cb(struct diff_queue_struct *q,
 		entry = hashmap_get_entry_from_hash(&s->file_map, hash, name,
 						    struct pathname_entry, ent);
 		if (!entry) {
+			if (s->skip_unseen)
+				continue;
+
 			add_file_item(s->files, name);
 
 			entry = xcalloc(sizeof(*entry), 1);
@@ -385,7 +389,7 @@ static void collect_changes_cb(struct diff_queue_struct *q,
 		}
 
 		file_item = entry->item;
-		adddel = s->phase == FROM_INDEX ?
+		adddel = s->mode == FROM_INDEX ?
 			&file_item->index : &file_item->worktree;
 		adddel->seen = 1;
 		adddel->add = stat.files[i]->added;
@@ -396,13 +400,22 @@ static void collect_changes_cb(struct diff_queue_struct *q,
 	free_diffstat_info(&stat);
 }
 
-static int get_modified_files(struct repository *r, struct string_list *files,
+enum modified_files_filter {
+	NO_FILTER = 0,
+	WORKTREE_ONLY = 1,
+	INDEX_ONLY = 2,
+};
+
+static int get_modified_files(struct repository *r,
+			      enum modified_files_filter filter,
+			      struct string_list *files,
 			      const struct pathspec *ps)
 {
 	struct object_id head_oid;
 	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
 					     &head_oid, NULL);
-	struct collection_status s = { FROM_WORKTREE };
+	struct collection_status s = { 0 };
+	int i;
 
 	if (discard_index(r->index) < 0 ||
 	    repo_read_index_preload(r, ps, 0) < 0)
@@ -412,10 +425,16 @@ static int get_modified_files(struct repository *r, struct string_list *files,
 	s.files = files;
 	hashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0);
 
-	for (s.phase = FROM_WORKTREE; s.phase <= FROM_INDEX; s.phase++) {
+	for (i = 0; i < 2; i++) {
 		struct rev_info rev;
 		struct setup_revision_opt opt = { 0 };
 
+		if (filter == INDEX_ONLY)
+			s.mode = (i == 0) ? FROM_INDEX : FROM_WORKTREE;
+		else
+			s.mode = (i == 0) ? FROM_WORKTREE : FROM_INDEX;
+		s.skip_unseen = filter && i;
+
 		opt.def = is_initial ?
 			empty_tree_oid_hex() : oid_to_hex(&head_oid);
 
@@ -429,7 +448,7 @@ static int get_modified_files(struct repository *r, struct string_list *files,
 		if (ps)
 			copy_pathspec(&rev.prune_data, ps);
 
-		if (s.phase == FROM_INDEX)
+		if (s.mode == FROM_INDEX)
 			run_diff_index(&rev, 1);
 		else {
 			rev.diffopt.flags.ignore_dirty_submodules = 1;
@@ -502,7 +521,7 @@ static void print_file_item(int i, struct string_list_item *item,
 static int run_status(struct add_i_state *s, const struct pathspec *ps,
 		      struct string_list *files, struct list_options *opts)
 {
-	if (get_modified_files(s->r, files, ps) < 0)
+	if (get_modified_files(s->r, NO_FILTER, files, ps) < 0)
 		return -1;
 
 	list(s, files, opts);
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 29, 2019, 21:11 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 3/9] built-in add -i: prepare for multi-selection commands

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

The `update`, `revert` and `add-untracked` commands allow selecting multiple entries. Let's extend the `list_and_choose()` function to accommodate those use cases.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 114 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 89 insertions(+), 25 deletions(-)
Show changes to add-interactive.c +89 −25
diff --git a/add-interactive.c b/add-interactive.c
index c62d63e35b..ea406e903b 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -72,15 +72,17 @@ static void init_add_i_state(struct add_i_state *s, struct repository *r)
 struct prefix_item_list {
 	struct string_list items;
 	struct string_list sorted;
+	int *selected; /* for multi-selections */
 	size_t min_length, max_length;
 };
 #define PREFIX_ITEM_LIST_INIT \
-	{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_NODUP, 1, 4 }
+	{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_NODUP, NULL, 1, 4 }
 
 static void prefix_item_list_clear(struct prefix_item_list *list)
 {
 	string_list_clear(&list->items, 1);
 	string_list_clear(&list->sorted, 0);
+	FREE_AND_NULL(list->selected);
 }
 
 static void extend_prefix_length(struct string_list_item *p,
@@ -182,11 +184,12 @@ static ssize_t find_unique(const char *string, struct prefix_item_list *list)
 struct list_options {
 	int columns;
 	const char *header;
-	void (*print_item)(int i, struct string_list_item *item, void *print_item_data);
+	void (*print_item)(int i, int selected, struct string_list_item *item,
+			   void *print_item_data);
 	void *print_item_data;
 };
 
-static void list(struct add_i_state *s, struct string_list *list,
+static void list(struct add_i_state *s, struct string_list *list, int *selected,
 		 struct list_options *opts)
 {
 	int i, last_lf = 0;
@@ -199,7 +202,8 @@ static void list(struct add_i_state *s, struct string_list *list,
 				 "%s", opts->header);
 
 	for (i = 0; i < list->nr; i++) {
-		opts->print_item(i, list->items + i, opts->print_item_data);
+		opts->print_item(i, selected ? selected[i] : 0, list->items + i,
+				 opts->print_item_data);
 
 		if ((opts->columns) && ((i + 1) % (opts->columns))) {
 			putchar('\t');
@@ -218,6 +222,10 @@ struct list_and_choose_options {
 	struct list_options list_opts;
 
 	const char *prompt;
+	enum {
+		SINGLETON = (1<<0),
+		IMMEDIATE = (1<<1),
+	} flags;
 	void (*print_help)(struct add_i_state *s);
 };
 
@@ -225,7 +233,8 @@ struct list_and_choose_options {
 #define LIST_AND_CHOOSE_QUIT  (-2)
 
 /*
- * Returns the selected index.
+ * Returns the selected index in singleton mode, the number of selected items
+ * otherwise.
  *
  * If an error occurred, returns `LIST_AND_CHOOSE_ERROR`. Upon EOF,
  * `LIST_AND_CHOOSE_QUIT` is returned.
@@ -234,8 +243,19 @@ static ssize_t list_and_choose(struct add_i_state *s,
 			       struct prefix_item_list *items,
 			       struct list_and_choose_options *opts)
 {
+	int singleton = opts->flags & SINGLETON;
+	int immediate = opts->flags & IMMEDIATE;
+
 	struct strbuf input = STRBUF_INIT;
-	ssize_t res = LIST_AND_CHOOSE_ERROR;
+	ssize_t res = singleton ? LIST_AND_CHOOSE_ERROR : 0;
+
+	if (!singleton) {
+		free(items->selected);
+		CALLOC_ARRAY(items->selected, items->items.nr);
+	}
+
+	if (singleton && !immediate)
+		BUG("singleton requires immediate");
 
 	find_unique_prefixes(items);
 
@@ -244,15 +264,16 @@ static ssize_t list_and_choose(struct add_i_state *s,
 
 		strbuf_reset(&input);
 
-		list(s, &items->items, &opts->list_opts);
+		list(s, &items->items, items->selected, &opts->list_opts);
 
 		color_fprintf(stdout, s->prompt_color, "%s", opts->prompt);
-		fputs("> ", stdout);
+		fputs(singleton ? "> " : ">> ", stdout);
 		fflush(stdout);
 
 		if (strbuf_getline(&input, stdin) == EOF) {
 			putchar('\n');
-			res = LIST_AND_CHOOSE_QUIT;
+			if (immediate)
+				res = LIST_AND_CHOOSE_QUIT;
 			break;
 		}
 		strbuf_trim(&input);
@@ -268,7 +289,9 @@ static ssize_t list_and_choose(struct add_i_state *s,
 		p = input.buf;
 		for (;;) {
 			size_t sep = strcspn(p, " \t\r\n,");
-			ssize_t index = -1;
+			int choose = 1;
+			/* `from` is inclusive, `to` is exclusive */
+			ssize_t from = -1, to = -1;
 
 			if (!sep) {
 				if (!*p)
@@ -277,30 +300,70 @@ static ssize_t list_and_choose(struct add_i_state *s,
 				continue;
 			}
 
-			if (isdigit(*p)) {
+			/* Input that begins with '-'; de-select */
+			if (*p == '-') {
+				choose = 0;
+				p++;
+				sep--;
+			}
+
+			if (sep == 1 && *p == '*') {
+				from = 0;
+				to = items->items.nr;
+			} else if (isdigit(*p)) {
 				char *endp;
-				index = strtoul(p, &endp, 10) - 1;
-				if (endp != p + sep)
-					index = -1;
+				/*
+				 * A range can be specified like 5-7 or 5-.
+				 *
+				 * Note: `from` is 0-based while the user input
+				 * is 1-based, hence we have to decrement by
+				 * one. We do not have to decrement `to` even
+				 * if it is 0-based because it is an exclusive
+				 * boundary.
+				 */
+				from = strtoul(p, &endp, 10) - 1;
+				if (endp == p + sep)
+					to = from + 1;
+				else if (*endp == '-') {
+					to = strtoul(++endp, &endp, 10);
+					/* extra characters after the range? */
+					if (endp != p + sep)
+						from = -1;
+				}
 			}
 
 			if (p[sep])
 				p[sep++] = '\0';
-			if (index < 0)
-				index = find_unique(p, items);
+			if (from < 0) {
+				from = find_unique(p, items);
+				if (from >= 0)
+					to = from + 1;
+			}
 
-			if (index < 0 || index >= items->items.nr)
+			if (from < 0 || from >= items->items.nr ||
+			    (singleton && from + 1 != to)) {
 				color_fprintf_ln(stdout, s->error_color,
 						 _("Huh (%s)?"), p);
-			else {
-				res = index;
+				break;
+			} else if (singleton) {
+				res = from;
 				break;
 			}
 
+			if (to > items->items.nr)
+				to = items->items.nr;
+
+			for (; from < to; from++)
+				if (items->selected[from] != choose) {
+					items->selected[from] = choose;
+					res += choose ? +1 : -1;
+				}
+
 			p += sep;
 		}
 
-		if (res != LIST_AND_CHOOSE_ERROR)
+		if ((immediate && res != LIST_AND_CHOOSE_ERROR) ||
+		    !strcmp(input.buf, "*"))
 			break;
 	}
 
@@ -500,7 +563,7 @@ struct print_file_item_data {
 	struct strbuf buf, index, worktree;
 };
 
-static void print_file_item(int i, struct string_list_item *item,
+static void print_file_item(int i, int selected, struct string_list_item *item,
 			    void *print_file_item_data)
 {
 	struct file_item *c = item->util;
@@ -515,7 +578,7 @@ static void print_file_item(int i, struct string_list_item *item,
 	strbuf_addf(&d->buf, d->modified_fmt,
 		    d->index.buf, d->worktree.buf, item->string);
 
-	printf(" %2d: %s", i + 1, d->buf.buf);
+	printf("%c%2d: %s", selected ? '*' : ' ', i + 1, d->buf.buf);
 }
 
 static int run_status(struct add_i_state *s, const struct pathspec *ps,
@@ -524,7 +587,7 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps,
 	if (get_modified_files(s->r, NO_FILTER, files, ps) < 0)
 		return -1;
 
-	list(s, files, opts);
+	list(s, files, NULL, opts);
 	putchar('\n');
 
 	return 0;
@@ -563,7 +626,8 @@ struct print_command_item_data {
 	const char *color, *reset;
 };
 
-static void print_command_item(int i, struct string_list_item *item,
+static void print_command_item(int i, int selected,
+			       struct string_list_item *item,
 			       void *print_command_item_data)
 {
 	struct print_command_item_data *d = print_command_item_data;
@@ -596,7 +660,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 	struct print_command_item_data data = { "[", "]" };
 	struct list_and_choose_options main_loop_opts = {
 		{ 4, N_("*** Commands ***"), print_command_item, &data },
-		N_("What now"), command_prompt_help
+		N_("What now"), SINGLETON | IMMEDIATE, command_prompt_help
 	};
 	struct {
 		const char *string;
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 29, 2019, 21:11 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 4/9] built-in add -i: implement the `update` command

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

After `status` and `help`, it is now time to port the `update` command to C, the second command that is shown in the main loop menu of `git add -i`.

This `git add -i` command is the first one which lets the user choose a subset of a list of files, and as such, this patch lays the groundwork for the other commands of that category:

- It teaches the `print_file_item()` function to show a unique prefix
  if we found any (the code to find it had been added already in the
  previous patch where we colored the unique prefixes of the main loop
  commands, but that patch uses the `print_command_item()` function to
  display the menu items).
- This patch also adds the help text that is shown when the user input
  to select items from the shown list could not be parsed.
- As `get_modified_files()` clears the list of files, it now has to take
  care of clearing the _full_ `prefix_item_list` lest the `sorted` and
  `selected` fields go stale and inconsistent.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 130 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 110 insertions(+), 20 deletions(-)
Show changes to add-interactive.c +110 −20
diff --git a/add-interactive.c b/add-interactive.c
index ea406e903b..1e34e88069 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -6,6 +6,7 @@
 #include "revision.h"
 #include "refs.h"
 #include "string-list.h"
+#include "lockfile.h"
 
 struct add_i_state {
 	struct repository *r;
@@ -377,6 +378,7 @@ struct adddel {
 };
 
 struct file_item {
+	size_t prefix_length;
 	struct adddel index, worktree;
 };
 
@@ -471,7 +473,7 @@ enum modified_files_filter {
 
 static int get_modified_files(struct repository *r,
 			      enum modified_files_filter filter,
-			      struct string_list *files,
+			      struct prefix_item_list *files,
 			      const struct pathspec *ps)
 {
 	struct object_id head_oid;
@@ -484,8 +486,8 @@ static int get_modified_files(struct repository *r,
 	    repo_read_index_preload(r, ps, 0) < 0)
 		return error(_("could not read index"));
 
-	string_list_clear(files, 1);
-	s.files = files;
+	prefix_item_list_clear(files);
+	s.files = &files->items;
 	hashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0);
 
 	for (i = 0; i < 2; i++) {
@@ -524,7 +526,7 @@ static int get_modified_files(struct repository *r,
 	hashmap_free_entries(&s.file_map, struct pathname_entry, ent);
 
 	/* While the diffs are ordered already, we ran *two* diffs... */
-	string_list_sort(files);
+	string_list_sort(&files->items);
 
 	return 0;
 }
@@ -559,8 +561,8 @@ static int is_valid_prefix(const char *prefix, size_t prefix_len)
 }
 
 struct print_file_item_data {
-	const char *modified_fmt;
-	struct strbuf buf, index, worktree;
+	const char *modified_fmt, *color, *reset;
+	struct strbuf buf, name, index, worktree;
 };
 
 static void print_file_item(int i, int selected, struct string_list_item *item,
@@ -568,34 +570,96 @@ static void print_file_item(int i, int selected, struct string_list_item *item,
 {
 	struct file_item *c = item->util;
 	struct print_file_item_data *d = print_file_item_data;
+	const char *highlighted = NULL;
 
 	strbuf_reset(&d->index);
 	strbuf_reset(&d->worktree);
 	strbuf_reset(&d->buf);
 
+	/* Format the item with the prefix highlighted. */
+	if (c->prefix_length > 0 &&
+	    is_valid_prefix(item->string, c->prefix_length)) {
+		strbuf_reset(&d->name);
+		strbuf_addf(&d->name, "%s%.*s%s%s", d->color,
+			    (int)c->prefix_length, item->string, d->reset,
+			    item->string + c->prefix_length);
+		highlighted = d->name.buf;
+	}
+
 	render_adddel(&d->worktree, &c->worktree, _("nothing"));
 	render_adddel(&d->index, &c->index, _("unchanged"));
-	strbuf_addf(&d->buf, d->modified_fmt,
-		    d->index.buf, d->worktree.buf, item->string);
+
+	strbuf_addf(&d->buf, d->modified_fmt, d->index.buf, d->worktree.buf,
+		    highlighted ? highlighted : item->string);
 
 	printf("%c%2d: %s", selected ? '*' : ' ', i + 1, d->buf.buf);
 }
 
 static int run_status(struct add_i_state *s, const struct pathspec *ps,
-		      struct string_list *files, struct list_options *opts)
+		      struct prefix_item_list *files,
+		      struct list_and_choose_options *opts)
 {
 	if (get_modified_files(s->r, NO_FILTER, files, ps) < 0)
 		return -1;
 
-	list(s, files, NULL, opts);
+	list(s, &files->items, NULL, &opts->list_opts);
 	putchar('\n');
 
 	return 0;
 }
 
+static int run_update(struct add_i_state *s, const struct pathspec *ps,
+		      struct prefix_item_list *files,
+		      struct list_and_choose_options *opts)
+{
+	int res = 0, fd;
+	size_t count, i;
+	struct lock_file index_lock;
+
+	if (get_modified_files(s->r, WORKTREE_ONLY, files, ps) < 0)
+		return -1;
+
+	if (!files->items.nr) {
+		putchar('\n');
+		return 0;
+	}
+
+	opts->prompt = N_("Update");
+	count = list_and_choose(s, files, opts);
+	if (count <= 0) {
+		putchar('\n');
+		return 0;
+	}
+
+	fd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR);
+	if (fd < 0) {
+		putchar('\n');
+		return -1;
+	}
+
+	for (i = 0; i < files->items.nr; i++) {
+		const char *name = files->items.items[i].string;
+		if (files->selected[i] &&
+		    add_file_to_index(s->r->index, name, 0) < 0) {
+			res = error(_("could not stage '%s'"), name);
+			break;
+		}
+	}
+
+	if (!res && write_locked_index(s->r->index, &index_lock, COMMIT_LOCK) < 0)
+		res = error(_("could not write index"));
+
+	if (!res)
+		printf(Q_("updated %d path\n",
+			  "updated %d paths\n", count), (int)count);
+
+	putchar('\n');
+	return res;
+}
+
 static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
-		    struct string_list *unused_files,
-		    struct list_options *unused_opts)
+		    struct prefix_item_list *unused_files,
+		    struct list_and_choose_options *unused_opts)
 {
 	color_fprintf_ln(stdout, s->help_color, "status        - %s",
 			 _("show paths with changes"));
@@ -613,9 +677,29 @@ static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
 	return 0;
 }
 
+static void choose_prompt_help(struct add_i_state *s)
+{
+	color_fprintf_ln(stdout, s->help_color, "%s",
+			 _("Prompt help:"));
+	color_fprintf_ln(stdout, s->help_color, "1          - %s",
+			 _("select a single item"));
+	color_fprintf_ln(stdout, s->help_color, "3-5        - %s",
+			 _("select a range of items"));
+	color_fprintf_ln(stdout, s->help_color, "2-3,6-9    - %s",
+			 _("select multiple ranges"));
+	color_fprintf_ln(stdout, s->help_color, "foo        - %s",
+			 _("select item based on unique prefix"));
+	color_fprintf_ln(stdout, s->help_color, "-...       - %s",
+			 _("unselect specified items"));
+	color_fprintf_ln(stdout, s->help_color, "*          - %s",
+			 _("choose all items"));
+	color_fprintf_ln(stdout, s->help_color, "           - %s",
+			 _("(empty) finish selecting"));
+}
+
 typedef int (*command_t)(struct add_i_state *s, const struct pathspec *ps,
-			 struct string_list *files,
-			 struct list_options *opts);
+			 struct prefix_item_list *files,
+			 struct list_and_choose_options *opts);
 
 struct command_item {
 	size_t prefix_length;
@@ -667,18 +751,21 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		command_t command;
 	} command_list[] = {
 		{ "status", run_status },
+		{ "update", run_update },
 		{ "help", run_help },
 	};
 	struct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;
 
 	struct print_file_item_data print_file_item_data = {
-		"%12s %12s %s", STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
+		"%12s %12s %s", NULL, NULL,
+		STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
 	};
-	struct list_options opts = {
-		0, NULL, print_file_item, &print_file_item_data
+	struct list_and_choose_options opts = {
+		{ 0, NULL, print_file_item, &print_file_item_data },
+		NULL, 0, choose_prompt_help
 	};
 	struct strbuf header = STRBUF_INIT;
-	struct string_list files = STRING_LIST_INIT_DUP;
+	struct prefix_item_list files = PREFIX_ITEM_LIST_INIT;
 	ssize_t i;
 	int res = 0;
 
@@ -699,11 +786,13 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		data.color = s.prompt_color;
 		data.reset = s.reset_color;
 	}
+	print_file_item_data.color = data.color;
+	print_file_item_data.reset = data.reset;
 
 	strbuf_addstr(&header, "      ");
 	strbuf_addf(&header, print_file_item_data.modified_fmt,
 		    _("staged"), _("unstaged"), _("path"));
-	opts.header = header.buf;
+	opts.list_opts.header = header.buf;
 
 	if (discard_index(r->index) < 0 ||
 	    repo_read_index(r) < 0 ||
@@ -727,8 +816,9 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		}
 	}
 
-	string_list_clear(&files, 1);
+	prefix_item_list_clear(&files);
 	strbuf_release(&print_file_item_data.buf);
+	strbuf_release(&print_file_item_data.name);
 	strbuf_release(&print_file_item_data.index);
 	strbuf_release(&print_file_item_data.worktree);
 	strbuf_release(&header);
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 29, 2019, 21:11 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 5/9] built-in add -i: re-implement `revert` in C

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

This is a relatively straight-forward port from the Perl version, with the notable exception that we imitate `git reset -- <paths>` in the C version rather than the convoluted `git ls-tree HEAD -- <paths> | git update-index --index-info` followed by `git update-index --force-remove -- <paths>` for the missed ones.

While at it, we fix the pretty obvious bug where the `revert` command offers to unstage files that do not have staged changes.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)
Show changes to add-interactive.c +109 −0
diff --git a/add-interactive.c b/add-interactive.c
index 1e34e88069..adab17a635 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -657,6 +657,114 @@ static int run_update(struct add_i_state *s, const struct pathspec *ps,
 	return res;
 }
 
+static void revert_from_diff(struct diff_queue_struct *q,
+			     struct diff_options *opt, void *data)
+{
+	int i, add_flags = ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE;
+
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filespec *one = q->queue[i]->one;
+		struct cache_entry *ce;
+
+		if (!(one->mode && !is_null_oid(&one->oid))) {
+			remove_file_from_index(opt->repo->index, one->path);
+			printf(_("note: %s is untracked now.\n"), one->path);
+		} else {
+			ce = make_cache_entry(opt->repo->index, one->mode,
+					      &one->oid, one->path, 0, 0);
+			if (!ce)
+				die(_("make_cache_entry failed for path '%s'"),
+				    one->path);
+			add_index_entry(opt->repo->index, ce, add_flags);
+		}
+	}
+}
+
+static int run_revert(struct add_i_state *s, const struct pathspec *ps,
+		      struct prefix_item_list *files,
+		      struct list_and_choose_options *opts)
+{
+	int res = 0, fd;
+	size_t count, i, j;
+
+	struct object_id oid;
+	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &oid,
+					     NULL);
+	struct lock_file index_lock;
+	const char **paths;
+	struct tree *tree;
+	struct diff_options diffopt = { NULL };
+
+	if (get_modified_files(s->r, INDEX_ONLY, files, ps) < 0)
+		return -1;
+
+	if (!files->items.nr) {
+		putchar('\n');
+		return 0;
+	}
+
+	opts->prompt = N_("Revert");
+	count = list_and_choose(s, files, opts);
+	if (count <= 0)
+		goto finish_revert;
+
+	fd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR);
+	if (fd < 0) {
+		res = -1;
+		goto finish_revert;
+	}
+
+	if (is_initial)
+		oidcpy(&oid, s->r->hash_algo->empty_tree);
+	else {
+		tree = parse_tree_indirect(&oid);
+		if (!tree) {
+			res = error(_("Could not parse HEAD^{tree}"));
+			goto finish_revert;
+		}
+		oidcpy(&oid, &tree->object.oid);
+	}
+
+	ALLOC_ARRAY(paths, count + 1);
+	for (i = j = 0; i < files->items.nr; i++)
+		if (files->selected[i])
+			paths[j++] = files->items.items[i].string;
+	paths[j] = NULL;
+
+	parse_pathspec(&diffopt.pathspec, 0,
+		       PATHSPEC_PREFER_FULL | PATHSPEC_LITERAL_PATH,
+		       NULL, paths);
+
+	diffopt.output_format = DIFF_FORMAT_CALLBACK;
+	diffopt.format_callback = revert_from_diff;
+	diffopt.flags.override_submodule_config = 1;
+	diffopt.repo = s->r;
+
+	if (do_diff_cache(&oid, &diffopt))
+		res = -1;
+	else {
+		diffcore_std(&diffopt);
+		diff_flush(&diffopt);
+	}
+	free(paths);
+	clear_pathspec(&diffopt.pathspec);
+
+	if (!res && write_locked_index(s->r->index, &index_lock,
+				       COMMIT_LOCK) < 0)
+		res = -1;
+	else
+		res = repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0, 1,
+						   NULL, NULL, NULL);
+
+	if (!res)
+		printf(Q_("reverted %d path\n",
+			  "reverted %d paths\n", count), (int)count);
+
+finish_revert:
+	putchar('\n');
+	return res;
+}
+
 static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
 		    struct prefix_item_list *unused_files,
 		    struct list_and_choose_options *unused_opts)
@@ -752,6 +860,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 	} command_list[] = {
 		{ "status", run_status },
 		{ "update", run_update },
+		{ "revert", run_revert },
 		{ "help", run_help },
 	};
 	struct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 29, 2019, 21:11 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 7/9] built-in add -i: implement the `patch` command

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

Well, it is not a full implementation yet. In the interest of making this easy to review (and easy to keep bugs out), we still hand off to the Perl script to do the actual work.

The `patch` functionality actually makes up for more than half of the 1,800+ lines of `git-add--interactive.perl`. It will be ported from Perl to C incrementally, later.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 91 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 84 insertions(+), 7 deletions(-)
Show changes to add-interactive.c +84 −7
diff --git a/add-interactive.c b/add-interactive.c
index a719d30b0b..cba9688bb5 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -8,6 +8,7 @@
 #include "string-list.h"
 #include "lockfile.h"
 #include "dir.h"
+#include "run-command.h"
 
 struct add_i_state {
 	struct repository *r;
@@ -375,7 +376,7 @@ static ssize_t list_and_choose(struct add_i_state *s,
 
 struct adddel {
 	uintmax_t add, del;
-	unsigned seen:1, binary:1;
+	unsigned seen:1, unmerged:1, binary:1;
 };
 
 struct file_item {
@@ -415,6 +416,7 @@ struct collection_status {
 	const char *reference;
 
 	unsigned skip_unseen:1;
+	size_t unmerged_count, binary_count;
 	struct string_list *files;
 	struct hashmap file_map;
 };
@@ -437,7 +439,7 @@ static void collect_changes_cb(struct diff_queue_struct *q,
 		int hash = strhash(name);
 		struct pathname_entry *entry;
 		struct file_item *file_item;
-		struct adddel *adddel;
+		struct adddel *adddel, *other_adddel;
 
 		entry = hashmap_get_entry_from_hash(&s->file_map, hash, name,
 						    struct pathname_entry, ent);
@@ -457,11 +459,21 @@ static void collect_changes_cb(struct diff_queue_struct *q,
 		file_item = entry->item;
 		adddel = s->mode == FROM_INDEX ?
 			&file_item->index : &file_item->worktree;
+		other_adddel = s->mode == FROM_INDEX ?
+			&file_item->worktree : &file_item->index;
 		adddel->seen = 1;
 		adddel->add = stat.files[i]->added;
 		adddel->del = stat.files[i]->deleted;
-		if (stat.files[i]->is_binary)
+		if (stat.files[i]->is_binary) {
+			if (!other_adddel->binary)
+				s->binary_count++;
 			adddel->binary = 1;
+		}
+		if (stat.files[i]->is_unmerged) {
+			if (!other_adddel->unmerged)
+				s->unmerged_count++;
+			adddel->unmerged = 1;
+		}
 	}
 	free_diffstat_info(&stat);
 }
@@ -475,7 +487,9 @@ enum modified_files_filter {
 static int get_modified_files(struct repository *r,
 			      enum modified_files_filter filter,
 			      struct prefix_item_list *files,
-			      const struct pathspec *ps)
+			      const struct pathspec *ps,
+			      size_t *unmerged_count,
+			      size_t *binary_count)
 {
 	struct object_id head_oid;
 	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
@@ -525,6 +539,10 @@ static int get_modified_files(struct repository *r,
 			clear_pathspec(&rev.prune_data);
 	}
 	hashmap_free_entries(&s.file_map, struct pathname_entry, ent);
+	if (unmerged_count)
+		*unmerged_count = s.unmerged_count;
+	if (binary_count)
+		*binary_count = s.binary_count;
 
 	/* While the diffs are ordered already, we ran *two* diffs... */
 	string_list_sort(&files->items);
@@ -607,7 +625,7 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps,
 		      struct prefix_item_list *files,
 		      struct list_and_choose_options *opts)
 {
-	if (get_modified_files(s->r, NO_FILTER, files, ps) < 0)
+	if (get_modified_files(s->r, NO_FILTER, files, ps, NULL, NULL) < 0)
 		return -1;
 
 	list(s, &files->items, NULL, &opts->list_opts);
@@ -624,7 +642,7 @@ static int run_update(struct add_i_state *s, const struct pathspec *ps,
 	size_t count, i;
 	struct lock_file index_lock;
 
-	if (get_modified_files(s->r, WORKTREE_ONLY, files, ps) < 0)
+	if (get_modified_files(s->r, WORKTREE_ONLY, files, ps, NULL, NULL) < 0)
 		return -1;
 
 	if (!files->items.nr) {
@@ -703,7 +721,7 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps,
 	struct tree *tree;
 	struct diff_options diffopt = { NULL };
 
-	if (get_modified_files(s->r, INDEX_ONLY, files, ps) < 0)
+	if (get_modified_files(s->r, INDEX_ONLY, files, ps, NULL, NULL) < 0)
 		return -1;
 
 	if (!files->items.nr) {
@@ -855,6 +873,64 @@ static int run_add_untracked(struct add_i_state *s, const struct pathspec *ps,
 	return res;
 }
 
+static int run_patch(struct add_i_state *s, const struct pathspec *ps,
+		     struct prefix_item_list *files,
+		     struct list_and_choose_options *opts)
+{
+	int res = 0;
+	ssize_t count, i, j;
+	size_t unmerged_count = 0, binary_count = 0;
+
+	if (get_modified_files(s->r, WORKTREE_ONLY, files, ps,
+			       &unmerged_count, &binary_count) < 0)
+		return -1;
+
+	if (unmerged_count || binary_count) {
+		for (i = j = 0; i < files->items.nr; i++) {
+			struct file_item *item = files->items.items[i].util;
+
+			if (item->index.binary || item->worktree.binary) {
+				free(item);
+				free(files->items.items[i].string);
+			} else if (item->index.unmerged ||
+				 item->worktree.unmerged) {
+				color_fprintf_ln(stderr, s->error_color,
+						 _("ignoring unmerged: %s"),
+						 files->items.items[i].string);
+				free(item);
+				free(files->items.items[i].string);
+			} else
+				files->items.items[j++] = files->items.items[i];
+		}
+		files->items.nr = j;
+	}
+
+	if (!files->items.nr) {
+		if (binary_count)
+			fprintf(stderr, _("Only binary files changed.\n"));
+		else
+			fprintf(stderr, _("No changes.\n"));
+		return 0;
+	}
+
+	opts->prompt = N_("Patch update");
+	count = list_and_choose(s, files, opts);
+	if (count >= 0) {
+		struct argv_array args = ARGV_ARRAY_INIT;
+
+		argv_array_pushl(&args, "git", "add--interactive", "--patch",
+				 "--", NULL);
+		for (i = 0; i < files->items.nr; i++)
+			if (files->selected[i])
+				argv_array_push(&args,
+						files->items.items[i].string);
+		res = run_command_v_opt(args.argv, 0);
+		argv_array_clear(&args);
+	}
+
+	return res;
+}
+
 static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
 		    struct prefix_item_list *unused_files,
 		    struct list_and_choose_options *unused_opts)
@@ -952,6 +1028,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		{ "update", run_update },
 		{ "revert", run_revert },
 		{ "add untracked", run_add_untracked },
+		{ "patch", run_patch },
 		{ "help", run_help },
 	};
 	struct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 29, 2019, 21:11 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 8/9] built-in add -i: re-implement the `diff` command

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

It is not only laziness that we simply spawn `git diff -p --cached` here: this command needs to use the pager, and the pager needs to exit when the diff is done. Currently we do not have any way to make that happen if we run the diff in-process. So let's just spawn.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
Show changes to add-interactive.c +42 −0
diff --git a/add-interactive.c b/add-interactive.c
index cba9688bb5..4d7d44a917 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -931,6 +931,47 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps,
 	return res;
 }
 
+static int run_diff(struct add_i_state *s, const struct pathspec *ps,
+		    struct prefix_item_list *files,
+		    struct list_and_choose_options *opts)
+{
+	int res = 0;
+	ssize_t count, i;
+
+	struct object_id oid;
+	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &oid,
+					     NULL);
+	if (get_modified_files(s->r, INDEX_ONLY, files, ps, NULL, NULL) < 0)
+		return -1;
+
+	if (!files->items.nr) {
+		putchar('\n');
+		return 0;
+	}
+
+	opts->prompt = N_("Review diff");
+	opts->flags = IMMEDIATE;
+	count = list_and_choose(s, files, opts);
+	opts->flags = 0;
+	if (count >= 0) {
+		struct argv_array args = ARGV_ARRAY_INIT;
+
+		argv_array_pushl(&args, "git", "diff", "-p", "--cached",
+				 oid_to_hex(!is_initial ? &oid :
+					    s->r->hash_algo->empty_tree),
+				 "--", NULL);
+		for (i = 0; i < files->items.nr; i++)
+			if (files->selected[i])
+				argv_array_push(&args,
+						files->items.items[i].string);
+		res = run_command_v_opt(args.argv, 0);
+		argv_array_clear(&args);
+	}
+
+	putchar('\n');
+	return res;
+}
+
 static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
 		    struct prefix_item_list *unused_files,
 		    struct list_and_choose_options *unused_opts)
@@ -1029,6 +1070,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		{ "revert", run_revert },
 		{ "add untracked", run_add_untracked },
 		{ "patch", run_patch },
+		{ "diff", run_diff },
 		{ "help", run_help },
 	};
 	struct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 29, 2019, 21:11 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 9/9] built-in add -i: offer the `quit` command

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

We do not really want to `exit()` here, of course, as this is safely libified code.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
Show changes to add-interactive.c +11 −5
diff --git a/add-interactive.c b/add-interactive.c
index 4d7d44a917..f395d54c08 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -1071,6 +1071,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		{ "add untracked", run_add_untracked },
 		{ "patch", run_patch },
 		{ "diff", run_diff },
+		{ "quit", NULL },
 		{ "help", run_help },
 	};
 	struct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;
@@ -1122,17 +1123,22 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 	res = run_status(&s, ps, &files, &opts);
 
 	for (;;) {
+		struct command_item *util;
+
 		i = list_and_choose(&s, &commands, &main_loop_opts);
-		if (i == LIST_AND_CHOOSE_QUIT) {
+		if (i < 0 || i >= commands.items.nr)
+			util = NULL;
+		else
+			util = commands.items.items[i].util;
+
+		if (i == LIST_AND_CHOOSE_QUIT || (util && !util->command)) {
 			printf(_("Bye.\n"));
 			res = 0;
 			break;
 		}
-		if (i != LIST_AND_CHOOSE_ERROR) {
-			struct command_item *util =
-				commands.items.items[i].util;
+
+		if (util)
 			res = util->command(&s, ps, &files, &opts);
-		}
 	}
 
 	prefix_item_list_clear(&files);
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Nov 29, 2019, 21:11 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 6/9] built-in add -i: re-implement `add-untracked` in C

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

This is yet another command, ported to C. It builds nicely on the support functions introduced for other commands, with the notable difference that only names are displayed for untracked files, no file type or diff summary.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)
Show changes to add-interactive.c +91 −0
diff --git a/add-interactive.c b/add-interactive.c
index adab17a635..a719d30b0b 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -7,6 +7,7 @@
 #include "refs.h"
 #include "string-list.h"
 #include "lockfile.h"
+#include "dir.h"
 
 struct add_i_state {
 	struct repository *r;
@@ -563,6 +564,7 @@ static int is_valid_prefix(const char *prefix, size_t prefix_len)
 struct print_file_item_data {
 	const char *modified_fmt, *color, *reset;
 	struct strbuf buf, name, index, worktree;
+	unsigned only_names:1;
 };
 
 static void print_file_item(int i, int selected, struct string_list_item *item,
@@ -586,6 +588,12 @@ static void print_file_item(int i, int selected, struct string_list_item *item,
 		highlighted = d->name.buf;
 	}
 
+	if (d->only_names) {
+		printf("%c%2d: %s", selected ? '*' : ' ', i + 1,
+		       highlighted ? highlighted : item->string);
+		return;
+	}
+
 	render_adddel(&d->worktree, &c->worktree, _("nothing"));
 	render_adddel(&d->index, &c->index, _("unchanged"));
 
@@ -765,6 +773,88 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps,
 	return res;
 }
 
+static int get_untracked_files(struct repository *r,
+			       struct prefix_item_list *files,
+			       const struct pathspec *ps)
+{
+	struct dir_struct dir = { 0 };
+	size_t i;
+	struct strbuf buf = STRBUF_INIT;
+
+	if (repo_read_index(r) < 0)
+		return error(_("could not read index"));
+
+	prefix_item_list_clear(files);
+	setup_standard_excludes(&dir);
+	add_pattern_list(&dir, EXC_CMDL, "--exclude option");
+	fill_directory(&dir, r->index, ps);
+
+	for (i = 0; i < dir.nr; i++) {
+		struct dir_entry *ent = dir.entries[i];
+
+		if (index_name_is_other(r->index, ent->name, ent->len)) {
+			strbuf_reset(&buf);
+			strbuf_add(&buf, ent->name, ent->len);
+			add_file_item(&files->items, buf.buf);
+		}
+	}
+
+	strbuf_release(&buf);
+	return 0;
+}
+
+static int run_add_untracked(struct add_i_state *s, const struct pathspec *ps,
+		      struct prefix_item_list *files,
+		      struct list_and_choose_options *opts)
+{
+	struct print_file_item_data *d = opts->list_opts.print_item_data;
+	int res = 0, fd;
+	size_t count, i;
+	struct lock_file index_lock;
+
+	if (get_untracked_files(s->r, files, ps) < 0)
+		return -1;
+
+	if (!files->items.nr) {
+		printf(_("No untracked files.\n"));
+		goto finish_add_untracked;
+	}
+
+	opts->prompt = N_("Add untracked");
+	d->only_names = 1;
+	count = list_and_choose(s, files, opts);
+	d->only_names = 0;
+	if (count <= 0)
+		goto finish_add_untracked;
+
+	fd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR);
+	if (fd < 0) {
+		res = -1;
+		goto finish_add_untracked;
+	}
+
+	for (i = 0; i < files->items.nr; i++) {
+		const char *name = files->items.items[i].string;
+		if (files->selected[i] &&
+		    add_file_to_index(s->r->index, name, 0) < 0) {
+			res = error(_("could not stage '%s'"), name);
+			break;
+		}
+	}
+
+	if (!res &&
+	    write_locked_index(s->r->index, &index_lock, COMMIT_LOCK) < 0)
+		res = error(_("could not write index"));
+
+	if (!res)
+		printf(Q_("added %d path\n",
+			  "added %d paths\n", count), (int)count);
+
+finish_add_untracked:
+	putchar('\n');
+	return res;
+}
+
 static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
 		    struct prefix_item_list *unused_files,
 		    struct list_and_choose_options *unused_opts)
@@ -861,6 +951,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 		{ "status", run_status },
 		{ "update", run_update },
 		{ "revert", run_revert },
+		{ "add untracked", run_add_untracked },
 		{ "help", run_help },
 	};
 	struct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;
-- 
gitgitgadget

← back to recent threads