{"thread":{"id":"52271","subject":"[PATCH 1/8] built-in add -i: allow filtering the modified files list","startedAt":"2019-11-15T12:36:28Z","lastAt":"2019-11-29T21:12:08Z","messageCount":29,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"386305","messageId":"1844c4d55e21c40cbdbfdd73c82b4a1a074ff184.1573821382.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.git.1573821382.gitgitgadget@gmail.com","subject":"[PATCH 1/8] built-in add -i: allow filtering the modified files list","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-15T12:36:15Z","receivedAt":"2019-11-15T12:36:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the `update` command of `git add -i`, we are primarily interested in the\nlist of modified files that have worktree (i.e. unstaged) changes.\n\nAt the same time, we need to determine _also_ the staged changes, to be\nable to produce the full added/deleted information.\n\nThe Perl script version of `git add -i` has a parameter of the\n`list_modified()` function for that matter. In C, we can be a lot more\nprecise, using an `enum`.\n\nThe C implementation of the filter also has an easier time to avoid\nunnecessary work, simply by using an adaptive order of the `diff-index`\nand `diff-files` phases, and then skipping files in the second phase\nwhen they have not been seen in the first phase.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 25 ++++++++++++++++++++++---\n 1 file changed, 22 insertions(+), 3 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 5d89863bab..8ec930ac15 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -347,6 +347,7 @@ struct collection_status {\n \n \tconst char *reference;\n \n+\tunsigned skip_unseen:1;\n \tstruct string_list *files;\n \tstruct hashmap file_map;\n };\n@@ -374,6 +375,9 @@ static void collect_changes_cb(struct diff_queue_struct *q,\n \t\tentry = hashmap_get_entry_from_hash(&s->file_map, hash, name,\n \t\t\t\t\t\t struct pathname_entry, ent);\n \t\tif (!entry) {\n+\t\t\tif (s->skip_unseen)\n+\t\t\t\tcontinue;\n+\n \t\t\tadd_file_item(s->files, name);\n \n \t\t\tentry = xcalloc(sizeof(*entry), 1);\n@@ -395,13 +399,22 @@ static void collect_changes_cb(struct diff_queue_struct *q,\n \tfree_diffstat_info(&stat);\n }\n \n-static int get_modified_files(struct repository *r, struct string_list *files,\n+enum modified_files_filter {\n+\tNO_FILTER = 0,\n+\tWORKTREE_ONLY = 1,\n+\tINDEX_ONLY = 2,\n+};\n+\n+static int get_modified_files(struct repository *r,\n+\t\t\t enum modified_files_filter filter,\n+\t\t\t struct string_list *files,\n \t\t\t const struct pathspec *ps)\n {\n \tstruct object_id head_oid;\n \tint is_initial = !resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n \t\t\t\t\t &head_oid, NULL);\n \tstruct collection_status s = { FROM_WORKTREE };\n+\tint i;\n \n \tif (discard_index(r->index) < 0 ||\n \t repo_read_index_preload(r, ps, 0) < 0)\n@@ -411,10 +424,16 @@ static int get_modified_files(struct repository *r, struct string_list *files,\n \ts.files = files;\n \thashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0);\n \n-\tfor (s.phase = FROM_WORKTREE; s.phase <= FROM_INDEX; s.phase++) {\n+\tfor (i = 0; i < 2; i++) {\n \t\tstruct rev_info rev;\n \t\tstruct setup_revision_opt opt = { 0 };\n \n+\t\tif (filter == INDEX_ONLY)\n+\t\t\ts.phase = i ? FROM_WORKTREE : FROM_INDEX;\n+\t\telse\n+\t\t\ts.phase = i ? FROM_INDEX : FROM_WORKTREE;\n+\t\ts.skip_unseen = filter && i;\n+\n \t\topt.def = is_initial ?\n \t\t\tempty_tree_oid_hex() : oid_to_hex(&head_oid);\n \n@@ -498,7 +517,7 @@ static void print_file_item(int i, struct string_list_item *item,\n static int run_status(struct add_i_state *s, const struct pathspec *ps,\n \t\t struct string_list *files, struct list_options *opts)\n {\n-\tif (get_modified_files(s->r, files, ps) < 0)\n+\tif (get_modified_files(s->r, NO_FILTER, files, ps) < 0)\n \t\treturn -1;\n \n \tlist(s, files, opts);\n-- \ngitgitgadget\n\n"},{"id":"386306","messageId":"pull.171.git.1573821382.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":null,"subject":"[PATCH 0/8] build-in add -i: implement all commands in the main loop","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-15T12:36:14Z","receivedAt":"2019-11-15T12:36:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Based on the js/builtin-add-i branch, this patch series implements the rest\nof the commands in git add -i's main loop: update, revert, add_untracked, \npatch, diff, and quit. Apart from quit, these commands are all very similar\nin that they first build a list of files, display it, let the user choose\nwhich ones to act on, and then perform the action.\n\nNote that the patch command is not actually converted to C, not completely\nat least: the built-in version simply hands off to git add--interactive \nafter letting the user select which files to act on.\n\nThe reason for this omission is practicality. Out of the 1,800+ lines of \ngit-add--interactive.perl, over a thousand deal just with the git add -p \npart. I did convert that functionality already (to be contributed in a\nseparate patch series, see https://github.com/gitgitgadget/git/pull/173),\ndiscovering that there is so little overlap between the git add --patch part\nand the rest of git add --interactive that I could put the former into a\ntotally different file: add-patch.c.\n\nJohannes Schindelin (8):\n built-in add -i: allow filtering the modified files list\n built-in add -i: prepare for multi-selection commands\n built-in add -i: implement the `update` command\n built-in add -i: re-implement `revert` in C\n built-in add -i: re-implement `add-untracked` in C\n built-in add -i: implement the `patch` command\n built-in add -i: re-implement the `diff` command\n built-in add -i: offer the `quit` command\n\n add-interactive.c | 608 +++++++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 553 insertions(+), 55 deletions(-)\n\n\nbase-commit: 2b5d5c1524d62add395da2b0ef50bbbe342362e4\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-171%2Fdscho%2Fadd-i-in-c-all-except-patch-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-171/dscho/add-i-in-c-all-except-patch-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/171\n-- \ngitgitgadget\n"},{"id":"386307","messageId":"cba5f1315234a1d310526e3032550d081034a0fa.1573821382.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.git.1573821382.gitgitgadget@gmail.com","subject":"[PATCH 3/8] built-in add -i: implement the `update` command","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-15T12:36:17Z","receivedAt":"2019-11-15T12:36:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAfter `status` and `help`, it is now time to port the `update` command\nto C, the second command that is shown in the main loop menu of `git add\n-i`.\n\nThis `git add -i` command is the first one which lets the user choose a\nsubset of a list of files, and as such, this patch lays the groundwork\nfor the other commands of that category:\n\n- It teaches the `print_file_item()` function to show a unique prefix\n if we found any (the code to find it had been added already in the\n previous patch where we colored the unique prefixes of the main loop\n commands, but that patch uses the `print_command_item()` function to\n display the menu items).\n\n- This patch also adds the help text that is shown when the user input\n to select items from the shown list could not be parsed.\n\n- As `get_modified_files()` clears the list of files, it now has to take\n care of clearing the _full_ `prefix_item_list` lest the `sorted` and\n `selected` fields go stale and inconsistent.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 130 +++++++++++++++++++++++++++++++++++++++-------\n 1 file changed, 110 insertions(+), 20 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 33a751150a..b0bda0cd2d 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -6,6 +6,7 @@\n #include \"revision.h\"\n #include \"refs.h\"\n #include \"string-list.h\"\n+#include \"lockfile.h\"\n \n struct add_i_state {\n \tstruct repository *r;\n@@ -376,6 +377,7 @@ struct adddel {\n };\n \n struct file_item {\n+\tsize_t prefix_length;\n \tstruct adddel index, worktree;\n };\n \n@@ -470,7 +472,7 @@ enum modified_files_filter {\n \n static int get_modified_files(struct repository *r,\n \t\t\t enum modified_files_filter filter,\n-\t\t\t struct string_list *files,\n+\t\t\t struct prefix_item_list *files,\n \t\t\t const struct pathspec *ps)\n {\n \tstruct object_id head_oid;\n@@ -483,8 +485,8 @@ static int get_modified_files(struct repository *r,\n \t repo_read_index_preload(r, ps, 0) < 0)\n \t\treturn error(_(\"could not read index\"));\n \n-\tstring_list_clear(files, 1);\n-\ts.files = files;\n+\tprefix_item_list_clear(files);\n+\ts.files = &files->items;\n \thashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0);\n \n \tfor (i = 0; i < 2; i++) {\n@@ -520,7 +522,7 @@ static int get_modified_files(struct repository *r,\n \thashmap_free_entries(&s.file_map, struct pathname_entry, ent);\n \n \t/* While the diffs are ordered already, we ran *two* diffs... */\n-\tstring_list_sort(files);\n+\tstring_list_sort(&files->items);\n \n \treturn 0;\n }\n@@ -555,8 +557,8 @@ static int is_valid_prefix(const char *prefix, size_t prefix_len)\n }\n \n struct print_file_item_data {\n-\tconst char *modified_fmt;\n-\tstruct strbuf buf, index, worktree;\n+\tconst char *modified_fmt, *color, *reset;\n+\tstruct strbuf buf, name, index, worktree;\n };\n \n static void print_file_item(int i, int selected, struct string_list_item *item,\n@@ -564,34 +566,96 @@ static void print_file_item(int i, int selected, struct string_list_item *item,\n {\n \tstruct file_item *c = item->util;\n \tstruct print_file_item_data *d = print_file_item_data;\n+\tconst char *highlighted = NULL;\n \n \tstrbuf_reset(&d->index);\n \tstrbuf_reset(&d->worktree);\n \tstrbuf_reset(&d->buf);\n \n+\t/* Format the item with the prefix highlighted. */\n+\tif (c->prefix_length > 0 &&\n+\t is_valid_prefix(item->string, c->prefix_length)) {\n+\t\tstrbuf_reset(&d->name);\n+\t\tstrbuf_addf(&d->name, \"%s%.*s%s%s\", d->color,\n+\t\t\t (int)c->prefix_length, item->string, d->reset,\n+\t\t\t item->string + c->prefix_length);\n+\t\thighlighted = d->name.buf;\n+\t}\n+\n \trender_adddel(&d->worktree, &c->worktree, _(\"nothing\"));\n \trender_adddel(&d->index, &c->index, _(\"unchanged\"));\n-\tstrbuf_addf(&d->buf, d->modified_fmt,\n-\t\t d->index.buf, d->worktree.buf, item->string);\n+\n+\tstrbuf_addf(&d->buf, d->modified_fmt, d->index.buf, d->worktree.buf,\n+\t\t highlighted ? highlighted : item->string);\n \n \tprintf(\"%c%2d: %s\", selected ? '*' : ' ', i + 1, d->buf.buf);\n }\n \n static int run_status(struct add_i_state *s, const struct pathspec *ps,\n-\t\t struct string_list *files, struct list_options *opts)\n+\t\t struct prefix_item_list *files,\n+\t\t struct list_and_choose_options *opts)\n {\n \tif (get_modified_files(s->r, NO_FILTER, files, ps) < 0)\n \t\treturn -1;\n \n-\tlist(s, files, NULL, opts);\n+\tlist(s, &files->items, NULL, &opts->list_opts);\n \tputchar('\\n');\n \n \treturn 0;\n }\n \n+static int run_update(struct add_i_state *s, const struct pathspec *ps,\n+\t\t struct prefix_item_list *files,\n+\t\t struct list_and_choose_options *opts)\n+{\n+\tint res = 0, fd;\n+\tsize_t count, i;\n+\tstruct lock_file index_lock;\n+\n+\tif (get_modified_files(s->r, WORKTREE_ONLY, files, ps) < 0)\n+\t\treturn -1;\n+\n+\tif (!files->items.nr) {\n+\t\tputchar('\\n');\n+\t\treturn 0;\n+\t}\n+\n+\topts->prompt = N_(\"Update\");\n+\tcount = list_and_choose(s, files, opts);\n+\tif (count <= 0) {\n+\t\tputchar('\\n');\n+\t\treturn 0;\n+\t}\n+\n+\tfd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR);\n+\tif (fd < 0) {\n+\t\tputchar('\\n');\n+\t\treturn -1;\n+\t}\n+\n+\tfor (i = 0; i < files->items.nr; i++) {\n+\t\tconst char *name = files->items.items[i].string;\n+\t\tif (files->selected[i] &&\n+\t\t add_file_to_index(s->r->index, name, 0) < 0) {\n+\t\t\tres = error(_(\"could not stage '%s'\"), name);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\tif (!res && write_locked_index(s->r->index, &index_lock, COMMIT_LOCK) < 0)\n+\t\tres = error(_(\"could not write index\"));\n+\n+\tif (!res)\n+\t\tprintf(Q_(\"updated %d path\\n\",\n+\t\t\t \"updated %d paths\\n\", count), (int)count);\n+\n+\tputchar('\\n');\n+\treturn res;\n+}\n+\n static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,\n-\t\t struct string_list *unused_files,\n-\t\t struct list_options *unused_opts)\n+\t\t struct prefix_item_list *unused_files,\n+\t\t struct list_and_choose_options *unused_opts)\n {\n \tcolor_fprintf_ln(stdout, s->help_color, \"status - %s\",\n \t\t\t _(\"show paths with changes\"));\n@@ -609,9 +673,29 @@ static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,\n \treturn 0;\n }\n \n+static void choose_prompt_help(struct add_i_state *s)\n+{\n+\tcolor_fprintf_ln(stdout, s->help_color, \"%s\",\n+\t\t\t _(\"Prompt help:\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \"1 - %s\",\n+\t\t\t _(\"select a single item\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \"3-5 - %s\",\n+\t\t\t _(\"select a range of items\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \"2-3,6-9 - %s\",\n+\t\t\t _(\"select multiple ranges\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \"foo - %s\",\n+\t\t\t _(\"select item based on unique prefix\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \"-... - %s\",\n+\t\t\t _(\"unselect specified items\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \"* - %s\",\n+\t\t\t _(\"choose all items\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \" - %s\",\n+\t\t\t _(\"(empty) finish selecting\"));\n+}\n+\n typedef int (*command_t)(struct add_i_state *s, const struct pathspec *ps,\n-\t\t\t struct string_list *files,\n-\t\t\t struct list_options *opts);\n+\t\t\t struct prefix_item_list *files,\n+\t\t\t struct list_and_choose_options *opts);\n \n struct command_item {\n \tsize_t prefix_length;\n@@ -663,18 +747,21 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\tcommand_t command;\n \t} command_list[] = {\n \t\t{ \"status\", run_status },\n+\t\t{ \"update\", run_update },\n \t\t{ \"help\", run_help },\n \t};\n \tstruct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;\n \n \tstruct print_file_item_data print_file_item_data = {\n-\t\t\"%12s %12s %s\", STRBUF_INIT, STRBUF_INIT, STRBUF_INIT\n+\t\t\"%12s %12s %s\", NULL, NULL,\n+\t\tSTRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT\n \t};\n-\tstruct list_options opts = {\n-\t\t0, NULL, print_file_item, &print_file_item_data\n+\tstruct list_and_choose_options opts = {\n+\t\t{ 0, NULL, print_file_item, &print_file_item_data },\n+\t\tNULL, 0, choose_prompt_help\n \t};\n \tstruct strbuf header = STRBUF_INIT;\n-\tstruct string_list files = STRING_LIST_INIT_DUP;\n+\tstruct prefix_item_list files = PREFIX_ITEM_LIST_INIT;\n \tssize_t i;\n \tint res = 0;\n \n@@ -695,11 +782,13 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\tdata.color = s.prompt_color;\n \t\tdata.reset = s.reset_color;\n \t}\n+\tprint_file_item_data.color = data.color;\n+\tprint_file_item_data.reset = data.reset;\n \n \tstrbuf_addstr(&header, \" \");\n \tstrbuf_addf(&header, print_file_item_data.modified_fmt,\n \t\t _(\"staged\"), _(\"unstaged\"), _(\"path\"));\n-\topts.header = header.buf;\n+\topts.list_opts.header = header.buf;\n \n \tif (discard_index(r->index) < 0 ||\n \t repo_read_index(r) < 0 ||\n@@ -723,8 +812,9 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\t}\n \t}\n \n-\tstring_list_clear(&files, 1);\n+\tprefix_item_list_clear(&files);\n \tstrbuf_release(&print_file_item_data.buf);\n+\tstrbuf_release(&print_file_item_data.name);\n \tstrbuf_release(&print_file_item_data.index);\n \tstrbuf_release(&print_file_item_data.worktree);\n \tstrbuf_release(&header);\n-- \ngitgitgadget\n\n"},{"id":"386308","messageId":"5c498795b354c8483f9344662085a8f8cca4580f.1573821382.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.git.1573821382.gitgitgadget@gmail.com","subject":"[PATCH 5/8] built-in add -i: re-implement `add-untracked` in C","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-15T12:36:19Z","receivedAt":"2019-11-15T12:36:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis is yet another command, ported to C. It builds nicely on the\nsupport functions introduced for other commands, with the notable\ndifference that only names are displayed for untracked files, no\nfile type or diff summary.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 91 insertions(+)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 191a10b97d..9ed4455a86 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -7,6 +7,7 @@\n #include \"refs.h\"\n #include \"string-list.h\"\n #include \"lockfile.h\"\n+#include \"dir.h\"\n \n struct add_i_state {\n \tstruct repository *r;\n@@ -559,6 +560,7 @@ static int is_valid_prefix(const char *prefix, size_t prefix_len)\n struct print_file_item_data {\n \tconst char *modified_fmt, *color, *reset;\n \tstruct strbuf buf, name, index, worktree;\n+\tunsigned only_names:1;\n };\n \n static void print_file_item(int i, int selected, struct string_list_item *item,\n@@ -582,6 +584,12 @@ static void print_file_item(int i, int selected, struct string_list_item *item,\n \t\thighlighted = d->name.buf;\n \t}\n \n+\tif (d->only_names) {\n+\t\tprintf(\"%c%2d: %s\", selected ? '*' : ' ', i + 1,\n+\t\t highlighted ? highlighted : item->string);\n+\t\treturn;\n+\t}\n+\n \trender_adddel(&d->worktree, &c->worktree, _(\"nothing\"));\n \trender_adddel(&d->index, &c->index, _(\"unchanged\"));\n \n@@ -761,6 +769,88 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps,\n \treturn res;\n }\n \n+static int get_untracked_files(struct repository *r,\n+\t\t\t struct prefix_item_list *files,\n+\t\t\t const struct pathspec *ps)\n+{\n+\tstruct dir_struct dir = { 0 };\n+\tsize_t i;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tif (repo_read_index(r) < 0)\n+\t\treturn error(_(\"could not read index\"));\n+\n+\tprefix_item_list_clear(files);\n+\tsetup_standard_excludes(&dir);\n+\tadd_pattern_list(&dir, EXC_CMDL, \"--exclude option\");\n+\tfill_directory(&dir, r->index, ps);\n+\n+\tfor (i = 0; i < dir.nr; i++) {\n+\t\tstruct dir_entry *ent = dir.entries[i];\n+\n+\t\tif (index_name_is_other(r->index, ent->name, ent->len)) {\n+\t\t\tstrbuf_reset(&buf);\n+\t\t\tstrbuf_add(&buf, ent->name, ent->len);\n+\t\t\tadd_file_item(&files->items, buf.buf);\n+\t\t}\n+\t}\n+\n+\tstrbuf_release(&buf);\n+\treturn 0;\n+}\n+\n+static int run_add_untracked(struct add_i_state *s, const struct pathspec *ps,\n+\t\t struct prefix_item_list *files,\n+\t\t struct list_and_choose_options *opts)\n+{\n+\tstruct print_file_item_data *d = opts->list_opts.print_item_data;\n+\tint res = 0, fd;\n+\tsize_t count, i;\n+\tstruct lock_file index_lock;\n+\n+\tif (get_untracked_files(s->r, files, ps) < 0)\n+\t\treturn -1;\n+\n+\tif (!files->items.nr) {\n+\t\tprintf(_(\"No untracked files.\\n\"));\n+\t\tgoto finish_add_untracked;\n+\t}\n+\n+\topts->prompt = N_(\"Add untracked\");\n+\td->only_names = 1;\n+\tcount = list_and_choose(s, files, opts);\n+\td->only_names = 0;\n+\tif (count <= 0)\n+\t\tgoto finish_add_untracked;\n+\n+\tfd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR);\n+\tif (fd < 0) {\n+\t\tres = -1;\n+\t\tgoto finish_add_untracked;\n+\t}\n+\n+\tfor (i = 0; i < files->items.nr; i++) {\n+\t\tconst char *name = files->items.items[i].string;\n+\t\tif (files->selected[i] &&\n+\t\t add_file_to_index(s->r->index, name, 0) < 0) {\n+\t\t\tres = error(_(\"could not stage '%s'\"), name);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\tif (!res &&\n+\t write_locked_index(s->r->index, &index_lock, COMMIT_LOCK) < 0)\n+\t\tres = error(_(\"could not write index\"));\n+\n+\tif (!res)\n+\t\tprintf(Q_(\"added %d path\\n\",\n+\t\t\t \"added %d paths\\n\", count), (int)count);\n+\n+finish_add_untracked:\n+\tputchar('\\n');\n+\treturn res;\n+}\n+\n static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,\n \t\t struct prefix_item_list *unused_files,\n \t\t struct list_and_choose_options *unused_opts)\n@@ -857,6 +947,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\t{ \"status\", run_status },\n \t\t{ \"update\", run_update },\n \t\t{ \"revert\", run_revert },\n+\t\t{ \"add untracked\", run_add_untracked },\n \t\t{ \"help\", run_help },\n \t};\n \tstruct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;\n-- \ngitgitgadget\n\n"},{"id":"386309","messageId":"5c31bbd24ab937f5906f967f007d5bcd305779e2.1573821382.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.git.1573821382.gitgitgadget@gmail.com","subject":"[PATCH 4/8] built-in add -i: re-implement `revert` in C","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-15T12:36:18Z","receivedAt":"2019-11-15T12:36:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis is a relatively straight-forward port from the Perl version, with\nthe notable exception that we imitate `git reset -- ` in the C\nversion rather than the convoluted `git ls-tree HEAD -- | git\nupdate-index --index-info` followed by `git update-index --force-remove\n-- ` for the missed ones.\n\nWhile at it, we fix the pretty obvious bug where the `revert` command\noffers to unstage files that do not have staged changes.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 109 insertions(+)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex b0bda0cd2d..191a10b97d 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -653,6 +653,114 @@ static int run_update(struct add_i_state *s, const struct pathspec *ps,\n \treturn res;\n }\n \n+static void revert_from_diff(struct diff_queue_struct *q,\n+\t\t\t struct diff_options *opt, void *data)\n+{\n+\tint i, add_flags = ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE;\n+\n+\tfor (i = 0; i < q->nr; i++) {\n+\t\tstruct diff_filespec *one = q->queue[i]->one;\n+\t\tstruct cache_entry *ce;\n+\n+\t\tif (!(one->mode && !is_null_oid(&one->oid))) {\n+\t\t\tremove_file_from_index(opt->repo->index, one->path);\n+\t\t\tprintf(_(\"note: %s is untracked now.\\n\"), one->path);\n+\t\t} else {\n+\t\t\tce = make_cache_entry(opt->repo->index, one->mode,\n+\t\t\t\t\t &one->oid, one->path, 0, 0);\n+\t\t\tif (!ce)\n+\t\t\t\tdie(_(\"make_cache_entry failed for path '%s'\"),\n+\t\t\t\t one->path);\n+\t\t\tadd_index_entry(opt->repo->index, ce, add_flags);\n+\t\t}\n+\t}\n+}\n+\n+static int run_revert(struct add_i_state *s, const struct pathspec *ps,\n+\t\t struct prefix_item_list *files,\n+\t\t struct list_and_choose_options *opts)\n+{\n+\tint res = 0, fd;\n+\tsize_t count, i, j;\n+\n+\tstruct object_id oid;\n+\tint is_initial = !resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, &oid,\n+\t\t\t\t\t NULL);\n+\tstruct lock_file index_lock;\n+\tconst char **paths;\n+\tstruct tree *tree;\n+\tstruct diff_options diffopt = { NULL };\n+\n+\tif (get_modified_files(s->r, INDEX_ONLY, files, ps) < 0)\n+\t\treturn -1;\n+\n+\tif (!files->items.nr) {\n+\t\tputchar('\\n');\n+\t\treturn 0;\n+\t}\n+\n+\topts->prompt = N_(\"Revert\");\n+\tcount = list_and_choose(s, files, opts);\n+\tif (count <= 0)\n+\t\tgoto finish_revert;\n+\n+\tfd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR);\n+\tif (fd < 0) {\n+\t\tres = -1;\n+\t\tgoto finish_revert;\n+\t}\n+\n+\tif (is_initial)\n+\t\toidcpy(&oid, s->r->hash_algo->empty_tree);\n+\telse {\n+\t\ttree = parse_tree_indirect(&oid);\n+\t\tif (!tree) {\n+\t\t\tres = error(_(\"Could not parse HEAD^{tree}\"));\n+\t\t\tgoto finish_revert;\n+\t\t}\n+\t\toidcpy(&oid, &tree->object.oid);\n+\t}\n+\n+\tALLOC_ARRAY(paths, count + 1);\n+\tfor (i = j = 0; i < files->items.nr; i++)\n+\t\tif (files->selected[i])\n+\t\t\tpaths[j++] = files->items.items[i].string;\n+\tpaths[j] = NULL;\n+\n+\tparse_pathspec(&diffopt.pathspec, 0,\n+\t\t PATHSPEC_PREFER_FULL | PATHSPEC_LITERAL_PATH,\n+\t\t NULL, paths);\n+\n+\tdiffopt.output_format = DIFF_FORMAT_CALLBACK;\n+\tdiffopt.format_callback = revert_from_diff;\n+\tdiffopt.flags.override_submodule_config = 1;\n+\tdiffopt.repo = s->r;\n+\n+\tif (do_diff_cache(&oid, &diffopt))\n+\t\tres = -1;\n+\telse {\n+\t\tdiffcore_std(&diffopt);\n+\t\tdiff_flush(&diffopt);\n+\t}\n+\tfree(paths);\n+\tclear_pathspec(&diffopt.pathspec);\n+\n+\tif (!res && write_locked_index(s->r->index, &index_lock,\n+\t\t\t\t COMMIT_LOCK) < 0)\n+\t\tres = -1;\n+\telse\n+\t\tres = repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0, 1,\n+\t\t\t\t\t\t NULL, NULL, NULL);\n+\n+\tif (!res)\n+\t\tprintf(Q_(\"reverted %d path\\n\",\n+\t\t\t \"reverted %d paths\\n\", count), (int)count);\n+\n+finish_revert:\n+\tputchar('\\n');\n+\treturn res;\n+}\n+\n static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,\n \t\t struct prefix_item_list *unused_files,\n \t\t struct list_and_choose_options *unused_opts)\n@@ -748,6 +856,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t} command_list[] = {\n \t\t{ \"status\", run_status },\n \t\t{ \"update\", run_update },\n+\t\t{ \"revert\", run_revert },\n \t\t{ \"help\", run_help },\n \t};\n \tstruct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;\n-- \ngitgitgadget\n\n"},{"id":"386310","messageId":"0c8a71e2e861f2ef657a363cc3da198f9062bf95.1573821382.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.git.1573821382.gitgitgadget@gmail.com","subject":"[PATCH 6/8] built-in add -i: implement the `patch` command","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-15T12:36:20Z","receivedAt":"2019-11-15T12:36:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWell, it is not a full implementation yet. In the interest of making\nthis easy to review (and easy to keep bugs out), we still hand off to\nthe Perl script to do the actual work.\n\nThe `patch` functionality actually makes up for more than half of the\n1,800+ lines of `git-add--interactive.perl`. It will be ported from Perl\nto C incrementally, later.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 91 +++++++++++++++++++++++++++++++++++++++++++----\n 1 file changed, 84 insertions(+), 7 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 9ed4455a86..fb8124fc57 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -8,6 +8,7 @@\n #include \"string-list.h\"\n #include \"lockfile.h\"\n #include \"dir.h\"\n+#include \"run-command.h\"\n \n struct add_i_state {\n \tstruct repository *r;\n@@ -374,7 +375,7 @@ static ssize_t list_and_choose(struct add_i_state *s,\n \n struct adddel {\n \tuintmax_t add, del;\n-\tunsigned seen:1, binary:1;\n+\tunsigned seen:1, unmerged:1, binary:1;\n };\n \n struct file_item {\n@@ -414,6 +415,7 @@ struct collection_status {\n \tconst char *reference;\n \n \tunsigned skip_unseen:1;\n+\tsize_t unmerged_count, binary_count;\n \tstruct string_list *files;\n \tstruct hashmap file_map;\n };\n@@ -436,7 +438,7 @@ static void collect_changes_cb(struct diff_queue_struct *q,\n \t\tint hash = strhash(name);\n \t\tstruct pathname_entry *entry;\n \t\tstruct file_item *file_item;\n-\t\tstruct adddel *adddel;\n+\t\tstruct adddel *adddel, *other_adddel;\n \n \t\tentry = hashmap_get_entry_from_hash(&s->file_map, hash, name,\n \t\t\t\t\t\t struct pathname_entry, ent);\n@@ -456,11 +458,21 @@ static void collect_changes_cb(struct diff_queue_struct *q,\n \t\tfile_item = entry->item;\n \t\tadddel = s->phase == FROM_INDEX ?\n \t\t\t&file_item->index : &file_item->worktree;\n+\t\tother_adddel = s->phase == FROM_INDEX ?\n+\t\t\t&file_item->worktree : &file_item->index;\n \t\tadddel->seen = 1;\n \t\tadddel->add = stat.files[i]->added;\n \t\tadddel->del = stat.files[i]->deleted;\n-\t\tif (stat.files[i]->is_binary)\n+\t\tif (stat.files[i]->is_binary) {\n+\t\t\tif (!other_adddel->binary)\n+\t\t\t\ts->binary_count++;\n \t\t\tadddel->binary = 1;\n+\t\t}\n+\t\tif (stat.files[i]->is_unmerged) {\n+\t\t\tif (!other_adddel->unmerged)\n+\t\t\t\ts->unmerged_count++;\n+\t\t\tadddel->unmerged = 1;\n+\t\t}\n \t}\n \tfree_diffstat_info(&stat);\n }\n@@ -474,7 +486,9 @@ enum modified_files_filter {\n static int get_modified_files(struct repository *r,\n \t\t\t enum modified_files_filter filter,\n \t\t\t struct prefix_item_list *files,\n-\t\t\t const struct pathspec *ps)\n+\t\t\t const struct pathspec *ps,\n+\t\t\t size_t *unmerged_count,\n+\t\t\t size_t *binary_count)\n {\n \tstruct object_id head_oid;\n \tint is_initial = !resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n@@ -521,6 +535,10 @@ static int get_modified_files(struct repository *r,\n \t\t}\n \t}\n \thashmap_free_entries(&s.file_map, struct pathname_entry, ent);\n+\tif (unmerged_count)\n+\t\t*unmerged_count = s.unmerged_count;\n+\tif (binary_count)\n+\t\t*binary_count = s.binary_count;\n \n \t/* While the diffs are ordered already, we ran *two* diffs... */\n \tstring_list_sort(&files->items);\n@@ -603,7 +621,7 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps,\n \t\t struct prefix_item_list *files,\n \t\t struct list_and_choose_options *opts)\n {\n-\tif (get_modified_files(s->r, NO_FILTER, files, ps) < 0)\n+\tif (get_modified_files(s->r, NO_FILTER, files, ps, NULL, NULL) < 0)\n \t\treturn -1;\n \n \tlist(s, &files->items, NULL, &opts->list_opts);\n@@ -620,7 +638,7 @@ static int run_update(struct add_i_state *s, const struct pathspec *ps,\n \tsize_t count, i;\n \tstruct lock_file index_lock;\n \n-\tif (get_modified_files(s->r, WORKTREE_ONLY, files, ps) < 0)\n+\tif (get_modified_files(s->r, WORKTREE_ONLY, files, ps, NULL, NULL) < 0)\n \t\treturn -1;\n \n \tif (!files->items.nr) {\n@@ -699,7 +717,7 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps,\n \tstruct tree *tree;\n \tstruct diff_options diffopt = { NULL };\n \n-\tif (get_modified_files(s->r, INDEX_ONLY, files, ps) < 0)\n+\tif (get_modified_files(s->r, INDEX_ONLY, files, ps, NULL, NULL) < 0)\n \t\treturn -1;\n \n \tif (!files->items.nr) {\n@@ -851,6 +869,64 @@ static int run_add_untracked(struct add_i_state *s, const struct pathspec *ps,\n \treturn res;\n }\n \n+static int run_patch(struct add_i_state *s, const struct pathspec *ps,\n+\t\t struct prefix_item_list *files,\n+\t\t struct list_and_choose_options *opts)\n+{\n+\tint res = 0;\n+\tssize_t count, i, j;\n+\tsize_t unmerged_count = 0, binary_count = 0;\n+\n+\tif (get_modified_files(s->r, WORKTREE_ONLY, files, ps,\n+\t\t\t &unmerged_count, &binary_count) < 0)\n+\t\treturn -1;\n+\n+\tif (unmerged_count || binary_count) {\n+\t\tfor (i = j = 0; i < files->items.nr; i++) {\n+\t\t\tstruct file_item *item = files->items.items[i].util;\n+\n+\t\t\tif (item->index.binary || item->worktree.binary) {\n+\t\t\t\tfree(item);\n+\t\t\t\tfree(files->items.items[i].string);\n+\t\t\t} else if (item->index.unmerged ||\n+\t\t\t\t item->worktree.unmerged) {\n+\t\t\t\tcolor_fprintf_ln(stderr, s->error_color,\n+\t\t\t\t\t\t _(\"ignoring unmerged: %s\"),\n+\t\t\t\t\t\t files->items.items[i].string);\n+\t\t\t\tfree(item);\n+\t\t\t\tfree(files->items.items[i].string);\n+\t\t\t} else\n+\t\t\t\tfiles->items.items[j++] = files->items.items[i];\n+\t\t}\n+\t\tfiles->items.nr = j;\n+\t}\n+\n+\tif (!files->items.nr) {\n+\t\tif (binary_count)\n+\t\t\tfprintf(stderr, _(\"Only binary files changed.\\n\"));\n+\t\telse\n+\t\t\tfprintf(stderr, _(\"No changes.\\n\"));\n+\t\treturn 0;\n+\t}\n+\n+\topts->prompt = N_(\"Patch update\");\n+\tcount = list_and_choose(s, files, opts);\n+\tif (count >= 0) {\n+\t\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\n+\t\targv_array_pushl(&args, \"git\", \"add--interactive\", \"--patch\",\n+\t\t\t\t \"--\", NULL);\n+\t\tfor (i = 0; i < files->items.nr; i++)\n+\t\t\tif (files->selected[i])\n+\t\t\t\targv_array_push(&args,\n+\t\t\t\t\t\tfiles->items.items[i].string);\n+\t\tres = run_command_v_opt(args.argv, 0);\n+\t\targv_array_clear(&args);\n+\t}\n+\n+\treturn res;\n+}\n+\n static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,\n \t\t struct prefix_item_list *unused_files,\n \t\t struct list_and_choose_options *unused_opts)\n@@ -948,6 +1024,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\t{ \"update\", run_update },\n \t\t{ \"revert\", run_revert },\n \t\t{ \"add untracked\", run_add_untracked },\n+\t\t{ \"patch\", run_patch },\n \t\t{ \"help\", run_help },\n \t};\n \tstruct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;\n-- \ngitgitgadget\n\n"},{"id":"386311","messageId":"dbcb340ededd67322506fd0459dad4200d7bd4ef.1573821382.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.git.1573821382.gitgitgadget@gmail.com","subject":"[PATCH 8/8] built-in add -i: offer the `quit` command","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-15T12:36:22Z","receivedAt":"2019-11-15T12:36:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe do not really want to `exit()` here, of course, as this is safely\nlibified code.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 16 +++++++++++-----\n 1 file changed, 11 insertions(+), 5 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex f33075b202..40bec9acbe 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -1067,6 +1067,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\t{ \"add untracked\", run_add_untracked },\n \t\t{ \"patch\", run_patch },\n \t\t{ \"diff\", run_diff },\n+\t\t{ \"quit\", NULL },\n \t\t{ \"help\", run_help },\n \t};\n \tstruct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;\n@@ -1118,17 +1119,22 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \tres = run_status(&s, ps, &files, &opts);\n \n \tfor (;;) {\n+\t\tstruct command_item *util;\n+\n \t\ti = list_and_choose(&s, &commands, &main_loop_opts);\n-\t\tif (i == LIST_AND_CHOOSE_QUIT) {\n+\t\tif (i < 0 || i >= commands.items.nr)\n+\t\t\tutil = NULL;\n+\t\telse\n+\t\t\tutil = commands.items.items[i].util;\n+\n+\t\tif (i == LIST_AND_CHOOSE_QUIT || (util && !util->command)) {\n \t\t\tprintf(_(\"Bye.\\n\"));\n \t\t\tres = 0;\n \t\t\tbreak;\n \t\t}\n-\t\tif (i != LIST_AND_CHOOSE_ERROR) {\n-\t\t\tstruct command_item *util =\n-\t\t\t\tcommands.items.items[i].util;\n+\n+\t\tif (util)\n \t\t\tres = util->command(&s, ps, &files, &opts);\n-\t\t}\n \t}\n \n \tprefix_item_list_clear(&files);\n-- \ngitgitgadget\n"},{"id":"386312","messageId":"499f3f19e026a60e050f2ed1bb2d820febc6b0a0.1573821382.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.git.1573821382.gitgitgadget@gmail.com","subject":"[PATCH 7/8] built-in add -i: re-implement the `diff` command","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-15T12:36:21Z","receivedAt":"2019-11-15T12:36:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIt is not only laziness that we simply spawn `git diff -p --cached`\nhere: this command needs to use the pager, and the pager needs to exit\nwhen the diff is done. Currently we do not have any way to make that\nhappen if we run the diff in-process. So let's just spawn.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 42 ++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 42 insertions(+)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex fb8124fc57..f33075b202 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -927,6 +927,47 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps,\n \treturn res;\n }\n \n+static int run_diff(struct add_i_state *s, const struct pathspec *ps,\n+\t\t struct prefix_item_list *files,\n+\t\t struct list_and_choose_options *opts)\n+{\n+\tint res = 0;\n+\tssize_t count, i;\n+\n+\tstruct object_id oid;\n+\tint is_initial = !resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, &oid,\n+\t\t\t\t\t NULL);\n+\tif (get_modified_files(s->r, INDEX_ONLY, files, ps, NULL, NULL) < 0)\n+\t\treturn -1;\n+\n+\tif (!files->items.nr) {\n+\t\tputchar('\\n');\n+\t\treturn 0;\n+\t}\n+\n+\topts->prompt = N_(\"Review diff\");\n+\topts->flags = IMMEDIATE;\n+\tcount = list_and_choose(s, files, opts);\n+\topts->flags = 0;\n+\tif (count >= 0) {\n+\t\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\n+\t\targv_array_pushl(&args, \"git\", \"diff\", \"-p\", \"--cached\",\n+\t\t\t\t oid_to_hex(!is_initial ? &oid :\n+\t\t\t\t\t s->r->hash_algo->empty_tree),\n+\t\t\t\t \"--\", NULL);\n+\t\tfor (i = 0; i < files->items.nr; i++)\n+\t\t\tif (files->selected[i])\n+\t\t\t\targv_array_push(&args,\n+\t\t\t\t\t\tfiles->items.items[i].string);\n+\t\tres = run_command_v_opt(args.argv, 0);\n+\t\targv_array_clear(&args);\n+\t}\n+\n+\tputchar('\\n');\n+\treturn res;\n+}\n+\n static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,\n \t\t struct prefix_item_list *unused_files,\n \t\t struct list_and_choose_options *unused_opts)\n@@ -1025,6 +1066,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\t{ \"revert\", run_revert },\n \t\t{ \"add untracked\", run_add_untracked },\n \t\t{ \"patch\", run_patch },\n+\t\t{ \"diff\", run_diff },\n \t\t{ \"help\", run_help },\n \t};\n \tstruct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;\n-- \ngitgitgadget\n\n"},{"id":"386313","messageId":"c324b47592dca2cd4967d62d2520a35bfcf04bae.1573821382.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.git.1573821382.gitgitgadget@gmail.com","subject":"[PATCH 2/8] built-in add -i: prepare for multi-selection commands","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-15T12:36:16Z","receivedAt":"2019-11-15T12:36:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe `upgrade`, `revert` and `add-untracked` commands allow selecting\nmultiple entries. Let's extend the `list_and_choose()` function to\naccommodate those use cases.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 114 ++++++++++++++++++++++++++++++++++++----------\n 1 file changed, 89 insertions(+), 25 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 8ec930ac15..33a751150a 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -72,15 +72,17 @@ static void init_add_i_state(struct add_i_state *s, struct repository *r)\n struct prefix_item_list {\n \tstruct string_list items;\n \tstruct string_list sorted;\n+\tint *selected; /* for multi-selections */\n \tsize_t min_length, max_length;\n };\n #define PREFIX_ITEM_LIST_INIT \\\n-\t{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_NODUP, 1, 4 }\n+\t{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_NODUP, NULL, 1, 4 }\n \n static void prefix_item_list_clear(struct prefix_item_list *list)\n {\n \tstring_list_clear(&list->items, 1);\n \tstring_list_clear(&list->sorted, 0);\n+\tFREE_AND_NULL(list->selected);\n }\n \n static void extend_prefix_length(struct string_list_item *p,\n@@ -182,11 +184,12 @@ static ssize_t find_unique(const char *string, struct prefix_item_list *list)\n struct list_options {\n \tint columns;\n \tconst char *header;\n-\tvoid (*print_item)(int i, struct string_list_item *item, void *print_item_data);\n+\tvoid (*print_item)(int i, int selected, struct string_list_item *item,\n+\t\t\t void *print_item_data);\n \tvoid *print_item_data;\n };\n \n-static void list(struct add_i_state *s, struct string_list *list,\n+static void list(struct add_i_state *s, struct string_list *list, int *selected,\n \t\t struct list_options *opts)\n {\n \tint i, last_lf = 0;\n@@ -199,7 +202,8 @@ static void list(struct add_i_state *s, struct string_list *list,\n \t\t\t\t \"%s\", opts->header);\n \n \tfor (i = 0; i < list->nr; i++) {\n-\t\topts->print_item(i, list->items + i, opts->print_item_data);\n+\t\topts->print_item(i, selected ? selected[i] : 0, list->items + i,\n+\t\t\t\t opts->print_item_data);\n \n \t\tif ((opts->columns) && ((i + 1) % (opts->columns))) {\n \t\t\tputchar('\\t');\n@@ -218,6 +222,10 @@ struct list_and_choose_options {\n \tstruct list_options list_opts;\n \n \tconst char *prompt;\n+\tenum {\n+\t\tSINGLETON = (1<<0),\n+\t\tIMMEDIATE = (1<<1),\n+\t} flags;\n \tvoid (*print_help)(struct add_i_state *s);\n };\n \n@@ -225,7 +233,8 @@ struct list_and_choose_options {\n #define LIST_AND_CHOOSE_QUIT (-2)\n \n /*\n- * Returns the selected index.\n+ * Returns the selected index in singleton mode, the number of selected items\n+ * otherwise.\n *\n * If an error occurred, returns `LIST_AND_CHOOSE_ERROR`. Upon EOF,\n * `LIST_AND_CHOOSE_QUIT` is returned.\n@@ -234,8 +243,19 @@ static ssize_t list_and_choose(struct add_i_state *s,\n \t\t\t struct prefix_item_list *items,\n \t\t\t struct list_and_choose_options *opts)\n {\n+\tint singleton = opts->flags & SINGLETON;\n+\tint immediate = opts->flags & IMMEDIATE;\n+\n \tstruct strbuf input = STRBUF_INIT;\n-\tssize_t res = LIST_AND_CHOOSE_ERROR;\n+\tssize_t res = singleton ? LIST_AND_CHOOSE_ERROR : 0;\n+\n+\tif (!singleton) {\n+\t\tfree(items->selected);\n+\t\tCALLOC_ARRAY(items->selected, items->items.nr);\n+\t}\n+\n+\tif (singleton && !immediate)\n+\t\tBUG(\"singleton requires immediate\");\n \n \tfind_unique_prefixes(items);\n \n@@ -244,15 +264,16 @@ static ssize_t list_and_choose(struct add_i_state *s,\n \n \t\tstrbuf_reset(&input);\n \n-\t\tlist(s, &items->items, &opts->list_opts);\n+\t\tlist(s, &items->items, items->selected, &opts->list_opts);\n \n \t\tcolor_fprintf(stdout, s->prompt_color, \"%s\", opts->prompt);\n-\t\tfputs(\"> \", stdout);\n+\t\tfputs(singleton ? \"> \" : \">> \", stdout);\n \t\tfflush(stdout);\n \n \t\tif (strbuf_getline(&input, stdin) == EOF) {\n \t\t\tputchar('\\n');\n-\t\t\tres = LIST_AND_CHOOSE_QUIT;\n+\t\t\tif (immediate)\n+\t\t\t\tres = LIST_AND_CHOOSE_QUIT;\n \t\t\tbreak;\n \t\t}\n \t\tstrbuf_trim(&input);\n@@ -268,7 +289,9 @@ static ssize_t list_and_choose(struct add_i_state *s,\n \t\tp = input.buf;\n \t\tfor (;;) {\n \t\t\tsize_t sep = strcspn(p, \" \\t\\r\\n,\");\n-\t\t\tssize_t index = -1;\n+\t\t\tint choose = 1;\n+\t\t\t/* `from` is inclusive, `to` is exclusive */\n+\t\t\tssize_t from = -1, to = -1;\n \n \t\t\tif (!sep) {\n \t\t\t\tif (!*p)\n@@ -277,29 +300,69 @@ static ssize_t list_and_choose(struct add_i_state *s,\n \t\t\t\tcontinue;\n \t\t\t}\n \n-\t\t\tif (isdigit(*p)) {\n+\t\t\t/* Input that begins with '-'; de-select */\n+\t\t\tif (*p == '-') {\n+\t\t\t\tchoose = 0;\n+\t\t\t\tp++;\n+\t\t\t\tsep--;\n+\t\t\t}\n+\n+\t\t\tif (sep == 1 && *p == '*') {\n+\t\t\t\tfrom = 0;\n+\t\t\t\tto = items->items.nr;\n+\t\t\t} else if (isdigit(*p)) {\n \t\t\t\tchar *endp;\n-\t\t\t\tindex = strtoul(p, &endp, 10) - 1;\n-\t\t\t\tif (endp != p + sep)\n-\t\t\t\t\tindex = -1;\n+\t\t\t\t/*\n+\t\t\t\t * A range can be specified like 5-7 or 5-.\n+\t\t\t\t *\n+\t\t\t\t * Note: `from` is 0-based while the user input\n+\t\t\t\t * is 1-based, hence we have to decrement by\n+\t\t\t\t * one. We do not have to decrement `to` even\n+\t\t\t\t * if it is 0-based because it is an exclusive\n+\t\t\t\t * boundary.\n+\t\t\t\t */\n+\t\t\t\tfrom = strtoul(p, &endp, 10) - 1;\n+\t\t\t\tif (endp == p + sep)\n+\t\t\t\t\tto = from + 1;\n+\t\t\t\telse if (*endp == '-') {\n+\t\t\t\t\tto = strtoul(++endp, &endp, 10);\n+\t\t\t\t\t/* extra characters after the range? */\n+\t\t\t\t\tif (endp != p + sep)\n+\t\t\t\t\t\tfrom = -1;\n+\t\t\t\t}\n \t\t\t}\n \n \t\t\tp[sep] = '\\0';\n-\t\t\tif (index < 0)\n-\t\t\t\tindex = find_unique(p, items);\n+\t\t\tif (from < 0) {\n+\t\t\t\tfrom = find_unique(p, items);\n+\t\t\t\tif (from >= 0)\n+\t\t\t\t\tto = from + 1;\n+\t\t\t}\n \n-\t\t\tif (index < 0 || index >= items->items.nr)\n+\t\t\tif (from < 0 || from >= items->items.nr ||\n+\t\t\t (singleton && from + 1 != to)) {\n \t\t\t\tcolor_fprintf_ln(stdout, s->error_color,\n \t\t\t\t\t\t _(\"Huh (%s)?\"), p);\n-\t\t\telse {\n-\t\t\t\tres = index;\n+\t\t\t\tbreak;\n+\t\t\t} else if (singleton) {\n+\t\t\t\tres = from;\n \t\t\t\tbreak;\n \t\t\t}\n \n+\t\t\tif (to > items->items.nr)\n+\t\t\t\tto = items->items.nr;\n+\n+\t\t\tfor (; from < to; from++)\n+\t\t\t\tif (items->selected[from] != choose) {\n+\t\t\t\t\titems->selected[from] = choose;\n+\t\t\t\t\tres += choose ? +1 : -1;\n+\t\t\t\t}\n+\n \t\t\tp += sep + 1;\n \t\t}\n \n-\t\tif (res != LIST_AND_CHOOSE_ERROR)\n+\t\tif ((immediate && res != LIST_AND_CHOOSE_ERROR) ||\n+\t\t !strcmp(input.buf, \"*\"))\n \t\t\tbreak;\n \t}\n \n@@ -496,7 +559,7 @@ struct print_file_item_data {\n \tstruct strbuf buf, index, worktree;\n };\n \n-static void print_file_item(int i, struct string_list_item *item,\n+static void print_file_item(int i, int selected, struct string_list_item *item,\n \t\t\t void *print_file_item_data)\n {\n \tstruct file_item *c = item->util;\n@@ -511,7 +574,7 @@ static void print_file_item(int i, struct string_list_item *item,\n \tstrbuf_addf(&d->buf, d->modified_fmt,\n \t\t d->index.buf, d->worktree.buf, item->string);\n \n-\tprintf(\" %2d: %s\", i + 1, d->buf.buf);\n+\tprintf(\"%c%2d: %s\", selected ? '*' : ' ', i + 1, d->buf.buf);\n }\n \n static int run_status(struct add_i_state *s, const struct pathspec *ps,\n@@ -520,7 +583,7 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps,\n \tif (get_modified_files(s->r, NO_FILTER, files, ps) < 0)\n \t\treturn -1;\n \n-\tlist(s, files, opts);\n+\tlist(s, files, NULL, opts);\n \tputchar('\\n');\n \n \treturn 0;\n@@ -559,7 +622,8 @@ struct print_command_item_data {\n \tconst char *color, *reset;\n };\n \n-static void print_command_item(int i, struct string_list_item *item,\n+static void print_command_item(int i, int selected,\n+\t\t\t struct string_list_item *item,\n \t\t\t void *print_command_item_data)\n {\n \tstruct print_command_item_data *d = print_command_item_data;\n@@ -592,7 +656,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \tstruct print_command_item_data data = { \"[\", \"]\" };\n \tstruct list_and_choose_options main_loop_opts = {\n \t\t{ 4, N_(\"*** Commands ***\"), print_command_item, &data },\n-\t\tN_(\"What now\"), command_prompt_help\n+\t\tN_(\"What now\"), SINGLETON | IMMEDIATE, command_prompt_help\n \t};\n \tstruct {\n \t\tconst char *string;\n-- \ngitgitgadget\n\n"},{"id":"386430","messageId":"xmqqwoby2ais.fsf@gitster-ct.c.googlers.com","threadId":"52271","inReplyTo":"pull.171.git.1573821382.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/8] build-in add -i: implement all commands in the main loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-18T02:17:31Z","receivedAt":"2019-11-18T02:17:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> Based on the js/builtin-add-i branch, this patch series implements the rest\n> of the commands in git add -i's main loop: update, revert, add_untracked, \n> patch, diff, and quit. Apart from quit, these commands are all very similar\n> in that they first build a list of files, display it, let the user choose\n> which ones to act on, and then perform the action.\n\nSo, this obviously depends on the previous \"let's start the main\nloop\" series, in which I recall you found at least one remaining bug\nafter sending the latest round.\n\nI'd prefer to queue this on top of the iteration of that series,\nwhich would hopefully become the final one, which would let me avoid\none rebase of this series ;-)\n\nThanks.\n"},{"id":"386431","messageId":"xmqqpnhp3oum.fsf@gitster-ct.c.googlers.com","threadId":"52271","inReplyTo":"xmqqwoby2ais.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/8] build-in add -i: implement all commands in the main loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-18T02:22:41Z","receivedAt":"2019-11-18T02:22:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n>> Based on the js/builtin-add-i branch, this patch series implements the rest\n>> of the commands in git add -i's main loop: update, revert, add_untracked, \n>> patch, diff, and quit. Apart from quit, these commands are all very similar\n>> in that they first build a list of files, display it, let the user choose\n>> which ones to act on, and then perform the action.\n>\n> So, this obviously depends on the previous \"let's start the main\n> loop\" series, in which I recall you found at least one remaining bug\n> after sending the latest round.\n>\n> I'd prefer to queue this on top of the iteration of that series,\n> which would hopefully become the final one, which would let me avoid\n> one rebase of this series ;-)\n>\n> Thanks.\n\nI found the v7 on the list, which is what I wanted to queue this on\ntop of. So my wait is over ;-)\n\nThanks.\n"},{"id":"386432","messageId":"xmqqlfsd3omz.fsf@gitster-ct.c.googlers.com","threadId":"52271","inReplyTo":"pull.171.git.1573821382.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/8] build-in add -i: implement all commands in the main loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-18T02:27:16Z","receivedAt":"2019-11-18T02:27:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> ... there is so little overlap between the git add --patch part\n> and the rest of git add --interactive that I could put the former\n> into a totally different file: add-patch.c.\n\nWe would want to call, after \"add -i in C\" stabilizes, directly into\n\"add -p\" machinery from different subcommands such as \"reset\",\n\"checkout\", etc., and bypass the rest of \"add -i\" when we do so. We\ncan credit the lack of overlap to the design that supports such\nusage while it is still in a scripted Porcelain.\n\nPreparing the \"add -p\" machinery in such a way that \"add -i\" merely\nis one of the users would make a lot of sense from organizational\npoint of view.\n\nThanks.\n"},{"id":"386463","messageId":"nycvar.QRO.7.76.6.1911181940020.46@tvgsbejvaqbjf.bet","threadId":"52271","inReplyTo":"xmqqlfsd3omz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/8] build-in add -i: implement all commands in the main loop","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-11-18T18:53:43Z","receivedAt":"2019-11-18T18:59:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 18 Nov 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > ... there is so little overlap between the git add --patch part\n> > and the rest of git add --interactive that I could put the former\n> > into a totally different file: add-patch.c.\n>\n> We would want to call, after \"add -i in C\" stabilizes, directly into\n> \"add -p\" machinery from different subcommands such as \"reset\",\n> \"checkout\", etc., and bypass the rest of \"add -i\" when we do so. We\n> can credit the lack of overlap to the design that supports such\n> usage while it is still in a scripted Porcelain.\n>\n> Preparing the \"add -p\" machinery in such a way that \"add -i\" merely\n> is one of the users would make a lot of sense from organizational\n> point of view.\n\nIf I understand you correctly, you mean that e.g. `git checkout -p` will\ncall the code from `add-patch.c` directly? That is indeed the case.\nAlready now, `git checkout -p` calls `run_add_interactive()` (see\nhttps://github.com/git/git/blob/v2.24.0/builtin/checkout.c#L463):\n\n\tif (opts->patch_mode) {\n\t\tconst char *patch_mode;\n\n\n\t\tif (opts->checkout_index && opts->checkout_worktree)\n\t\t\tpatch_mode = \"--patch=checkout\";\n\t\telse if (opts->checkout_index && !opts->checkout_worktree)\n\t\t\tpatch_mode = \"--patch=reset\";\n\t\telse if (!opts->checkout_index && opts->checkout_worktree)\n\t\t\tpatch_mode = \"--patch=worktree\";\n\t\telse\n\t\t\tBUG(\"either flag must have been set, worktree=%d, index=%d\",\n\t\t\t opts->checkout_worktree, opts->checkout_index);\n\t\treturn run_add_interactive(revision, patch_mode, &opts->pathspec);\n\t}\n\nAnd with the changes in PR #174 (i.e. three patch series later, see\nhttps://github.com/gitgitgadget/git/pull/174/files#diff-1f1eb09e4703764f9f9b9f1f1e67eb97R205-R218\nfor details), this will call `run_add_p()`, i.e. the newly-added code in\n`add-patch.c` directly:\n\n\t\tif (!strcmp(patch_mode, \"--patch\"))\n\t\t\tmode = ADD_P_STAGE;\n\t\telse if (!strcmp(patch_mode, \"--patch=stash\"))\n\t\t\tmode = ADD_P_STASH;\n\t\telse if (!strcmp(patch_mode, \"--patch=reset\"))\n\t\t\tmode = ADD_P_RESET;\n\t\telse if (!strcmp(patch_mode, \"--patch=checkout\"))\n\t\t\tmode = ADD_P_CHECKOUT;\n\t\telse if (!strcmp(patch_mode, \"--patch=worktree\"))\n\t\t\tmode = ADD_P_WORKTREE;\n\t\telse\n\t\t\tdie(\"'%s' not supported\", patch_mode);\n\n\t\treturn !!run_add_p(the_repository, mode, revision, pathspec);\n\nNow that I see it, it does not look the most elegant that the\n`patch_mode` is a string instead of that enum, but I think that will be\nrelatively easy to fix once the legacy `git-add--interactive.perl` will\nhave been decomissioned.\n\nCiao,\nDscho\n"},{"id":"386464","messageId":"nycvar.QRO.7.76.6.1911181958190.46@tvgsbejvaqbjf.bet","threadId":"52271","inReplyTo":"xmqqpnhp3oum.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/8] build-in add -i: implement all commands in the main loop","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-11-18T19:22:56Z","receivedAt":"2019-11-18T19:23:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 18 Nov 2019, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n>\n> > \"Johannes Schindelin via GitGitGadget\" \n> > writes:\n> >\n> >> Based on the js/builtin-add-i branch, this patch series implements the rest\n> >> of the commands in git add -i's main loop: update, revert, add_untracked,\n> >> patch, diff, and quit. Apart from quit, these commands are all very similar\n> >> in that they first build a list of files, display it, let the user choose\n> >> which ones to act on, and then perform the action.\n> >\n> > So, this obviously depends on the previous \"let's start the main\n> > loop\" series, in which I recall you found at least one remaining bug\n> > after sending the latest round.\n> >\n> > I'd prefer to queue this on top of the iteration of that series,\n> > which would hopefully become the final one, which would let me avoid\n> > one rebase of this series ;-)\n> >\n> > Thanks.\n>\n> I found the v7 on the list, which is what I wanted to queue this on\n> top of. So my wait is over ;-)\n\nYes, sorry, I did not think that I was opaque but I was.\n\nSo to clarify: I will always rebase the built-in add -i/-p patch series\non top of what you put on gitster/git. That is, this here patch series\nwill always be rebased on top of js/builtin-add-i, the next patch series\nwill always be rebased on top of js/builtin-add-i-cmds, etc\n\nCiao,\nDscho\n"},{"id":"386495","messageId":"xmqq36ek1wnt.fsf@gitster-ct.c.googlers.com","threadId":"52271","inReplyTo":"nycvar.QRO.7.76.6.1911181940020.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 0/8] build-in add -i: implement all commands in the main loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-19T01:29:10Z","receivedAt":"2019-11-19T01:29:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> Preparing the \"add -p\" machinery in such a way that \"add -i\" merely\n>> is one of the users would make a lot of sense from organizational\n>> point of view.\n>\n> If I understand you correctly, you mean that e.g. `git checkout -p` will\n> call the code from `add-patch.c` directly?...\n\nYup, and it is a good organization that the \"add -p\" machinery is\nheld not too intimate with the rest of \"add -i\" but treats \"add -i\"\nas just one of its users from the beginning, which is what you did\nhere.\n\nIn case it wasn't clear, I am saying that I like the resulting\norganization of these 8 patches.\n\n\n\n"},{"id":"386704","messageId":"xmqqeey18xha.fsf@gitster-ct.c.googlers.com","threadId":"52271","inReplyTo":"1844c4d55e21c40cbdbfdd73c82b4a1a074ff184.1573821382.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/8] built-in add -i: allow filtering the modified files list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-21T08:06:25Z","receivedAt":"2019-11-21T08:06:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> +enum modified_files_filter {\n> +\tNO_FILTER = 0,\n> +\tWORKTREE_ONLY = 1,\n> +\tINDEX_ONLY = 2,\n> +};\n> +\n> +static int get_modified_files(struct repository *r,\n> +\t\t\t enum modified_files_filter filter,\n> +\t\t\t struct string_list *files,\n> \t\t\t const struct pathspec *ps)\n> {\n> \tstruct object_id head_oid;\n> \tint is_initial = !resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n> \t\t\t\t\t &head_oid, NULL);\n> \tstruct collection_status s = { FROM_WORKTREE };\n\nWill have a comment on this later.\n\n> +\tint i;\n> \n> \tif (discard_index(r->index) < 0 ||\n> \t repo_read_index_preload(r, ps, 0) < 0)\n> @@ -411,10 +424,16 @@ static int get_modified_files(struct repository *r, struct string_list *files,\n> \ts.files = files;\n> \thashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0);\n> \n> -\tfor (s.phase = FROM_WORKTREE; s.phase <= FROM_INDEX; s.phase++) {\n> +\tfor (i = 0; i < 2; i++) {\n> \t\tstruct rev_info rev;\n> \t\tstruct setup_revision_opt opt = { 0 };\n> \n> +\t\tif (filter == INDEX_ONLY)\n> +\t\t\ts.phase = i ? FROM_WORKTREE : FROM_INDEX;\n> +\t\telse\n> +\t\t\ts.phase = i ? FROM_INDEX : FROM_WORKTREE;\n> +\t\ts.skip_unseen = filter && i;\n\n;-)\n\nLooks somewhat crazy but it works---when the caller wants to do\n'index-only' for example we are not interested in paths that did not\nappear in the INDEX side of the diff, so we run FROM_INDEX diff first\nand then the let next one (i.e. FROM_WORKTREE diff) be skipped for\npaths that are not in the result of the first one. To me personally,\nI would find the tertially expession written like this\n\n\ts.phase = (i == 0) ? FROM_INDEX : FROM_WORKTREE;\n\nmuch easier to follow, as it matches the order which ones are done\nfirst.\n\nAlso I notice two things.\n\n - It used to make 100% sense to call this field .phase because we\n always processed the first phase and then on to the second phase,\n where the first one was called WORKTREE and the second one was\n called INDEX. In the new world order after this step, the name\n .phase no longer makes any sense. Sometimes we run diff-files in\n phase 0 and diff-index in phase 1, but some other times we run\n diff-index in phase 0 and diff-files in phase 0. The variable\n that has the closest meaning to the 'phase' is the newly\n introduced 'i'.\n\n - The initialization of the local \"struct collection_status s\" at\n the beginning of the function still uses .phase = FROM_WORKTREE\n which might be somewhat misleading. We cannot remove the whole\n initialization, as the original used to nul initialize the other\n fields in this structure and I suspect the code still relies on\n it, but the initialization of .phase in particular no longer has\n any effect; it always is assigned inside the loop before the\n field gets used.\n\n\n> \t\topt.def = is_initial ?\n> \t\t\tempty_tree_oid_hex() : oid_to_hex(&head_oid);\n\nBy the way, this is not a new issue introduced by this patch, but I\nnotice copy_pathspec() is used twice on the same rev.prune_data in\nthis functino. Who is responsible for releasing the resource held\nin this struct?\n"},{"id":"386705","messageId":"xmqqa78p8x6c.fsf@gitster-ct.c.googlers.com","threadId":"52271","inReplyTo":"c324b47592dca2cd4967d62d2520a35bfcf04bae.1573821382.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/8] built-in add -i: prepare for multi-selection commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-21T08:12:59Z","receivedAt":"2019-11-21T08:13:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> The `upgrade`, `revert` and `add-untracked` commands allow selecting\n> multiple entries. Let's extend the `list_and_choose()` function to\n> accommodate those use cases.\n\nupgrade???\n\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> add-interactive.c | 114 ++++++++++++++++++++++++++++++++++++----------\n> 1 file changed, 89 insertions(+), 25 deletions(-)\n>\n> diff --git a/add-interactive.c b/add-interactive.c\n> index 8ec930ac15..33a751150a 100644\n> --- a/add-interactive.c\n> +++ b/add-interactive.c\n> @@ -72,15 +72,17 @@ static void init_add_i_state(struct add_i_state *s, struct repository *r)\n> struct prefix_item_list {\n> \tstruct string_list items;\n> \tstruct string_list sorted;\n> +\tint *selected; /* for multi-selections */\n> \tsize_t min_length, max_length;\n> };\n\nOK. The shape of the data is enough to tell us what you did in this\npatch and I think it makes sense ;-)\n\n"},{"id":"387000","messageId":"nycvar.QRO.7.76.6.1911251558200.31080@tvgsbejvaqbjf.bet","threadId":"52271","inReplyTo":"xmqqeey18xha.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/8] built-in add -i: allow filtering the modified files list","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-11-25T15:20:26Z","receivedAt":"2019-11-25T15:20:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 21 Nov 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > +enum modified_files_filter {\n> > +\tNO_FILTER = 0,\n> > +\tWORKTREE_ONLY = 1,\n> > +\tINDEX_ONLY = 2,\n> > +};\n> > +\n> > +static int get_modified_files(struct repository *r,\n> > +\t\t\t enum modified_files_filter filter,\n> > +\t\t\t struct string_list *files,\n> > \t\t\t const struct pathspec *ps)\n> > {\n> > \tstruct object_id head_oid;\n> > \tint is_initial = !resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n> > \t\t\t\t\t &head_oid, NULL);\n> > \tstruct collection_status s = { FROM_WORKTREE };\n>\n> Will have a comment on this later.\n\nYes, you're right, this initialization does not make sense. I changed it\nto `{ 0 }` because I still want the struct to be zero'ed out.\n\n> > +\tint i;\n> >\n> > \tif (discard_index(r->index) < 0 ||\n> > \t repo_read_index_preload(r, ps, 0) < 0)\n> > @@ -411,10 +424,16 @@ static int get_modified_files(struct repository *r, struct string_list *files,\n> > \ts.files = files;\n> > \thashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0);\n> >\n> > -\tfor (s.phase = FROM_WORKTREE; s.phase <= FROM_INDEX; s.phase++) {\n> > +\tfor (i = 0; i < 2; i++) {\n> > \t\tstruct rev_info rev;\n> > \t\tstruct setup_revision_opt opt = { 0 };\n> >\n> > +\t\tif (filter == INDEX_ONLY)\n> > +\t\t\ts.phase = i ? FROM_WORKTREE : FROM_INDEX;\n> > +\t\telse\n> > +\t\t\ts.phase = i ? FROM_INDEX : FROM_WORKTREE;\n> > +\t\ts.skip_unseen = filter && i;\n>\n> ;-)\n>\n> Looks somewhat crazy but it works---when the caller wants to do\n> 'index-only' for example we are not interested in paths that did not\n> appear in the INDEX side of the diff, so we run FROM_INDEX diff first\n> and then the let next one (i.e. FROM_WORKTREE diff) be skipped for\n> paths that are not in the result of the first one. To me personally,\n> I would find the tertially expession written like this\n>\n> \ts.phase = (i == 0) ? FROM_INDEX : FROM_WORKTREE;\n>\n> much easier to follow, as it matches the order which ones are done\n> first.\n\nSure, that reverses the order, but it makes it more intuitive because `i\n== 0` happens first. Changed.\n\n> Also I notice two things.\n>\n> - It used to make 100% sense to call this field .phase because we\n> always processed the first phase and then on to the second phase,\n> where the first one was called WORKTREE and the second one was\n> called INDEX. In the new world order after this step, the name\n> .phase no longer makes any sense. Sometimes we run diff-files in\n> phase 0 and diff-index in phase 1, but some other times we run\n> diff-index in phase 0 and diff-files in phase 0. The variable\n> that has the closest meaning to the 'phase' is the newly\n> introduced 'i'.\n\nI renamed it to `mode` in this commit. While I usually frown on renaming\nfields in the same commit as introducing changes in behavior, I think in\nthis case it's kind of okay because it does not add many lines.\n\n> - The initialization of the local \"struct collection_status s\" at\n> the beginning of the function still uses .phase = FROM_WORKTREE\n> which might be somewhat misleading. We cannot remove the whole\n> initialization, as the original used to nul initialize the other\n> fields in this structure and I suspect the code still relies on\n> it, but the initialization of .phase in particular no longer has\n> any effect; it always is assigned inside the loop before the\n> field gets used.\n>\n>\n> > \t\topt.def = is_initial ?\n> > \t\t\tempty_tree_oid_hex() : oid_to_hex(&head_oid);\n>\n> By the way, this is not a new issue introduced by this patch, but I\n> notice copy_pathspec() is used twice on the same rev.prune_data in\n> this functino. Who is responsible for releasing the resource held\n> in this struct?\n\nGood point. I assumed that the diff machinery would take care of this, but\nit does not. So I introduced another patch to fix up what is already in\n`next`.\n\nCiao,\nDscho\n"},{"id":"387001","messageId":"nycvar.QRO.7.76.6.1911251621180.31080@tvgsbejvaqbjf.bet","threadId":"52271","inReplyTo":"xmqqa78p8x6c.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/8] built-in add -i: prepare for multi-selection commands","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-11-25T15:21:55Z","receivedAt":"2019-11-25T15:22:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 21 Nov 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > From: Johannes Schindelin \n> >\n> > The `upgrade`, `revert` and `add-untracked` commands allow selecting\n> > multiple entries. Let's extend the `list_and_choose()` function to\n> > accommodate those use cases.\n>\n> upgrade???\n\nWhoooooops. \"update\", of course.\n\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > add-interactive.c | 114 ++++++++++++++++++++++++++++++++++++----------\n> > 1 file changed, 89 insertions(+), 25 deletions(-)\n> >\n> > diff --git a/add-interactive.c b/add-interactive.c\n> > index 8ec930ac15..33a751150a 100644\n> > --- a/add-interactive.c\n> > +++ b/add-interactive.c\n> > @@ -72,15 +72,17 @@ static void init_add_i_state(struct add_i_state *s, struct repository *r)\n> > struct prefix_item_list {\n> > \tstruct string_list items;\n> > \tstruct string_list sorted;\n> > +\tint *selected; /* for multi-selections */\n> > \tsize_t min_length, max_length;\n> > };\n>\n> OK. The shape of the data is enough to tell us what you did in this\n> patch and I think it makes sense ;-)\n\n:-)\n\nThanks,\nDscho\n"},{"id":"387308","messageId":"pull.171.v2.git.1575061909.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.git.1573821382.gitgitgadget@gmail.com","subject":"[PATCH v2 0/9] built-in add -i: implement all commands in the main loop","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-29T21:11:40Z","receivedAt":"2019-11-29T21:11:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Based on the js/builtin-add-i branch, this patch series implements the rest\nof the commands in git add -i's main loop: update, revert, add_untracked, \npatch, diff, and quit. Apart from quit, these commands are all very similar\nin that they first build a list of files, display it, let the user choose\nwhich ones to act on, and then perform the action.\n\nNote that the patch command is not actually converted to C, not completely\nat least: the built-in version simply hands off to git add--interactive \nafter letting the user select which files to act on.\n\nThe reason for this omission is practicality. Out of the 1,800+ lines of \ngit-add--interactive.perl, over a thousand deal just with the git add -p \npart. I did convert that functionality already (to be contributed in a\nseparate patch series, see https://github.com/gitgitgadget/git/pull/173),\ndiscovering that there is so little overlap between the git add --patch part\nand the rest of git add --interactive that I could put the former into a\ntotally different file: add-patch.c.\n\nChanges since v1:\n\n * As an introductory commit, we now release the pathspecs that we passed to\n the diff machinery.\n * The ternary in 1/8 (now 1/9) was adjusted from i ? to (i == 0) ? to make\n the flow more intuitive.\n * The commit message of 2/8 (now 3/9) no longer talks about an upgrade \n command (which never existed)but about the update command (that I\n actually meant).\n\nJohannes Schindelin (9):\n add-interactive: make sure to release `rev.prune_data`\n built-in add -i: allow filtering the modified files list\n built-in add -i: prepare for multi-selection commands\n built-in add -i: implement the `update` command\n built-in add -i: re-implement `revert` in C\n built-in add -i: re-implement `add-untracked` in C\n built-in add -i: implement the `patch` command\n built-in add -i: re-implement the `diff` command\n built-in add -i: offer the `quit` command\n\n add-interactive.c | 619 +++++++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 560 insertions(+), 59 deletions(-)\n\n\nbase-commit: 8c159044625e46de67cd8467f07424f38eb8301e\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-171%2Fdscho%2Fadd-i-in-c-all-except-patch-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-171/dscho/add-i-in-c-all-except-patch-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/171\n\nRange-diff vs v1:\n\n -: ---------- > 1: 1e02bf2d63 add-interactive: make sure to release `rev.prune_data`\n 1: 1844c4d55e ! 2: fab098d86e built-in add -i: allow filtering the modified files list\n @@ -17,12 +17,21 @@\n and `diff-files` phases, and then skipping files in the second phase\n when they have not been seen in the first phase.\n \n + Seeing as we change the meaning of the `phase` field, we rename it to\n + `mode` to reflect that the order depends on the exact invocation of the\n + `git add -i` command.\n +\n Signed-off-by: Johannes Schindelin \n \n diff --git a/add-interactive.c b/add-interactive.c\n --- a/add-interactive.c\n +++ b/add-interactive.c\n @@\n + }\n + \n + struct collection_status {\n +-\tenum { FROM_WORKTREE = 0, FROM_INDEX = 1 } phase;\n ++\tenum { FROM_WORKTREE = 0, FROM_INDEX = 1 } mode;\n \n \tconst char *reference;\n \n @@ -40,6 +49,15 @@\n \t\t\tadd_file_item(s->files, name);\n \n \t\t\tentry = xcalloc(sizeof(*entry), 1);\n +@@\n + \t\t}\n + \n + \t\tfile_item = entry->item;\n +-\t\tadddel = s->phase == FROM_INDEX ?\n ++\t\tadddel = s->mode == FROM_INDEX ?\n + \t\t\t&file_item->index : &file_item->worktree;\n + \t\tadddel->seen = 1;\n + \t\tadddel->add = stat.files[i]->added;\n @@\n \tfree_diffstat_info(&stat);\n }\n @@ -59,7 +77,8 @@\n \tstruct object_id head_oid;\n \tint is_initial = !resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n \t\t\t\t\t &head_oid, NULL);\n - \tstruct collection_status s = { FROM_WORKTREE };\n +-\tstruct collection_status s = { FROM_WORKTREE };\n ++\tstruct collection_status s = { 0 };\n +\tint i;\n \n \tif (discard_index(r->index) < 0 ||\n @@ -74,14 +93,23 @@\n \t\tstruct setup_revision_opt opt = { 0 };\n \n +\t\tif (filter == INDEX_ONLY)\n -+\t\t\ts.phase = i ? FROM_WORKTREE : FROM_INDEX;\n ++\t\t\ts.mode = (i == 0) ? FROM_INDEX : FROM_WORKTREE;\n +\t\telse\n -+\t\t\ts.phase = i ? FROM_INDEX : FROM_WORKTREE;\n ++\t\t\ts.mode = (i == 0) ? FROM_WORKTREE : FROM_INDEX;\n +\t\ts.skip_unseen = filter && i;\n +\n \t\topt.def = is_initial ?\n \t\t\tempty_tree_oid_hex() : oid_to_hex(&head_oid);\n \n +@@\n + \t\tif (ps)\n + \t\t\tcopy_pathspec(&rev.prune_data, ps);\n + \n +-\t\tif (s.phase == FROM_INDEX)\n ++\t\tif (s.mode == FROM_INDEX)\n + \t\t\trun_diff_index(&rev, 1);\n + \t\telse {\n + \t\t\trev.diffopt.flags.ignore_dirty_submodules = 1;\n @@\n static int run_status(struct add_i_state *s, const struct pathspec *ps,\n \t\t struct string_list *files, struct list_options *opts)\n 2: c324b47592 ! 3: 58a581f4ee built-in add -i: prepare for multi-selection commands\n @@ -2,7 +2,7 @@\n \n built-in add -i: prepare for multi-selection commands\n \n - The `upgrade`, `revert` and `add-untracked` commands allow selecting\n + The `update`, `revert` and `add-untracked` commands allow selecting\n multiple entries. Let's extend the `list_and_choose()` function to\n accommodate those use cases.\n \n @@ -168,7 +168,8 @@\n +\t\t\t\t}\n \t\t\t}\n \n - \t\t\tp[sep] = '\\0';\n + \t\t\tif (p[sep])\n + \t\t\t\tp[sep++] = '\\0';\n -\t\t\tif (index < 0)\n -\t\t\t\tindex = find_unique(p, items);\n +\t\t\tif (from < 0) {\n @@ -199,7 +200,7 @@\n +\t\t\t\t\tres += choose ? +1 : -1;\n +\t\t\t\t}\n +\n - \t\t\tp += sep + 1;\n + \t\t\tp += sep;\n \t\t}\n \n -\t\tif (res != LIST_AND_CHOOSE_ERROR)\n 3: cba5f13152 = 4: c311a29c77 built-in add -i: implement the `update` command\n 4: 5c31bbd24a = 5: f70723a160 built-in add -i: re-implement `revert` in C\n 5: 5c498795b3 = 6: 952fbc8f79 built-in add -i: re-implement `add-untracked` in C\n 6: 0c8a71e2e8 ! 7: cbd10da523 built-in add -i: implement the `patch` command\n @@ -51,9 +51,9 @@\n \t\t\t\t\t\t struct pathname_entry, ent);\n @@\n \t\tfile_item = entry->item;\n - \t\tadddel = s->phase == FROM_INDEX ?\n + \t\tadddel = s->mode == FROM_INDEX ?\n \t\t\t&file_item->index : &file_item->worktree;\n -+\t\tother_adddel = s->phase == FROM_INDEX ?\n ++\t\tother_adddel = s->mode == FROM_INDEX ?\n +\t\t\t&file_item->worktree : &file_item->index;\n \t\tadddel->seen = 1;\n \t\tadddel->add = stat.files[i]->added;\n @@ -84,7 +84,7 @@\n \tstruct object_id head_oid;\n \tint is_initial = !resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n @@\n - \t\t}\n + \t\t\tclear_pathspec(&rev.prune_data);\n \t}\n \thashmap_free_entries(&s.file_map, struct pathname_entry, ent);\n +\tif (unmerged_count)\n 7: 499f3f19e0 = 8: e4907112e4 built-in add -i: re-implement the `diff` command\n 8: dbcb340ede = 9: 5ba6cd31ab built-in add -i: offer the `quit` command\n\n-- \ngitgitgadget\n"},{"id":"387309","messageId":"1e02bf2d63e99a6416238999b646998af960bc4b.1575061909.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.v2.git.1575061909.gitgitgadget@gmail.com","subject":"[PATCH v2 1/9] add-interactive: make sure to release `rev.prune_data`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-29T21:11:41Z","receivedAt":"2019-11-29T21:11:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nDuring a review, Junio Hamano pointed out that the `rev.prune_data` was\ncopied from another pathspec but never cleaned up.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex d6cb98cd40..de2fccb0ef 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -435,6 +435,9 @@ static int get_modified_files(struct repository *r, struct string_list *files,\n \t\t\trev.diffopt.flags.ignore_dirty_submodules = 1;\n \t\t\trun_diff_files(&rev, 0);\n \t\t}\n+\n+\t\tif (ps)\n+\t\t\tclear_pathspec(&rev.prune_data);\n \t}\n \thashmap_free_entries(&s.file_map, struct pathname_entry, ent);\n \n-- \ngitgitgadget\n\n"},{"id":"387310","messageId":"fab098d86eb47d8ef108e62080db87c1381b2e56.1575061909.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.v2.git.1575061909.gitgitgadget@gmail.com","subject":"[PATCH v2 2/9] built-in add -i: allow filtering the modified files list","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-29T21:11:42Z","receivedAt":"2019-11-29T21:11:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the `update` command of `git add -i`, we are primarily interested in the\nlist of modified files that have worktree (i.e. unstaged) changes.\n\nAt the same time, we need to determine _also_ the staged changes, to be\nable to produce the full added/deleted information.\n\nThe Perl script version of `git add -i` has a parameter of the\n`list_modified()` function for that matter. In C, we can be a lot more\nprecise, using an `enum`.\n\nThe C implementation of the filter also has an easier time to avoid\nunnecessary work, simply by using an adaptive order of the `diff-index`\nand `diff-files` phases, and then skipping files in the second phase\nwhen they have not been seen in the first phase.\n\nSeeing as we change the meaning of the `phase` field, we rename it to\n`mode` to reflect that the order depends on the exact invocation of the\n`git add -i` command.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 33 ++++++++++++++++++++++++++-------\n 1 file changed, 26 insertions(+), 7 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex de2fccb0ef..c62d63e35b 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -344,10 +344,11 @@ static int pathname_entry_cmp(const void *unused_cmp_data,\n }\n \n struct collection_status {\n-\tenum { FROM_WORKTREE = 0, FROM_INDEX = 1 } phase;\n+\tenum { FROM_WORKTREE = 0, FROM_INDEX = 1 } mode;\n \n \tconst char *reference;\n \n+\tunsigned skip_unseen:1;\n \tstruct string_list *files;\n \tstruct hashmap file_map;\n };\n@@ -375,6 +376,9 @@ static void collect_changes_cb(struct diff_queue_struct *q,\n \t\tentry = hashmap_get_entry_from_hash(&s->file_map, hash, name,\n \t\t\t\t\t\t struct pathname_entry, ent);\n \t\tif (!entry) {\n+\t\t\tif (s->skip_unseen)\n+\t\t\t\tcontinue;\n+\n \t\t\tadd_file_item(s->files, name);\n \n \t\t\tentry = xcalloc(sizeof(*entry), 1);\n@@ -385,7 +389,7 @@ static void collect_changes_cb(struct diff_queue_struct *q,\n \t\t}\n \n \t\tfile_item = entry->item;\n-\t\tadddel = s->phase == FROM_INDEX ?\n+\t\tadddel = s->mode == FROM_INDEX ?\n \t\t\t&file_item->index : &file_item->worktree;\n \t\tadddel->seen = 1;\n \t\tadddel->add = stat.files[i]->added;\n@@ -396,13 +400,22 @@ static void collect_changes_cb(struct diff_queue_struct *q,\n \tfree_diffstat_info(&stat);\n }\n \n-static int get_modified_files(struct repository *r, struct string_list *files,\n+enum modified_files_filter {\n+\tNO_FILTER = 0,\n+\tWORKTREE_ONLY = 1,\n+\tINDEX_ONLY = 2,\n+};\n+\n+static int get_modified_files(struct repository *r,\n+\t\t\t enum modified_files_filter filter,\n+\t\t\t struct string_list *files,\n \t\t\t const struct pathspec *ps)\n {\n \tstruct object_id head_oid;\n \tint is_initial = !resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n \t\t\t\t\t &head_oid, NULL);\n-\tstruct collection_status s = { FROM_WORKTREE };\n+\tstruct collection_status s = { 0 };\n+\tint i;\n \n \tif (discard_index(r->index) < 0 ||\n \t repo_read_index_preload(r, ps, 0) < 0)\n@@ -412,10 +425,16 @@ static int get_modified_files(struct repository *r, struct string_list *files,\n \ts.files = files;\n \thashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0);\n \n-\tfor (s.phase = FROM_WORKTREE; s.phase <= FROM_INDEX; s.phase++) {\n+\tfor (i = 0; i < 2; i++) {\n \t\tstruct rev_info rev;\n \t\tstruct setup_revision_opt opt = { 0 };\n \n+\t\tif (filter == INDEX_ONLY)\n+\t\t\ts.mode = (i == 0) ? FROM_INDEX : FROM_WORKTREE;\n+\t\telse\n+\t\t\ts.mode = (i == 0) ? FROM_WORKTREE : FROM_INDEX;\n+\t\ts.skip_unseen = filter && i;\n+\n \t\topt.def = is_initial ?\n \t\t\tempty_tree_oid_hex() : oid_to_hex(&head_oid);\n \n@@ -429,7 +448,7 @@ static int get_modified_files(struct repository *r, struct string_list *files,\n \t\tif (ps)\n \t\t\tcopy_pathspec(&rev.prune_data, ps);\n \n-\t\tif (s.phase == FROM_INDEX)\n+\t\tif (s.mode == FROM_INDEX)\n \t\t\trun_diff_index(&rev, 1);\n \t\telse {\n \t\t\trev.diffopt.flags.ignore_dirty_submodules = 1;\n@@ -502,7 +521,7 @@ static void print_file_item(int i, struct string_list_item *item,\n static int run_status(struct add_i_state *s, const struct pathspec *ps,\n \t\t struct string_list *files, struct list_options *opts)\n {\n-\tif (get_modified_files(s->r, files, ps) < 0)\n+\tif (get_modified_files(s->r, NO_FILTER, files, ps) < 0)\n \t\treturn -1;\n \n \tlist(s, files, opts);\n-- \ngitgitgadget\n\n"},{"id":"387311","messageId":"58a581f4ee020f3557bbca889355ba54b7399b56.1575061909.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.v2.git.1575061909.gitgitgadget@gmail.com","subject":"[PATCH v2 3/9] built-in add -i: prepare for multi-selection commands","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-29T21:11:43Z","receivedAt":"2019-11-29T21:11:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe `update`, `revert` and `add-untracked` commands allow selecting\nmultiple entries. Let's extend the `list_and_choose()` function to\naccommodate those use cases.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 114 ++++++++++++++++++++++++++++++++++++----------\n 1 file changed, 89 insertions(+), 25 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex c62d63e35b..ea406e903b 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -72,15 +72,17 @@ static void init_add_i_state(struct add_i_state *s, struct repository *r)\n struct prefix_item_list {\n \tstruct string_list items;\n \tstruct string_list sorted;\n+\tint *selected; /* for multi-selections */\n \tsize_t min_length, max_length;\n };\n #define PREFIX_ITEM_LIST_INIT \\\n-\t{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_NODUP, 1, 4 }\n+\t{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_NODUP, NULL, 1, 4 }\n \n static void prefix_item_list_clear(struct prefix_item_list *list)\n {\n \tstring_list_clear(&list->items, 1);\n \tstring_list_clear(&list->sorted, 0);\n+\tFREE_AND_NULL(list->selected);\n }\n \n static void extend_prefix_length(struct string_list_item *p,\n@@ -182,11 +184,12 @@ static ssize_t find_unique(const char *string, struct prefix_item_list *list)\n struct list_options {\n \tint columns;\n \tconst char *header;\n-\tvoid (*print_item)(int i, struct string_list_item *item, void *print_item_data);\n+\tvoid (*print_item)(int i, int selected, struct string_list_item *item,\n+\t\t\t void *print_item_data);\n \tvoid *print_item_data;\n };\n \n-static void list(struct add_i_state *s, struct string_list *list,\n+static void list(struct add_i_state *s, struct string_list *list, int *selected,\n \t\t struct list_options *opts)\n {\n \tint i, last_lf = 0;\n@@ -199,7 +202,8 @@ static void list(struct add_i_state *s, struct string_list *list,\n \t\t\t\t \"%s\", opts->header);\n \n \tfor (i = 0; i < list->nr; i++) {\n-\t\topts->print_item(i, list->items + i, opts->print_item_data);\n+\t\topts->print_item(i, selected ? selected[i] : 0, list->items + i,\n+\t\t\t\t opts->print_item_data);\n \n \t\tif ((opts->columns) && ((i + 1) % (opts->columns))) {\n \t\t\tputchar('\\t');\n@@ -218,6 +222,10 @@ struct list_and_choose_options {\n \tstruct list_options list_opts;\n \n \tconst char *prompt;\n+\tenum {\n+\t\tSINGLETON = (1<<0),\n+\t\tIMMEDIATE = (1<<1),\n+\t} flags;\n \tvoid (*print_help)(struct add_i_state *s);\n };\n \n@@ -225,7 +233,8 @@ struct list_and_choose_options {\n #define LIST_AND_CHOOSE_QUIT (-2)\n \n /*\n- * Returns the selected index.\n+ * Returns the selected index in singleton mode, the number of selected items\n+ * otherwise.\n *\n * If an error occurred, returns `LIST_AND_CHOOSE_ERROR`. Upon EOF,\n * `LIST_AND_CHOOSE_QUIT` is returned.\n@@ -234,8 +243,19 @@ static ssize_t list_and_choose(struct add_i_state *s,\n \t\t\t struct prefix_item_list *items,\n \t\t\t struct list_and_choose_options *opts)\n {\n+\tint singleton = opts->flags & SINGLETON;\n+\tint immediate = opts->flags & IMMEDIATE;\n+\n \tstruct strbuf input = STRBUF_INIT;\n-\tssize_t res = LIST_AND_CHOOSE_ERROR;\n+\tssize_t res = singleton ? LIST_AND_CHOOSE_ERROR : 0;\n+\n+\tif (!singleton) {\n+\t\tfree(items->selected);\n+\t\tCALLOC_ARRAY(items->selected, items->items.nr);\n+\t}\n+\n+\tif (singleton && !immediate)\n+\t\tBUG(\"singleton requires immediate\");\n \n \tfind_unique_prefixes(items);\n \n@@ -244,15 +264,16 @@ static ssize_t list_and_choose(struct add_i_state *s,\n \n \t\tstrbuf_reset(&input);\n \n-\t\tlist(s, &items->items, &opts->list_opts);\n+\t\tlist(s, &items->items, items->selected, &opts->list_opts);\n \n \t\tcolor_fprintf(stdout, s->prompt_color, \"%s\", opts->prompt);\n-\t\tfputs(\"> \", stdout);\n+\t\tfputs(singleton ? \"> \" : \">> \", stdout);\n \t\tfflush(stdout);\n \n \t\tif (strbuf_getline(&input, stdin) == EOF) {\n \t\t\tputchar('\\n');\n-\t\t\tres = LIST_AND_CHOOSE_QUIT;\n+\t\t\tif (immediate)\n+\t\t\t\tres = LIST_AND_CHOOSE_QUIT;\n \t\t\tbreak;\n \t\t}\n \t\tstrbuf_trim(&input);\n@@ -268,7 +289,9 @@ static ssize_t list_and_choose(struct add_i_state *s,\n \t\tp = input.buf;\n \t\tfor (;;) {\n \t\t\tsize_t sep = strcspn(p, \" \\t\\r\\n,\");\n-\t\t\tssize_t index = -1;\n+\t\t\tint choose = 1;\n+\t\t\t/* `from` is inclusive, `to` is exclusive */\n+\t\t\tssize_t from = -1, to = -1;\n \n \t\t\tif (!sep) {\n \t\t\t\tif (!*p)\n@@ -277,30 +300,70 @@ static ssize_t list_and_choose(struct add_i_state *s,\n \t\t\t\tcontinue;\n \t\t\t}\n \n-\t\t\tif (isdigit(*p)) {\n+\t\t\t/* Input that begins with '-'; de-select */\n+\t\t\tif (*p == '-') {\n+\t\t\t\tchoose = 0;\n+\t\t\t\tp++;\n+\t\t\t\tsep--;\n+\t\t\t}\n+\n+\t\t\tif (sep == 1 && *p == '*') {\n+\t\t\t\tfrom = 0;\n+\t\t\t\tto = items->items.nr;\n+\t\t\t} else if (isdigit(*p)) {\n \t\t\t\tchar *endp;\n-\t\t\t\tindex = strtoul(p, &endp, 10) - 1;\n-\t\t\t\tif (endp != p + sep)\n-\t\t\t\t\tindex = -1;\n+\t\t\t\t/*\n+\t\t\t\t * A range can be specified like 5-7 or 5-.\n+\t\t\t\t *\n+\t\t\t\t * Note: `from` is 0-based while the user input\n+\t\t\t\t * is 1-based, hence we have to decrement by\n+\t\t\t\t * one. We do not have to decrement `to` even\n+\t\t\t\t * if it is 0-based because it is an exclusive\n+\t\t\t\t * boundary.\n+\t\t\t\t */\n+\t\t\t\tfrom = strtoul(p, &endp, 10) - 1;\n+\t\t\t\tif (endp == p + sep)\n+\t\t\t\t\tto = from + 1;\n+\t\t\t\telse if (*endp == '-') {\n+\t\t\t\t\tto = strtoul(++endp, &endp, 10);\n+\t\t\t\t\t/* extra characters after the range? */\n+\t\t\t\t\tif (endp != p + sep)\n+\t\t\t\t\t\tfrom = -1;\n+\t\t\t\t}\n \t\t\t}\n \n \t\t\tif (p[sep])\n \t\t\t\tp[sep++] = '\\0';\n-\t\t\tif (index < 0)\n-\t\t\t\tindex = find_unique(p, items);\n+\t\t\tif (from < 0) {\n+\t\t\t\tfrom = find_unique(p, items);\n+\t\t\t\tif (from >= 0)\n+\t\t\t\t\tto = from + 1;\n+\t\t\t}\n \n-\t\t\tif (index < 0 || index >= items->items.nr)\n+\t\t\tif (from < 0 || from >= items->items.nr ||\n+\t\t\t (singleton && from + 1 != to)) {\n \t\t\t\tcolor_fprintf_ln(stdout, s->error_color,\n \t\t\t\t\t\t _(\"Huh (%s)?\"), p);\n-\t\t\telse {\n-\t\t\t\tres = index;\n+\t\t\t\tbreak;\n+\t\t\t} else if (singleton) {\n+\t\t\t\tres = from;\n \t\t\t\tbreak;\n \t\t\t}\n \n+\t\t\tif (to > items->items.nr)\n+\t\t\t\tto = items->items.nr;\n+\n+\t\t\tfor (; from < to; from++)\n+\t\t\t\tif (items->selected[from] != choose) {\n+\t\t\t\t\titems->selected[from] = choose;\n+\t\t\t\t\tres += choose ? +1 : -1;\n+\t\t\t\t}\n+\n \t\t\tp += sep;\n \t\t}\n \n-\t\tif (res != LIST_AND_CHOOSE_ERROR)\n+\t\tif ((immediate && res != LIST_AND_CHOOSE_ERROR) ||\n+\t\t !strcmp(input.buf, \"*\"))\n \t\t\tbreak;\n \t}\n \n@@ -500,7 +563,7 @@ struct print_file_item_data {\n \tstruct strbuf buf, index, worktree;\n };\n \n-static void print_file_item(int i, struct string_list_item *item,\n+static void print_file_item(int i, int selected, struct string_list_item *item,\n \t\t\t void *print_file_item_data)\n {\n \tstruct file_item *c = item->util;\n@@ -515,7 +578,7 @@ static void print_file_item(int i, struct string_list_item *item,\n \tstrbuf_addf(&d->buf, d->modified_fmt,\n \t\t d->index.buf, d->worktree.buf, item->string);\n \n-\tprintf(\" %2d: %s\", i + 1, d->buf.buf);\n+\tprintf(\"%c%2d: %s\", selected ? '*' : ' ', i + 1, d->buf.buf);\n }\n \n static int run_status(struct add_i_state *s, const struct pathspec *ps,\n@@ -524,7 +587,7 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps,\n \tif (get_modified_files(s->r, NO_FILTER, files, ps) < 0)\n \t\treturn -1;\n \n-\tlist(s, files, opts);\n+\tlist(s, files, NULL, opts);\n \tputchar('\\n');\n \n \treturn 0;\n@@ -563,7 +626,8 @@ struct print_command_item_data {\n \tconst char *color, *reset;\n };\n \n-static void print_command_item(int i, struct string_list_item *item,\n+static void print_command_item(int i, int selected,\n+\t\t\t struct string_list_item *item,\n \t\t\t void *print_command_item_data)\n {\n \tstruct print_command_item_data *d = print_command_item_data;\n@@ -596,7 +660,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \tstruct print_command_item_data data = { \"[\", \"]\" };\n \tstruct list_and_choose_options main_loop_opts = {\n \t\t{ 4, N_(\"*** Commands ***\"), print_command_item, &data },\n-\t\tN_(\"What now\"), command_prompt_help\n+\t\tN_(\"What now\"), SINGLETON | IMMEDIATE, command_prompt_help\n \t};\n \tstruct {\n \t\tconst char *string;\n-- \ngitgitgadget\n\n"},{"id":"387312","messageId":"c311a29c77f0c05bcb455d387a15ee9c77d765e4.1575061909.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.v2.git.1575061909.gitgitgadget@gmail.com","subject":"[PATCH v2 4/9] built-in add -i: implement the `update` command","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-29T21:11:44Z","receivedAt":"2019-11-29T21:11:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAfter `status` and `help`, it is now time to port the `update` command\nto C, the second command that is shown in the main loop menu of `git add\n-i`.\n\nThis `git add -i` command is the first one which lets the user choose a\nsubset of a list of files, and as such, this patch lays the groundwork\nfor the other commands of that category:\n\n- It teaches the `print_file_item()` function to show a unique prefix\n if we found any (the code to find it had been added already in the\n previous patch where we colored the unique prefixes of the main loop\n commands, but that patch uses the `print_command_item()` function to\n display the menu items).\n\n- This patch also adds the help text that is shown when the user input\n to select items from the shown list could not be parsed.\n\n- As `get_modified_files()` clears the list of files, it now has to take\n care of clearing the _full_ `prefix_item_list` lest the `sorted` and\n `selected` fields go stale and inconsistent.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 130 +++++++++++++++++++++++++++++++++++++++-------\n 1 file changed, 110 insertions(+), 20 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex ea406e903b..1e34e88069 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -6,6 +6,7 @@\n #include \"revision.h\"\n #include \"refs.h\"\n #include \"string-list.h\"\n+#include \"lockfile.h\"\n \n struct add_i_state {\n \tstruct repository *r;\n@@ -377,6 +378,7 @@ struct adddel {\n };\n \n struct file_item {\n+\tsize_t prefix_length;\n \tstruct adddel index, worktree;\n };\n \n@@ -471,7 +473,7 @@ enum modified_files_filter {\n \n static int get_modified_files(struct repository *r,\n \t\t\t enum modified_files_filter filter,\n-\t\t\t struct string_list *files,\n+\t\t\t struct prefix_item_list *files,\n \t\t\t const struct pathspec *ps)\n {\n \tstruct object_id head_oid;\n@@ -484,8 +486,8 @@ static int get_modified_files(struct repository *r,\n \t repo_read_index_preload(r, ps, 0) < 0)\n \t\treturn error(_(\"could not read index\"));\n \n-\tstring_list_clear(files, 1);\n-\ts.files = files;\n+\tprefix_item_list_clear(files);\n+\ts.files = &files->items;\n \thashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0);\n \n \tfor (i = 0; i < 2; i++) {\n@@ -524,7 +526,7 @@ static int get_modified_files(struct repository *r,\n \thashmap_free_entries(&s.file_map, struct pathname_entry, ent);\n \n \t/* While the diffs are ordered already, we ran *two* diffs... */\n-\tstring_list_sort(files);\n+\tstring_list_sort(&files->items);\n \n \treturn 0;\n }\n@@ -559,8 +561,8 @@ static int is_valid_prefix(const char *prefix, size_t prefix_len)\n }\n \n struct print_file_item_data {\n-\tconst char *modified_fmt;\n-\tstruct strbuf buf, index, worktree;\n+\tconst char *modified_fmt, *color, *reset;\n+\tstruct strbuf buf, name, index, worktree;\n };\n \n static void print_file_item(int i, int selected, struct string_list_item *item,\n@@ -568,34 +570,96 @@ static void print_file_item(int i, int selected, struct string_list_item *item,\n {\n \tstruct file_item *c = item->util;\n \tstruct print_file_item_data *d = print_file_item_data;\n+\tconst char *highlighted = NULL;\n \n \tstrbuf_reset(&d->index);\n \tstrbuf_reset(&d->worktree);\n \tstrbuf_reset(&d->buf);\n \n+\t/* Format the item with the prefix highlighted. */\n+\tif (c->prefix_length > 0 &&\n+\t is_valid_prefix(item->string, c->prefix_length)) {\n+\t\tstrbuf_reset(&d->name);\n+\t\tstrbuf_addf(&d->name, \"%s%.*s%s%s\", d->color,\n+\t\t\t (int)c->prefix_length, item->string, d->reset,\n+\t\t\t item->string + c->prefix_length);\n+\t\thighlighted = d->name.buf;\n+\t}\n+\n \trender_adddel(&d->worktree, &c->worktree, _(\"nothing\"));\n \trender_adddel(&d->index, &c->index, _(\"unchanged\"));\n-\tstrbuf_addf(&d->buf, d->modified_fmt,\n-\t\t d->index.buf, d->worktree.buf, item->string);\n+\n+\tstrbuf_addf(&d->buf, d->modified_fmt, d->index.buf, d->worktree.buf,\n+\t\t highlighted ? highlighted : item->string);\n \n \tprintf(\"%c%2d: %s\", selected ? '*' : ' ', i + 1, d->buf.buf);\n }\n \n static int run_status(struct add_i_state *s, const struct pathspec *ps,\n-\t\t struct string_list *files, struct list_options *opts)\n+\t\t struct prefix_item_list *files,\n+\t\t struct list_and_choose_options *opts)\n {\n \tif (get_modified_files(s->r, NO_FILTER, files, ps) < 0)\n \t\treturn -1;\n \n-\tlist(s, files, NULL, opts);\n+\tlist(s, &files->items, NULL, &opts->list_opts);\n \tputchar('\\n');\n \n \treturn 0;\n }\n \n+static int run_update(struct add_i_state *s, const struct pathspec *ps,\n+\t\t struct prefix_item_list *files,\n+\t\t struct list_and_choose_options *opts)\n+{\n+\tint res = 0, fd;\n+\tsize_t count, i;\n+\tstruct lock_file index_lock;\n+\n+\tif (get_modified_files(s->r, WORKTREE_ONLY, files, ps) < 0)\n+\t\treturn -1;\n+\n+\tif (!files->items.nr) {\n+\t\tputchar('\\n');\n+\t\treturn 0;\n+\t}\n+\n+\topts->prompt = N_(\"Update\");\n+\tcount = list_and_choose(s, files, opts);\n+\tif (count <= 0) {\n+\t\tputchar('\\n');\n+\t\treturn 0;\n+\t}\n+\n+\tfd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR);\n+\tif (fd < 0) {\n+\t\tputchar('\\n');\n+\t\treturn -1;\n+\t}\n+\n+\tfor (i = 0; i < files->items.nr; i++) {\n+\t\tconst char *name = files->items.items[i].string;\n+\t\tif (files->selected[i] &&\n+\t\t add_file_to_index(s->r->index, name, 0) < 0) {\n+\t\t\tres = error(_(\"could not stage '%s'\"), name);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\tif (!res && write_locked_index(s->r->index, &index_lock, COMMIT_LOCK) < 0)\n+\t\tres = error(_(\"could not write index\"));\n+\n+\tif (!res)\n+\t\tprintf(Q_(\"updated %d path\\n\",\n+\t\t\t \"updated %d paths\\n\", count), (int)count);\n+\n+\tputchar('\\n');\n+\treturn res;\n+}\n+\n static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,\n-\t\t struct string_list *unused_files,\n-\t\t struct list_options *unused_opts)\n+\t\t struct prefix_item_list *unused_files,\n+\t\t struct list_and_choose_options *unused_opts)\n {\n \tcolor_fprintf_ln(stdout, s->help_color, \"status - %s\",\n \t\t\t _(\"show paths with changes\"));\n@@ -613,9 +677,29 @@ static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,\n \treturn 0;\n }\n \n+static void choose_prompt_help(struct add_i_state *s)\n+{\n+\tcolor_fprintf_ln(stdout, s->help_color, \"%s\",\n+\t\t\t _(\"Prompt help:\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \"1 - %s\",\n+\t\t\t _(\"select a single item\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \"3-5 - %s\",\n+\t\t\t _(\"select a range of items\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \"2-3,6-9 - %s\",\n+\t\t\t _(\"select multiple ranges\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \"foo - %s\",\n+\t\t\t _(\"select item based on unique prefix\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \"-... - %s\",\n+\t\t\t _(\"unselect specified items\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \"* - %s\",\n+\t\t\t _(\"choose all items\"));\n+\tcolor_fprintf_ln(stdout, s->help_color, \" - %s\",\n+\t\t\t _(\"(empty) finish selecting\"));\n+}\n+\n typedef int (*command_t)(struct add_i_state *s, const struct pathspec *ps,\n-\t\t\t struct string_list *files,\n-\t\t\t struct list_options *opts);\n+\t\t\t struct prefix_item_list *files,\n+\t\t\t struct list_and_choose_options *opts);\n \n struct command_item {\n \tsize_t prefix_length;\n@@ -667,18 +751,21 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\tcommand_t command;\n \t} command_list[] = {\n \t\t{ \"status\", run_status },\n+\t\t{ \"update\", run_update },\n \t\t{ \"help\", run_help },\n \t};\n \tstruct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;\n \n \tstruct print_file_item_data print_file_item_data = {\n-\t\t\"%12s %12s %s\", STRBUF_INIT, STRBUF_INIT, STRBUF_INIT\n+\t\t\"%12s %12s %s\", NULL, NULL,\n+\t\tSTRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT\n \t};\n-\tstruct list_options opts = {\n-\t\t0, NULL, print_file_item, &print_file_item_data\n+\tstruct list_and_choose_options opts = {\n+\t\t{ 0, NULL, print_file_item, &print_file_item_data },\n+\t\tNULL, 0, choose_prompt_help\n \t};\n \tstruct strbuf header = STRBUF_INIT;\n-\tstruct string_list files = STRING_LIST_INIT_DUP;\n+\tstruct prefix_item_list files = PREFIX_ITEM_LIST_INIT;\n \tssize_t i;\n \tint res = 0;\n \n@@ -699,11 +786,13 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\tdata.color = s.prompt_color;\n \t\tdata.reset = s.reset_color;\n \t}\n+\tprint_file_item_data.color = data.color;\n+\tprint_file_item_data.reset = data.reset;\n \n \tstrbuf_addstr(&header, \" \");\n \tstrbuf_addf(&header, print_file_item_data.modified_fmt,\n \t\t _(\"staged\"), _(\"unstaged\"), _(\"path\"));\n-\topts.header = header.buf;\n+\topts.list_opts.header = header.buf;\n \n \tif (discard_index(r->index) < 0 ||\n \t repo_read_index(r) < 0 ||\n@@ -727,8 +816,9 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\t}\n \t}\n \n-\tstring_list_clear(&files, 1);\n+\tprefix_item_list_clear(&files);\n \tstrbuf_release(&print_file_item_data.buf);\n+\tstrbuf_release(&print_file_item_data.name);\n \tstrbuf_release(&print_file_item_data.index);\n \tstrbuf_release(&print_file_item_data.worktree);\n \tstrbuf_release(&header);\n-- \ngitgitgadget\n\n"},{"id":"387313","messageId":"f70723a160c1ecddf87349fc99af25f7718e1413.1575061909.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.v2.git.1575061909.gitgitgadget@gmail.com","subject":"[PATCH v2 5/9] built-in add -i: re-implement `revert` in C","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-29T21:11:45Z","receivedAt":"2019-11-29T21:12:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis is a relatively straight-forward port from the Perl version, with\nthe notable exception that we imitate `git reset -- ` in the C\nversion rather than the convoluted `git ls-tree HEAD -- | git\nupdate-index --index-info` followed by `git update-index --force-remove\n-- ` for the missed ones.\n\nWhile at it, we fix the pretty obvious bug where the `revert` command\noffers to unstage files that do not have staged changes.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 109 insertions(+)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 1e34e88069..adab17a635 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -657,6 +657,114 @@ static int run_update(struct add_i_state *s, const struct pathspec *ps,\n \treturn res;\n }\n \n+static void revert_from_diff(struct diff_queue_struct *q,\n+\t\t\t struct diff_options *opt, void *data)\n+{\n+\tint i, add_flags = ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE;\n+\n+\tfor (i = 0; i < q->nr; i++) {\n+\t\tstruct diff_filespec *one = q->queue[i]->one;\n+\t\tstruct cache_entry *ce;\n+\n+\t\tif (!(one->mode && !is_null_oid(&one->oid))) {\n+\t\t\tremove_file_from_index(opt->repo->index, one->path);\n+\t\t\tprintf(_(\"note: %s is untracked now.\\n\"), one->path);\n+\t\t} else {\n+\t\t\tce = make_cache_entry(opt->repo->index, one->mode,\n+\t\t\t\t\t &one->oid, one->path, 0, 0);\n+\t\t\tif (!ce)\n+\t\t\t\tdie(_(\"make_cache_entry failed for path '%s'\"),\n+\t\t\t\t one->path);\n+\t\t\tadd_index_entry(opt->repo->index, ce, add_flags);\n+\t\t}\n+\t}\n+}\n+\n+static int run_revert(struct add_i_state *s, const struct pathspec *ps,\n+\t\t struct prefix_item_list *files,\n+\t\t struct list_and_choose_options *opts)\n+{\n+\tint res = 0, fd;\n+\tsize_t count, i, j;\n+\n+\tstruct object_id oid;\n+\tint is_initial = !resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, &oid,\n+\t\t\t\t\t NULL);\n+\tstruct lock_file index_lock;\n+\tconst char **paths;\n+\tstruct tree *tree;\n+\tstruct diff_options diffopt = { NULL };\n+\n+\tif (get_modified_files(s->r, INDEX_ONLY, files, ps) < 0)\n+\t\treturn -1;\n+\n+\tif (!files->items.nr) {\n+\t\tputchar('\\n');\n+\t\treturn 0;\n+\t}\n+\n+\topts->prompt = N_(\"Revert\");\n+\tcount = list_and_choose(s, files, opts);\n+\tif (count <= 0)\n+\t\tgoto finish_revert;\n+\n+\tfd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR);\n+\tif (fd < 0) {\n+\t\tres = -1;\n+\t\tgoto finish_revert;\n+\t}\n+\n+\tif (is_initial)\n+\t\toidcpy(&oid, s->r->hash_algo->empty_tree);\n+\telse {\n+\t\ttree = parse_tree_indirect(&oid);\n+\t\tif (!tree) {\n+\t\t\tres = error(_(\"Could not parse HEAD^{tree}\"));\n+\t\t\tgoto finish_revert;\n+\t\t}\n+\t\toidcpy(&oid, &tree->object.oid);\n+\t}\n+\n+\tALLOC_ARRAY(paths, count + 1);\n+\tfor (i = j = 0; i < files->items.nr; i++)\n+\t\tif (files->selected[i])\n+\t\t\tpaths[j++] = files->items.items[i].string;\n+\tpaths[j] = NULL;\n+\n+\tparse_pathspec(&diffopt.pathspec, 0,\n+\t\t PATHSPEC_PREFER_FULL | PATHSPEC_LITERAL_PATH,\n+\t\t NULL, paths);\n+\n+\tdiffopt.output_format = DIFF_FORMAT_CALLBACK;\n+\tdiffopt.format_callback = revert_from_diff;\n+\tdiffopt.flags.override_submodule_config = 1;\n+\tdiffopt.repo = s->r;\n+\n+\tif (do_diff_cache(&oid, &diffopt))\n+\t\tres = -1;\n+\telse {\n+\t\tdiffcore_std(&diffopt);\n+\t\tdiff_flush(&diffopt);\n+\t}\n+\tfree(paths);\n+\tclear_pathspec(&diffopt.pathspec);\n+\n+\tif (!res && write_locked_index(s->r->index, &index_lock,\n+\t\t\t\t COMMIT_LOCK) < 0)\n+\t\tres = -1;\n+\telse\n+\t\tres = repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0, 1,\n+\t\t\t\t\t\t NULL, NULL, NULL);\n+\n+\tif (!res)\n+\t\tprintf(Q_(\"reverted %d path\\n\",\n+\t\t\t \"reverted %d paths\\n\", count), (int)count);\n+\n+finish_revert:\n+\tputchar('\\n');\n+\treturn res;\n+}\n+\n static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,\n \t\t struct prefix_item_list *unused_files,\n \t\t struct list_and_choose_options *unused_opts)\n@@ -752,6 +860,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t} command_list[] = {\n \t\t{ \"status\", run_status },\n \t\t{ \"update\", run_update },\n+\t\t{ \"revert\", run_revert },\n \t\t{ \"help\", run_help },\n \t};\n \tstruct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;\n-- \ngitgitgadget\n\n"},{"id":"387314","messageId":"cbd10da523a0d1f17ad90628b3867fd498dd3675.1575061909.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.v2.git.1575061909.gitgitgadget@gmail.com","subject":"[PATCH v2 7/9] built-in add -i: implement the `patch` command","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-29T21:11:47Z","receivedAt":"2019-11-29T21:12:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWell, it is not a full implementation yet. In the interest of making\nthis easy to review (and easy to keep bugs out), we still hand off to\nthe Perl script to do the actual work.\n\nThe `patch` functionality actually makes up for more than half of the\n1,800+ lines of `git-add--interactive.perl`. It will be ported from Perl\nto C incrementally, later.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 91 +++++++++++++++++++++++++++++++++++++++++++----\n 1 file changed, 84 insertions(+), 7 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex a719d30b0b..cba9688bb5 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -8,6 +8,7 @@\n #include \"string-list.h\"\n #include \"lockfile.h\"\n #include \"dir.h\"\n+#include \"run-command.h\"\n \n struct add_i_state {\n \tstruct repository *r;\n@@ -375,7 +376,7 @@ static ssize_t list_and_choose(struct add_i_state *s,\n \n struct adddel {\n \tuintmax_t add, del;\n-\tunsigned seen:1, binary:1;\n+\tunsigned seen:1, unmerged:1, binary:1;\n };\n \n struct file_item {\n@@ -415,6 +416,7 @@ struct collection_status {\n \tconst char *reference;\n \n \tunsigned skip_unseen:1;\n+\tsize_t unmerged_count, binary_count;\n \tstruct string_list *files;\n \tstruct hashmap file_map;\n };\n@@ -437,7 +439,7 @@ static void collect_changes_cb(struct diff_queue_struct *q,\n \t\tint hash = strhash(name);\n \t\tstruct pathname_entry *entry;\n \t\tstruct file_item *file_item;\n-\t\tstruct adddel *adddel;\n+\t\tstruct adddel *adddel, *other_adddel;\n \n \t\tentry = hashmap_get_entry_from_hash(&s->file_map, hash, name,\n \t\t\t\t\t\t struct pathname_entry, ent);\n@@ -457,11 +459,21 @@ static void collect_changes_cb(struct diff_queue_struct *q,\n \t\tfile_item = entry->item;\n \t\tadddel = s->mode == FROM_INDEX ?\n \t\t\t&file_item->index : &file_item->worktree;\n+\t\tother_adddel = s->mode == FROM_INDEX ?\n+\t\t\t&file_item->worktree : &file_item->index;\n \t\tadddel->seen = 1;\n \t\tadddel->add = stat.files[i]->added;\n \t\tadddel->del = stat.files[i]->deleted;\n-\t\tif (stat.files[i]->is_binary)\n+\t\tif (stat.files[i]->is_binary) {\n+\t\t\tif (!other_adddel->binary)\n+\t\t\t\ts->binary_count++;\n \t\t\tadddel->binary = 1;\n+\t\t}\n+\t\tif (stat.files[i]->is_unmerged) {\n+\t\t\tif (!other_adddel->unmerged)\n+\t\t\t\ts->unmerged_count++;\n+\t\t\tadddel->unmerged = 1;\n+\t\t}\n \t}\n \tfree_diffstat_info(&stat);\n }\n@@ -475,7 +487,9 @@ enum modified_files_filter {\n static int get_modified_files(struct repository *r,\n \t\t\t enum modified_files_filter filter,\n \t\t\t struct prefix_item_list *files,\n-\t\t\t const struct pathspec *ps)\n+\t\t\t const struct pathspec *ps,\n+\t\t\t size_t *unmerged_count,\n+\t\t\t size_t *binary_count)\n {\n \tstruct object_id head_oid;\n \tint is_initial = !resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n@@ -525,6 +539,10 @@ static int get_modified_files(struct repository *r,\n \t\t\tclear_pathspec(&rev.prune_data);\n \t}\n \thashmap_free_entries(&s.file_map, struct pathname_entry, ent);\n+\tif (unmerged_count)\n+\t\t*unmerged_count = s.unmerged_count;\n+\tif (binary_count)\n+\t\t*binary_count = s.binary_count;\n \n \t/* While the diffs are ordered already, we ran *two* diffs... */\n \tstring_list_sort(&files->items);\n@@ -607,7 +625,7 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps,\n \t\t struct prefix_item_list *files,\n \t\t struct list_and_choose_options *opts)\n {\n-\tif (get_modified_files(s->r, NO_FILTER, files, ps) < 0)\n+\tif (get_modified_files(s->r, NO_FILTER, files, ps, NULL, NULL) < 0)\n \t\treturn -1;\n \n \tlist(s, &files->items, NULL, &opts->list_opts);\n@@ -624,7 +642,7 @@ static int run_update(struct add_i_state *s, const struct pathspec *ps,\n \tsize_t count, i;\n \tstruct lock_file index_lock;\n \n-\tif (get_modified_files(s->r, WORKTREE_ONLY, files, ps) < 0)\n+\tif (get_modified_files(s->r, WORKTREE_ONLY, files, ps, NULL, NULL) < 0)\n \t\treturn -1;\n \n \tif (!files->items.nr) {\n@@ -703,7 +721,7 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps,\n \tstruct tree *tree;\n \tstruct diff_options diffopt = { NULL };\n \n-\tif (get_modified_files(s->r, INDEX_ONLY, files, ps) < 0)\n+\tif (get_modified_files(s->r, INDEX_ONLY, files, ps, NULL, NULL) < 0)\n \t\treturn -1;\n \n \tif (!files->items.nr) {\n@@ -855,6 +873,64 @@ static int run_add_untracked(struct add_i_state *s, const struct pathspec *ps,\n \treturn res;\n }\n \n+static int run_patch(struct add_i_state *s, const struct pathspec *ps,\n+\t\t struct prefix_item_list *files,\n+\t\t struct list_and_choose_options *opts)\n+{\n+\tint res = 0;\n+\tssize_t count, i, j;\n+\tsize_t unmerged_count = 0, binary_count = 0;\n+\n+\tif (get_modified_files(s->r, WORKTREE_ONLY, files, ps,\n+\t\t\t &unmerged_count, &binary_count) < 0)\n+\t\treturn -1;\n+\n+\tif (unmerged_count || binary_count) {\n+\t\tfor (i = j = 0; i < files->items.nr; i++) {\n+\t\t\tstruct file_item *item = files->items.items[i].util;\n+\n+\t\t\tif (item->index.binary || item->worktree.binary) {\n+\t\t\t\tfree(item);\n+\t\t\t\tfree(files->items.items[i].string);\n+\t\t\t} else if (item->index.unmerged ||\n+\t\t\t\t item->worktree.unmerged) {\n+\t\t\t\tcolor_fprintf_ln(stderr, s->error_color,\n+\t\t\t\t\t\t _(\"ignoring unmerged: %s\"),\n+\t\t\t\t\t\t files->items.items[i].string);\n+\t\t\t\tfree(item);\n+\t\t\t\tfree(files->items.items[i].string);\n+\t\t\t} else\n+\t\t\t\tfiles->items.items[j++] = files->items.items[i];\n+\t\t}\n+\t\tfiles->items.nr = j;\n+\t}\n+\n+\tif (!files->items.nr) {\n+\t\tif (binary_count)\n+\t\t\tfprintf(stderr, _(\"Only binary files changed.\\n\"));\n+\t\telse\n+\t\t\tfprintf(stderr, _(\"No changes.\\n\"));\n+\t\treturn 0;\n+\t}\n+\n+\topts->prompt = N_(\"Patch update\");\n+\tcount = list_and_choose(s, files, opts);\n+\tif (count >= 0) {\n+\t\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\n+\t\targv_array_pushl(&args, \"git\", \"add--interactive\", \"--patch\",\n+\t\t\t\t \"--\", NULL);\n+\t\tfor (i = 0; i < files->items.nr; i++)\n+\t\t\tif (files->selected[i])\n+\t\t\t\targv_array_push(&args,\n+\t\t\t\t\t\tfiles->items.items[i].string);\n+\t\tres = run_command_v_opt(args.argv, 0);\n+\t\targv_array_clear(&args);\n+\t}\n+\n+\treturn res;\n+}\n+\n static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,\n \t\t struct prefix_item_list *unused_files,\n \t\t struct list_and_choose_options *unused_opts)\n@@ -952,6 +1028,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\t{ \"update\", run_update },\n \t\t{ \"revert\", run_revert },\n \t\t{ \"add untracked\", run_add_untracked },\n+\t\t{ \"patch\", run_patch },\n \t\t{ \"help\", run_help },\n \t};\n \tstruct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;\n-- \ngitgitgadget\n\n"},{"id":"387316","messageId":"e4907112e4b7e053b1436b8bcb8337deb5309463.1575061909.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.v2.git.1575061909.gitgitgadget@gmail.com","subject":"[PATCH v2 8/9] built-in add -i: re-implement the `diff` command","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-29T21:11:48Z","receivedAt":"2019-11-29T21:12:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIt is not only laziness that we simply spawn `git diff -p --cached`\nhere: this command needs to use the pager, and the pager needs to exit\nwhen the diff is done. Currently we do not have any way to make that\nhappen if we run the diff in-process. So let's just spawn.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 42 ++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 42 insertions(+)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex cba9688bb5..4d7d44a917 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -931,6 +931,47 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps,\n \treturn res;\n }\n \n+static int run_diff(struct add_i_state *s, const struct pathspec *ps,\n+\t\t struct prefix_item_list *files,\n+\t\t struct list_and_choose_options *opts)\n+{\n+\tint res = 0;\n+\tssize_t count, i;\n+\n+\tstruct object_id oid;\n+\tint is_initial = !resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, &oid,\n+\t\t\t\t\t NULL);\n+\tif (get_modified_files(s->r, INDEX_ONLY, files, ps, NULL, NULL) < 0)\n+\t\treturn -1;\n+\n+\tif (!files->items.nr) {\n+\t\tputchar('\\n');\n+\t\treturn 0;\n+\t}\n+\n+\topts->prompt = N_(\"Review diff\");\n+\topts->flags = IMMEDIATE;\n+\tcount = list_and_choose(s, files, opts);\n+\topts->flags = 0;\n+\tif (count >= 0) {\n+\t\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\n+\t\targv_array_pushl(&args, \"git\", \"diff\", \"-p\", \"--cached\",\n+\t\t\t\t oid_to_hex(!is_initial ? &oid :\n+\t\t\t\t\t s->r->hash_algo->empty_tree),\n+\t\t\t\t \"--\", NULL);\n+\t\tfor (i = 0; i < files->items.nr; i++)\n+\t\t\tif (files->selected[i])\n+\t\t\t\targv_array_push(&args,\n+\t\t\t\t\t\tfiles->items.items[i].string);\n+\t\tres = run_command_v_opt(args.argv, 0);\n+\t\targv_array_clear(&args);\n+\t}\n+\n+\tputchar('\\n');\n+\treturn res;\n+}\n+\n static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,\n \t\t struct prefix_item_list *unused_files,\n \t\t struct list_and_choose_options *unused_opts)\n@@ -1029,6 +1070,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\t{ \"revert\", run_revert },\n \t\t{ \"add untracked\", run_add_untracked },\n \t\t{ \"patch\", run_patch },\n+\t\t{ \"diff\", run_diff },\n \t\t{ \"help\", run_help },\n \t};\n \tstruct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;\n-- \ngitgitgadget\n\n"},{"id":"387315","messageId":"5ba6cd31ab5bba6828f4e195ebd17de9c0758ba7.1575061909.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.v2.git.1575061909.gitgitgadget@gmail.com","subject":"[PATCH v2 9/9] built-in add -i: offer the `quit` command","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-29T21:11:49Z","receivedAt":"2019-11-29T21:12:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe do not really want to `exit()` here, of course, as this is safely\nlibified code.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 16 +++++++++++-----\n 1 file changed, 11 insertions(+), 5 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 4d7d44a917..f395d54c08 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -1071,6 +1071,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\t{ \"add untracked\", run_add_untracked },\n \t\t{ \"patch\", run_patch },\n \t\t{ \"diff\", run_diff },\n+\t\t{ \"quit\", NULL },\n \t\t{ \"help\", run_help },\n \t};\n \tstruct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;\n@@ -1122,17 +1123,22 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \tres = run_status(&s, ps, &files, &opts);\n \n \tfor (;;) {\n+\t\tstruct command_item *util;\n+\n \t\ti = list_and_choose(&s, &commands, &main_loop_opts);\n-\t\tif (i == LIST_AND_CHOOSE_QUIT) {\n+\t\tif (i < 0 || i >= commands.items.nr)\n+\t\t\tutil = NULL;\n+\t\telse\n+\t\t\tutil = commands.items.items[i].util;\n+\n+\t\tif (i == LIST_AND_CHOOSE_QUIT || (util && !util->command)) {\n \t\t\tprintf(_(\"Bye.\\n\"));\n \t\t\tres = 0;\n \t\t\tbreak;\n \t\t}\n-\t\tif (i != LIST_AND_CHOOSE_ERROR) {\n-\t\t\tstruct command_item *util =\n-\t\t\t\tcommands.items.items[i].util;\n+\n+\t\tif (util)\n \t\t\tres = util->command(&s, ps, &files, &opts);\n-\t\t}\n \t}\n \n \tprefix_item_list_clear(&files);\n-- \ngitgitgadget\n"},{"id":"387317","messageId":"952fbc8f79760554b3f45ca9a935541a5dbd1a33.1575061909.git.gitgitgadget@gmail.com","threadId":"52271","inReplyTo":"pull.171.v2.git.1575061909.gitgitgadget@gmail.com","subject":"[PATCH v2 6/9] built-in add -i: re-implement `add-untracked` in C","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-29T21:11:46Z","receivedAt":"2019-11-29T21:12:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis is yet another command, ported to C. It builds nicely on the\nsupport functions introduced for other commands, with the notable\ndifference that only names are displayed for untracked files, no\nfile type or diff summary.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 91 insertions(+)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex adab17a635..a719d30b0b 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -7,6 +7,7 @@\n #include \"refs.h\"\n #include \"string-list.h\"\n #include \"lockfile.h\"\n+#include \"dir.h\"\n \n struct add_i_state {\n \tstruct repository *r;\n@@ -563,6 +564,7 @@ static int is_valid_prefix(const char *prefix, size_t prefix_len)\n struct print_file_item_data {\n \tconst char *modified_fmt, *color, *reset;\n \tstruct strbuf buf, name, index, worktree;\n+\tunsigned only_names:1;\n };\n \n static void print_file_item(int i, int selected, struct string_list_item *item,\n@@ -586,6 +588,12 @@ static void print_file_item(int i, int selected, struct string_list_item *item,\n \t\thighlighted = d->name.buf;\n \t}\n \n+\tif (d->only_names) {\n+\t\tprintf(\"%c%2d: %s\", selected ? '*' : ' ', i + 1,\n+\t\t highlighted ? highlighted : item->string);\n+\t\treturn;\n+\t}\n+\n \trender_adddel(&d->worktree, &c->worktree, _(\"nothing\"));\n \trender_adddel(&d->index, &c->index, _(\"unchanged\"));\n \n@@ -765,6 +773,88 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps,\n \treturn res;\n }\n \n+static int get_untracked_files(struct repository *r,\n+\t\t\t struct prefix_item_list *files,\n+\t\t\t const struct pathspec *ps)\n+{\n+\tstruct dir_struct dir = { 0 };\n+\tsize_t i;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tif (repo_read_index(r) < 0)\n+\t\treturn error(_(\"could not read index\"));\n+\n+\tprefix_item_list_clear(files);\n+\tsetup_standard_excludes(&dir);\n+\tadd_pattern_list(&dir, EXC_CMDL, \"--exclude option\");\n+\tfill_directory(&dir, r->index, ps);\n+\n+\tfor (i = 0; i < dir.nr; i++) {\n+\t\tstruct dir_entry *ent = dir.entries[i];\n+\n+\t\tif (index_name_is_other(r->index, ent->name, ent->len)) {\n+\t\t\tstrbuf_reset(&buf);\n+\t\t\tstrbuf_add(&buf, ent->name, ent->len);\n+\t\t\tadd_file_item(&files->items, buf.buf);\n+\t\t}\n+\t}\n+\n+\tstrbuf_release(&buf);\n+\treturn 0;\n+}\n+\n+static int run_add_untracked(struct add_i_state *s, const struct pathspec *ps,\n+\t\t struct prefix_item_list *files,\n+\t\t struct list_and_choose_options *opts)\n+{\n+\tstruct print_file_item_data *d = opts->list_opts.print_item_data;\n+\tint res = 0, fd;\n+\tsize_t count, i;\n+\tstruct lock_file index_lock;\n+\n+\tif (get_untracked_files(s->r, files, ps) < 0)\n+\t\treturn -1;\n+\n+\tif (!files->items.nr) {\n+\t\tprintf(_(\"No untracked files.\\n\"));\n+\t\tgoto finish_add_untracked;\n+\t}\n+\n+\topts->prompt = N_(\"Add untracked\");\n+\td->only_names = 1;\n+\tcount = list_and_choose(s, files, opts);\n+\td->only_names = 0;\n+\tif (count <= 0)\n+\t\tgoto finish_add_untracked;\n+\n+\tfd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR);\n+\tif (fd < 0) {\n+\t\tres = -1;\n+\t\tgoto finish_add_untracked;\n+\t}\n+\n+\tfor (i = 0; i < files->items.nr; i++) {\n+\t\tconst char *name = files->items.items[i].string;\n+\t\tif (files->selected[i] &&\n+\t\t add_file_to_index(s->r->index, name, 0) < 0) {\n+\t\t\tres = error(_(\"could not stage '%s'\"), name);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\tif (!res &&\n+\t write_locked_index(s->r->index, &index_lock, COMMIT_LOCK) < 0)\n+\t\tres = error(_(\"could not write index\"));\n+\n+\tif (!res)\n+\t\tprintf(Q_(\"added %d path\\n\",\n+\t\t\t \"added %d paths\\n\", count), (int)count);\n+\n+finish_add_untracked:\n+\tputchar('\\n');\n+\treturn res;\n+}\n+\n static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,\n \t\t struct prefix_item_list *unused_files,\n \t\t struct list_and_choose_options *unused_opts)\n@@ -861,6 +951,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\t{ \"status\", run_status },\n \t\t{ \"update\", run_update },\n \t\t{ \"revert\", run_revert },\n+\t\t{ \"add untracked\", run_add_untracked },\n \t\t{ \"help\", run_help },\n \t};\n \tstruct prefix_item_list commands = PREFIX_ITEM_LIST_INIT;\n-- \ngitgitgadget\n\n"}]}