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> -- gitgitgadget
