From ed53346b92ea9573e07d588243da303a8b63c610 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 7 Mar 2019 13:05:42 +0100 Subject: [PATCH 01/11] Start to implement a built-in version of `git add --interactive` This is hardly the first conversion of a Git command that is implemented as a script to a built-in. So far, the most successful strategy for such conversions has been to add a built-in helper and call that for more and more functionality from the script, as more and more parts are converted. With the interactive add, we choose a different strategy. The sole reason for this is that on Windows (where such a conversion has the most benefits in terms of speed and robustness) we face the very specific problem that a `system()` call in Perl seems to close `stdin` in the parent process when the spawned process consumes even one character from `stdin`. And that just does not work for us here, as it would stop the main loop as soon as any interactive command was performed by the helper. Which is almost all of the commands in `git add -i`. It is almost as if Perl told us once again that it does not want us to use it on Windows. Instead, we follow the opposite route where we start with a bare-bones version of the built-in interactive add, guarded by the new `add.interactive.useBuiltin` config variable, and then add more and more functionality to it, until it is feature complete. At this point, the built-in version of `git add -i` only states that it cannot do anything yet ;-) Signed-off-by: Johannes Schindelin --- Documentation/config/add.txt | 5 +++++ Makefile | 1 + add-interactive.c | 7 +++++++ add-interactive.h | 8 ++++++++ builtin/add.c | 10 ++++++++++ t/README | 4 ++++ 6 files changed, 35 insertions(+) create mode 100644 add-interactive.c create mode 100644 add-interactive.h diff --git a/Documentation/config/add.txt b/Documentation/config/add.txt index 4d753f006e..c9f748f81c 100644 --- a/Documentation/config/add.txt +++ b/Documentation/config/add.txt @@ -5,3 +5,8 @@ add.ignore-errors (deprecated):: option of linkgit:git-add[1]. `add.ignore-errors` is deprecated, as it does not follow the usual naming convention for configuration variables. + +add.interactive.useBuiltin:: + [EXPERIMENTAL] Set to `true` to use the experimental built-in + implementation of the interactive version of linkgit:git-add[1] + instead of the Perl script version. Is `false` by default. diff --git a/Makefile b/Makefile index c5240942f2..18e656a32f 100644 --- a/Makefile +++ b/Makefile @@ -848,6 +848,7 @@ LIB_H = $(shell $(FIND) . \ -name '*.h' -print) LIB_OBJS += abspath.o +LIB_OBJS += add-interactive.o LIB_OBJS += advice.o LIB_OBJS += alias.o LIB_OBJS += alloc.o diff --git a/add-interactive.c b/add-interactive.c new file mode 100644 index 0000000000..482e458dc6 --- /dev/null +++ b/add-interactive.c @@ -0,0 +1,7 @@ +#include "cache.h" +#include "add-interactive.h" + +int run_add_i(struct repository *r, const struct pathspec *ps) +{ + die(_("No commands are available in the built-in `git add -i` yet!")); +} diff --git a/add-interactive.h b/add-interactive.h new file mode 100644 index 0000000000..7043b8741d --- /dev/null +++ b/add-interactive.h @@ -0,0 +1,8 @@ +#ifndef ADD_INTERACTIVE_H +#define ADD_INTERACTIVE_H + +struct repository; +struct pathspec; +int run_add_i(struct repository *r, const struct pathspec *ps); + +#endif diff --git a/builtin/add.c b/builtin/add.c index db2dfa4350..e19d66e894 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -20,6 +20,7 @@ #include "bulk-checkin.h" #include "argv-array.h" #include "submodule.h" +#include "add-interactive.h" static const char * const builtin_add_usage[] = { N_("git add [] [--] ..."), @@ -185,6 +186,14 @@ int run_add_interactive(const char *revision, const char *patch_mode, { int status, i; struct argv_array argv = ARGV_ARRAY_INIT; + int use_builtin_add_i = + git_env_bool("GIT_TEST_ADD_I_USE_BUILTIN", -1); + if (use_builtin_add_i < 0) + git_config_get_bool("add.interactive.usebuiltin", + &use_builtin_add_i); + + if (use_builtin_add_i == 1 && !patch_mode) + return !!run_add_i(the_repository, pathspec); argv_array_push(&argv, "add--interactive"); if (patch_mode) @@ -319,6 +328,7 @@ static int add_config(const char *var, const char *value, void *cb) ignore_add_errors = git_config_bool(var, value); return 0; } + return git_default_config(var, value, cb); } diff --git a/t/README b/t/README index 886bbec5bc..6408a1847e 100644 --- a/t/README +++ b/t/README @@ -383,6 +383,10 @@ GIT_TEST_REBASE_USE_BUILTIN=, when false, disables the builtin version of git-rebase. See 'rebase.useBuiltin' in git-config(1). +GIT_TEST_ADD_I_USE_BUILTIN=, when true, enables the +builtin version of git add -i. See 'add.interactive.useBuiltin' in +git-config(1). + GIT_TEST_INDEX_THREADS= enables exercising the multi-threaded loading of the index for the whole test suite by bypassing the default number of cache entries and thread minimums. Setting this to 1 will make the From bc99009fbf0e01e9bbe77aa6410489beadf454dc Mon Sep 17 00:00:00 2001 From: Daniel Ferreira Date: Tue, 16 May 2017 01:00:31 -0300 Subject: [PATCH 02/11] diff: export diffstat interface Make the diffstat interface (namely, the diffstat_t struct and compute_diffstat) no longer be internal to diff.c and allow it to be used by other parts of git. This is helpful for code that may want to easily extract information from files using the diff machinery, while flushing it differently from how the show_* functions used by diff_flush() do it. One example is the builtin implementation of git-add--interactive's status. Signed-off-by: Daniel Ferreira Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- diff.c | 37 +++++++++++++++---------------------- diff.h | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/diff.c b/diff.c index 5306c48652..daa5f3a736 100644 --- a/diff.c +++ b/diff.c @@ -2489,22 +2489,6 @@ static void pprint_rename(struct strbuf *name, const char *a, const char *b) } } -struct diffstat_t { - int nr; - int alloc; - struct diffstat_file { - char *from_name; - char *name; - char *print_name; - const char *comments; - unsigned is_unmerged:1; - unsigned is_binary:1; - unsigned is_renamed:1; - unsigned is_interesting:1; - uintmax_t added, deleted; - } **files; -}; - static struct diffstat_file *diffstat_add(struct diffstat_t *diffstat, const char *name_a, const char *name_b) @@ -6001,12 +5985,7 @@ void diff_flush(struct diff_options *options) dirstat_by_line) { struct diffstat_t diffstat; - memset(&diffstat, 0, sizeof(struct diffstat_t)); - for (i = 0; i < q->nr; i++) { - struct diff_filepair *p = q->queue[i]; - if (check_pair_status(p)) - diff_flush_stat(p, options, &diffstat); - } + compute_diffstat(options, &diffstat, q); if (output_format & DIFF_FORMAT_NUMSTAT) show_numstat(&diffstat, options); if (output_format & DIFF_FORMAT_DIFFSTAT) @@ -6306,6 +6285,20 @@ static int is_submodule_ignored(const char *path, struct diff_options *options) return ignored; } +void compute_diffstat(struct diff_options *options, + struct diffstat_t *diffstat, + struct diff_queue_struct *q) +{ + int i; + + memset(diffstat, 0, sizeof(struct diffstat_t)); + for (i = 0; i < q->nr; i++) { + struct diff_filepair *p = q->queue[i]; + if (check_pair_status(p)) + diff_flush_stat(p, options, diffstat); + } +} + void diff_addremove(struct diff_options *options, int addremove, unsigned mode, const struct object_id *oid, diff --git a/diff.h b/diff.h index b512d0477a..ae9bedfab8 100644 --- a/diff.h +++ b/diff.h @@ -240,6 +240,22 @@ void diff_emit_submodule_error(struct diff_options *o, const char *err); void diff_emit_submodule_pipethrough(struct diff_options *o, const char *line, int len); +struct diffstat_t { + int nr; + int alloc; + struct diffstat_file { + char *from_name; + char *name; + char *print_name; + const char *comments; + unsigned is_unmerged:1; + unsigned is_binary:1; + unsigned is_renamed:1; + unsigned is_interesting:1; + uintmax_t added, deleted; + } **files; +}; + enum color_diff { DIFF_RESET = 0, DIFF_CONTEXT = 1, @@ -328,6 +344,9 @@ void diff_change(struct diff_options *, struct diff_filepair *diff_unmerge(struct diff_options *, const char *path); +void compute_diffstat(struct diff_options *options, struct diffstat_t *diffstat, + struct diff_queue_struct *q); + #define DIFF_SETUP_REVERSE 1 #define DIFF_SETUP_USE_SIZE_CACHE 4 From 5e23c0756b5ee543437d0dbf4e9f685df6341bdf Mon Sep 17 00:00:00 2001 From: Daniel Ferreira Date: Tue, 16 May 2017 01:00:32 -0300 Subject: [PATCH 03/11] built-in add -i: implement the `status` command This implements the `status` command of `git add -i`. The data structures introduced in this commit will be extended as needed later. At this point, we re-implement only part of the `list_and_choose()` function of the Perl script `git-add--interactive.perl` and call it `list()`. It does not yet color anything, or do columns, or allow user input. Over the course of the next commits, we will introduce a `list_and_choose()` function that uses `list()` to display the list of options and let the user choose one or more of the displayed items. This will be used to implement the main loop of the built-in `git add -i`, at which point the new `status` command can actually be used. Note that we pass the list of items as a `struct item **` as opposed to a `struct item *`, to allow for the actual items to contain much more information than merely the name. Signed-off-by: Daniel Ferreira Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- add-interactive.c | 265 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 264 insertions(+), 1 deletion(-) diff --git a/add-interactive.c b/add-interactive.c index 482e458dc6..59b28011f7 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -1,7 +1,270 @@ #include "cache.h" #include "add-interactive.h" +#include "diffcore.h" +#include "revision.h" +#include "refs.h" + +struct item { + const char *name; +}; + +struct list_options { + const char *header; + void (*print_item)(int i, struct item *item, void *print_item_data); + void *print_item_data; +}; + +static void list(struct item **list, size_t nr, struct list_options *opts) +{ + int i; + + if (!nr) + return; + + if (opts->header) + printf("%s\n", opts->header); + + for (i = 0; i < nr; i++) { + opts->print_item(i, list[i], opts->print_item_data); + putchar('\n'); + } +} + +struct adddel { + uintmax_t add, del; + unsigned seen:1, binary:1; +}; + +struct file_list { + struct file_item { + struct item item; + struct adddel index, worktree; + } **file; + size_t nr, alloc; +}; + +static void add_file_item(struct file_list *list, const char *name) +{ + struct file_item *item; + + FLEXPTR_ALLOC_STR(item, item.name, name); + + ALLOC_GROW(list->file, list->nr + 1, list->alloc); + list->file[list->nr++] = item; +} + +static void reset_file_list(struct file_list *list) +{ + size_t i; + + for (i = 0; i < list->nr; i++) + free(list->file[i]); + list->nr = 0; +} + +static void release_file_list(struct file_list *list) +{ + reset_file_list(list); + FREE_AND_NULL(list->file); + list->alloc = 0; +} + +static int file_item_cmp(const void *a, const void *b) +{ + const struct file_item * const *f1 = a; + const struct file_item * const *f2 = b; + + return strcmp((*f1)->item.name, (*f2)->item.name); +} + +struct pathname_entry { + struct hashmap_entry ent; + size_t index; + char pathname[FLEX_ARRAY]; +}; + +static int pathname_entry_cmp(const void *unused_cmp_data, + const void *entry, const void *entry_or_key, + const void *pathname) +{ + const struct pathname_entry *e1 = entry, *e2 = entry_or_key; + + return strcmp(e1->pathname, + pathname ? (const char *)pathname : e2->pathname); +} + +struct collection_status { + enum { FROM_WORKTREE = 0, FROM_INDEX = 1 } phase; + + const char *reference; + + struct file_list *list; + struct hashmap file_map; +}; + +static void collect_changes_cb(struct diff_queue_struct *q, + struct diff_options *options, + void *data) +{ + struct collection_status *s = data; + struct diffstat_t stat = { 0 }; + int i; + + if (!q->nr) + return; + + compute_diffstat(options, &stat, q); + + for (i = 0; i < stat.nr; i++) { + const char *name = stat.files[i]->name; + int hash = strhash(name); + struct pathname_entry *entry; + size_t file_index; + struct file_item *file; + struct adddel *adddel; + + entry = hashmap_get_from_hash(&s->file_map, hash, name); + if (entry) + file_index = entry->index; + else { + FLEX_ALLOC_STR(entry, pathname, name); + hashmap_entry_init(entry, hash); + entry->index = file_index = s->list->nr; + hashmap_add(&s->file_map, entry); + + add_file_item(s->list, name); + } + file = s->list->file[file_index]; + + adddel = s->phase == FROM_INDEX ? &file->index : &file->worktree; + adddel->seen = 1; + adddel->add = stat.files[i]->added; + adddel->del = stat.files[i]->deleted; + if (stat.files[i]->is_binary) + adddel->binary = 1; + } +} + +static int get_modified_files(struct repository *r, struct file_list *list, + 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 }; + + if (repo_read_index_preload(r, ps, 0) < 0) + return error(_("could not read index")); + + s.list = list; + hashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0); + + for (s.phase = FROM_WORKTREE; s.phase <= FROM_INDEX; s.phase++) { + struct rev_info rev; + struct setup_revision_opt opt = { 0 }; + + opt.def = is_initial ? + empty_tree_oid_hex() : oid_to_hex(&head_oid); + + init_revisions(&rev, NULL); + setup_revisions(0, NULL, &rev, &opt); + + rev.diffopt.output_format = DIFF_FORMAT_CALLBACK; + rev.diffopt.format_callback = collect_changes_cb; + rev.diffopt.format_callback_data = &s; + + if (ps) + copy_pathspec(&rev.prune_data, ps); + + if (s.phase == FROM_INDEX) + run_diff_index(&rev, 1); + else { + rev.diffopt.flags.ignore_dirty_submodules = 1; + run_diff_files(&rev, 0); + } + } + hashmap_free(&s.file_map, 1); + + /* While the diffs are ordered already, we ran *two* diffs... */ + QSORT(list->file, list->nr, file_item_cmp); + + return 0; +} + +static void populate_wi_changes(struct strbuf *buf, + struct adddel *ad, const char *no_changes) +{ + if (ad->binary) + strbuf_addstr(buf, _("binary")); + else if (ad->seen) + strbuf_addf(buf, "+%"PRIuMAX"/-%"PRIuMAX, + (uintmax_t)ad->add, (uintmax_t)ad->del); + else + strbuf_addstr(buf, no_changes); +} + +struct print_file_item_data { + const char *modified_fmt; + struct strbuf buf, index, worktree; +}; + +static void print_file_item(int i, struct item *item, + void *print_file_item_data) +{ + struct file_item *c = (struct file_item *)item; + struct print_file_item_data *d = print_file_item_data; + + strbuf_reset(&d->index); + strbuf_reset(&d->worktree); + strbuf_reset(&d->buf); + + populate_wi_changes(&d->worktree, &c->worktree, _("nothing")); + populate_wi_changes(&d->index, &c->index, _("unchanged")); + strbuf_addf(&d->buf, d->modified_fmt, + d->index.buf, d->worktree.buf, item->name); + + printf(" %2d: %s", i + 1, d->buf.buf); +} + +static int run_status(struct repository *r, const struct pathspec *ps, + struct file_list *files, struct list_options *opts) +{ + reset_file_list(files); + + if (get_modified_files(r, files, ps) < 0) + return -1; + + if (files->nr) + list((struct item **)files->file, files->nr, opts); + putchar('\n'); + + return 0; +} int run_add_i(struct repository *r, const struct pathspec *ps) { - die(_("No commands are available in the built-in `git add -i` yet!")); + struct print_file_item_data print_file_item_data = { + "%12s %12s %s", STRBUF_INIT, STRBUF_INIT, STRBUF_INIT + }; + struct list_options opts = { + NULL, print_file_item, &print_file_item_data + }; + struct strbuf header = STRBUF_INIT; + struct file_list files = { NULL }; + int res = 0; + + strbuf_addstr(&header, " "); + strbuf_addf(&header, print_file_item_data.modified_fmt, + _("staged"), _("unstaged"), _("path")); + opts.header = header.buf; + + res = run_status(r, ps, &files, &opts); + + release_file_list(&files); + strbuf_release(&print_file_item_data.buf); + strbuf_release(&print_file_item_data.index); + strbuf_release(&print_file_item_data.worktree); + strbuf_release(&header); + + return res; } From 8cafc6ae8d2dd434f46751874074940f13412b9f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 27 Mar 2019 23:46:46 +0100 Subject: [PATCH 04/11] built-in add -i: refresh the index before running `status` This is what the Perl version does, and therefore it is what the built-in version should do, too. Signed-off-by: Johannes Schindelin --- add-interactive.c | 4 +++- repository.c | 19 +++++++++++++++++++ repository.h | 7 +++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/add-interactive.c b/add-interactive.c index 59b28011f7..2dbf29dee2 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -258,7 +258,9 @@ int run_add_i(struct repository *r, const struct pathspec *ps) _("staged"), _("unstaged"), _("path")); opts.header = header.buf; - res = run_status(r, ps, &files, &opts); + repo_refresh_and_write_index(r, REFRESH_QUIET, 1); + if (run_status(r, ps, &files, &opts) < 0) + res = -1; release_file_list(&files); strbuf_release(&print_file_item_data.buf); diff --git a/repository.c b/repository.c index 65e6f8b8fd..c90f310093 100644 --- a/repository.c +++ b/repository.c @@ -272,3 +272,22 @@ int repo_hold_locked_index(struct repository *repo, BUG("the repo hasn't been setup"); return hold_lock_file_for_update(lf, repo->index_file, flags); } + +int repo_refresh_and_write_index(struct repository *r, + unsigned int flags, int gentle) +{ + struct lock_file lock_file = LOCK_INIT; + int fd; + + if (repo_read_index_preload(r, NULL, 0) < 0) + return error(_("could not read index")); + fd = repo_hold_locked_index(r, &lock_file, 0); + if (!gentle && fd < 0) + return error(_("could not lock index for writing")); + refresh_index(r->index, flags, NULL, NULL, NULL); + if (0 <= fd) + repo_update_index_if_able(r, &lock_file); + rollback_lock_file(&lock_file); + + return 0; +} diff --git a/repository.h b/repository.h index 8981649d43..fb49e0e328 100644 --- a/repository.h +++ b/repository.h @@ -154,5 +154,12 @@ int repo_read_index_unmerged(struct repository *); */ void repo_update_index_if_able(struct repository *, struct lock_file *); +/* + * Refresh the index and write it out. If the index file could not be + * locked, error out, except in gentle mode. The flags will be passed + * through to refresh_index(). + */ +int repo_refresh_and_write_index(struct repository *r, + unsigned int flags, int gentle); #endif /* REPOSITORY_H */ From 83d92a9762b4c10d74d804a6104b5ae76330ca17 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 May 2019 23:10:52 +0200 Subject: [PATCH 05/11] built-in add -i: color the header in the `status` command For simplicity, we only implemented the `status` command without colors. This patch starts adding color, matching what the Perl script `git-add--interactive.perl` does. Original-Patch-By: Daniel Ferreira Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- add-interactive.c | 60 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 6 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 2dbf29dee2..6c2fca12c1 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -1,9 +1,51 @@ #include "cache.h" #include "add-interactive.h" +#include "color.h" +#include "config.h" #include "diffcore.h" #include "revision.h" #include "refs.h" +struct add_i_state { + struct repository *r; + int use_color; + char header_color[COLOR_MAXLEN]; +}; + +static void init_color(struct repository *r, struct add_i_state *s, + const char *slot_name, char *dst, + const char *default_color) +{ + char *key = xstrfmt("color.interactive.%s", slot_name); + const char *value; + + if (!s->use_color) + dst[0] = '\0'; + else if (repo_config_get_value(r, key, &value) || + color_parse(value, dst)) + strlcpy(dst, default_color, COLOR_MAXLEN); + + free(key); +} + +static int init_add_i_state(struct repository *r, struct add_i_state *s) +{ + const char *value; + + s->r = r; + + if (repo_config_get_value(r, "color.interactive", &value)) + s->use_color = -1; + else + s->use_color = + git_config_colorbool("color.interactive", value); + s->use_color = want_color(s->use_color); + + init_color(r, s, "header", s->header_color, GIT_COLOR_BOLD); + + return 0; +} + struct item { const char *name; }; @@ -14,7 +56,8 @@ struct list_options { void *print_item_data; }; -static void list(struct item **list, size_t nr, struct list_options *opts) +static void list(struct item **list, size_t nr, + struct add_i_state *s, struct list_options *opts) { int i; @@ -22,7 +65,8 @@ static void list(struct item **list, size_t nr, struct list_options *opts) return; if (opts->header) - printf("%s\n", opts->header); + color_fprintf_ln(stdout, s->header_color, + "%s", opts->header); for (i = 0; i < nr; i++) { opts->print_item(i, list[i], opts->print_item_data); @@ -226,16 +270,16 @@ static void print_file_item(int i, struct item *item, printf(" %2d: %s", i + 1, d->buf.buf); } -static int run_status(struct repository *r, const struct pathspec *ps, +static int run_status(struct add_i_state *s, const struct pathspec *ps, struct file_list *files, struct list_options *opts) { reset_file_list(files); - if (get_modified_files(r, files, ps) < 0) + if (get_modified_files(s->r, files, ps) < 0) return -1; if (files->nr) - list((struct item **)files->file, files->nr, opts); + list((struct item **)files->file, files->nr, s, opts); putchar('\n'); return 0; @@ -243,6 +287,7 @@ static int run_status(struct repository *r, const struct pathspec *ps, int run_add_i(struct repository *r, const struct pathspec *ps) { + struct add_i_state s = { NULL }; struct print_file_item_data print_file_item_data = { "%12s %12s %s", STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; @@ -253,13 +298,16 @@ int run_add_i(struct repository *r, const struct pathspec *ps) struct file_list files = { NULL }; int res = 0; + if (init_add_i_state(r, &s)) + return error("could not parse `add -i` config"); + strbuf_addstr(&header, " "); strbuf_addf(&header, print_file_item_data.modified_fmt, _("staged"), _("unstaged"), _("path")); opts.header = header.buf; repo_refresh_and_write_index(r, REFRESH_QUIET, 1); - if (run_status(r, ps, &files, &opts) < 0) + if (run_status(&s, ps, &files, &opts) < 0) res = -1; release_file_list(&files); From db978391e8acab8cb3dccb52b25981f86220a4f8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 7 Mar 2019 13:05:42 +0100 Subject: [PATCH 06/11] built-in add -i: implement the main loop The reason why we did not start with the main loop to begin with is that it is the first user of `list_and_choose()`, which uses the `list()` function that we conveniently introduced for use by the `status` command. Apart from the "and choose" part, there are more differences between the way the `status` command calls the `list_and_choose()` function in the Perl version of `git add -i` compared to the other callers of said function. The most important ones: - The list is not only shown, but the user is also asked to make a choice, possibly selecting multiple entries. - The list of items is prefixed with a marker indicating what items have been selected, if multi-selection is allowed. - Initially, for each item a unique prefix (if there exists any within the given parameters) is determined, and shown in the list, and accepted as a shortcut for the selection. These features will be implemented later, except the part where the user can choose a command. At this stage, though, the built-in `git add -i` still only supports the `status` command, with the remaining commands to follow over the course of the next commits. In addition, we also modify `list()` to support displaying the commands in columns, even if there is currently only one. The Perl script `git-add--interactive.perl` mixed the purposes of the "list" and the "and choose" part into the same function. In the C version, we will keep them separate instead, calling the `list()` function from the `list_and_choose()` function. Note that we only have a prompt ending in a single ">" at this stage; later commits will add commands that display a double ">>" to indicate that the user is in a different loop than the main one. Signed-off-by: Johannes Schindelin --- add-interactive.c | 125 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 122 insertions(+), 3 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 6c2fca12c1..33d10d25ef 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -51,6 +51,7 @@ struct item { }; struct list_options { + int columns; const char *header; void (*print_item)(int i, struct item *item, void *print_item_data); void *print_item_data; @@ -59,7 +60,7 @@ struct list_options { static void list(struct item **list, size_t nr, struct add_i_state *s, struct list_options *opts) { - int i; + int i, last_lf = 0; if (!nr) return; @@ -70,8 +71,91 @@ static void list(struct item **list, size_t nr, for (i = 0; i < nr; i++) { opts->print_item(i, list[i], opts->print_item_data); - putchar('\n'); + + if ((opts->columns) && ((i + 1) % (opts->columns))) { + putchar('\t'); + last_lf = 0; + } + else { + putchar('\n'); + last_lf = 1; + } } + + if (!last_lf) + putchar('\n'); +} +struct list_and_choose_options { + struct list_options list_opts; + + const char *prompt; +}; + +/* + * Returns the selected index. + */ +static ssize_t list_and_choose(struct item **items, size_t nr, + struct add_i_state *s, + struct list_and_choose_options *opts) +{ + struct strbuf input = STRBUF_INIT; + ssize_t res = -1; + + for (;;) { + char *p, *endp; + + strbuf_reset(&input); + + list(items, nr, s, &opts->list_opts); + + printf("%s%s", opts->prompt, "> "); + fflush(stdout); + + if (strbuf_getline(&input, stdin) == EOF) { + putchar('\n'); + res = -2; + break; + } + strbuf_trim(&input); + + if (!input.len) + break; + + p = input.buf; + for (;;) { + size_t sep = strcspn(p, " \t\r\n,"); + ssize_t index = -1; + + if (!sep) { + if (!*p) + break; + p++; + continue; + } + + if (isdigit(*p)) { + index = strtoul(p, &endp, 10) - 1; + if (endp != p + sep) + index = -1; + } + + p[sep] = '\0'; + if (index < 0 || index >= nr) + printf(_("Huh (%s)?\n"), p); + else { + res = index; + break; + } + + p += sep + 1; + } + + if (res >= 0) + break; + } + + strbuf_release(&input); + return res; } struct adddel { @@ -285,17 +369,40 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps, return 0; } +static void print_command_item(int i, struct item *item, + void *print_command_item_data) +{ + printf(" %2d: %s", i + 1, item->name); +} + +struct command_item { + struct item item; + int (*command)(struct add_i_state *s, const struct pathspec *ps, + struct file_list *files, struct list_options *opts); +}; + int run_add_i(struct repository *r, const struct pathspec *ps) { struct add_i_state s = { NULL }; + struct list_and_choose_options main_loop_opts = { + { 4, N_("*** Commands ***"), print_command_item, NULL }, + N_("What now") + }; + struct command_item + status = { { "status" }, run_status }; + struct command_item *commands[] = { + &status + }; + struct print_file_item_data print_file_item_data = { "%12s %12s %s", STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; struct list_options opts = { - NULL, print_file_item, &print_file_item_data + 0, NULL, print_file_item, &print_file_item_data }; struct strbuf header = STRBUF_INIT; struct file_list files = { NULL }; + ssize_t i; int res = 0; if (init_add_i_state(r, &s)) @@ -310,6 +417,18 @@ int run_add_i(struct repository *r, const struct pathspec *ps) if (run_status(&s, ps, &files, &opts) < 0) res = -1; + for (;;) { + i = list_and_choose((struct item **)commands, + ARRAY_SIZE(commands), &s, &main_loop_opts); + if (i < -1) { + printf(_("Bye.\n")); + res = 0; + break; + } + if (i >= 0) + res = commands[i]->command(&s, ps, &files, &opts); + } + release_file_list(&files); strbuf_release(&print_file_item_data.buf); strbuf_release(&print_file_item_data.index); From d7245b92d85790db9b25b238e58ef1b12ce9a20b Mon Sep 17 00:00:00 2001 From: Slavica Djukic Date: Wed, 27 Mar 2019 01:08:50 +0100 Subject: [PATCH 07/11] Add a function to determine unique prefixes for a list of strings In the `git add -i` command, we show unique prefixes of the commands and files, to give an indication what prefix would select them. Naturally, the C implementation looks a lot different than the Perl implementation: in Perl, a trie is much easier implemented, while we already have a pretty neat hashmap implementation in C that we use for the purpose of storing (not necessarily unique) prefixes. The idea: for each item that we add, we generate prefixes starting with the first letter, then the first two letters, then three, etc, until we find a prefix that is unique (or until the prefix length would be longer than we want). If we encounter a previously-unique prefix on the way, we adjust that item's prefix to make it unique again (or we mark it as having no unique prefix if we failed to find one). These partial prefixes are stored in a hash map (for quick lookup times). To make sure that this function works as expected, we add a test using a special-purpose test helper that was added for that purpose. Note: We expect the list of prefix items to be passed in as a list of pointers rather than as regular list to avoid having to copy information (the actual items will most likely contain more information than just the name and the length of the unique prefix, but passing in `struct prefix_item *` would not allow for that). Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- Makefile | 2 + prefix-map.c | 111 +++++++++++++++++++++++++++++++++++++ prefix-map.h | 40 +++++++++++++ t/helper/test-prefix-map.c | 58 +++++++++++++++++++ t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + t/t0016-prefix-map.sh | 10 ++++ 7 files changed, 223 insertions(+) create mode 100644 prefix-map.c create mode 100644 prefix-map.h create mode 100644 t/helper/test-prefix-map.c create mode 100755 t/t0016-prefix-map.sh diff --git a/Makefile b/Makefile index 18e656a32f..8299b3f17d 100644 --- a/Makefile +++ b/Makefile @@ -754,6 +754,7 @@ TEST_BUILTINS_OBJS += test-online-cpus.o TEST_BUILTINS_OBJS += test-parse-options.o TEST_BUILTINS_OBJS += test-path-utils.o TEST_BUILTINS_OBJS += test-pkt-line.o +TEST_BUILTINS_OBJS += test-prefix-map.o TEST_BUILTINS_OBJS += test-prio-queue.o TEST_BUILTINS_OBJS += test-reach.o TEST_BUILTINS_OBJS += test-read-cache.o @@ -967,6 +968,7 @@ LIB_OBJS += patch-ids.o LIB_OBJS += path.o LIB_OBJS += pathspec.o LIB_OBJS += pkt-line.o +LIB_OBJS += prefix-map.o LIB_OBJS += preload-index.o LIB_OBJS += pretty.o LIB_OBJS += prio-queue.o diff --git a/prefix-map.c b/prefix-map.c new file mode 100644 index 0000000000..3c5ae4ae0a --- /dev/null +++ b/prefix-map.c @@ -0,0 +1,111 @@ +#include "cache.h" +#include "prefix-map.h" + +static int map_cmp(const void *unused_cmp_data, + const void *entry, + const void *entry_or_key, + const void *unused_keydata) +{ + const struct prefix_map_entry *a = entry; + const struct prefix_map_entry *b = entry_or_key; + + return a->prefix_length != b->prefix_length || + strncmp(a->name, b->name, a->prefix_length); +} + +static void add_prefix_entry(struct hashmap *map, const char *name, + size_t prefix_length, struct prefix_item *item) +{ + struct prefix_map_entry *result = xmalloc(sizeof(*result)); + result->name = name; + result->prefix_length = prefix_length; + result->item = item; + hashmap_entry_init(result, memhash(name, prefix_length)); + hashmap_add(map, result); +} + +static void init_prefix_map(struct prefix_map *prefix_map, + int min_prefix_length, int max_prefix_length) +{ + hashmap_init(&prefix_map->map, map_cmp, NULL, 0); + prefix_map->min_length = min_prefix_length; + prefix_map->max_length = max_prefix_length; +} + +static void add_prefix_item(struct prefix_map *prefix_map, + struct prefix_item *item) +{ + struct prefix_map_entry *e = xmalloc(sizeof(*e)), *e2; + int j; + + e->item = item; + e->name = e->item->name; + + for (j = prefix_map->min_length; j <= prefix_map->max_length; j++) { + if (!isascii(e->name[j])) { + free(e); + break; + } + + e->prefix_length = j; + hashmap_entry_init(e, memhash(e->name, j)); + e2 = hashmap_get(&prefix_map->map, e, NULL); + if (!e2) { + /* prefix is unique so far */ + e->item->prefix_length = j; + hashmap_add(&prefix_map->map, e); + break; + } + + if (!e2->item) + continue; /* non-unique prefix */ + + if (j != e2->item->prefix_length) + BUG("unexpected prefix length: %d != %d", + (int)j, (int)e2->item->prefix_length); + + /* skip common prefix */ + for (; j < prefix_map->max_length && e->name[j]; j++) { + if (e->item->name[j] != e2->item->name[j]) + break; + add_prefix_entry(&prefix_map->map, e->name, j + 1, + NULL); + } + + /* e2 no longer refers to a unique prefix */ + if (j < prefix_map->max_length && e2->name[j]) { + /* found a new unique prefix for e2's item */ + e2->item->prefix_length = j + 1; + add_prefix_entry(&prefix_map->map, e2->name, j + 1, + e2->item); + } + else + e2->item->prefix_length = 0; + e2->item = NULL; + + if (j < prefix_map->max_length && e->name[j]) { + /* found a unique prefix for the item */ + e->item->prefix_length = j + 1; + add_prefix_entry(&prefix_map->map, e->name, j + 1, + e->item); + } else { + /* item has no (short enough) unique prefix */ + e->item->prefix_length = 0; + free(e); + } + + break; + } +} + +void find_unique_prefixes(struct prefix_item **list, size_t nr, + int min_length, int max_length) +{ + int i; + struct prefix_map prefix_map; + + init_prefix_map(&prefix_map, min_length, max_length); + for (i = 0; i < nr; i++) + add_prefix_item(&prefix_map, list[i]); + hashmap_free(&prefix_map.map, 1); +} diff --git a/prefix-map.h b/prefix-map.h new file mode 100644 index 0000000000..ce3b8a4a32 --- /dev/null +++ b/prefix-map.h @@ -0,0 +1,40 @@ +#ifndef PREFIX_MAP_H +#define PREFIX_MAP_H + +#include "hashmap.h" + +struct prefix_item { + const char *name; + size_t prefix_length; +}; + +struct prefix_map_entry { + struct hashmap_entry e; + const char *name; + size_t prefix_length; + /* if item is NULL, the prefix is not unique */ + struct prefix_item *item; +}; + +struct prefix_map { + struct hashmap map; + int min_length, max_length; +}; + +/* + * Find unique prefixes in a given list of strings. + * + * Typically, the `struct prefix_item` information will be but a field in the + * actual item struct; For this reason, the `list` parameter is specified as a + * list of pointers to the items. + * + * The `min_length`/`max_length` parameters define what length the unique + * prefixes should have. + * + * If no unique prefix could be found for a given item, its `prefix_length` + * will be set to 0. + */ +void find_unique_prefixes(struct prefix_item **list, size_t nr, + int min_length, int max_length); + +#endif diff --git a/t/helper/test-prefix-map.c b/t/helper/test-prefix-map.c new file mode 100644 index 0000000000..3f1c90eaf0 --- /dev/null +++ b/t/helper/test-prefix-map.c @@ -0,0 +1,58 @@ +#include "test-tool.h" +#include "cache.h" +#include "prefix-map.h" + +static size_t test_count, failed_count; + +static void check(int succeeded, const char *file, size_t line_no, + const char *fmt, ...) +{ + va_list ap; + + test_count++; + if (succeeded) + return; + + va_start(ap, fmt); + fprintf(stderr, "%s:%d: ", file, (int)line_no); + vfprintf(stderr, fmt, ap); + fputc('\n', stderr); + va_end(ap); + + failed_count++; +} + +#define EXPECT_SIZE_T_EQUALS(expect, actual, hint) \ + check(expect == actual, __FILE__, __LINE__, \ + "size_t's do not match: %" \ + PRIdMAX " != %" PRIdMAX " (%s) (%s)", \ + (intmax_t)expect, (intmax_t)actual, #actual, hint) + +int cmd__prefix_map(int argc, const char **argv) +{ +#define NR 5 + struct prefix_item items[NR] = { + { "unique" }, + { "hell" }, + { "hello" }, + { "wok" }, + { "world" }, + }; + struct prefix_item *list[NR] = { + items, items + 1, items + 2, items + 3, items + 4 + }; + + find_unique_prefixes(list, NR, 1, 3); + +#define EXPECT_PREFIX_LENGTH_EQUALS(expect, index) \ + EXPECT_SIZE_T_EQUALS(expect, list[index]->prefix_length, \ + list[index]->name) + + EXPECT_PREFIX_LENGTH_EQUALS(1, 0); + EXPECT_PREFIX_LENGTH_EQUALS(0, 1); + EXPECT_PREFIX_LENGTH_EQUALS(0, 2); + EXPECT_PREFIX_LENGTH_EQUALS(3, 3); + EXPECT_PREFIX_LENGTH_EQUALS(3, 4); + + return !!failed_count; +} diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c index 99db7409b8..d6a92a8699 100644 --- a/t/helper/test-tool.c +++ b/t/helper/test-tool.c @@ -32,6 +32,7 @@ static struct test_cmd cmds[] = { { "parse-options", cmd__parse_options }, { "path-utils", cmd__path_utils }, { "pkt-line", cmd__pkt_line }, + { "prefix-map", cmd__prefix_map }, { "prio-queue", cmd__prio_queue }, { "reach", cmd__reach }, { "read-cache", cmd__read_cache }, diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h index 25abed1cf2..33a089ee4e 100644 --- a/t/helper/test-tool.h +++ b/t/helper/test-tool.h @@ -29,6 +29,7 @@ int cmd__online_cpus(int argc, const char **argv); int cmd__parse_options(int argc, const char **argv); int cmd__path_utils(int argc, const char **argv); int cmd__pkt_line(int argc, const char **argv); +int cmd__prefix_map(int argc, const char **argv); int cmd__prio_queue(int argc, const char **argv); int cmd__reach(int argc, const char **argv); int cmd__read_cache(int argc, const char **argv); diff --git a/t/t0016-prefix-map.sh b/t/t0016-prefix-map.sh new file mode 100755 index 0000000000..187fa92aec --- /dev/null +++ b/t/t0016-prefix-map.sh @@ -0,0 +1,10 @@ +#!/bin/sh + +test_description='basic tests for prefix map' +. ./test-lib.sh + +test_expect_success 'prefix map' ' + test-tool prefix-map +' + +test_done From 573f9f415493f2a06d82177ba29aa73382835032 Mon Sep 17 00:00:00 2001 From: Slavica Djukic Date: Wed, 27 Feb 2019 12:31:53 +0100 Subject: [PATCH 08/11] built-in add -i: show unique prefixes of the commands Just like in the Perl script `git-add--interactive.perl`, for each command a unique prefix is determined (if there exists any within the given parameters), and shown in the list, and accepted as a shortcut for the command. We use the prefix map implementation that we just added in the previous commit for that purpose. Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- add-interactive.c | 69 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 13 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 33d10d25ef..44ab670f7f 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -5,6 +5,7 @@ #include "diffcore.h" #include "revision.h" #include "refs.h" +#include "prefix-map.h" struct add_i_state { struct repository *r; @@ -46,18 +47,32 @@ static int init_add_i_state(struct repository *r, struct add_i_state *s) return 0; } -struct item { - const char *name; -}; +static ssize_t find_unique(const char *string, + struct prefix_item **list, size_t nr) +{ + ssize_t found = -1, i; + + for (i = 0; i < nr; i++) { + struct prefix_item *item = list[i]; + if (!starts_with(item->name, string)) + continue; + if (found >= 0) + return -1; + found = i; + } + + return found; +} struct list_options { int columns; const char *header; - void (*print_item)(int i, struct item *item, void *print_item_data); + void (*print_item)(int i, struct prefix_item *item, + void *print_item_data); void *print_item_data; }; -static void list(struct item **list, size_t nr, +static void list(struct prefix_item **list, size_t nr, struct add_i_state *s, struct list_options *opts) { int i, last_lf = 0; @@ -94,13 +109,15 @@ struct list_and_choose_options { /* * Returns the selected index. */ -static ssize_t list_and_choose(struct item **items, size_t nr, +static ssize_t list_and_choose(struct prefix_item **items, size_t nr, struct add_i_state *s, struct list_and_choose_options *opts) { struct strbuf input = STRBUF_INIT; ssize_t res = -1; + find_unique_prefixes(items, nr, 1, 4); + for (;;) { char *p, *endp; @@ -140,6 +157,9 @@ static ssize_t list_and_choose(struct item **items, size_t nr, } p[sep] = '\0'; + if (index < 0) + index = find_unique(p, items, nr); + if (index < 0 || index >= nr) printf(_("Huh (%s)?\n"), p); else { @@ -165,7 +185,7 @@ struct adddel { struct file_list { struct file_item { - struct item item; + struct prefix_item item; struct adddel index, worktree; } **file; size_t nr, alloc; @@ -331,12 +351,29 @@ static void populate_wi_changes(struct strbuf *buf, strbuf_addstr(buf, no_changes); } +/* filters out prefixes which have special meaning to list_and_choose() */ +static int is_valid_prefix(const char *prefix, size_t prefix_len) +{ + return prefix_len && prefix && + /* + * We expect `prefix` to be NUL terminated, therefore this + * `strcspn()` call is okay, even if it might do much more + * work than strictly necessary. + */ + strcspn(prefix, " \t\r\n,") >= prefix_len && /* separators */ + *prefix != '-' && /* deselection */ + !isdigit(*prefix) && /* selection */ + (prefix_len != 1 || + (*prefix != '*' && /* "all" wildcard */ + *prefix != '?')); /* prompt help */ +} + struct print_file_item_data { const char *modified_fmt; struct strbuf buf, index, worktree; }; -static void print_file_item(int i, struct item *item, +static void print_file_item(int i, struct prefix_item *item, void *print_file_item_data) { struct file_item *c = (struct file_item *)item; @@ -363,20 +400,26 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps, return -1; if (files->nr) - list((struct item **)files->file, files->nr, s, opts); + list((struct prefix_item **)files->file, files->nr, s, opts); putchar('\n'); return 0; } -static void print_command_item(int i, struct item *item, +static void print_command_item(int i, struct prefix_item *item, void *print_command_item_data) { - printf(" %2d: %s", i + 1, item->name); + if (!item->prefix_length || + !is_valid_prefix(item->name, item->prefix_length)) + printf(" %2d: %s", i + 1, item->name); + else + printf(" %3d: [%.*s]%s", i + 1, + (int)item->prefix_length, item->name, + item->name + item->prefix_length); } struct command_item { - struct item item; + struct prefix_item item; int (*command)(struct add_i_state *s, const struct pathspec *ps, struct file_list *files, struct list_options *opts); }; @@ -418,7 +461,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps) res = -1; for (;;) { - i = list_and_choose((struct item **)commands, + i = list_and_choose((struct prefix_item **)commands, ARRAY_SIZE(commands), &s, &main_loop_opts); if (i < -1) { printf(_("Bye.\n")); From f48c70e2db5054ae202841892488faafa8c5c92a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 6 Mar 2019 00:06:57 +0100 Subject: [PATCH 09/11] built-in add -i: support `?` (prompt help) With this change, we print out the same colored help text that the Perl-based `git add -i` prints in the main loop when question mark is entered. Signed-off-by: Johannes Schindelin --- add-interactive.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/add-interactive.c b/add-interactive.c index 44ab670f7f..ac8af70c85 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -11,6 +11,7 @@ struct add_i_state { struct repository *r; int use_color; char header_color[COLOR_MAXLEN]; + char help_color[COLOR_MAXLEN]; }; static void init_color(struct repository *r, struct add_i_state *s, @@ -43,6 +44,7 @@ static int init_add_i_state(struct repository *r, struct add_i_state *s) s->use_color = want_color(s->use_color); init_color(r, s, "header", s->header_color, GIT_COLOR_BOLD); + init_color(r, s, "help", s->help_color, GIT_COLOR_BOLD_RED); return 0; } @@ -104,6 +106,7 @@ struct list_and_choose_options { struct list_options list_opts; const char *prompt; + void (*print_help)(struct add_i_state *s); }; /* @@ -138,6 +141,11 @@ static ssize_t list_and_choose(struct prefix_item **items, size_t nr, if (!input.len) break; + if (!strcmp(input.buf, "?")) { + opts->print_help(s); + continue; + } + p = input.buf; for (;;) { size_t sep = strcspn(p, " \t\r\n,"); @@ -424,12 +432,24 @@ struct command_item { struct file_list *files, struct list_options *opts); }; +static void command_prompt_help(struct add_i_state *s) +{ + const char *help_color = s->help_color; + color_fprintf_ln(stdout, help_color, "%s", _("Prompt help:")); + color_fprintf_ln(stdout, help_color, "1 - %s", + _("select a numbered item")); + color_fprintf_ln(stdout, help_color, "foo - %s", + _("select item based on unique prefix")); + color_fprintf_ln(stdout, help_color, " - %s", + _("(empty) select nothing")); +} + int run_add_i(struct repository *r, const struct pathspec *ps) { struct add_i_state s = { NULL }; struct list_and_choose_options main_loop_opts = { { 4, N_("*** Commands ***"), print_command_item, NULL }, - N_("What now") + N_("What now"), command_prompt_help }; struct command_item status = { { "status" }, run_status }; From 9cf326024af1ae6f21ebabd473f862053874201c Mon Sep 17 00:00:00 2001 From: Slavica Djukic Date: Sun, 3 Mar 2019 13:19:27 +0100 Subject: [PATCH 10/11] built-in add -i: use color in the main loop The error messages as well as the unique prefixes are colored in `git add -i` by default; We need to do the same in the built-in version. Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- add-interactive.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index ac8af70c85..dfd1b6b160 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -12,6 +12,9 @@ struct add_i_state { int use_color; char header_color[COLOR_MAXLEN]; char help_color[COLOR_MAXLEN]; + char prompt_color[COLOR_MAXLEN]; + char error_color[COLOR_MAXLEN]; + char reset_color[COLOR_MAXLEN]; }; static void init_color(struct repository *r, struct add_i_state *s, @@ -45,6 +48,9 @@ static int init_add_i_state(struct repository *r, struct add_i_state *s) init_color(r, s, "header", s->header_color, GIT_COLOR_BOLD); init_color(r, s, "help", s->help_color, GIT_COLOR_BOLD_RED); + init_color(r, s, "prompt", s->prompt_color, GIT_COLOR_BOLD_BLUE); + init_color(r, s, "error", s->error_color, GIT_COLOR_BOLD_RED); + init_color(r, s, "reset", s->reset_color, GIT_COLOR_RESET); return 0; } @@ -128,7 +134,8 @@ static ssize_t list_and_choose(struct prefix_item **items, size_t nr, list(items, nr, s, &opts->list_opts); - printf("%s%s", opts->prompt, "> "); + color_fprintf(stdout, s->prompt_color, "%s", opts->prompt); + fputs("> ", stdout); fflush(stdout); if (strbuf_getline(&input, stdin) == EOF) { @@ -169,7 +176,8 @@ static ssize_t list_and_choose(struct prefix_item **items, size_t nr, index = find_unique(p, items, nr); if (index < 0 || index >= nr) - printf(_("Huh (%s)?\n"), p); + color_fprintf_ln(stdout, s->error_color, + _("Huh (%s)?"), p); else { res = index; break; @@ -414,15 +422,21 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps, return 0; } +struct print_command_item_data { + const char *color, *reset; +}; + static void print_command_item(int i, struct prefix_item *item, void *print_command_item_data) { + struct print_command_item_data *d = print_command_item_data; + if (!item->prefix_length || !is_valid_prefix(item->name, item->prefix_length)) printf(" %2d: %s", i + 1, item->name); else - printf(" %3d: [%.*s]%s", i + 1, - (int)item->prefix_length, item->name, + printf(" %2d: %s%.*s%s%s", i + 1, + d->color, (int)item->prefix_length, item->name, d->reset, item->name + item->prefix_length); } @@ -447,8 +461,9 @@ static void command_prompt_help(struct add_i_state *s) int run_add_i(struct repository *r, const struct pathspec *ps) { struct add_i_state s = { NULL }; + struct print_command_item_data data; struct list_and_choose_options main_loop_opts = { - { 4, N_("*** Commands ***"), print_command_item, NULL }, + { 4, N_("*** Commands ***"), print_command_item, &data }, N_("What now"), command_prompt_help }; struct command_item @@ -471,6 +486,18 @@ int run_add_i(struct repository *r, const struct pathspec *ps) if (init_add_i_state(r, &s)) return error("could not parse `add -i` config"); + /* + * When color was asked for, use the prompt color for + * highlighting, otherwise use square brackets. + */ + if (s.use_color) { + data.color = s.prompt_color; + data.reset = s.reset_color; + } else { + data.color = "["; + data.reset = "]"; + } + strbuf_addstr(&header, " "); strbuf_addf(&header, print_file_item_data.modified_fmt, _("staged"), _("unstaged"), _("path")); From ddb913e1947949e5442cb3b7c5559238b8535754 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 7 Mar 2019 01:12:07 +0100 Subject: [PATCH 11/11] built-in add -i: implement the `help` command This imitates the code to show the help text from the Perl script `git-add--interactive.perl` in the built-in version. To make sure that it renders exactly like the Perl version of `git add -i`, we also add a test case for that to `t3701-add-interactive.sh`. Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- add-interactive.c | 27 +++++++++++++++++++++++++-- t/t3701-add-interactive.sh | 24 ++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index dfd1b6b160..e690ca57fa 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -422,6 +422,27 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps, return 0; } +static int run_help(struct add_i_state *s, const struct pathspec *ps, + struct file_list *files, struct list_options *opts) +{ + const char *help_color = s->help_color; + + color_fprintf_ln(stdout, help_color, "status - %s", + _("show paths with changes")); + color_fprintf_ln(stdout, help_color, "update - %s", + _("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")); + color_fprintf_ln(stdout, help_color, "patch - %s", + _("pick hunks and update selectively")); + color_fprintf_ln(stdout, help_color, "diff - %s", + _("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")); + + return 0; +} + struct print_command_item_data { const char *color, *reset; }; @@ -467,9 +488,11 @@ int run_add_i(struct repository *r, const struct pathspec *ps) N_("What now"), command_prompt_help }; struct command_item - status = { { "status" }, run_status }; + status = { { "status" }, run_status }, + help = { { "help" }, run_help }; struct command_item *commands[] = { - &status + &status, + &help }; struct print_file_item_data print_file_item_data = { diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 65dfbc033a..91aaef2932 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -639,4 +639,28 @@ test_expect_success 'add -p patch editing works with pathological context lines' test_cmp expected-2 actual ' +test_expect_success 'show help from add--helper' ' + git reset --hard && + cat >expect <<-EOF && + + *** Commands *** + 1: status 2: update 3: revert 4: add untracked + 5: patch 6: diff 7: quit 8: help + What now> status - show paths with changes + update - add working tree state to the staged set of changes + revert - revert staged set of changes back to the HEAD version + patch - pick hunks and update selectively + diff - view diff between HEAD and index + add untracked - add contents of untracked files to the staged set of changes + *** Commands *** + 1: status 2: update 3: revert 4: add untracked + 5: patch 6: diff 7: quit 8: help + What now>$SP + Bye. + EOF + test_write_lines h | GIT_PAGER_IN_USE=true TERM=vt100 git add -i >actual.colored && + test_decode_color actual && + test_i18ncmp expect actual +' + test_done