[PATCH v5 00/10] Turn git add-i into built-in

["Slavica Đukić via GitGitGadget" <gitgitgadget@gmail.com>]

This is the first version of a patch series to start porting
git-add--interactive from Perl to C. Daniel Ferreira's patch series used as
a head start:
https://public-inbox.org/git/1494907234-28903-1-git-send-email-bnmvco@gmail.com/t/#u

Changes since v4:

 * rename print_modified to list_modifed
 * the big change was implementing list_and_choose, which resulted in code
   refactoring, i.e. separating list_modified and status_cmd and making
   status_cmd use both list_modified and list_and_choose
 * implement struct choice instead of struct stuff_item as main data
   structure for list_and_choose
 * introduce list_only option and implement support for !list_only users
 * introduce highlighting of unique prefixes

Note that authorship handling is slightly changed. In some of the commits, I
used Original-patch-by instead of listing Daniel Ferreira as author.

Also, I would like to point out that my Outreachy internship is going to
finish on March 4 and I would really appreciate reviews before it does.

Daniel Ferreira (3):
  diff: export diffstat interface
  add--helper: create builtin helper for interactive add
  add--interactive.perl: use add--helper --status for status_cmd

Slavica Djukic (7):
  add-interactive.c: implement list_modified
  add-interactive.c: implement list_and_choose
  add-interactive.c: implement status command
  add-interactive.c: add support for list_only option
  add-interactive.c: implement show-help command
  t3701-add-interactive: test add_i_show_help()
  add--interactive.perl: use add--helper --show-help for help_cmd

 .gitignore                 |   1 +
 Makefile                   |   2 +
 add-interactive.c          | 819 +++++++++++++++++++++++++++++++++++++
 add-interactive.h          |  10 +
 builtin.h                  |   1 +
 builtin/add--helper.c      |  43 ++
 diff.c                     |  36 +-
 diff.h                     |  18 +
 git-add--interactive.perl  |  17 +-
 git.c                      |   1 +
 t/t3701-add-interactive.sh |  24 ++
 11 files changed, 937 insertions(+), 35 deletions(-)
 create mode 100644 add-interactive.c
 create mode 100644 add-interactive.h
 create mode 100644 builtin/add--helper.c


base-commit: ca1b4116483b397e78483376296bcd23916ab553
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-103%2FslavicaDj%2Fadd-i-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-103/slavicaDj/add-i-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/103

Range-diff vs v4:

  1:  737767b6f4 !  1:  d839f0c082 diff: export diffstat interface
     @@ -11,6 +11,7 @@
          how the show_* functions used by diff_flush() do it. One example is the
          builtin implementation of git-add--interactive's status.
      
     +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
          Signed-off-by: Daniel Ferreira <bnmvco@gmail.com>
          Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
      
  2:  91b1963125 !  2:  304c3863b1 add--helper: create builtin helper for interactive add
     @@ -2,8 +2,8 @@
      
          add--helper: create builtin helper for interactive add
      
     -    Create a builtin helper for git-add--interactive, which right now is not
     -    able to do anything.
     +    Create a builtin helper for git-add--interactive, which at this point
     +    is not doing anything.
      
          This is the first step in an effort to convert git-add--interactive.perl
          to a C builtin, in search for better portability, expressibility and
     @@ -13,6 +13,7 @@
          remove the last "big" Git script to have Perl as a dependency, allowing
          most Git users to have a NOPERL build running without big losses.
      
     +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
          Signed-off-by: Daniel Ferreira <bnmvco@gmail.com>
          Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
      
  3:  d247ef69fe !  3:  8790ffaa39 add-interactive.c: implement status command
     @@ -1,17 +1,12 @@
     -Author: Daniel Ferreira <bnmvco@gmail.com>
     +Author: Slavica Djukic <slawica92@hotmail.com>
      
     -    add-interactive.c: implement status command
     +    add-interactive.c: implement list_modified
      
     -    Add new files: add-interactive.c and add-interactive.h, which
     -    will be used for implementing "application logic" of git add -i,
     -    whereas add--helper.c will be used mostly for parsing the command line.
     -    We're a bit lax with the command-line parsing, as the command is
     -    intended to be called only by one internal user: the add--interactive script.
     +    Implement list_modified from Perl, which will be used
     +    by most of the *_cmd functions, including status in the
     +    following commit.
      
     -    Implement add --interactive's status command in add-interactive.c and
     -    use it in builtin add--helper.c.
     -
     -    It prints a numstat comparing changed files between a) the worktree and
     +    It lists a numstat comparing changed files between a) the worktree and
          the index; b) the index and the HEAD.
      
          To do so, we use run_diff_index() and run_diff_files() to get changed
     @@ -19,11 +14,13 @@
          combination of a hashmap and qsort() to print the result in
          O(n) + O(n lg n) complexity.
      
     -    This is the first interactive add command implemented in C of those
     -    anticipated by the previous commit, which introduced
     -    the add--helper built-in.
     +    Add new file: add-interactive.c which will be used for implementing
     +    "application logic" of git add -i (alongside with add-interactive.h,
     +    added in later commit), whereas add--helper.c will be used mostly
     +    for parsing the command line.
      
     -    Signed-off-by: Daniel Ferreira <bnmvco@gmail.com>
     +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
     +    Original-patch-by: Daniel Ferreira <bnmvco@gmail.com>
          Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
      
       diff --git a/Makefile b/Makefile
     @@ -43,7 +40,6 @@
       --- /dev/null
       +++ b/add-interactive.c
      @@
     -+#include "add-interactive.h"
      +#include "cache.h"
      +#include "commit.h"
      +#include "color.h"
     @@ -51,8 +47,6 @@
      +#include "diffcore.h"
      +#include "revision.h"
      +
     -+#define HEADER_INDENT "      "
     -+
      +enum collection_phase {
      +	WORKTREE,
      +	INDEX
     @@ -75,66 +69,8 @@
      +	struct hashmap file_map;
      +};
      +
     -+static int use_color = -1;
     -+enum color_add_i {
     -+	COLOR_PROMPT,
     -+	COLOR_HEADER,
     -+	COLOR_HELP,
     -+	COLOR_ERROR
     -+};
     -+
     -+static char colors[][COLOR_MAXLEN] = {
     -+	GIT_COLOR_BOLD_BLUE, /* Prompt */
     -+	GIT_COLOR_BOLD,      /* Header */
     -+	GIT_COLOR_BOLD_RED,  /* Help */
     -+	GIT_COLOR_BOLD_RED   /* Error */
     -+};
     -+
     -+static const char *get_color(enum color_add_i ix)
     -+{
     -+	if (want_color(use_color))
     -+		return colors[ix];
     -+	return "";
     -+}
     -+
     -+static int parse_color_slot(const char *slot)
     -+{
     -+	if (!strcasecmp(slot, "prompt"))
     -+		return COLOR_PROMPT;
     -+	if (!strcasecmp(slot, "header"))
     -+		return COLOR_HEADER;
     -+	if (!strcasecmp(slot, "help"))
     -+		return COLOR_HELP;
     -+	if (!strcasecmp(slot, "error"))
     -+		return COLOR_ERROR;
     -+
     -+	return -1;
     -+}
     -+
     -+int add_i_config(const char *var,
     -+		const char *value, void *cbdata)
     -+{
     -+	const char *name;
     -+
     -+	if (!strcmp(var, "color.interactive")) {
     -+		use_color = git_config_colorbool(var, value);
     -+		return 0;
     -+	}
     -+
     -+	if (skip_prefix(var, "color.interactive.", &name)) {
     -+		int slot = parse_color_slot(name);
     -+		if (slot < 0)
     -+			return 0;
     -+		if (!value)
     -+			return config_error_nonbool(var);
     -+		return color_parse(value, colors[slot]);
     -+	}
     -+
     -+	return git_default_config(var, value, cbdata);
     -+}
     -+
      +static int hash_cmp(const void *unused_cmp_data, const void *entry,
     -+			const void *entry_or_key, const void *keydata)
     ++		    const void *entry_or_key, const void *keydata)
      +{
      +	const struct file_stat *e1 = entry, *e2 = entry_or_key;
      +	const char *name = keydata ? keydata : e2->name;
     @@ -151,8 +87,8 @@
      +}
      +
      +static void collect_changes_cb(struct diff_queue_struct *q,
     -+					 struct diff_options *options,
     -+					 void *data)
     ++			       struct diff_options *options,
     ++			       void *data)
      +{
      +	struct collection_status *s = data;
      +	struct diffstat_t stat = { 0 };
     @@ -222,128 +158,73 @@
      +	run_diff_index(&rev, 1);
      +}
      +
     -+void add_i_print_modified(void)
     ++static int is_inital_commit(void)
      +{
     -+	int i = 0;
     -+	struct collection_status s;
     -+	/* TRANSLATORS: you can adjust this to align "git add -i" status menu */
     -+	const char *modified_fmt = _("%12s %12s %s");
     -+	const char *header_color = get_color(COLOR_HEADER);
      +	struct object_id sha1;
     ++	if (get_oid("HEAD", &sha1))
     ++		return 1;
     ++	return 0;
     ++}
     ++
     ++static const char *get_diff_reference(void)
     ++{
     ++	if(is_inital_commit())
     ++		return empty_tree_oid_hex();
     ++	return "HEAD";
     ++}
     ++
     ++static void filter_files(const char *filter, struct hashmap *file_map,
     ++			 struct file_stat **files)
     ++{
     ++
     ++	for (int i = 0; i < hashmap_get_size(file_map); i++) {
     ++		struct file_stat *f = files[i];
     ++
     ++		if ((!(f->worktree.added || f->worktree.deleted)) &&
     ++		   (!strcmp(filter, "file-only")))
     ++				hashmap_remove(file_map, f, NULL);
      +
     ++		if ((!(f->index.added || f->index.deleted)) &&
     ++		   (!strcmp(filter, "index-only")))
     ++				hashmap_remove(file_map, f, NULL);
     ++	}
     ++}
     ++
     ++static struct collection_status *list_modified(struct repository *r, const char *filter)
     ++{
     ++	int i = 0;
     ++	struct collection_status *s = xcalloc(1, sizeof(*s));
      +	struct hashmap_iter iter;
      +	struct file_stat **files;
      +	struct file_stat *entry;
      +
     -+	if (read_cache() < 0)
     -+		return;
     ++	if (repo_read_index(r) < 0) {
     ++		printf("\n");
     ++		return NULL;
     ++	}
      +
     -+	s.reference = !get_oid("HEAD", &sha1) ? "HEAD": empty_tree_oid_hex();
     -+	hashmap_init(&s.file_map, hash_cmp, NULL, 0);
     ++	s->reference = get_diff_reference();
     ++	hashmap_init(&s->file_map, hash_cmp, NULL, 0);
      +
     -+	collect_changes_worktree(&s);
     -+	collect_changes_index(&s);
     ++	collect_changes_worktree(s);
     ++	collect_changes_index(s);
      +
     -+	if (hashmap_get_size(&s.file_map) < 1) {
     ++	if (hashmap_get_size(&s->file_map) < 1) {
      +		printf("\n");
     -+		return;
     ++		return NULL;
      +	}
      +
     -+	printf(HEADER_INDENT);
     -+	color_fprintf(stdout, header_color, modified_fmt, _("staged"),
     -+			_("unstaged"), _("path"));
     -+	printf("\n");
     -+
     -+	hashmap_iter_init(&s.file_map, &iter);
     ++	hashmap_iter_init(&s->file_map, &iter);
      +
     -+	files = xcalloc(hashmap_get_size(&s.file_map), sizeof(struct file_stat *));
     ++	files = xcalloc(hashmap_get_size(&s->file_map), sizeof(struct file_stat *));
      +	while ((entry = hashmap_iter_next(&iter))) {
      +		files[i++] = entry;
      +	}
     -+	QSORT(files, hashmap_get_size(&s.file_map), alphabetical_cmp);
     ++	QSORT(files, hashmap_get_size(&s->file_map), alphabetical_cmp);
      +
     -+	for (i = 0; i < hashmap_get_size(&s.file_map); i++) {
     -+		struct file_stat *f = files[i];
     -+
     -+		char worktree_changes[50];
     -+		char index_changes[50];
     -+
     -+		if (f->worktree.added || f->worktree.deleted)
     -+			snprintf(worktree_changes, 50, "+%"PRIuMAX"/-%"PRIuMAX, f->worktree.added,
     -+					f->worktree.deleted);
     -+		else
     -+			snprintf(worktree_changes, 50, "%s", _("nothing"));
     -+
     -+		if (f->index.added || f->index.deleted)
     -+			snprintf(index_changes, 50, "+%"PRIuMAX"/-%"PRIuMAX, f->index.added,
     -+					f->index.deleted);
     -+		else
     -+			snprintf(index_changes, 50, "%s", _("unchanged"));
     -+
     -+		printf(" %2d: ", i + 1);
     -+		printf(modified_fmt, index_changes, worktree_changes, f->name);
     -+		printf("\n");
     -+	}
     -+	printf("\n");
     ++	if (filter)
     ++		filter_files(filter, &s->file_map, files);
      +
      +	free(files);
     -+	hashmap_free(&s.file_map, 1);
     ++	return s;
      +}
     -
     - diff --git a/add-interactive.h b/add-interactive.h
     - new file mode 100644
     - --- /dev/null
     - +++ b/add-interactive.h
     -@@
     -+#ifndef ADD_INTERACTIVE_H
     -+#define ADD_INTERACTIVE_H
     -+
     -+int add_i_config(const char *var, const char *value, void *cbdata);
     -+
     -+void add_i_print_modified(void);
     -+
     -+#endif
     - \ No newline at end of file
     -
     - diff --git a/builtin/add--helper.c b/builtin/add--helper.c
     - --- a/builtin/add--helper.c
     - +++ b/builtin/add--helper.c
     -@@
     -+#include "add-interactive.h"
     - #include "builtin.h"
     -+#include "config.h"
     -+#include "revision.h"
     -+
     -+static const char * const builtin_add_helper_usage[] = {
     -+	N_("git add-interactive--helper <command>"),
     -+	NULL
     -+};
     -+
     -+enum cmd_mode {
     -+	DEFAULT = 0,
     -+	STATUS
     -+};
     - 
     - int cmd_add__helper(int argc, const char **argv, const char *prefix)
     - {
     -+	enum cmd_mode mode = DEFAULT;
     -+
     -+	struct option options[] = {
     -+		OPT_CMDMODE(0, "status", &mode,
     -+			 N_("print status information with diffstat"), STATUS),
     -+		OPT_END()
     -+	};
     -+
     -+	git_config(add_i_config, NULL);
     -+	argc = parse_options(argc, argv, NULL, options,
     -+			     builtin_add_helper_usage,
     -+			     PARSE_OPT_KEEP_ARGV0);
     -+
     -+	if (mode == STATUS)
     -+		add_i_print_modified();
     -+	else
     -+		usage_with_options(builtin_add_helper_usage,
     -+				   options);
     -+
     - 	return 0;
     - }
  -:  ---------- >  4:  a97b29d274 add-interactive.c: implement list_and_choose
  -:  ---------- >  5:  9a72aabe6c add-interactive.c: implement status command
  4:  fb3f9378ac !  6:  883963ee6e add--interactive.perl: use add--helper --status for status_cmd
     @@ -14,6 +14,7 @@
          the fact that it would be hard to test, we'll pass on adding
          a regression test for this.
      
     +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
          Signed-off-by: Daniel Ferreira <bnmvco@gmail.com>
          Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
          Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
  -:  ---------- >  7:  7912f37517 add-interactive.c: add support for list_only option
  5:  ab16afd1d5 !  8:  441321fc3d add-interactive.c: implement show-help command
     @@ -10,31 +10,32 @@
          handle_builtin and re-routed to the help command, without ever
          calling cmd_add__helper().
      
     +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
          Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
      
       diff --git a/add-interactive.c b/add-interactive.c
       --- a/add-interactive.c
       +++ b/add-interactive.c
      @@
     - 	free(files);
     - 	hashmap_free(&s.file_map, 1);
     + 	free(s);
     + 	free_choices(&choices);
       }
      +
      +void add_i_show_help(void)
      +{
      +	const char *help_color = get_color(COLOR_HELP);
      +	color_fprintf_ln(stdout, help_color, "status        - %s",
     -+			_("show paths with changes"));
     ++			 _("show paths with changes"));
      +	color_fprintf_ln(stdout, help_color, "update        - %s",
     -+			_("add working tree state to the staged set of changes"));
     ++			 _("add working tree state to the staged set of changes"));
      +	color_fprintf_ln(stdout, help_color, "revert        - %s",
     -+			_("revert staged set of changes back to the HEAD version"));
     ++			 _("revert staged set of changes back to the HEAD version"));
      +	color_fprintf_ln(stdout, help_color, "patch         - %s",
     -+			_("pick hunks and update selectively"));
     ++			 _("pick hunks and update selectively"));
      +	color_fprintf_ln(stdout, help_color, "diff          - %s",
     -+			_("view diff between HEAD and index"));
     ++			 _("view diff between HEAD and index"));
      +	color_fprintf_ln(stdout, help_color, "add untracked - %s",
     -+			_("add contents of untracked files to the staged set of changes"));
     ++			 _("add contents of untracked files to the staged set of changes"));
      +}
      
       diff --git a/add-interactive.h b/add-interactive.h
     @@ -42,13 +43,11 @@
       +++ b/add-interactive.h
      @@
       
     - void add_i_print_modified(void);
     + void add_i_status(void);
       
     --#endif
     - \ No newline at end of file
      +void add_i_show_help(void);
      +
     -+#endif
     + #endif
      
       diff --git a/builtin/add--helper.c b/builtin/add--helper.c
       --- a/builtin/add--helper.c
     @@ -66,16 +65,16 @@
      @@
       	struct option options[] = {
       		OPT_CMDMODE(0, "status", &mode,
     - 			 N_("print status information with diffstat"), STATUS),
     + 			    N_("print status information with diffstat"), STATUS),
      +		OPT_CMDMODE(0, "show-help", &mode,
     -+			 N_("show help"), HELP),
     ++			    N_("show help"), HELP),
       		OPT_END()
       	};
       
      @@
       
       	if (mode == STATUS)
     - 		add_i_print_modified();
     + 		add_i_status();
      +	else if (mode == HELP)
      +		add_i_show_help();
       	else
  6:  0a27304a84 !  9:  315ae8b211 t3701-add-interactive: test add_i_show_help()
     @@ -11,6 +11,7 @@
          Prefix git add -i call with GIT_PAGER_IN_USE=true TERM=vt100
          to force colored output on Windows.
      
     +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
          Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
      
       diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
  7:  ca2a7c4375 ! 10:  2b4bdce730 add--interactive.perl: use add--helper --show-help for help_cmd
     @@ -12,6 +12,7 @@
          to print the numstat, also here we forgo adding a regression test:
          the Perl script is on its way out (and this patch is part of that journey).
      
     +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
          Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
          Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
      

-- 
gitgitgadget