End of Outreachy internship

[Slavica Djukic <slavicadj.ip2018@gmail.com>]

Hi everyone,

Today is officially the last day of my Outreachy internship.
I wanted to say a few things about my experience and future of the
project I worked on.
Project was "Turn git add -i into built-in" with Johannes Schindelin
as my mentor.

I truly had amazing time, I learned so much about
coding and Git itself, went to Git Merge and FOSDEM and had a chance
to meet great people.
I've never contributed before to open-source, and now that I did,
I find it invaluable experience.
Although my obligations at this point don't allow me to work on
the project, I will try my best to contribute in the future to Git
or/and other great open-source projects.
That said, my mentor Johannes Schindelin will continue to work on
the project from now on.

What is implemented in this project so far:
commands: status and help, helper functions like highlight_prefix(),
find_unique_prefixes(), is_valid_prefix(), etc., but most importantly:
list_and_choose() and list_modified().
Most of the add -i commands will use those two functions to collect,
show the data and let the user make a choice.

What is left to be done:
In the current patch series, I got a lot of suggestion from Junio,
and I applied all of them -- the only thing left is correcting
list_and_choose more to make it type-independent.
In the future, all the other commands: update, revert, add untracked,
patch, diff and quit.

I would like to thank all reviewers for useful suggestions and
comprehension.
And thousand thanks to Johannes Schindelin, who turned out to be
the most amazing mentor.

-Slavica Djukic

On 20-Feb-19 12:41 PM, Slavica Đukić via GitGitGadget wrote:
> 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>
>        
>