End of Outreachy internship

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx/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@xxxxxx>
           Signed-off-by: Daniel Ferreira <bnmvco@xxxxxxxxx>
           Signed-off-by: Slavica Djukic <slawica92@xxxxxxxxxxx>
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@xxxxxx>
           Signed-off-by: Daniel Ferreira <bnmvco@xxxxxxxxx>
           Signed-off-by: Slavica Djukic <slawica92@xxxxxxxxxxx>
3: d247ef69fe ! 3: 8790ffaa39 add-interactive.c: implement status command
      @@ -1,17 +1,12 @@
      -Author: Daniel Ferreira <bnmvco@xxxxxxxxx>
      +Author: Slavica Djukic <slawica92@xxxxxxxxxxx>
- 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@xxxxxxxxx>
      +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
      +    Original-patch-by: Daniel Ferreira <bnmvco@xxxxxxxxx>
           Signed-off-by: Slavica Djukic <slawica92@xxxxxxxxxxx>
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@xxxxxx>
           Signed-off-by: Daniel Ferreira <bnmvco@xxxxxxxxx>
           Signed-off-by: Slavica Djukic <slawica92@xxxxxxxxxxx>
           Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
   -:  ---------- >  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@xxxxxx>
           Signed-off-by: Slavica Djukic <slawica92@xxxxxxxxxxx>
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@xxxxxx>
           Signed-off-by: Slavica Djukic <slawica92@xxxxxxxxxxx>
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@xxxxxx>
           Signed-off-by: Slavica Djukic <slawica92@xxxxxxxxxxx>
           Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux