From ed53346b92ea9573e07d588243da303a8b63c610 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 7 Mar 2019 13:05:42 +0100 Subject: [PATCH 01/69] Start to implement a built-in version of `git add --interactive` This is hardly the first conversion of a Git command that is implemented as a script to a built-in. So far, the most successful strategy for such conversions has been to add a built-in helper and call that for more and more functionality from the script, as more and more parts are converted. With the interactive add, we choose a different strategy. The sole reason for this is that on Windows (where such a conversion has the most benefits in terms of speed and robustness) we face the very specific problem that a `system()` call in Perl seems to close `stdin` in the parent process when the spawned process consumes even one character from `stdin`. And that just does not work for us here, as it would stop the main loop as soon as any interactive command was performed by the helper. Which is almost all of the commands in `git add -i`. It is almost as if Perl told us once again that it does not want us to use it on Windows. Instead, we follow the opposite route where we start with a bare-bones version of the built-in interactive add, guarded by the new `add.interactive.useBuiltin` config variable, and then add more and more functionality to it, until it is feature complete. At this point, the built-in version of `git add -i` only states that it cannot do anything yet ;-) Signed-off-by: Johannes Schindelin --- Documentation/config/add.txt | 5 +++++ Makefile | 1 + add-interactive.c | 7 +++++++ add-interactive.h | 8 ++++++++ builtin/add.c | 10 ++++++++++ t/README | 4 ++++ 6 files changed, 35 insertions(+) create mode 100644 add-interactive.c create mode 100644 add-interactive.h diff --git a/Documentation/config/add.txt b/Documentation/config/add.txt index 4d753f006ec1ef..c9f748f81cb1c7 100644 --- a/Documentation/config/add.txt +++ b/Documentation/config/add.txt @@ -5,3 +5,8 @@ add.ignore-errors (deprecated):: option of linkgit:git-add[1]. `add.ignore-errors` is deprecated, as it does not follow the usual naming convention for configuration variables. + +add.interactive.useBuiltin:: + [EXPERIMENTAL] Set to `true` to use the experimental built-in + implementation of the interactive version of linkgit:git-add[1] + instead of the Perl script version. Is `false` by default. diff --git a/Makefile b/Makefile index c5240942f29e78..18e656a32f9919 100644 --- a/Makefile +++ b/Makefile @@ -848,6 +848,7 @@ LIB_H = $(shell $(FIND) . \ -name '*.h' -print) LIB_OBJS += abspath.o +LIB_OBJS += add-interactive.o LIB_OBJS += advice.o LIB_OBJS += alias.o LIB_OBJS += alloc.o diff --git a/add-interactive.c b/add-interactive.c new file mode 100644 index 00000000000000..482e458dc60f5c --- /dev/null +++ b/add-interactive.c @@ -0,0 +1,7 @@ +#include "cache.h" +#include "add-interactive.h" + +int run_add_i(struct repository *r, const struct pathspec *ps) +{ + die(_("No commands are available in the built-in `git add -i` yet!")); +} diff --git a/add-interactive.h b/add-interactive.h new file mode 100644 index 00000000000000..7043b8741d7bd3 --- /dev/null +++ b/add-interactive.h @@ -0,0 +1,8 @@ +#ifndef ADD_INTERACTIVE_H +#define ADD_INTERACTIVE_H + +struct repository; +struct pathspec; +int run_add_i(struct repository *r, const struct pathspec *ps); + +#endif diff --git a/builtin/add.c b/builtin/add.c index db2dfa43502d00..e19d66e8942440 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -20,6 +20,7 @@ #include "bulk-checkin.h" #include "argv-array.h" #include "submodule.h" +#include "add-interactive.h" static const char * const builtin_add_usage[] = { N_("git add [] [--] ..."), @@ -185,6 +186,14 @@ int run_add_interactive(const char *revision, const char *patch_mode, { int status, i; struct argv_array argv = ARGV_ARRAY_INIT; + int use_builtin_add_i = + git_env_bool("GIT_TEST_ADD_I_USE_BUILTIN", -1); + if (use_builtin_add_i < 0) + git_config_get_bool("add.interactive.usebuiltin", + &use_builtin_add_i); + + if (use_builtin_add_i == 1 && !patch_mode) + return !!run_add_i(the_repository, pathspec); argv_array_push(&argv, "add--interactive"); if (patch_mode) @@ -319,6 +328,7 @@ static int add_config(const char *var, const char *value, void *cb) ignore_add_errors = git_config_bool(var, value); return 0; } + return git_default_config(var, value, cb); } diff --git a/t/README b/t/README index 886bbec5bc8e40..6408a1847e28d9 100644 --- a/t/README +++ b/t/README @@ -383,6 +383,10 @@ GIT_TEST_REBASE_USE_BUILTIN=, when false, disables the builtin version of git-rebase. See 'rebase.useBuiltin' in git-config(1). +GIT_TEST_ADD_I_USE_BUILTIN=, when true, enables the +builtin version of git add -i. See 'add.interactive.useBuiltin' in +git-config(1). + GIT_TEST_INDEX_THREADS= enables exercising the multi-threaded loading of the index for the whole test suite by bypassing the default number of cache entries and thread minimums. Setting this to 1 will make the From bc99009fbf0e01e9bbe77aa6410489beadf454dc Mon Sep 17 00:00:00 2001 From: Daniel Ferreira Date: Tue, 16 May 2017 01:00:31 -0300 Subject: [PATCH 02/69] diff: export diffstat interface Make the diffstat interface (namely, the diffstat_t struct and compute_diffstat) no longer be internal to diff.c and allow it to be used by other parts of git. This is helpful for code that may want to easily extract information from files using the diff machinery, while flushing it differently from how the show_* functions used by diff_flush() do it. One example is the builtin implementation of git-add--interactive's status. Signed-off-by: Daniel Ferreira Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- diff.c | 37 +++++++++++++++---------------------- diff.h | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/diff.c b/diff.c index 5306c48652db59..daa5f3a736fee8 100644 --- a/diff.c +++ b/diff.c @@ -2489,22 +2489,6 @@ static void pprint_rename(struct strbuf *name, const char *a, const char *b) } } -struct diffstat_t { - int nr; - int alloc; - struct diffstat_file { - char *from_name; - char *name; - char *print_name; - const char *comments; - unsigned is_unmerged:1; - unsigned is_binary:1; - unsigned is_renamed:1; - unsigned is_interesting:1; - uintmax_t added, deleted; - } **files; -}; - static struct diffstat_file *diffstat_add(struct diffstat_t *diffstat, const char *name_a, const char *name_b) @@ -6001,12 +5985,7 @@ void diff_flush(struct diff_options *options) dirstat_by_line) { struct diffstat_t diffstat; - memset(&diffstat, 0, sizeof(struct diffstat_t)); - for (i = 0; i < q->nr; i++) { - struct diff_filepair *p = q->queue[i]; - if (check_pair_status(p)) - diff_flush_stat(p, options, &diffstat); - } + compute_diffstat(options, &diffstat, q); if (output_format & DIFF_FORMAT_NUMSTAT) show_numstat(&diffstat, options); if (output_format & DIFF_FORMAT_DIFFSTAT) @@ -6306,6 +6285,20 @@ static int is_submodule_ignored(const char *path, struct diff_options *options) return ignored; } +void compute_diffstat(struct diff_options *options, + struct diffstat_t *diffstat, + struct diff_queue_struct *q) +{ + int i; + + memset(diffstat, 0, sizeof(struct diffstat_t)); + for (i = 0; i < q->nr; i++) { + struct diff_filepair *p = q->queue[i]; + if (check_pair_status(p)) + diff_flush_stat(p, options, diffstat); + } +} + void diff_addremove(struct diff_options *options, int addremove, unsigned mode, const struct object_id *oid, diff --git a/diff.h b/diff.h index b512d0477ac3a4..ae9bedfab8c9bd 100644 --- a/diff.h +++ b/diff.h @@ -240,6 +240,22 @@ void diff_emit_submodule_error(struct diff_options *o, const char *err); void diff_emit_submodule_pipethrough(struct diff_options *o, const char *line, int len); +struct diffstat_t { + int nr; + int alloc; + struct diffstat_file { + char *from_name; + char *name; + char *print_name; + const char *comments; + unsigned is_unmerged:1; + unsigned is_binary:1; + unsigned is_renamed:1; + unsigned is_interesting:1; + uintmax_t added, deleted; + } **files; +}; + enum color_diff { DIFF_RESET = 0, DIFF_CONTEXT = 1, @@ -328,6 +344,9 @@ void diff_change(struct diff_options *, struct diff_filepair *diff_unmerge(struct diff_options *, const char *path); +void compute_diffstat(struct diff_options *options, struct diffstat_t *diffstat, + struct diff_queue_struct *q); + #define DIFF_SETUP_REVERSE 1 #define DIFF_SETUP_USE_SIZE_CACHE 4 From 5e23c0756b5ee543437d0dbf4e9f685df6341bdf Mon Sep 17 00:00:00 2001 From: Daniel Ferreira Date: Tue, 16 May 2017 01:00:32 -0300 Subject: [PATCH 03/69] built-in add -i: implement the `status` command This implements the `status` command of `git add -i`. The data structures introduced in this commit will be extended as needed later. At this point, we re-implement only part of the `list_and_choose()` function of the Perl script `git-add--interactive.perl` and call it `list()`. It does not yet color anything, or do columns, or allow user input. Over the course of the next commits, we will introduce a `list_and_choose()` function that uses `list()` to display the list of options and let the user choose one or more of the displayed items. This will be used to implement the main loop of the built-in `git add -i`, at which point the new `status` command can actually be used. Note that we pass the list of items as a `struct item **` as opposed to a `struct item *`, to allow for the actual items to contain much more information than merely the name. Signed-off-by: Daniel Ferreira Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- add-interactive.c | 265 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 264 insertions(+), 1 deletion(-) diff --git a/add-interactive.c b/add-interactive.c index 482e458dc60f5c..59b28011f77432 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -1,7 +1,270 @@ #include "cache.h" #include "add-interactive.h" +#include "diffcore.h" +#include "revision.h" +#include "refs.h" + +struct item { + const char *name; +}; + +struct list_options { + const char *header; + void (*print_item)(int i, struct item *item, void *print_item_data); + void *print_item_data; +}; + +static void list(struct item **list, size_t nr, struct list_options *opts) +{ + int i; + + if (!nr) + return; + + if (opts->header) + printf("%s\n", opts->header); + + for (i = 0; i < nr; i++) { + opts->print_item(i, list[i], opts->print_item_data); + putchar('\n'); + } +} + +struct adddel { + uintmax_t add, del; + unsigned seen:1, binary:1; +}; + +struct file_list { + struct file_item { + struct item item; + struct adddel index, worktree; + } **file; + size_t nr, alloc; +}; + +static void add_file_item(struct file_list *list, const char *name) +{ + struct file_item *item; + + FLEXPTR_ALLOC_STR(item, item.name, name); + + ALLOC_GROW(list->file, list->nr + 1, list->alloc); + list->file[list->nr++] = item; +} + +static void reset_file_list(struct file_list *list) +{ + size_t i; + + for (i = 0; i < list->nr; i++) + free(list->file[i]); + list->nr = 0; +} + +static void release_file_list(struct file_list *list) +{ + reset_file_list(list); + FREE_AND_NULL(list->file); + list->alloc = 0; +} + +static int file_item_cmp(const void *a, const void *b) +{ + const struct file_item * const *f1 = a; + const struct file_item * const *f2 = b; + + return strcmp((*f1)->item.name, (*f2)->item.name); +} + +struct pathname_entry { + struct hashmap_entry ent; + size_t index; + char pathname[FLEX_ARRAY]; +}; + +static int pathname_entry_cmp(const void *unused_cmp_data, + const void *entry, const void *entry_or_key, + const void *pathname) +{ + const struct pathname_entry *e1 = entry, *e2 = entry_or_key; + + return strcmp(e1->pathname, + pathname ? (const char *)pathname : e2->pathname); +} + +struct collection_status { + enum { FROM_WORKTREE = 0, FROM_INDEX = 1 } phase; + + const char *reference; + + struct file_list *list; + struct hashmap file_map; +}; + +static void collect_changes_cb(struct diff_queue_struct *q, + struct diff_options *options, + void *data) +{ + struct collection_status *s = data; + struct diffstat_t stat = { 0 }; + int i; + + if (!q->nr) + return; + + compute_diffstat(options, &stat, q); + + for (i = 0; i < stat.nr; i++) { + const char *name = stat.files[i]->name; + int hash = strhash(name); + struct pathname_entry *entry; + size_t file_index; + struct file_item *file; + struct adddel *adddel; + + entry = hashmap_get_from_hash(&s->file_map, hash, name); + if (entry) + file_index = entry->index; + else { + FLEX_ALLOC_STR(entry, pathname, name); + hashmap_entry_init(entry, hash); + entry->index = file_index = s->list->nr; + hashmap_add(&s->file_map, entry); + + add_file_item(s->list, name); + } + file = s->list->file[file_index]; + + adddel = s->phase == FROM_INDEX ? &file->index : &file->worktree; + adddel->seen = 1; + adddel->add = stat.files[i]->added; + adddel->del = stat.files[i]->deleted; + if (stat.files[i]->is_binary) + adddel->binary = 1; + } +} + +static int get_modified_files(struct repository *r, struct file_list *list, + const struct pathspec *ps) +{ + struct object_id head_oid; + int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, + &head_oid, NULL); + struct collection_status s = { FROM_WORKTREE }; + + if (repo_read_index_preload(r, ps, 0) < 0) + return error(_("could not read index")); + + s.list = list; + hashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0); + + for (s.phase = FROM_WORKTREE; s.phase <= FROM_INDEX; s.phase++) { + struct rev_info rev; + struct setup_revision_opt opt = { 0 }; + + opt.def = is_initial ? + empty_tree_oid_hex() : oid_to_hex(&head_oid); + + init_revisions(&rev, NULL); + setup_revisions(0, NULL, &rev, &opt); + + rev.diffopt.output_format = DIFF_FORMAT_CALLBACK; + rev.diffopt.format_callback = collect_changes_cb; + rev.diffopt.format_callback_data = &s; + + if (ps) + copy_pathspec(&rev.prune_data, ps); + + if (s.phase == FROM_INDEX) + run_diff_index(&rev, 1); + else { + rev.diffopt.flags.ignore_dirty_submodules = 1; + run_diff_files(&rev, 0); + } + } + hashmap_free(&s.file_map, 1); + + /* While the diffs are ordered already, we ran *two* diffs... */ + QSORT(list->file, list->nr, file_item_cmp); + + return 0; +} + +static void populate_wi_changes(struct strbuf *buf, + struct adddel *ad, const char *no_changes) +{ + if (ad->binary) + strbuf_addstr(buf, _("binary")); + else if (ad->seen) + strbuf_addf(buf, "+%"PRIuMAX"/-%"PRIuMAX, + (uintmax_t)ad->add, (uintmax_t)ad->del); + else + strbuf_addstr(buf, no_changes); +} + +struct print_file_item_data { + const char *modified_fmt; + struct strbuf buf, index, worktree; +}; + +static void print_file_item(int i, struct item *item, + void *print_file_item_data) +{ + struct file_item *c = (struct file_item *)item; + struct print_file_item_data *d = print_file_item_data; + + strbuf_reset(&d->index); + strbuf_reset(&d->worktree); + strbuf_reset(&d->buf); + + populate_wi_changes(&d->worktree, &c->worktree, _("nothing")); + populate_wi_changes(&d->index, &c->index, _("unchanged")); + strbuf_addf(&d->buf, d->modified_fmt, + d->index.buf, d->worktree.buf, item->name); + + printf(" %2d: %s", i + 1, d->buf.buf); +} + +static int run_status(struct repository *r, const struct pathspec *ps, + struct file_list *files, struct list_options *opts) +{ + reset_file_list(files); + + if (get_modified_files(r, files, ps) < 0) + return -1; + + if (files->nr) + list((struct item **)files->file, files->nr, opts); + putchar('\n'); + + return 0; +} int run_add_i(struct repository *r, const struct pathspec *ps) { - die(_("No commands are available in the built-in `git add -i` yet!")); + struct print_file_item_data print_file_item_data = { + "%12s %12s %s", STRBUF_INIT, STRBUF_INIT, STRBUF_INIT + }; + struct list_options opts = { + NULL, print_file_item, &print_file_item_data + }; + struct strbuf header = STRBUF_INIT; + struct file_list files = { NULL }; + int res = 0; + + strbuf_addstr(&header, " "); + strbuf_addf(&header, print_file_item_data.modified_fmt, + _("staged"), _("unstaged"), _("path")); + opts.header = header.buf; + + res = run_status(r, ps, &files, &opts); + + release_file_list(&files); + strbuf_release(&print_file_item_data.buf); + strbuf_release(&print_file_item_data.index); + strbuf_release(&print_file_item_data.worktree); + strbuf_release(&header); + + return res; } From 8cafc6ae8d2dd434f46751874074940f13412b9f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 27 Mar 2019 23:46:46 +0100 Subject: [PATCH 04/69] built-in add -i: refresh the index before running `status` This is what the Perl version does, and therefore it is what the built-in version should do, too. Signed-off-by: Johannes Schindelin --- add-interactive.c | 4 +++- repository.c | 19 +++++++++++++++++++ repository.h | 7 +++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/add-interactive.c b/add-interactive.c index 59b28011f77432..2dbf29dee235bb 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -258,7 +258,9 @@ int run_add_i(struct repository *r, const struct pathspec *ps) _("staged"), _("unstaged"), _("path")); opts.header = header.buf; - res = run_status(r, ps, &files, &opts); + repo_refresh_and_write_index(r, REFRESH_QUIET, 1); + if (run_status(r, ps, &files, &opts) < 0) + res = -1; release_file_list(&files); strbuf_release(&print_file_item_data.buf); diff --git a/repository.c b/repository.c index 65e6f8b8fdfcf8..c90f310093737d 100644 --- a/repository.c +++ b/repository.c @@ -272,3 +272,22 @@ int repo_hold_locked_index(struct repository *repo, BUG("the repo hasn't been setup"); return hold_lock_file_for_update(lf, repo->index_file, flags); } + +int repo_refresh_and_write_index(struct repository *r, + unsigned int flags, int gentle) +{ + struct lock_file lock_file = LOCK_INIT; + int fd; + + if (repo_read_index_preload(r, NULL, 0) < 0) + return error(_("could not read index")); + fd = repo_hold_locked_index(r, &lock_file, 0); + if (!gentle && fd < 0) + return error(_("could not lock index for writing")); + refresh_index(r->index, flags, NULL, NULL, NULL); + if (0 <= fd) + repo_update_index_if_able(r, &lock_file); + rollback_lock_file(&lock_file); + + return 0; +} diff --git a/repository.h b/repository.h index 8981649d43736e..fb49e0e3289013 100644 --- a/repository.h +++ b/repository.h @@ -154,5 +154,12 @@ int repo_read_index_unmerged(struct repository *); */ void repo_update_index_if_able(struct repository *, struct lock_file *); +/* + * Refresh the index and write it out. If the index file could not be + * locked, error out, except in gentle mode. The flags will be passed + * through to refresh_index(). + */ +int repo_refresh_and_write_index(struct repository *r, + unsigned int flags, int gentle); #endif /* REPOSITORY_H */ From cad52c8f876a0154ac98419b393922962f150956 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 7 May 2019 10:49:00 +0200 Subject: [PATCH 05/69] fixup! mingw (git_terminal_prompt): turn on echo explictly The `add -p` changes conflict with this commit, so let's revert it for now. Besides, it would appear that the rationale of the change is not sound: in a quick test in a PowerShell, it seems that echo *is* the default. To verify, test, run this in a PowerShell session: git -c alias.s="! unset GIT_CONFIG_PARAMETERS; printf `"hostname=123\\n\\n`" | GIT_ASKPASS= ./git \ -c credential.helper= --exec-path=`$PWD credential fill " s This reverts commit b9990a392775401e0600971fefa1529098342dce. Signed-off-by: Johannes Schindelin --- compat/terminal.c | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/compat/terminal.c b/compat/terminal.c index 00eb4c51470831..561d339f44c0b8 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -77,26 +77,17 @@ static void restore_term(void) hconin = INVALID_HANDLE_VALUE; } -static int set_echo(int echo) +static int disable_echo(void) { - DWORD new_cmode; - - if (hconin == INVALID_HANDLE_VALUE) - hconin = CreateFile("CONIN$", GENERIC_READ | GENERIC_WRITE, - FILE_SHARE_READ, NULL, OPEN_EXISTING, - FILE_ATTRIBUTE_NORMAL, NULL); + hconin = CreateFile("CONIN$", GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); if (hconin == INVALID_HANDLE_VALUE) return -1; GetConsoleMode(hconin, &cmode); - new_cmode = cmode | ENABLE_LINE_INPUT; - if (echo) - new_cmode |= ENABLE_ECHO_INPUT; - else - new_cmode &= ~ENABLE_ECHO_INPUT; - sigchain_push_common(restore_term_on_signal); - if (!SetConsoleMode(hconin, new_cmode)) { + if (!SetConsoleMode(hconin, cmode & (~ENABLE_ECHO_INPUT))) { CloseHandle(hconin); hconin = INVALID_HANDLE_VALUE; return -1; @@ -105,11 +96,6 @@ static int set_echo(int echo) return 0; } -static int disable_echo(void) -{ - return set_echo(0); -} - static char *shell_prompt(const char *prompt, int echo) { const char *read_input[] = { @@ -183,8 +169,6 @@ char *git_terminal_prompt(const char *prompt, int echo) if (result) return result; - if (echo && set_echo(1)) - return NULL; #endif input_fh = fopen(INPUT_PATH, "r" FORCE_TEXT); From fa27842fbc7f0913572c56879754f0d5a80d812d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 7 May 2019 14:54:34 +0200 Subject: [PATCH 06/69] fixup! mingw (git_terminal_prompt): work around BusyBox & WSL issues We do not actually need this, as `bash` is not a BusyBox applet, and in the unlikely event anybody would call `git.exe` from within WSL, they would use `/cmd/git.exe` which puts the MSYS2 `bash.exe` into the `PATH` before the WSL `bash`. This reverts commit 040d9dbeb7e10ee413cbbe16ea621c43056b1d11. Signed-off-by: Johannes Schindelin --- compat/terminal.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/compat/terminal.c b/compat/terminal.c index 561d339f44c0b8..f124263818470c 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -101,10 +101,8 @@ static char *shell_prompt(const char *prompt, int echo) const char *read_input[] = { /* Note: call 'bash' explicitly, as 'read -s' is bash-specific */ "bash", "-c", echo ? - "test \"a$SHELL\" != \"a${SHELL%.exe}\" || exit 127; cat >/dev/tty &&" - " read -r line /dev/tty &&" - " read -r -s line /dev/tty", + "cat >/dev/tty && read -r line /dev/tty && read -r -s line /dev/tty", NULL }; struct child_process child = CHILD_PROCESS_INIT; @@ -140,10 +138,7 @@ static char *shell_prompt(const char *prompt, int echo) close(child.out); code = finish_command(&child); if (code) { - if (code != 127) - error("failed to execute prompt script (exit code %d)", - code); - + error("failed to execute prompt script (exit code %d)", code); return NULL; } From ddd2fdba8e38c56117e3719fe61750f4e9c224f3 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 7 May 2019 15:01:17 +0200 Subject: [PATCH 07/69] Temporarily back out compat/terminal.c changes The "hack" to use `bash`'s `read -r` function to read reliably from `/dev/tty` conflicts with the upcoming built-in `add -p`, to support single key stroke commands. Let's back out the changes, and then re-apply them on top of `add -p`, consolidated. Signed-off-by: Johannes Schindelin --- compat/terminal.c | 63 ----------------------------------------------- 1 file changed, 63 deletions(-) diff --git a/compat/terminal.c b/compat/terminal.c index f124263818470c..fa13ee672db33e 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -1,12 +1,7 @@ -#ifndef NO_INTTYPES_H -#include -#endif #include "git-compat-util.h" -#include "run-command.h" #include "compat/terminal.h" #include "sigchain.h" #include "strbuf.h" -#include "cache.h" #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE) @@ -96,55 +91,6 @@ static int disable_echo(void) return 0; } -static char *shell_prompt(const char *prompt, int echo) -{ - const char *read_input[] = { - /* Note: call 'bash' explicitly, as 'read -s' is bash-specific */ - "bash", "-c", echo ? - "cat >/dev/tty && read -r line /dev/tty && read -r -s line /dev/tty", - NULL - }; - struct child_process child = CHILD_PROCESS_INIT; - static struct strbuf buffer = STRBUF_INIT; - int prompt_len = strlen(prompt), len = -1, code; - - child.argv = read_input; - child.in = -1; - child.out = -1; - child.silent_exec_failure = 1; - - if (start_command(&child)) - return NULL; - - if (write_in_full(child.in, prompt, prompt_len) != prompt_len) { - error("could not write to prompt script"); - close(child.in); - goto ret; - } - close(child.in); - - strbuf_reset(&buffer); - len = strbuf_read(&buffer, child.out, 1024); - if (len < 0) { - error("could not read from prompt script"); - goto ret; - } - - strbuf_strip_suffix(&buffer, "\n"); - strbuf_strip_suffix(&buffer, "\r"); - -ret: - close(child.out); - code = finish_command(&child); - if (code) { - error("failed to execute prompt script (exit code %d)", code); - return NULL; - } - - return len < 0 ? NULL : buffer.buf; -} - #endif #ifndef FORCE_TEXT @@ -157,15 +103,6 @@ char *git_terminal_prompt(const char *prompt, int echo) int r; FILE *input_fh, *output_fh; -#ifdef GIT_WINDOWS_NATIVE - - /* try shell_prompt first, fall back to CONIN/OUT if bash is missing */ - char *result = shell_prompt(prompt, echo); - if (result) - return result; - -#endif - input_fh = fopen(INPUT_PATH, "r" FORCE_TEXT); if (!input_fh) return NULL; From 46f5bade0f3c107f5d8fd07deae3cd3bbe688c1d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 7 May 2019 15:40:45 +0200 Subject: [PATCH 08/69] Temporarily revert part of "tests: add a special setup [...]" This conflicts with the `add-p` series and will be reapplied on top of the merge later. This reverts commit dc72b086531aeddc6a8b0bfab9d89d9a16b90b5f. Signed-off-by: Johannes Schindelin --- t/README | 4 ---- 1 file changed, 4 deletions(-) diff --git a/t/README b/t/README index cef23b9c18c362..8ea6f69b1f63f3 100644 --- a/t/README +++ b/t/README @@ -383,10 +383,6 @@ GIT_TEST_REBASE_USE_BUILTIN=, when false, disables the builtin version of git-rebase. See 'rebase.useBuiltin' in git-config(1). -GIT_TEST_STASH_USE_BUILTIN=, when false, disables the -built-in version of git-stash. See 'stash.useBuiltin' in -git-config(1). - GIT_TEST_INDEX_THREADS= enables exercising the multi-threaded loading of the index for the whole test suite by bypassing the default number of cache entries and thread minimums. Setting this to 1 will make the From a5ae46fd778e693a9421113fbc4fe8ac0bd9a652 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 7 May 2019 15:42:17 +0200 Subject: [PATCH 09/69] Temporarily revert "t3701: verify that we can add *lots* of files interactively" This conflicts with the `add-p` series and will be reapplied on top of the merge later. This reverts commit c8c1c27aed4431ae44a758f8436a1a916ad6b477. Signed-off-by: Johannes Schindelin --- t/t3701-add-interactive.sh | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index f9789ac02b08a7..65dfbc033a027d 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -639,25 +639,4 @@ test_expect_success 'add -p patch editing works with pathological context lines' test_cmp expected-2 actual ' -test_expect_success EXPENSIVE 'add -i with a lot of files' ' - git reset --hard && - x160=0123456789012345678901234567890123456789 && - x160=$x160$x160$x160$x160 && - y= && - i=0 && - while test $i -le 200 - do - name=$(printf "%s%03d" $x160 $i) && - echo $name >$name && - git add -N $name && - y="${y}y$LF" && - i=$(($i+1)) || - break - done && - echo "$y" | git add -p -- . && - git diff --cached >staged && - test_line_count = 1407 staged && - git reset --hard -' - test_done From 83d92a9762b4c10d74d804a6104b5ae76330ca17 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 May 2019 23:10:52 +0200 Subject: [PATCH 10/69] built-in add -i: color the header in the `status` command For simplicity, we only implemented the `status` command without colors. This patch starts adding color, matching what the Perl script `git-add--interactive.perl` does. Original-Patch-By: Daniel Ferreira Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- add-interactive.c | 60 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 6 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 2dbf29dee235bb..6c2fca12c14e32 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -1,9 +1,51 @@ #include "cache.h" #include "add-interactive.h" +#include "color.h" +#include "config.h" #include "diffcore.h" #include "revision.h" #include "refs.h" +struct add_i_state { + struct repository *r; + int use_color; + char header_color[COLOR_MAXLEN]; +}; + +static void init_color(struct repository *r, struct add_i_state *s, + const char *slot_name, char *dst, + const char *default_color) +{ + char *key = xstrfmt("color.interactive.%s", slot_name); + const char *value; + + if (!s->use_color) + dst[0] = '\0'; + else if (repo_config_get_value(r, key, &value) || + color_parse(value, dst)) + strlcpy(dst, default_color, COLOR_MAXLEN); + + free(key); +} + +static int init_add_i_state(struct repository *r, struct add_i_state *s) +{ + const char *value; + + s->r = r; + + if (repo_config_get_value(r, "color.interactive", &value)) + s->use_color = -1; + else + s->use_color = + git_config_colorbool("color.interactive", value); + s->use_color = want_color(s->use_color); + + init_color(r, s, "header", s->header_color, GIT_COLOR_BOLD); + + return 0; +} + struct item { const char *name; }; @@ -14,7 +56,8 @@ struct list_options { void *print_item_data; }; -static void list(struct item **list, size_t nr, struct list_options *opts) +static void list(struct item **list, size_t nr, + struct add_i_state *s, struct list_options *opts) { int i; @@ -22,7 +65,8 @@ static void list(struct item **list, size_t nr, struct list_options *opts) return; if (opts->header) - printf("%s\n", opts->header); + color_fprintf_ln(stdout, s->header_color, + "%s", opts->header); for (i = 0; i < nr; i++) { opts->print_item(i, list[i], opts->print_item_data); @@ -226,16 +270,16 @@ static void print_file_item(int i, struct item *item, printf(" %2d: %s", i + 1, d->buf.buf); } -static int run_status(struct repository *r, const struct pathspec *ps, +static int run_status(struct add_i_state *s, const struct pathspec *ps, struct file_list *files, struct list_options *opts) { reset_file_list(files); - if (get_modified_files(r, files, ps) < 0) + if (get_modified_files(s->r, files, ps) < 0) return -1; if (files->nr) - list((struct item **)files->file, files->nr, opts); + list((struct item **)files->file, files->nr, s, opts); putchar('\n'); return 0; @@ -243,6 +287,7 @@ static int run_status(struct repository *r, const struct pathspec *ps, int run_add_i(struct repository *r, const struct pathspec *ps) { + struct add_i_state s = { NULL }; struct print_file_item_data print_file_item_data = { "%12s %12s %s", STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; @@ -253,13 +298,16 @@ int run_add_i(struct repository *r, const struct pathspec *ps) struct file_list files = { NULL }; int res = 0; + if (init_add_i_state(r, &s)) + return error("could not parse `add -i` config"); + strbuf_addstr(&header, " "); strbuf_addf(&header, print_file_item_data.modified_fmt, _("staged"), _("unstaged"), _("path")); opts.header = header.buf; repo_refresh_and_write_index(r, REFRESH_QUIET, 1); - if (run_status(r, ps, &files, &opts) < 0) + if (run_status(&s, ps, &files, &opts) < 0) res = -1; release_file_list(&files); From db978391e8acab8cb3dccb52b25981f86220a4f8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 7 Mar 2019 13:05:42 +0100 Subject: [PATCH 11/69] built-in add -i: implement the main loop The reason why we did not start with the main loop to begin with is that it is the first user of `list_and_choose()`, which uses the `list()` function that we conveniently introduced for use by the `status` command. Apart from the "and choose" part, there are more differences between the way the `status` command calls the `list_and_choose()` function in the Perl version of `git add -i` compared to the other callers of said function. The most important ones: - The list is not only shown, but the user is also asked to make a choice, possibly selecting multiple entries. - The list of items is prefixed with a marker indicating what items have been selected, if multi-selection is allowed. - Initially, for each item a unique prefix (if there exists any within the given parameters) is determined, and shown in the list, and accepted as a shortcut for the selection. These features will be implemented later, except the part where the user can choose a command. At this stage, though, the built-in `git add -i` still only supports the `status` command, with the remaining commands to follow over the course of the next commits. In addition, we also modify `list()` to support displaying the commands in columns, even if there is currently only one. The Perl script `git-add--interactive.perl` mixed the purposes of the "list" and the "and choose" part into the same function. In the C version, we will keep them separate instead, calling the `list()` function from the `list_and_choose()` function. Note that we only have a prompt ending in a single ">" at this stage; later commits will add commands that display a double ">>" to indicate that the user is in a different loop than the main one. Signed-off-by: Johannes Schindelin --- add-interactive.c | 123 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 121 insertions(+), 2 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 6c2fca12c14e32..33d10d25efa751 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -51,6 +51,7 @@ struct item { }; struct list_options { + int columns; const char *header; void (*print_item)(int i, struct item *item, void *print_item_data); void *print_item_data; @@ -59,7 +60,7 @@ struct list_options { static void list(struct item **list, size_t nr, struct add_i_state *s, struct list_options *opts) { - int i; + int i, last_lf = 0; if (!nr) return; @@ -70,8 +71,91 @@ static void list(struct item **list, size_t nr, for (i = 0; i < nr; i++) { opts->print_item(i, list[i], opts->print_item_data); + + if ((opts->columns) && ((i + 1) % (opts->columns))) { + putchar('\t'); + last_lf = 0; + } + else { + putchar('\n'); + last_lf = 1; + } + } + + if (!last_lf) putchar('\n'); +} +struct list_and_choose_options { + struct list_options list_opts; + + const char *prompt; +}; + +/* + * Returns the selected index. + */ +static ssize_t list_and_choose(struct item **items, size_t nr, + struct add_i_state *s, + struct list_and_choose_options *opts) +{ + struct strbuf input = STRBUF_INIT; + ssize_t res = -1; + + for (;;) { + char *p, *endp; + + strbuf_reset(&input); + + list(items, nr, s, &opts->list_opts); + + printf("%s%s", opts->prompt, "> "); + fflush(stdout); + + if (strbuf_getline(&input, stdin) == EOF) { + putchar('\n'); + res = -2; + break; + } + strbuf_trim(&input); + + if (!input.len) + break; + + p = input.buf; + for (;;) { + size_t sep = strcspn(p, " \t\r\n,"); + ssize_t index = -1; + + if (!sep) { + if (!*p) + break; + p++; + continue; + } + + if (isdigit(*p)) { + index = strtoul(p, &endp, 10) - 1; + if (endp != p + sep) + index = -1; + } + + p[sep] = '\0'; + if (index < 0 || index >= nr) + printf(_("Huh (%s)?\n"), p); + else { + res = index; + break; + } + + p += sep + 1; + } + + if (res >= 0) + break; } + + strbuf_release(&input); + return res; } struct adddel { @@ -285,17 +369,40 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps, return 0; } +static void print_command_item(int i, struct item *item, + void *print_command_item_data) +{ + printf(" %2d: %s", i + 1, item->name); +} + +struct command_item { + struct item item; + int (*command)(struct add_i_state *s, const struct pathspec *ps, + struct file_list *files, struct list_options *opts); +}; + int run_add_i(struct repository *r, const struct pathspec *ps) { struct add_i_state s = { NULL }; + struct list_and_choose_options main_loop_opts = { + { 4, N_("*** Commands ***"), print_command_item, NULL }, + N_("What now") + }; + struct command_item + status = { { "status" }, run_status }; + struct command_item *commands[] = { + &status + }; + struct print_file_item_data print_file_item_data = { "%12s %12s %s", STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; struct list_options opts = { - NULL, print_file_item, &print_file_item_data + 0, NULL, print_file_item, &print_file_item_data }; struct strbuf header = STRBUF_INIT; struct file_list files = { NULL }; + ssize_t i; int res = 0; if (init_add_i_state(r, &s)) @@ -310,6 +417,18 @@ int run_add_i(struct repository *r, const struct pathspec *ps) if (run_status(&s, ps, &files, &opts) < 0) res = -1; + for (;;) { + i = list_and_choose((struct item **)commands, + ARRAY_SIZE(commands), &s, &main_loop_opts); + if (i < -1) { + printf(_("Bye.\n")); + res = 0; + break; + } + if (i >= 0) + res = commands[i]->command(&s, ps, &files, &opts); + } + release_file_list(&files); strbuf_release(&print_file_item_data.buf); strbuf_release(&print_file_item_data.index); From d7245b92d85790db9b25b238e58ef1b12ce9a20b Mon Sep 17 00:00:00 2001 From: Slavica Djukic Date: Wed, 27 Mar 2019 01:08:50 +0100 Subject: [PATCH 12/69] Add a function to determine unique prefixes for a list of strings In the `git add -i` command, we show unique prefixes of the commands and files, to give an indication what prefix would select them. Naturally, the C implementation looks a lot different than the Perl implementation: in Perl, a trie is much easier implemented, while we already have a pretty neat hashmap implementation in C that we use for the purpose of storing (not necessarily unique) prefixes. The idea: for each item that we add, we generate prefixes starting with the first letter, then the first two letters, then three, etc, until we find a prefix that is unique (or until the prefix length would be longer than we want). If we encounter a previously-unique prefix on the way, we adjust that item's prefix to make it unique again (or we mark it as having no unique prefix if we failed to find one). These partial prefixes are stored in a hash map (for quick lookup times). To make sure that this function works as expected, we add a test using a special-purpose test helper that was added for that purpose. Note: We expect the list of prefix items to be passed in as a list of pointers rather than as regular list to avoid having to copy information (the actual items will most likely contain more information than just the name and the length of the unique prefix, but passing in `struct prefix_item *` would not allow for that). Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- Makefile | 2 + prefix-map.c | 111 +++++++++++++++++++++++++++++++++++++ prefix-map.h | 40 +++++++++++++ t/helper/test-prefix-map.c | 58 +++++++++++++++++++ t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + t/t0016-prefix-map.sh | 10 ++++ 7 files changed, 223 insertions(+) create mode 100644 prefix-map.c create mode 100644 prefix-map.h create mode 100644 t/helper/test-prefix-map.c create mode 100755 t/t0016-prefix-map.sh diff --git a/Makefile b/Makefile index 18e656a32f9919..8299b3f17da832 100644 --- a/Makefile +++ b/Makefile @@ -754,6 +754,7 @@ TEST_BUILTINS_OBJS += test-online-cpus.o TEST_BUILTINS_OBJS += test-parse-options.o TEST_BUILTINS_OBJS += test-path-utils.o TEST_BUILTINS_OBJS += test-pkt-line.o +TEST_BUILTINS_OBJS += test-prefix-map.o TEST_BUILTINS_OBJS += test-prio-queue.o TEST_BUILTINS_OBJS += test-reach.o TEST_BUILTINS_OBJS += test-read-cache.o @@ -967,6 +968,7 @@ LIB_OBJS += patch-ids.o LIB_OBJS += path.o LIB_OBJS += pathspec.o LIB_OBJS += pkt-line.o +LIB_OBJS += prefix-map.o LIB_OBJS += preload-index.o LIB_OBJS += pretty.o LIB_OBJS += prio-queue.o diff --git a/prefix-map.c b/prefix-map.c new file mode 100644 index 00000000000000..3c5ae4ae0aecb3 --- /dev/null +++ b/prefix-map.c @@ -0,0 +1,111 @@ +#include "cache.h" +#include "prefix-map.h" + +static int map_cmp(const void *unused_cmp_data, + const void *entry, + const void *entry_or_key, + const void *unused_keydata) +{ + const struct prefix_map_entry *a = entry; + const struct prefix_map_entry *b = entry_or_key; + + return a->prefix_length != b->prefix_length || + strncmp(a->name, b->name, a->prefix_length); +} + +static void add_prefix_entry(struct hashmap *map, const char *name, + size_t prefix_length, struct prefix_item *item) +{ + struct prefix_map_entry *result = xmalloc(sizeof(*result)); + result->name = name; + result->prefix_length = prefix_length; + result->item = item; + hashmap_entry_init(result, memhash(name, prefix_length)); + hashmap_add(map, result); +} + +static void init_prefix_map(struct prefix_map *prefix_map, + int min_prefix_length, int max_prefix_length) +{ + hashmap_init(&prefix_map->map, map_cmp, NULL, 0); + prefix_map->min_length = min_prefix_length; + prefix_map->max_length = max_prefix_length; +} + +static void add_prefix_item(struct prefix_map *prefix_map, + struct prefix_item *item) +{ + struct prefix_map_entry *e = xmalloc(sizeof(*e)), *e2; + int j; + + e->item = item; + e->name = e->item->name; + + for (j = prefix_map->min_length; j <= prefix_map->max_length; j++) { + if (!isascii(e->name[j])) { + free(e); + break; + } + + e->prefix_length = j; + hashmap_entry_init(e, memhash(e->name, j)); + e2 = hashmap_get(&prefix_map->map, e, NULL); + if (!e2) { + /* prefix is unique so far */ + e->item->prefix_length = j; + hashmap_add(&prefix_map->map, e); + break; + } + + if (!e2->item) + continue; /* non-unique prefix */ + + if (j != e2->item->prefix_length) + BUG("unexpected prefix length: %d != %d", + (int)j, (int)e2->item->prefix_length); + + /* skip common prefix */ + for (; j < prefix_map->max_length && e->name[j]; j++) { + if (e->item->name[j] != e2->item->name[j]) + break; + add_prefix_entry(&prefix_map->map, e->name, j + 1, + NULL); + } + + /* e2 no longer refers to a unique prefix */ + if (j < prefix_map->max_length && e2->name[j]) { + /* found a new unique prefix for e2's item */ + e2->item->prefix_length = j + 1; + add_prefix_entry(&prefix_map->map, e2->name, j + 1, + e2->item); + } + else + e2->item->prefix_length = 0; + e2->item = NULL; + + if (j < prefix_map->max_length && e->name[j]) { + /* found a unique prefix for the item */ + e->item->prefix_length = j + 1; + add_prefix_entry(&prefix_map->map, e->name, j + 1, + e->item); + } else { + /* item has no (short enough) unique prefix */ + e->item->prefix_length = 0; + free(e); + } + + break; + } +} + +void find_unique_prefixes(struct prefix_item **list, size_t nr, + int min_length, int max_length) +{ + int i; + struct prefix_map prefix_map; + + init_prefix_map(&prefix_map, min_length, max_length); + for (i = 0; i < nr; i++) + add_prefix_item(&prefix_map, list[i]); + hashmap_free(&prefix_map.map, 1); +} diff --git a/prefix-map.h b/prefix-map.h new file mode 100644 index 00000000000000..ce3b8a4a321b97 --- /dev/null +++ b/prefix-map.h @@ -0,0 +1,40 @@ +#ifndef PREFIX_MAP_H +#define PREFIX_MAP_H + +#include "hashmap.h" + +struct prefix_item { + const char *name; + size_t prefix_length; +}; + +struct prefix_map_entry { + struct hashmap_entry e; + const char *name; + size_t prefix_length; + /* if item is NULL, the prefix is not unique */ + struct prefix_item *item; +}; + +struct prefix_map { + struct hashmap map; + int min_length, max_length; +}; + +/* + * Find unique prefixes in a given list of strings. + * + * Typically, the `struct prefix_item` information will be but a field in the + * actual item struct; For this reason, the `list` parameter is specified as a + * list of pointers to the items. + * + * The `min_length`/`max_length` parameters define what length the unique + * prefixes should have. + * + * If no unique prefix could be found for a given item, its `prefix_length` + * will be set to 0. + */ +void find_unique_prefixes(struct prefix_item **list, size_t nr, + int min_length, int max_length); + +#endif diff --git a/t/helper/test-prefix-map.c b/t/helper/test-prefix-map.c new file mode 100644 index 00000000000000..3f1c90eaf04361 --- /dev/null +++ b/t/helper/test-prefix-map.c @@ -0,0 +1,58 @@ +#include "test-tool.h" +#include "cache.h" +#include "prefix-map.h" + +static size_t test_count, failed_count; + +static void check(int succeeded, const char *file, size_t line_no, + const char *fmt, ...) +{ + va_list ap; + + test_count++; + if (succeeded) + return; + + va_start(ap, fmt); + fprintf(stderr, "%s:%d: ", file, (int)line_no); + vfprintf(stderr, fmt, ap); + fputc('\n', stderr); + va_end(ap); + + failed_count++; +} + +#define EXPECT_SIZE_T_EQUALS(expect, actual, hint) \ + check(expect == actual, __FILE__, __LINE__, \ + "size_t's do not match: %" \ + PRIdMAX " != %" PRIdMAX " (%s) (%s)", \ + (intmax_t)expect, (intmax_t)actual, #actual, hint) + +int cmd__prefix_map(int argc, const char **argv) +{ +#define NR 5 + struct prefix_item items[NR] = { + { "unique" }, + { "hell" }, + { "hello" }, + { "wok" }, + { "world" }, + }; + struct prefix_item *list[NR] = { + items, items + 1, items + 2, items + 3, items + 4 + }; + + find_unique_prefixes(list, NR, 1, 3); + +#define EXPECT_PREFIX_LENGTH_EQUALS(expect, index) \ + EXPECT_SIZE_T_EQUALS(expect, list[index]->prefix_length, \ + list[index]->name) + + EXPECT_PREFIX_LENGTH_EQUALS(1, 0); + EXPECT_PREFIX_LENGTH_EQUALS(0, 1); + EXPECT_PREFIX_LENGTH_EQUALS(0, 2); + EXPECT_PREFIX_LENGTH_EQUALS(3, 3); + EXPECT_PREFIX_LENGTH_EQUALS(3, 4); + + return !!failed_count; +} diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c index 99db7409b81289..d6a92a8699501d 100644 --- a/t/helper/test-tool.c +++ b/t/helper/test-tool.c @@ -32,6 +32,7 @@ static struct test_cmd cmds[] = { { "parse-options", cmd__parse_options }, { "path-utils", cmd__path_utils }, { "pkt-line", cmd__pkt_line }, + { "prefix-map", cmd__prefix_map }, { "prio-queue", cmd__prio_queue }, { "reach", cmd__reach }, { "read-cache", cmd__read_cache }, diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h index 25abed1cf2acc9..33a089ee4e2a05 100644 --- a/t/helper/test-tool.h +++ b/t/helper/test-tool.h @@ -29,6 +29,7 @@ int cmd__online_cpus(int argc, const char **argv); int cmd__parse_options(int argc, const char **argv); int cmd__path_utils(int argc, const char **argv); int cmd__pkt_line(int argc, const char **argv); +int cmd__prefix_map(int argc, const char **argv); int cmd__prio_queue(int argc, const char **argv); int cmd__reach(int argc, const char **argv); int cmd__read_cache(int argc, const char **argv); diff --git a/t/t0016-prefix-map.sh b/t/t0016-prefix-map.sh new file mode 100755 index 00000000000000..187fa92aecf35d --- /dev/null +++ b/t/t0016-prefix-map.sh @@ -0,0 +1,10 @@ +#!/bin/sh + +test_description='basic tests for prefix map' +. ./test-lib.sh + +test_expect_success 'prefix map' ' + test-tool prefix-map +' + +test_done From 573f9f415493f2a06d82177ba29aa73382835032 Mon Sep 17 00:00:00 2001 From: Slavica Djukic Date: Wed, 27 Feb 2019 12:31:53 +0100 Subject: [PATCH 13/69] built-in add -i: show unique prefixes of the commands Just like in the Perl script `git-add--interactive.perl`, for each command a unique prefix is determined (if there exists any within the given parameters), and shown in the list, and accepted as a shortcut for the command. We use the prefix map implementation that we just added in the previous commit for that purpose. Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- add-interactive.c | 69 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 13 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 33d10d25efa751..44ab670f7ff2a7 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -5,6 +5,7 @@ #include "diffcore.h" #include "revision.h" #include "refs.h" +#include "prefix-map.h" struct add_i_state { struct repository *r; @@ -46,18 +47,32 @@ static int init_add_i_state(struct repository *r, struct add_i_state *s) return 0; } -struct item { - const char *name; -}; +static ssize_t find_unique(const char *string, + struct prefix_item **list, size_t nr) +{ + ssize_t found = -1, i; + + for (i = 0; i < nr; i++) { + struct prefix_item *item = list[i]; + if (!starts_with(item->name, string)) + continue; + if (found >= 0) + return -1; + found = i; + } + + return found; +} struct list_options { int columns; const char *header; - void (*print_item)(int i, struct item *item, void *print_item_data); + void (*print_item)(int i, struct prefix_item *item, + void *print_item_data); void *print_item_data; }; -static void list(struct item **list, size_t nr, +static void list(struct prefix_item **list, size_t nr, struct add_i_state *s, struct list_options *opts) { int i, last_lf = 0; @@ -94,13 +109,15 @@ struct list_and_choose_options { /* * Returns the selected index. */ -static ssize_t list_and_choose(struct item **items, size_t nr, +static ssize_t list_and_choose(struct prefix_item **items, size_t nr, struct add_i_state *s, struct list_and_choose_options *opts) { struct strbuf input = STRBUF_INIT; ssize_t res = -1; + find_unique_prefixes(items, nr, 1, 4); + for (;;) { char *p, *endp; @@ -140,6 +157,9 @@ static ssize_t list_and_choose(struct item **items, size_t nr, } p[sep] = '\0'; + if (index < 0) + index = find_unique(p, items, nr); + if (index < 0 || index >= nr) printf(_("Huh (%s)?\n"), p); else { @@ -165,7 +185,7 @@ struct adddel { struct file_list { struct file_item { - struct item item; + struct prefix_item item; struct adddel index, worktree; } **file; size_t nr, alloc; @@ -331,12 +351,29 @@ static void populate_wi_changes(struct strbuf *buf, strbuf_addstr(buf, no_changes); } +/* filters out prefixes which have special meaning to list_and_choose() */ +static int is_valid_prefix(const char *prefix, size_t prefix_len) +{ + return prefix_len && prefix && + /* + * We expect `prefix` to be NUL terminated, therefore this + * `strcspn()` call is okay, even if it might do much more + * work than strictly necessary. + */ + strcspn(prefix, " \t\r\n,") >= prefix_len && /* separators */ + *prefix != '-' && /* deselection */ + !isdigit(*prefix) && /* selection */ + (prefix_len != 1 || + (*prefix != '*' && /* "all" wildcard */ + *prefix != '?')); /* prompt help */ +} + struct print_file_item_data { const char *modified_fmt; struct strbuf buf, index, worktree; }; -static void print_file_item(int i, struct item *item, +static void print_file_item(int i, struct prefix_item *item, void *print_file_item_data) { struct file_item *c = (struct file_item *)item; @@ -363,20 +400,26 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps, return -1; if (files->nr) - list((struct item **)files->file, files->nr, s, opts); + list((struct prefix_item **)files->file, files->nr, s, opts); putchar('\n'); return 0; } -static void print_command_item(int i, struct item *item, +static void print_command_item(int i, struct prefix_item *item, void *print_command_item_data) { - printf(" %2d: %s", i + 1, item->name); + if (!item->prefix_length || + !is_valid_prefix(item->name, item->prefix_length)) + printf(" %2d: %s", i + 1, item->name); + else + printf(" %3d: [%.*s]%s", i + 1, + (int)item->prefix_length, item->name, + item->name + item->prefix_length); } struct command_item { - struct item item; + struct prefix_item item; int (*command)(struct add_i_state *s, const struct pathspec *ps, struct file_list *files, struct list_options *opts); }; @@ -418,7 +461,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps) res = -1; for (;;) { - i = list_and_choose((struct item **)commands, + i = list_and_choose((struct prefix_item **)commands, ARRAY_SIZE(commands), &s, &main_loop_opts); if (i < -1) { printf(_("Bye.\n")); From f48c70e2db5054ae202841892488faafa8c5c92a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 6 Mar 2019 00:06:57 +0100 Subject: [PATCH 14/69] built-in add -i: support `?` (prompt help) With this change, we print out the same colored help text that the Perl-based `git add -i` prints in the main loop when question mark is entered. Signed-off-by: Johannes Schindelin --- add-interactive.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/add-interactive.c b/add-interactive.c index 44ab670f7ff2a7..ac8af70c85c7a3 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -11,6 +11,7 @@ struct add_i_state { struct repository *r; int use_color; char header_color[COLOR_MAXLEN]; + char help_color[COLOR_MAXLEN]; }; static void init_color(struct repository *r, struct add_i_state *s, @@ -43,6 +44,7 @@ static int init_add_i_state(struct repository *r, struct add_i_state *s) s->use_color = want_color(s->use_color); init_color(r, s, "header", s->header_color, GIT_COLOR_BOLD); + init_color(r, s, "help", s->help_color, GIT_COLOR_BOLD_RED); return 0; } @@ -104,6 +106,7 @@ struct list_and_choose_options { struct list_options list_opts; const char *prompt; + void (*print_help)(struct add_i_state *s); }; /* @@ -138,6 +141,11 @@ static ssize_t list_and_choose(struct prefix_item **items, size_t nr, if (!input.len) break; + if (!strcmp(input.buf, "?")) { + opts->print_help(s); + continue; + } + p = input.buf; for (;;) { size_t sep = strcspn(p, " \t\r\n,"); @@ -424,12 +432,24 @@ struct command_item { struct file_list *files, struct list_options *opts); }; +static void command_prompt_help(struct add_i_state *s) +{ + const char *help_color = s->help_color; + color_fprintf_ln(stdout, help_color, "%s", _("Prompt help:")); + color_fprintf_ln(stdout, help_color, "1 - %s", + _("select a numbered item")); + color_fprintf_ln(stdout, help_color, "foo - %s", + _("select item based on unique prefix")); + color_fprintf_ln(stdout, help_color, " - %s", + _("(empty) select nothing")); +} + int run_add_i(struct repository *r, const struct pathspec *ps) { struct add_i_state s = { NULL }; struct list_and_choose_options main_loop_opts = { { 4, N_("*** Commands ***"), print_command_item, NULL }, - N_("What now") + N_("What now"), command_prompt_help }; struct command_item status = { { "status" }, run_status }; From 9cf326024af1ae6f21ebabd473f862053874201c Mon Sep 17 00:00:00 2001 From: Slavica Djukic Date: Sun, 3 Mar 2019 13:19:27 +0100 Subject: [PATCH 15/69] built-in add -i: use color in the main loop The error messages as well as the unique prefixes are colored in `git add -i` by default; We need to do the same in the built-in version. Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- add-interactive.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index ac8af70c85c7a3..dfd1b6b1601d38 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -12,6 +12,9 @@ struct add_i_state { int use_color; char header_color[COLOR_MAXLEN]; char help_color[COLOR_MAXLEN]; + char prompt_color[COLOR_MAXLEN]; + char error_color[COLOR_MAXLEN]; + char reset_color[COLOR_MAXLEN]; }; static void init_color(struct repository *r, struct add_i_state *s, @@ -45,6 +48,9 @@ static int init_add_i_state(struct repository *r, struct add_i_state *s) init_color(r, s, "header", s->header_color, GIT_COLOR_BOLD); init_color(r, s, "help", s->help_color, GIT_COLOR_BOLD_RED); + init_color(r, s, "prompt", s->prompt_color, GIT_COLOR_BOLD_BLUE); + init_color(r, s, "error", s->error_color, GIT_COLOR_BOLD_RED); + init_color(r, s, "reset", s->reset_color, GIT_COLOR_RESET); return 0; } @@ -128,7 +134,8 @@ static ssize_t list_and_choose(struct prefix_item **items, size_t nr, list(items, nr, s, &opts->list_opts); - printf("%s%s", opts->prompt, "> "); + color_fprintf(stdout, s->prompt_color, "%s", opts->prompt); + fputs("> ", stdout); fflush(stdout); if (strbuf_getline(&input, stdin) == EOF) { @@ -169,7 +176,8 @@ static ssize_t list_and_choose(struct prefix_item **items, size_t nr, index = find_unique(p, items, nr); if (index < 0 || index >= nr) - printf(_("Huh (%s)?\n"), p); + color_fprintf_ln(stdout, s->error_color, + _("Huh (%s)?"), p); else { res = index; break; @@ -414,15 +422,21 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps, return 0; } +struct print_command_item_data { + const char *color, *reset; +}; + static void print_command_item(int i, struct prefix_item *item, void *print_command_item_data) { + struct print_command_item_data *d = print_command_item_data; + if (!item->prefix_length || !is_valid_prefix(item->name, item->prefix_length)) printf(" %2d: %s", i + 1, item->name); else - printf(" %3d: [%.*s]%s", i + 1, - (int)item->prefix_length, item->name, + printf(" %2d: %s%.*s%s%s", i + 1, + d->color, (int)item->prefix_length, item->name, d->reset, item->name + item->prefix_length); } @@ -447,8 +461,9 @@ static void command_prompt_help(struct add_i_state *s) int run_add_i(struct repository *r, const struct pathspec *ps) { struct add_i_state s = { NULL }; + struct print_command_item_data data; struct list_and_choose_options main_loop_opts = { - { 4, N_("*** Commands ***"), print_command_item, NULL }, + { 4, N_("*** Commands ***"), print_command_item, &data }, N_("What now"), command_prompt_help }; struct command_item @@ -471,6 +486,18 @@ int run_add_i(struct repository *r, const struct pathspec *ps) if (init_add_i_state(r, &s)) return error("could not parse `add -i` config"); + /* + * When color was asked for, use the prompt color for + * highlighting, otherwise use square brackets. + */ + if (s.use_color) { + data.color = s.prompt_color; + data.reset = s.reset_color; + } else { + data.color = "["; + data.reset = "]"; + } + strbuf_addstr(&header, " "); strbuf_addf(&header, print_file_item_data.modified_fmt, _("staged"), _("unstaged"), _("path")); From ddb913e1947949e5442cb3b7c5559238b8535754 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 7 Mar 2019 01:12:07 +0100 Subject: [PATCH 16/69] built-in add -i: implement the `help` command This imitates the code to show the help text from the Perl script `git-add--interactive.perl` in the built-in version. To make sure that it renders exactly like the Perl version of `git add -i`, we also add a test case for that to `t3701-add-interactive.sh`. Signed-off-by: Slavica Djukic Signed-off-by: Johannes Schindelin --- add-interactive.c | 27 +++++++++++++++++++++++++-- t/t3701-add-interactive.sh | 24 ++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index dfd1b6b1601d38..e690ca57fae969 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -422,6 +422,27 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps, return 0; } +static int run_help(struct add_i_state *s, const struct pathspec *ps, + struct file_list *files, struct list_options *opts) +{ + const char *help_color = s->help_color; + + color_fprintf_ln(stdout, help_color, "status - %s", + _("show paths with changes")); + color_fprintf_ln(stdout, help_color, "update - %s", + _("add working tree state to the staged set of changes")); + color_fprintf_ln(stdout, help_color, "revert - %s", + _("revert staged set of changes back to the HEAD version")); + color_fprintf_ln(stdout, help_color, "patch - %s", + _("pick hunks and update selectively")); + color_fprintf_ln(stdout, help_color, "diff - %s", + _("view diff between HEAD and index")); + color_fprintf_ln(stdout, help_color, "add untracked - %s", + _("add contents of untracked files to the staged set of changes")); + + return 0; +} + struct print_command_item_data { const char *color, *reset; }; @@ -467,9 +488,11 @@ int run_add_i(struct repository *r, const struct pathspec *ps) N_("What now"), command_prompt_help }; struct command_item - status = { { "status" }, run_status }; + status = { { "status" }, run_status }, + help = { { "help" }, run_help }; struct command_item *commands[] = { - &status + &status, + &help }; struct print_file_item_data print_file_item_data = { diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 65dfbc033a027d..91aaef29323276 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -639,4 +639,28 @@ test_expect_success 'add -p patch editing works with pathological context lines' test_cmp expected-2 actual ' +test_expect_success 'show help from add--helper' ' + git reset --hard && + cat >expect <<-EOF && + + *** Commands *** + 1: status 2: update 3: revert 4: add untracked + 5: patch 6: diff 7: quit 8: help + What now> status - show paths with changes + update - add working tree state to the staged set of changes + revert - revert staged set of changes back to the HEAD version + patch - pick hunks and update selectively + diff - view diff between HEAD and index + add untracked - add contents of untracked files to the staged set of changes + *** Commands *** + 1: status 2: update 3: revert 4: add untracked + 5: patch 6: diff 7: quit 8: help + What now>$SP + Bye. + EOF + test_write_lines h | GIT_PAGER_IN_USE=true TERM=vt100 git add -i >actual.colored && + test_decode_color actual && + test_i18ncmp expect actual +' + test_done From e65167d79fe30ccc7433a1317369bf95d131a854 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 5 Mar 2019 23:33:55 +0100 Subject: [PATCH 17/69] built-in add -i: allow filtering the modified files list In `update` command of `git add -i`, we are primarily interested in the list of modified files that have worktree (i.e. unstaged) changes. The Perl script version of `git add -i` has a parameter of the `list_modified()` function for that matter. In C, we can be a lot more precise, using an `enum`. The C implementation of the filter also has an easier time to avoid unnecessary work, simply by using an adaptive order of the `diff-index` and `diff-files` calls, and then not adding unnecessary entries in the first place. Signed-off-by: Johannes Schindelin --- add-interactive.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index e690ca57fae969..f62f0c5f06c6ef 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -262,6 +262,7 @@ struct collection_status { const char *reference; + unsigned skip_unseen:1; struct file_list *list; struct hashmap file_map; }; @@ -290,6 +291,8 @@ static void collect_changes_cb(struct diff_queue_struct *q, entry = hashmap_get_from_hash(&s->file_map, hash, name); if (entry) file_index = entry->index; + else if (s->skip_unseen) + continue; else { FLEX_ALLOC_STR(entry, pathname, name); hashmap_entry_init(entry, hash); @@ -309,13 +312,22 @@ static void collect_changes_cb(struct diff_queue_struct *q, } } -static int get_modified_files(struct repository *r, struct file_list *list, +enum modified_files_filter { + NO_FILTER = 0, + WORKTREE_ONLY = 1, + INDEX_ONLY = 2, +}; + +static int get_modified_files(struct repository *r, + enum modified_files_filter filter, + struct file_list *list, const struct pathspec *ps) { struct object_id head_oid; int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &head_oid, NULL); struct collection_status s = { FROM_WORKTREE }; + int i; if (repo_read_index_preload(r, ps, 0) < 0) return error(_("could not read index")); @@ -323,10 +335,16 @@ static int get_modified_files(struct repository *r, struct file_list *list, s.list = list; hashmap_init(&s.file_map, pathname_entry_cmp, NULL, 0); - for (s.phase = FROM_WORKTREE; s.phase <= FROM_INDEX; s.phase++) { + for (i = 0; i < 2; i++) { struct rev_info rev; struct setup_revision_opt opt = { 0 }; + if (filter == INDEX_ONLY) + s.phase = i ? FROM_WORKTREE : FROM_INDEX; + else + s.phase = i ? FROM_INDEX : FROM_WORKTREE; + s.skip_unseen = filter && i; + opt.def = is_initial ? empty_tree_oid_hex() : oid_to_hex(&head_oid); @@ -412,7 +430,7 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps, { reset_file_list(files); - if (get_modified_files(s->r, files, ps) < 0) + if (get_modified_files(s->r, 0, files, ps) < 0) return -1; if (files->nr) From 7a479f418510d57562af12441ffa421cdb404b7d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 7 Mar 2019 21:36:11 +0100 Subject: [PATCH 18/69] built-in add -i: prepare for multi-selection commands The `upgrade`, `revert` and `add-untracked` commands allow selecting multiple entries. Let's extend the `list_and_choose()` function to accommodate those use cases. Signed-off-by: Johannes Schindelin --- add-interactive.c | 104 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 77 insertions(+), 27 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index f62f0c5f06c6ef..1e79b280be87cd 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -75,12 +75,12 @@ static ssize_t find_unique(const char *string, struct list_options { int columns; const char *header; - void (*print_item)(int i, struct prefix_item *item, + void (*print_item)(int i, int selected, struct prefix_item *item, void *print_item_data); void *print_item_data; }; -static void list(struct prefix_item **list, size_t nr, +static void list(struct prefix_item **list, int *selected, size_t nr, struct add_i_state *s, struct list_options *opts) { int i, last_lf = 0; @@ -93,7 +93,8 @@ static void list(struct prefix_item **list, size_t nr, "%s", opts->header); for (i = 0; i < nr; i++) { - opts->print_item(i, list[i], opts->print_item_data); + opts->print_item(i, selected ? selected[i] : 0, list[i], + opts->print_item_data); if ((opts->columns) && ((i + 1) % (opts->columns))) { putchar('\t'); @@ -112,18 +113,32 @@ struct list_and_choose_options { struct list_options list_opts; const char *prompt; + enum { + SINGLETON = (1<<0), + IMMEDIATE = (1<<1), + } flags; void (*print_help)(struct add_i_state *s); }; /* - * Returns the selected index. + * Returns the selected index in singleton mode, the number of selected items + * otherwise. */ -static ssize_t list_and_choose(struct prefix_item **items, size_t nr, - struct add_i_state *s, +static ssize_t list_and_choose(struct prefix_item **items, int *selected, + size_t nr, struct add_i_state *s, struct list_and_choose_options *opts) { + int singleton = opts->flags & SINGLETON; + int immediate = opts->flags & IMMEDIATE; + struct strbuf input = STRBUF_INIT; - ssize_t res = -1; + ssize_t res = singleton ? -1 : 0; + + if (!selected && !singleton) + BUG("need a selected array in non-singleton mode"); + + if (singleton && !immediate) + BUG("singleton requires immediate"); find_unique_prefixes(items, nr, 1, 4); @@ -132,15 +147,16 @@ static ssize_t list_and_choose(struct prefix_item **items, size_t nr, strbuf_reset(&input); - list(items, nr, s, &opts->list_opts); + list(items, selected, nr, s, &opts->list_opts); color_fprintf(stdout, s->prompt_color, "%s", opts->prompt); - fputs("> ", stdout); + fputs(singleton ? "> " : ">> ", stdout); fflush(stdout); if (strbuf_getline(&input, stdin) == EOF) { putchar('\n'); - res = -2; + if (immediate) + res = -2; break; } strbuf_trim(&input); @@ -156,7 +172,9 @@ static ssize_t list_and_choose(struct prefix_item **items, size_t nr, p = input.buf; for (;;) { size_t sep = strcspn(p, " \t\r\n,"); - ssize_t index = -1; + int choose = 1; + /* `from` is inclusive, `to` is exclusive */ + ssize_t from = -1, to = -1; if (!sep) { if (!*p) @@ -165,28 +183,59 @@ static ssize_t list_and_choose(struct prefix_item **items, size_t nr, continue; } - if (isdigit(*p)) { - index = strtoul(p, &endp, 10) - 1; - if (endp != p + sep) - index = -1; + /* Input that begins with '-'; unchoose */ + if (*p == '-') { + choose = 0; + p++; + sep--; + } + + if (sep == 1 && *p == '*') { + from = 0; + to = nr; + } else if (isdigit(*p)) { + /* A range can be specified like 5-7 or 5-. */ + from = strtoul(p, &endp, 10) - 1; + if (endp == p + sep) + to = from + 1; + else if (*endp == '-') { + to = strtoul(++endp, &endp, 10); + /* extra characters after the range? */ + if (endp != p + sep) + from = -1; + } } p[sep] = '\0'; - if (index < 0) - index = find_unique(p, items, nr); + if (from < 0) { + from = find_unique(p, items, nr); + if (from >= 0) + to = from + 1; + } - if (index < 0 || index >= nr) + if (from < 0 || from >= nr || + (singleton && from + 1 != to)) { color_fprintf_ln(stdout, s->error_color, _("Huh (%s)?"), p); - else { - res = index; + break; + } else if (singleton) { + res = from; break; } + if (to > nr) + to = nr; + + for (; from < to; from++) + if (selected[from] != choose) { + selected[from] = choose; + res += choose ? +1 : -1; + } + p += sep + 1; } - if (res >= 0) + if ((immediate && res >= 0) || !strcmp(input.buf, "*")) break; } @@ -407,7 +456,7 @@ struct print_file_item_data { struct strbuf buf, index, worktree; }; -static void print_file_item(int i, struct prefix_item *item, +static void print_file_item(int i, int selected, struct prefix_item *item, void *print_file_item_data) { struct file_item *c = (struct file_item *)item; @@ -422,7 +471,7 @@ static void print_file_item(int i, struct prefix_item *item, strbuf_addf(&d->buf, d->modified_fmt, d->index.buf, d->worktree.buf, item->name); - printf(" %2d: %s", i + 1, d->buf.buf); + printf("%c%2d: %s", selected ? '*' : ' ', i + 1, d->buf.buf); } static int run_status(struct add_i_state *s, const struct pathspec *ps, @@ -434,7 +483,8 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps, return -1; if (files->nr) - list((struct prefix_item **)files->file, files->nr, s, opts); + list((struct prefix_item **)files->file, NULL, files->nr, + s, opts); putchar('\n'); return 0; @@ -465,7 +515,7 @@ struct print_command_item_data { const char *color, *reset; }; -static void print_command_item(int i, struct prefix_item *item, +static void print_command_item(int i, int selected, struct prefix_item *item, void *print_command_item_data) { struct print_command_item_data *d = print_command_item_data; @@ -503,7 +553,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps) struct print_command_item_data data; struct list_and_choose_options main_loop_opts = { { 4, N_("*** Commands ***"), print_command_item, &data }, - N_("What now"), command_prompt_help + N_("What now"), SINGLETON | IMMEDIATE, command_prompt_help }; struct command_item status = { { "status" }, run_status }, @@ -549,7 +599,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps) res = -1; for (;;) { - i = list_and_choose((struct prefix_item **)commands, + i = list_and_choose((struct prefix_item **)commands, NULL, ARRAY_SIZE(commands), &s, &main_loop_opts); if (i < -1) { printf(_("Bye.\n")); From 41e36a5cce8fe11e1668c543d42044192b54556c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 5 Mar 2019 23:58:49 +0100 Subject: [PATCH 19/69] built-in add -i: implement the `update` command After `status` and `help`, it is now turn to port the `update` command to C, the second command that is shown in the main loop menu of `git add -i`. This `git add -i` command is the first one which lets the user choose a subset of a list of files, and as such, this patch lays the groundwork for the other commands of that category: - It teaches the `print_file_item()` function to show a unique prefix if we found any (the code to find it had been added already in the previous patch where we colored the unique prefixes of the main loop commands, but that patch uses the `print_command_item()` function to display the menu items). - This patch also adds the help text that is shown when the user input to select items from the shown list could not be parsed. Signed-off-by: Johannes Schindelin --- add-interactive.c | 124 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 112 insertions(+), 12 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 1e79b280be87cd..67c6ef03b551cf 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -6,6 +6,7 @@ #include "revision.h" #include "refs.h" #include "prefix-map.h" +#include "lockfile.h" struct add_i_state { struct repository *r; @@ -452,8 +453,8 @@ static int is_valid_prefix(const char *prefix, size_t prefix_len) } struct print_file_item_data { - const char *modified_fmt; - struct strbuf buf, index, worktree; + const char *modified_fmt, *color, *reset; + struct strbuf buf, name, index, worktree; }; static void print_file_item(int i, int selected, struct prefix_item *item, @@ -461,21 +462,34 @@ static void print_file_item(int i, int selected, struct prefix_item *item, { struct file_item *c = (struct file_item *)item; struct print_file_item_data *d = print_file_item_data; + const char *highlighted = NULL; strbuf_reset(&d->index); strbuf_reset(&d->worktree); strbuf_reset(&d->buf); + /* Format the item with the prefix highlighted. */ + if (item->prefix_length > 0 && + is_valid_prefix(item->name, item->prefix_length)) { + strbuf_reset(&d->name); + strbuf_addf(&d->name, "%s%.*s%s%s", d->color, + (int)item->prefix_length, item->name, d->reset, + item->name + item->prefix_length); + highlighted = d->name.buf; + } + populate_wi_changes(&d->worktree, &c->worktree, _("nothing")); populate_wi_changes(&d->index, &c->index, _("unchanged")); strbuf_addf(&d->buf, d->modified_fmt, - d->index.buf, d->worktree.buf, item->name); + d->index.buf, d->worktree.buf, + highlighted ? highlighted : item->name); printf("%c%2d: %s", selected ? '*' : ' ', i + 1, d->buf.buf); } static int run_status(struct add_i_state *s, const struct pathspec *ps, - struct file_list *files, struct list_options *opts) + struct file_list *files, + struct list_and_choose_options *opts) { reset_file_list(files); @@ -484,14 +498,72 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps, if (files->nr) list((struct prefix_item **)files->file, NULL, files->nr, - s, opts); + s, &opts->list_opts); putchar('\n'); return 0; } +static int run_update(struct add_i_state *s, const struct pathspec *ps, + struct file_list *files, + struct list_and_choose_options *opts) +{ + int res = 0, fd, *selected = NULL; + size_t count, i; + struct lock_file index_lock; + + reset_file_list(files); + + if (get_modified_files(s->r, WORKTREE_ONLY, files, ps) < 0) + return -1; + + if (!files->nr) { + putchar('\n'); + return 0; + } + + opts->prompt = N_("Update"); + CALLOC_ARRAY(selected, files->nr); + + count = list_and_choose((struct prefix_item **)files->file, + selected, files->nr, s, opts); + if (count <= 0) { + putchar('\n'); + free(selected); + return 0; + } + + fd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR); + if (fd < 0) { + putchar('\n'); + free(selected); + return -1; + } + + for (i = 0; i < files->nr; i++) { + const char *name = files->file[i]->item.name; + if (selected[i] && + add_file_to_index(s->r->index, name, 0) < 0) { + res = error(_("could not stage '%s'"), name); + break; + } + } + + if (!res && write_locked_index(s->r->index, &index_lock, COMMIT_LOCK) < 0) + res = error(_("could not write index")); + + if (!res) + printf(Q_("updated %d path\n", + "updated %d paths\n", count), (int)count); + + putchar('\n'); + free(selected); + return res; +} + static int run_help(struct add_i_state *s, const struct pathspec *ps, - struct file_list *files, struct list_options *opts) + struct file_list *files, + struct list_and_choose_options *opts) { const char *help_color = s->help_color; @@ -511,6 +583,27 @@ static int run_help(struct add_i_state *s, const struct pathspec *ps, return 0; } +static void choose_prompt_help(struct add_i_state *s) +{ + const char *help_color = s->help_color; + color_fprintf_ln(stdout, help_color, "%s", + _("Prompt help:")); + color_fprintf_ln(stdout, help_color, "1 - %s", + _("select a single item")); + color_fprintf_ln(stdout, help_color, "3-5 - %s", + _("select a range of items")); + color_fprintf_ln(stdout, help_color, "2-3,6-9 - %s", + _("select multiple ranges")); + color_fprintf_ln(stdout, help_color, "foo - %s", + _("select item based on unique prefix")); + color_fprintf_ln(stdout, help_color, "-... - %s", + _("unselect specified items")); + color_fprintf_ln(stdout, help_color, "* - %s", + _("choose all items")); + color_fprintf_ln(stdout, help_color, " - %s", + _("(empty) finish selecting")); +} + struct print_command_item_data { const char *color, *reset; }; @@ -532,7 +625,8 @@ static void print_command_item(int i, int selected, struct prefix_item *item, struct command_item { struct prefix_item item; int (*command)(struct add_i_state *s, const struct pathspec *ps, - struct file_list *files, struct list_options *opts); + struct file_list *files, + struct list_and_choose_options *opts); }; static void command_prompt_help(struct add_i_state *s) @@ -557,17 +651,20 @@ int run_add_i(struct repository *r, const struct pathspec *ps) }; struct command_item status = { { "status" }, run_status }, + update = { { "update" }, run_update }, help = { { "help" }, run_help }; struct command_item *commands[] = { - &status, + &status, &update, &help }; struct print_file_item_data print_file_item_data = { - "%12s %12s %s", STRBUF_INIT, STRBUF_INIT, STRBUF_INIT + "%12s %12s %s", NULL, NULL, + STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; - struct list_options opts = { - 0, NULL, print_file_item, &print_file_item_data + struct list_and_choose_options opts = { + { 0, NULL, print_file_item, &print_file_item_data }, + NULL, 0, choose_prompt_help }; struct strbuf header = STRBUF_INIT; struct file_list files = { NULL }; @@ -588,11 +685,13 @@ int run_add_i(struct repository *r, const struct pathspec *ps) data.color = "["; data.reset = "]"; } + print_file_item_data.color = data.color; + print_file_item_data.reset = data.reset; strbuf_addstr(&header, " "); strbuf_addf(&header, print_file_item_data.modified_fmt, _("staged"), _("unstaged"), _("path")); - opts.header = header.buf; + opts.list_opts.header = header.buf; repo_refresh_and_write_index(r, REFRESH_QUIET, 1); if (run_status(&s, ps, &files, &opts) < 0) @@ -612,6 +711,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps) release_file_list(&files); strbuf_release(&print_file_item_data.buf); + strbuf_release(&print_file_item_data.name); strbuf_release(&print_file_item_data.index); strbuf_release(&print_file_item_data.worktree); strbuf_release(&header); From c6ba59eed34ee68ef9f597f28af46f28910a0e8f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 6 Mar 2019 23:06:13 +0100 Subject: [PATCH 20/69] built-in add -i: re-implement `revert` in C This is a relatively straight-forward port from the Perl version, with the notable exception that we imitate `git reset -- ` in the C version rather than the convoluted `git ls-tree HEAD -- | git update-index --index-info` followed by `git update-index --force-remove -- ` for the missed ones. While at it, we fix the pretty obvious bug where the `revert` command offers to unstage files that do not have staged changes. Signed-off-by: Johannes Schindelin --- add-interactive.c | 114 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 113 insertions(+), 1 deletion(-) diff --git a/add-interactive.c b/add-interactive.c index 67c6ef03b551cf..ad0759968f0bd2 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -561,6 +561,117 @@ static int run_update(struct add_i_state *s, const struct pathspec *ps, return res; } +static void revert_from_diff(struct diff_queue_struct *q, + struct diff_options *opt, void *data) +{ + int i, add_flags = ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE; + + for (i = 0; i < q->nr; i++) { + struct diff_filespec *one = q->queue[i]->one; + struct cache_entry *ce; + + if (!(one->mode && !is_null_oid(&one->oid))) { + remove_file_from_index(opt->repo->index, one->path); + printf(_("note: %s is untracked now.\n"), one->path); + } else { + ce = make_cache_entry(opt->repo->index, one->mode, + &one->oid, one->path, 0, 0); + if (!ce) + die(_("make_cache_entry failed for path '%s'"), + one->path); + add_index_entry(opt->repo->index, ce, add_flags); + } + } +} + +static int run_revert(struct add_i_state *s, const struct pathspec *ps, + struct file_list *files, + struct list_and_choose_options *opts) +{ + int res = 0, fd, *selected = NULL; + size_t count, i, j; + + struct object_id oid; + int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &oid, + NULL); + struct lock_file index_lock; + const char **paths; + struct tree *tree; + struct diff_options diffopt = { NULL }; + + reset_file_list(files); + if (get_modified_files(s->r, INDEX_ONLY, files, ps) < 0) + return -1; + + if (!files->nr) { + putchar('\n'); + return 0; + } + + opts->prompt = N_("Revert"); + CALLOC_ARRAY(selected, files->nr); + + count = list_and_choose((struct prefix_item **)files->file, + selected, files->nr, s, opts); + if (count <= 0) + goto finish_revert; + + fd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR); + if (fd < 0) { + res = -1; + goto finish_revert; + } + + if (is_initial) + oidcpy(&oid, s->r->hash_algo->empty_tree); + else { + tree = parse_tree_indirect(&oid); + if (!tree) { + res = error(_("Could not parse HEAD^{tree}")); + goto finish_revert; + } + oidcpy(&oid, &tree->object.oid); + } + + ALLOC_ARRAY(paths, count + 1); + for (i = j = 0; i < files->nr; i++) + if (selected[i]) + paths[j++] = files->file[i]->item.name; + paths[j] = NULL; + + parse_pathspec(&diffopt.pathspec, 0, + PATHSPEC_PREFER_FULL | PATHSPEC_LITERAL_PATH, + NULL, paths); + + diffopt.output_format = DIFF_FORMAT_CALLBACK; + diffopt.format_callback = revert_from_diff; + diffopt.flags.override_submodule_config = 1; + diffopt.repo = s->r; + + if (do_diff_cache(&oid, &diffopt)) + res = -1; + else { + diffcore_std(&diffopt); + diff_flush(&diffopt); + } + free(paths); + clear_pathspec(&diffopt.pathspec); + + if (!res && write_locked_index(s->r->index, &index_lock, + COMMIT_LOCK) < 0) + res = -1; + else + res = repo_refresh_and_write_index(s->r, REFRESH_QUIET, 1); + if (!res) + printf(Q_("reverted %d path\n", + "reverted %d paths\n", count), (int)count); + +finish_revert: + putchar('\n'); + free(selected); + return res; +} + static int run_help(struct add_i_state *s, const struct pathspec *ps, struct file_list *files, struct list_and_choose_options *opts) @@ -652,9 +763,10 @@ int run_add_i(struct repository *r, const struct pathspec *ps) struct command_item status = { { "status" }, run_status }, update = { { "update" }, run_update }, + revert = { { "revert" }, run_revert }, help = { { "help" }, run_help }; struct command_item *commands[] = { - &status, &update, + &status, &update, &revert, &help }; From 3df0c464c8bdad624c4753ebb63ffb214054a00b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 7 Mar 2019 00:59:24 +0100 Subject: [PATCH 21/69] built-in add -i: re-implement `add-untracked` in C This is yet another command, ported to C. It builds nicely on the support functions introduced for other commands, with the notable difference that only names are displayed for untracked files, no file type or diff summary. Signed-off-by: Johannes Schindelin --- add-interactive.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 97 insertions(+), 1 deletion(-) diff --git a/add-interactive.c b/add-interactive.c index ad0759968f0bd2..d674a04e43bc05 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -7,6 +7,8 @@ #include "refs.h" #include "prefix-map.h" #include "lockfile.h" +#include "pathspec.h" +#include "dir.h" struct add_i_state { struct repository *r; @@ -455,6 +457,7 @@ static int is_valid_prefix(const char *prefix, size_t prefix_len) struct print_file_item_data { const char *modified_fmt, *color, *reset; struct strbuf buf, name, index, worktree; + unsigned only_names:1; }; static void print_file_item(int i, int selected, struct prefix_item *item, @@ -478,6 +481,12 @@ static void print_file_item(int i, int selected, struct prefix_item *item, highlighted = d->name.buf; } + if (d->only_names) { + printf("%c%2d: %s", selected ? '*' : ' ', i + 1, + highlighted ? highlighted : item->name); + return; + } + populate_wi_changes(&d->worktree, &c->worktree, _("nothing")); populate_wi_changes(&d->index, &c->index, _("unchanged")); strbuf_addf(&d->buf, d->modified_fmt, @@ -672,6 +681,92 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps, return res; } +static int get_untracked_files(struct repository *r, struct file_list *list, + const struct pathspec *ps) +{ + struct dir_struct dir = { 0 }; + size_t i; + + if (repo_read_index(r) < 0) + return error(_("could not read index")); + + setup_standard_excludes(&dir); + add_exclude_list(&dir, EXC_CMDL, "--exclude option"); + fill_directory(&dir, r->index, ps); + + for (i = 0; i < dir.nr; i++) { + struct dir_entry *ent = dir.entries[i]; + + if (index_name_is_other(r->index, ent->name, ent->len)) { + struct file_item *item; + + FLEXPTR_ALLOC_MEM(item, item.name, ent->name, ent->len); + + ALLOC_GROW(list->file, list->nr + 1, list->alloc); + list->file[list->nr++] = item; + } + } + + return 0; +} + +static int run_add_untracked(struct add_i_state *s, const struct pathspec *ps, + struct file_list *files, + struct list_and_choose_options *opts) +{ + struct print_file_item_data *d = opts->list_opts.print_item_data; + int res = 0, fd, *selected = NULL; + size_t count, i; + + struct lock_file index_lock; + + reset_file_list(files); + if (get_untracked_files(s->r, files, ps) < 0) + return -1; + + if (!files->nr) { + printf(_("No untracked files.\n")); + goto finish_add_untracked; + } + + opts->prompt = N_("Add untracked"); + CALLOC_ARRAY(selected, files->nr); + + d->only_names = 1; + count = list_and_choose((struct prefix_item **)files->file, + selected, files->nr, s, opts); + d->only_names = 0; + if (count <= 0) + goto finish_add_untracked; + + fd = repo_hold_locked_index(s->r, &index_lock, LOCK_REPORT_ON_ERROR); + if (fd < 0) { + res = -1; + goto finish_add_untracked; + } + + for (i = 0; i < files->nr; i++) { + const char *name = files->file[i]->item.name; + if (selected[i] && + add_file_to_index(s->r->index, name, 0) < 0) { + res = error(_("could not stage '%s'"), name); + break; + } + } + + if (!res && write_locked_index(s->r->index, &index_lock, COMMIT_LOCK) < 0) + res = error(_("could not write index")); + + if (!res) + printf(Q_("added %d path\n", + "added %d paths\n", count), (int)count); + +finish_add_untracked: + putchar('\n'); + free(selected); + return res; +} + static int run_help(struct add_i_state *s, const struct pathspec *ps, struct file_list *files, struct list_and_choose_options *opts) @@ -764,9 +859,10 @@ int run_add_i(struct repository *r, const struct pathspec *ps) status = { { "status" }, run_status }, update = { { "update" }, run_update }, revert = { { "revert" }, run_revert }, + add_untracked = { { "add untracked" }, run_add_untracked }, help = { { "help" }, run_help }; struct command_item *commands[] = { - &status, &update, &revert, + &status, &update, &revert, &add_untracked, &help }; From c599406c1036f35c8fe63b2152142e00e6ddc345 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 11 Mar 2019 10:07:49 +0100 Subject: [PATCH 22/69] built-in add -i: implement the `patch` command Well, it is not a full implementation yet. In the interest of making this easy to review (and easy to keep bugs out), we still hand off to the Perl script to do the actual work. The `patch` functionality actually makes up for more than half of the 1,800+ lines of `git-add--interactive.perl`. It will be ported from Perl to C incrementally, later. Signed-off-by: Johannes Schindelin --- add-interactive.c | 97 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 88 insertions(+), 9 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index d674a04e43bc05..c57f6e2fe29c45 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -9,6 +9,8 @@ #include "lockfile.h" #include "pathspec.h" #include "dir.h" +#include "argv-array.h" +#include "run-command.h" struct add_i_state { struct repository *r; @@ -248,7 +250,7 @@ static ssize_t list_and_choose(struct prefix_item **items, int *selected, struct adddel { uintmax_t add, del; - unsigned seen:1, binary:1; + unsigned seen:1, unmerged:1, binary:1; }; struct file_list { @@ -315,6 +317,7 @@ struct collection_status { const char *reference; unsigned skip_unseen:1; + size_t unmerged_count, binary_count; struct file_list *list; struct hashmap file_map; }; @@ -338,12 +341,14 @@ static void collect_changes_cb(struct diff_queue_struct *q, struct pathname_entry *entry; size_t file_index; struct file_item *file; - struct adddel *adddel; + struct adddel *adddel, *other_adddel; entry = hashmap_get_from_hash(&s->file_map, hash, name); - if (entry) + if (entry) { + if (entry->index == (size_t)-1) + continue; file_index = entry->index; - else if (s->skip_unseen) + } else if (s->skip_unseen) continue; else { FLEX_ALLOC_STR(entry, pathname, name); @@ -356,11 +361,20 @@ static void collect_changes_cb(struct diff_queue_struct *q, file = s->list->file[file_index]; adddel = s->phase == FROM_INDEX ? &file->index : &file->worktree; + other_adddel = s->phase == FROM_INDEX ? &file->worktree : &file->index; adddel->seen = 1; adddel->add = stat.files[i]->added; adddel->del = stat.files[i]->deleted; - if (stat.files[i]->is_binary) + if (stat.files[i]->is_binary) { + if (!other_adddel->binary) + s->binary_count++; adddel->binary = 1; + } + if (stat.files[i]->is_unmerged) { + if (!other_adddel->unmerged) + s->unmerged_count++; + adddel->unmerged = 1; + } } } @@ -372,6 +386,8 @@ enum modified_files_filter { static int get_modified_files(struct repository *r, enum modified_files_filter filter, + size_t *unmerged_count, + size_t *binary_count, struct file_list *list, const struct pathspec *ps) { @@ -418,6 +434,10 @@ static int get_modified_files(struct repository *r, } } hashmap_free(&s.file_map, 1); + if (unmerged_count) + *unmerged_count = s.unmerged_count; + if (binary_count) + *binary_count = s.binary_count; /* While the diffs are ordered already, we ran *two* diffs... */ QSORT(list->file, list->nr, file_item_cmp); @@ -502,7 +522,7 @@ static int run_status(struct add_i_state *s, const struct pathspec *ps, { reset_file_list(files); - if (get_modified_files(s->r, 0, files, ps) < 0) + if (get_modified_files(s->r, 0, NULL, NULL, files, ps) < 0) return -1; if (files->nr) @@ -523,7 +543,7 @@ static int run_update(struct add_i_state *s, const struct pathspec *ps, reset_file_list(files); - if (get_modified_files(s->r, WORKTREE_ONLY, files, ps) < 0) + if (get_modified_files(s->r, WORKTREE_ONLY, NULL, NULL, files, ps) < 0) return -1; if (!files->nr) { @@ -609,7 +629,7 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps, struct diff_options diffopt = { NULL }; reset_file_list(files); - if (get_modified_files(s->r, INDEX_ONLY, files, ps) < 0) + if (get_modified_files(s->r, INDEX_ONLY, NULL, NULL, files, ps) < 0) return -1; if (!files->nr) { @@ -767,6 +787,64 @@ static int run_add_untracked(struct add_i_state *s, const struct pathspec *ps, return res; } +static int run_patch(struct add_i_state *s, const struct pathspec *ps, + struct file_list *files, + struct list_and_choose_options *opts) +{ + struct prefix_item **items = (struct prefix_item **)files->file; + int res = 0, *selected = NULL; + ssize_t count, i, j; + size_t unmerged_count = 0, binary_count = 0; + + reset_file_list(files); + if (get_modified_files(s->r, WORKTREE_ONLY, &unmerged_count, + &binary_count, files, ps) < 0) + return -1; + + if (unmerged_count || binary_count) { + for (i = j = 0; i < files->nr; i++) + if (files->file[i]->index.binary || + files->file[i]->worktree.binary) + free(items[i]); + else if (files->file[i]->index.unmerged || + files->file[i]->worktree.unmerged) { + color_fprintf_ln(stderr, s->error_color, + _("ignoring unmerged: %s"), + files->file[i]->item.name); + free(items[i]); + } else + items[j++] = items[i]; + files->nr = j; + } + + if (!files->nr) { + if (binary_count) + fprintf(stderr, _("Only binary files changed.\n")); + else + fprintf(stderr, _("No changes.\n")); + return 0; + } + + opts->prompt = N_("Patch update"); + CALLOC_ARRAY(selected, files->nr); + + count = list_and_choose(items, selected, files->nr, s, opts); + if (count >= 0) { + struct argv_array args = ARGV_ARRAY_INIT; + + argv_array_pushl(&args, "git", "add--interactive", "--patch", + "--", NULL); + for (i = 0; i < files->nr; i++) + if (selected[i]) + argv_array_push(&args, items[i]->name); + res = run_command_v_opt(args.argv, 0); + argv_array_clear(&args); + } + + free(selected); + return res; +} + static int run_help(struct add_i_state *s, const struct pathspec *ps, struct file_list *files, struct list_and_choose_options *opts) @@ -860,10 +938,11 @@ int run_add_i(struct repository *r, const struct pathspec *ps) update = { { "update" }, run_update }, revert = { { "revert" }, run_revert }, add_untracked = { { "add untracked" }, run_add_untracked }, + patch = { { "patch" }, run_patch }, help = { { "help" }, run_help }; struct command_item *commands[] = { &status, &update, &revert, &add_untracked, - &help + &patch, &help }; struct print_file_item_data print_file_item_data = { From 523512215ffd2db31b0c7ea7999013d8257e9e72 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 8 Mar 2019 15:58:27 +0100 Subject: [PATCH 23/69] built-in add -i: re-implement the `diff` command It is not only laziness that we simply spawn `git diff -p --cached` here: this command needs to use the pager, and the pager needs to exit when the diff is done. Currently we do not have any way to make that happen if we run the diff in-process. So let's just spawn. Signed-off-by: Johannes Schindelin --- add-interactive.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/add-interactive.c b/add-interactive.c index c57f6e2fe29c45..5de13edc4c4d38 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -845,6 +845,51 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, return res; } +static int run_diff(struct add_i_state *s, const struct pathspec *ps, + struct file_list *files, + struct list_and_choose_options *opts) +{ + struct prefix_item **items = (struct prefix_item **)files->file; + int res = 0, *selected = NULL; + ssize_t count, i; + + struct object_id oid; + int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &oid, + NULL); + reset_file_list(files); + if (get_modified_files(s->r, INDEX_ONLY, NULL, NULL, files, ps) < 0) + return -1; + + if (!files->nr) { + putchar('\n'); + return 0; + } + + opts->prompt = N_("Review diff"); + CALLOC_ARRAY(selected, files->nr); + + opts->flags = IMMEDIATE; + count = list_and_choose(items, selected, files->nr, s, opts); + opts->flags = 0; + if (count >= 0) { + struct argv_array args = ARGV_ARRAY_INIT; + + argv_array_pushl(&args, "git", "diff", "-p", "--cached", + oid_to_hex(!is_initial ? &oid : + s->r->hash_algo->empty_tree), + "--", NULL); + for (i = 0; i < files->nr; i++) + if (selected[i]) + argv_array_push(&args, items[i]->name); + res = run_command_v_opt(args.argv, 0); + argv_array_clear(&args); + } + + putchar('\n'); + free(selected); + return res; +} + static int run_help(struct add_i_state *s, const struct pathspec *ps, struct file_list *files, struct list_and_choose_options *opts) @@ -939,10 +984,11 @@ int run_add_i(struct repository *r, const struct pathspec *ps) revert = { { "revert" }, run_revert }, add_untracked = { { "add untracked" }, run_add_untracked }, patch = { { "patch" }, run_patch }, + diff = { { "diff" }, run_diff }, help = { { "help" }, run_help }; struct command_item *commands[] = { &status, &update, &revert, &add_untracked, - &patch, &help + &patch, &diff, &help }; struct print_file_item_data print_file_item_data = { From 2e33d358c8aa1d63925ebfd476b488edd8e906df Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 7 Mar 2019 23:37:23 +0100 Subject: [PATCH 24/69] built-in add -i: offer the `quit` command We do not really want to `exit()` here, of course, as this is safely libified code. Signed-off-by: Johannes Schindelin --- add-interactive.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 5de13edc4c4d38..d9bb60ee7af370 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -985,10 +985,11 @@ int run_add_i(struct repository *r, const struct pathspec *ps) add_untracked = { { "add untracked" }, run_add_untracked }, patch = { { "patch" }, run_patch }, diff = { { "diff" }, run_diff }, + quit = { { "quit" }, NULL }, help = { { "help" }, run_help }; struct command_item *commands[] = { &status, &update, &revert, &add_untracked, - &patch, &diff, &help + &patch, &diff, &quit, &help }; struct print_file_item_data print_file_item_data = { @@ -1033,7 +1034,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps) for (;;) { i = list_and_choose((struct prefix_item **)commands, NULL, ARRAY_SIZE(commands), &s, &main_loop_opts); - if (i < -1) { + if (i < -1 || (i >= 0 && !commands[i]->command)) { printf(_("Bye.\n")); res = 0; break; From 45b77161157c0ac71c79cdee8c39328d993781c2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 3 Apr 2019 22:15:39 +0200 Subject: [PATCH 25/69] t3701: add a test for advanced split-hunk editing In this developer's workflows, it often happens that a hunk needs to be edited in a way that adds lines, and even reduces the context Let's add a regression test for this. Note that just like the preceding test case, the new test case is *not* handled gracefully by the current `git add -p`. It will be handled correctly by the upcoming built-in `git add -p`, though. Signed-off-by: Johannes Schindelin --- t/t3701-add-interactive.sh | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 91aaef29323276..986b48f3ad11bf 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -403,6 +403,28 @@ test_expect_failure 'split hunk "add -p (no, yes, edit)"' ' ! grep "^+31" actual ' +test_expect_failure 'edit, adding lines to the first hunk' ' + test_write_lines 10 11 20 30 40 50 51 60 >test && + git reset && + tr _ " " >patch <<-EOF && + @@ -1,5 +1,6 @@ + _10 + +11 + +12 + _20 + +21 + +22 + _30 + EOF + # test sequence is s(plit), e(dit), n(o) + # q n q q is there to make sure we exit at the end. + printf "%s\n" s e n q n q q | + EDITOR=./fake_editor.sh git add -p 2>error && + test_must_be_empty error && + git diff --cached >actual && + grep "^+22" actual +' + test_expect_success 'patch mode ignores unmerged entries' ' git reset --hard && test_commit conflict && From 7e9ef10adbd17f30bebff31fa2054fcd7ee4aed3 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 20:05:24 +0100 Subject: [PATCH 26/69] t3701: avoid depending on the TTY prerequisite The TTY prerequisite is a rather heavy one: it not only requires Perl to work, but also the IO/Pty.pm module (with native support, and it requires pseudo terminals, too). In particular, test cases marked with the TTY prerequisite would be skipped in Git for Windows' SDK. In the case of `git add -p`, we do not actually need that big a hammer, as we do not want to test any functionality that requires a pseudo terminal; all we want is to talk the interactive add command to use color, even when being called from within the test suite. And we found exactly such a trick earlier already: when we added a test case to verify that the main loop of `git add -i` is colored appropriately. Let's use that trick instead of the TTY prerequisite. Signed-off-by: Johannes Schindelin --- t/t3701-add-interactive.sh | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 986b48f3ad11bf..26ef5105819582 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -23,6 +23,17 @@ diff_cmp () { test_cmp "$1.filtered" "$2.filtered" } +# This function uses a trick to manipulate the interactive add to use color: +# the `want_color()` function special-cases the situation where a pager was +# spawned and Git now wants to output colored text: to detect that situation, +# the environment variable `GIT_PAGER_IN_USE` is set. However, color is +# suppressed despite that environment variable if the `TERM` variable +# indicates a dumb terminal, so we set that variable, too. + +force_color () { + env GIT_PAGER_IN_USE=true TERM=vt100 "$@" +} + test_expect_success 'setup (initial)' ' echo content >file && git add file && @@ -451,35 +462,35 @@ test_expect_success 'patch mode ignores unmerged entries' ' diff_cmp expected diff ' -test_expect_success TTY 'diffs can be colorized' ' +test_expect_success 'diffs can be colorized' ' git reset --hard && echo content >test && - printf y | test_terminal git add -p >output 2>&1 && + printf y | force_color git add -p >output 2>&1 && # We do not want to depend on the exact coloring scheme # git uses for diffs, so just check that we saw some kind of color. grep "$(printf "\\033")" output ' -test_expect_success TTY 'diffFilter filters diff' ' +test_expect_success 'diffFilter filters diff' ' git reset --hard && echo content >test && test_config interactive.diffFilter "sed s/^/foo:/" && - printf y | test_terminal git add -p >output 2>&1 && + printf y | force_color git add -p >output 2>&1 && # avoid depending on the exact coloring or content of the prompts, # and just make sure we saw our diff prefixed grep foo:.*content output ' -test_expect_success TTY 'detect bogus diffFilter output' ' +test_expect_success 'detect bogus diffFilter output' ' git reset --hard && echo content >test && test_config interactive.diffFilter "echo too-short" && - printf y | test_must_fail test_terminal git add -p + printf y | test_must_fail force_color git add -p ' test_expect_success 'patch-mode via -i prompts for files' ' @@ -680,7 +691,7 @@ test_expect_success 'show help from add--helper' ' What now>$SP Bye. EOF - test_write_lines h | GIT_PAGER_IN_USE=true TERM=vt100 git add -i >actual.colored && + test_write_lines h | force_color git add -i >actual.colored && test_decode_color actual && test_i18ncmp expect actual ' From 5051b44a39588efadd76a2a600db9067f209cc9e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 11 Mar 2019 22:01:22 +0100 Subject: [PATCH 27/69] built-in add -i: start implementing the `patch` functionality in C In the previous steps, we re-implemented the main loop of `git add -i` in C, and most of the commands. Notably, we left out the actual functionality of `patch`, as the relevant code makes up more than half of `git-add--interactive.perl`, and is actually pretty independent of the rest of the commands. With this commit, we start to tackle that `patch` part. For better separation of concerns, we keep the code in a separate file, `add-patch.c`. The new code is still guarded behind the `add.interactive.useBuiltin` config setting, and for the moment, it can only be called via `git add -p`. The actual functionality follows the original implementation of 5cde71d64aff (git-add --interactive, 2006-12-10), but not too closely (for example, we use string offsets rather than copying strings around, and we also remember which previous/next hunk was undecided, rather than looking again when the user asked to jump there). As a further deviation from that commit, We also use a comma instead of a slash to separate the available commands in the prompt, as the current version of the Perl script does this, and we also add a line about the question mark ("print help") to the help text. Signed-off-by: Johannes Schindelin --- Makefile | 1 + add-interactive.h | 1 + add-patch.c | 260 ++++++++++++++++++++++++++++++++++++++++++++++ builtin/add.c | 10 +- 4 files changed, 270 insertions(+), 2 deletions(-) create mode 100644 add-patch.c diff --git a/Makefile b/Makefile index 8299b3f17da832..c9cb20d5c5127c 100644 --- a/Makefile +++ b/Makefile @@ -850,6 +850,7 @@ LIB_H = $(shell $(FIND) . \ LIB_OBJS += abspath.o LIB_OBJS += add-interactive.o +LIB_OBJS += add-patch.o LIB_OBJS += advice.o LIB_OBJS += alias.o LIB_OBJS += alloc.o diff --git a/add-interactive.h b/add-interactive.h index 7043b8741d7bd3..0e3d93acc93264 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -4,5 +4,6 @@ struct repository; struct pathspec; int run_add_i(struct repository *r, const struct pathspec *ps); +int run_add_p(struct repository *r, const struct pathspec *ps); #endif diff --git a/add-patch.c b/add-patch.c new file mode 100644 index 00000000000000..7ff5d9e764c8cd --- /dev/null +++ b/add-patch.c @@ -0,0 +1,260 @@ +#include "cache.h" +#include "add-interactive.h" +#include "strbuf.h" +#include "run-command.h" +#include "argv-array.h" +#include "pathspec.h" + +struct hunk { + size_t start, end; + enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use; +}; + +struct add_p_state { + struct repository *r; + struct strbuf answer, buf; + + /* parsed diff */ + struct strbuf plain; + struct hunk head; + struct hunk *hunk; + size_t hunk_nr, hunk_alloc; +}; + +static void setup_child_process(struct child_process *cp, + struct add_p_state *s, ...) +{ + va_list ap; + const char *arg; + + va_start(ap, s); + while((arg = va_arg(ap, const char *))) + argv_array_push(&cp->args, arg); + va_end(ap); + + cp->git_cmd = 1; + argv_array_pushf(&cp->env_array, + INDEX_ENVIRONMENT "=%s", s->r->index_file); +} + +static int parse_diff(struct add_p_state *s, const struct pathspec *ps) +{ + struct strbuf *plain = &s->plain; + struct child_process cp = CHILD_PROCESS_INIT; + char *p, *pend; + size_t i; + struct hunk *hunk = NULL; + int res; + + /* Use `--no-color` explicitly, just in case `diff.color = always`. */ + setup_child_process(&cp, s, + "diff-files", "-p", "--no-color", "--", NULL); + for (i = 0; i < ps->nr; i++) + argv_array_push(&cp.args, ps->items[i].original); + + res = capture_command(&cp, plain, 0); + if (res) + return error(_("could not parse diff")); + if (!plain->len) + return 0; + strbuf_complete_line(plain); + + /* parse hunks */ + p = plain->buf; + pend = p + plain->len; + while (p != pend) { + char *eol = memchr(p, '\n', pend - p); + if (!eol) + eol = pend; + + if (starts_with(p, "diff ")) { + if (p != plain->buf) + BUG("multi-file diff not yet handled"); + hunk = &s->head; + } else if (p == plain->buf) + BUG("diff starts with unexpected line:\n" + "%.*s\n", (int)(eol - p), p); + else if (starts_with(p, "@@ ")) { + s->hunk_nr++; + ALLOC_GROW(s->hunk, s->hunk_nr, + s->hunk_alloc); + hunk = s->hunk + s->hunk_nr - 1; + memset(hunk, 0, sizeof(*hunk)); + + hunk->start = p - plain->buf; + } + + p = eol == pend ? pend : eol + 1; + hunk->end = p - plain->buf; + } + + return 0; +} + +static void render_hunk(struct add_p_state *s, struct hunk *hunk, + struct strbuf *out) +{ + strbuf_add(out, s->plain.buf + hunk->start, + hunk->end - hunk->start); +} + +static void reassemble_patch(struct add_p_state *s, struct strbuf *out) +{ + struct hunk *hunk; + size_t i; + + render_hunk(s, &s->head, out); + + for (i = 0; i < s->hunk_nr; i++) { + hunk = s->hunk + i; + if (hunk->use == USE_HUNK) + render_hunk(s, hunk, out); + } +} + +static const char help_patch_text[] = +N_("y - stage this hunk\n" + "n - do not stage this hunk\n" + "a - stage this and all the remaining hunks\n" + "d - do not stage this hunk nor any of the remaining hunks\n" + "j - leave this hunk undecided, see next undecided hunk\n" + "J - leave this hunk undecided, see next hunk\n" + "k - leave this hunk undecided, see previous undecided hunk\n" + "K - leave this hunk undecided, see previous hunk\n" + "? - print help\n"); + +static int patch_update_file(struct add_p_state *s) +{ + size_t hunk_index = 0; + ssize_t i, undecided_previous, undecided_next; + struct hunk *hunk; + char ch; + struct child_process cp = CHILD_PROCESS_INIT; + + if (!s->hunk_nr) + return 0; + + strbuf_reset(&s->buf); + render_hunk(s, &s->head, &s->buf); + fputs(s->buf.buf, stdout); + for (;;) { + if (hunk_index >= s->hunk_nr) + hunk_index = 0; + hunk = s->hunk + hunk_index; + + undecided_previous = -1; + for (i = hunk_index - 1; i >= 0; i--) + if (s->hunk[i].use == UNDECIDED_HUNK) { + undecided_previous = i; + break; + } + + undecided_next = -1; + for (i = hunk_index + 1; i < s->hunk_nr; i++) + if (s->hunk[i].use == UNDECIDED_HUNK) { + undecided_next = i; + break; + } + + /* Everything decided? */ + if (undecided_previous < 0 && undecided_next < 0 && + hunk->use != UNDECIDED_HUNK) + break; + + strbuf_reset(&s->buf); + render_hunk(s, hunk, &s->buf); + fputs(s->buf.buf, stdout); + + strbuf_reset(&s->buf); + if (undecided_previous >= 0) + strbuf_addstr(&s->buf, ",k"); + if (hunk_index) + strbuf_addstr(&s->buf, ",K"); + if (undecided_next >= 0) + strbuf_addstr(&s->buf, ",j"); + if (hunk_index + 1 < s->hunk_nr) + strbuf_addstr(&s->buf, ",J"); + printf(_("Stage this hunk [y,n,a,d%s,?]? "), s->buf.buf); + fflush(stdout); + if (strbuf_getline(&s->answer, stdin) == EOF) + break; + strbuf_trim_trailing_newline(&s->answer); + + if (!s->answer.len) + continue; + ch = tolower(s->answer.buf[0]); + if (ch == 'y') { + hunk->use = USE_HUNK; +soft_increment: + while (++hunk_index < s->hunk_nr && + s->hunk[hunk_index].use + != UNDECIDED_HUNK) + ; /* continue looking */ + } else if (ch == 'n') { + hunk->use = SKIP_HUNK; + goto soft_increment; + } else if (ch == 'a') { + for (; hunk_index < s->hunk_nr; hunk_index++) { + hunk = s->hunk + hunk_index; + if (hunk->use == UNDECIDED_HUNK) + hunk->use = USE_HUNK; + } + } else if (ch == 'd') { + for (; hunk_index < s->hunk_nr; hunk_index++) { + hunk = s->hunk + hunk_index; + if (hunk->use == UNDECIDED_HUNK) + hunk->use = SKIP_HUNK; + } + } else if (hunk_index && s->answer.buf[0] == 'K') + hunk_index--; + else if (hunk_index + 1 < s->hunk_nr && + s->answer.buf[0] == 'J') + hunk_index++; + else if (undecided_previous >= 0 && + s->answer.buf[0] == 'k') + hunk_index = undecided_previous; + else if (undecided_next >= 0 && s->answer.buf[0] == 'j') + hunk_index = undecided_next; + else + puts(_(help_patch_text)); + } + + /* Any hunk to be used? */ + for (i = 0; i < s->hunk_nr; i++) + if (s->hunk[i].use == USE_HUNK) + break; + + if (i < s->hunk_nr) { + /* At least one hunk selected: apply */ + strbuf_reset(&s->buf); + reassemble_patch(s, &s->buf); + + setup_child_process(&cp, s, "apply", "--cached", NULL); + if (pipe_command(&cp, s->buf.buf, s->buf.len, + NULL, 0, NULL, 0)) + error(_("'git apply --cached' failed")); + repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0); + } + + putchar('\n'); + return 0; +} + +int run_add_p(struct repository *r, const struct pathspec *ps) +{ + struct add_p_state s = { r, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; + + if (repo_refresh_and_write_index(r, REFRESH_QUIET, 0) < 0 || + parse_diff(&s, ps) < 0) { + strbuf_release(&s.plain); + return -1; + } + + if (s.hunk_nr) + patch_update_file(&s); + + strbuf_release(&s.answer); + strbuf_release(&s.buf); + strbuf_release(&s.plain); + return 0; +} diff --git a/builtin/add.c b/builtin/add.c index e19d66e8942440..79edf17bf2bf95 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -192,8 +192,14 @@ int run_add_interactive(const char *revision, const char *patch_mode, git_config_get_bool("add.interactive.usebuiltin", &use_builtin_add_i); - if (use_builtin_add_i == 1 && !patch_mode) - return !!run_add_i(the_repository, pathspec); + if (use_builtin_add_i == 1) { + if (!patch_mode) + return !!run_add_i(the_repository, pathspec); + if (strcmp(patch_mode, "--patch")) + die("'%s' not yet supported in the built-in add -p", + patch_mode); + return !!run_add_p(the_repository, pathspec); + } argv_array_push(&argv, "add--interactive"); if (patch_mode) From 792305785bea17550dcfaec5346e7dc4f3c96279 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 20:36:47 +0100 Subject: [PATCH 28/69] t3701: add a test for the different `add -p` prompts The `git add -p` command offers different prompts for regular diff hunks vs mode change pseudo hunks vs diffs deleting files. Let's cover this in the regresion test suite, in preparation for re-implementing `git add -p` in C. For the mode change prompt, we use a trick that lets this test case pass even on systems without executable bit, i.e. where `core.filemode = false` (such as Windows): we first add the file to the index with `git add --chmod=+x`, and then call `git add -p` with `core.filemode` forced to `true`. The file on disk has no executable bit set, therefore we will see a mode change. Signed-off-by: Johannes Schindelin --- t/t3701-add-interactive.sh | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 26ef5105819582..f2a34b3c4152b3 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -105,7 +105,6 @@ test_expect_success 'revert works (commit)' ' grep "unchanged *+3/-0 file" output ' - test_expect_success 'setup expected' ' cat >expected <<-\EOF EOF @@ -274,6 +273,24 @@ test_expect_success FILEMODE 'stage mode and hunk' ' # end of tests disabled when filemode is not usable +test_expect_success 'different prompts for mode change/deleted' ' + git reset --hard && + >file && + >deleted && + git add --chmod=+x file deleted && + echo changed >file && + rm deleted && + test_write_lines n n n | + git -c core.filemode=true add -p >actual && + sed -n "s/^\(Stage .*?\).*/\1/p" actual >actual.filtered && + cat >expect <<-\EOF && + Stage deletion [y,n,q,a,d,?]? + Stage mode change [y,n,q,a,d,j,J,g,/,?]? + Stage this hunk [y,n,q,a,d,K,g,/,e,?]? + EOF + test_cmp expect actual.filtered +' + test_expect_success 'setup again' ' git reset --hard && test_chmod +x file && From cf4bbd3f84ca63d88bbe16c29a9e1ea6f07a4c4b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 12 Mar 2019 12:52:51 +0100 Subject: [PATCH 29/69] built-in add -i: wire up the new C code for the `patch` command The code in `git-add--interactive.perl` that takes care of the `patch` command can look quite intimidating. There are so many modes in which it can be called, for example. But for the `patch` command in `git add -i`, only one mode is relevant: the `stage` mode. And we just implemented the beginnings of that mode in C so far. So let's use it when `add.interactive.useBuiltin=true`. Now, while the code in `add-patch.c` is far from reaching feature parity with the code in `git-add--interactive.perl` (color is not implemented, the diff algorithm cannot be configured, the colored diff cannot be post-processed via `interactive.diffFilter`, many commands are unimplemented yet, etc), hooking it all up with the part of `git add -i` that is already converted to C makes it easier to test and develop it. Note: at this stage, both the `add.interactive.useBuiltin` config setting is still safely opt-in, and will probably be fore quite some time, to allow for thorough testing "in the wild" without adversely affecting existing users. Signed-off-by: Johannes Schindelin --- add-interactive.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index d9bb60ee7af370..bebe2c8237a1e8 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -831,14 +831,17 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, count = list_and_choose(items, selected, files->nr, s, opts); if (count >= 0) { struct argv_array args = ARGV_ARRAY_INIT; + struct pathspec ps_selected = { 0 }; - argv_array_pushl(&args, "git", "add--interactive", "--patch", - "--", NULL); for (i = 0; i < files->nr; i++) if (selected[i]) argv_array_push(&args, items[i]->name); - res = run_command_v_opt(args.argv, 0); + parse_pathspec(&ps_selected, + PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL, + PATHSPEC_LITERAL_PATH, "", args.argv); + res = run_add_p(s->r, &ps_selected); argv_array_clear(&args); + clear_pathspec(&ps_selected); } free(selected); From f0c70487cc7aab408e8a717dde7afedc00755b77 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 24 Mar 2019 22:51:02 +0100 Subject: [PATCH 30/69] t3701: verify the shown messages when nothing can be added In preparation for re-implementing `git add -p` in pure C (where we will purposefully keep the implementation of `git add -p` separate from the implementation of `git add -i`), let's verify that the user is told the same things as in the Perl version when the diff file is either empty or contains only entries about binary files. Signed-off-by: Johannes Schindelin --- t/t3701-add-interactive.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index f2a34b3c4152b3..8e26fb984455ff 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -291,6 +291,17 @@ test_expect_success 'different prompts for mode change/deleted' ' test_cmp expect actual.filtered ' +test_expect_success 'correct message when there is nothing to do' ' + git reset --hard && + git add -p 2>err && + test_i18ngrep "No changes" err && + printf "\\0123" >binary && + git add binary && + printf "\\0abc" >binary && + git add -p 2>err && + test_i18ngrep "Only binary files changed" err +' + test_expect_success 'setup again' ' git reset --hard && test_chmod +x file && From e79b65b87e76cb8cf0e53b9e2ab149af8198638c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 12 Mar 2019 15:42:10 +0100 Subject: [PATCH 31/69] built-in add -p: show colored hunks by default Just like the Perl version, we now generate two diffs if `color.diff` is set: one with and one without color. Then we parse them in parallel and record which hunks start at which offsets in both. Note that this is a (slight) deviation from the way the Perl version did it: we are no longer reading the output of `diff-files` line by line (which is more natural for Perl than for C), but in one go, and parse everything later, so we might just as well do it in synchrony. Signed-off-by: Johannes Schindelin --- add-patch.c | 79 +++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 62 insertions(+), 17 deletions(-) diff --git a/add-patch.c b/add-patch.c index 7ff5d9e764c8cd..a137036a7e3523 100644 --- a/add-patch.c +++ b/add-patch.c @@ -4,9 +4,10 @@ #include "run-command.h" #include "argv-array.h" #include "pathspec.h" +#include "color.h" struct hunk { - size_t start, end; + size_t start, end, colored_start, colored_end; enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use; }; @@ -15,7 +16,7 @@ struct add_p_state { struct strbuf answer, buf; /* parsed diff */ - struct strbuf plain; + struct strbuf plain, colored; struct hunk head; struct hunk *hunk; size_t hunk_nr, hunk_alloc; @@ -39,26 +40,50 @@ static void setup_child_process(struct child_process *cp, static int parse_diff(struct add_p_state *s, const struct pathspec *ps) { - struct strbuf *plain = &s->plain; + struct argv_array args = ARGV_ARRAY_INIT; + struct strbuf *plain = &s->plain, *colored = NULL; struct child_process cp = CHILD_PROCESS_INIT; - char *p, *pend; - size_t i; + char *p, *pend, *colored_p = NULL, *colored_pend = NULL; + size_t i, color_arg_index; struct hunk *hunk = NULL; int res; /* Use `--no-color` explicitly, just in case `diff.color = always`. */ - setup_child_process(&cp, s, - "diff-files", "-p", "--no-color", "--", NULL); + argv_array_pushl(&args, "diff-files", "-p", "--no-color", "--", NULL); + color_arg_index = args.argc - 2; for (i = 0; i < ps->nr; i++) - argv_array_push(&cp.args, ps->items[i].original); + argv_array_push(&args, ps->items[i].original); + setup_child_process(&cp, s, NULL); + cp.argv = args.argv; res = capture_command(&cp, plain, 0); - if (res) + if (res) { + argv_array_clear(&args); return error(_("could not parse diff")); - if (!plain->len) + } + if (!plain->len) { + argv_array_clear(&args); return 0; + } strbuf_complete_line(plain); + if (want_color_fd(1, -1)) { + struct child_process colored_cp = CHILD_PROCESS_INIT; + + setup_child_process(&colored_cp, s, NULL); + xsnprintf((char *)args.argv[color_arg_index], 8, "--color"); + colored_cp.argv = args.argv; + colored = &s->colored; + res = capture_command(&colored_cp, colored, 0); + argv_array_clear(&args); + if (res) + return error(_("could not parse colored diff")); + strbuf_complete_line(colored); + colored_p = colored->buf; + colored_pend = colored_p + colored->len; + } + argv_array_clear(&args); + /* parse hunks */ p = plain->buf; pend = p + plain->len; @@ -82,20 +107,37 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) memset(hunk, 0, sizeof(*hunk)); hunk->start = p - plain->buf; + if (colored) + hunk->colored_start = colored_p - colored->buf; } p = eol == pend ? pend : eol + 1; hunk->end = p - plain->buf; + + if (colored) { + char *colored_eol = memchr(colored_p, '\n', + colored_pend - colored_p); + if (colored_eol) + colored_p = colored_eol + 1; + else + colored_p = colored_pend; + + hunk->colored_end = colored_p - colored->buf; + } } return 0; } static void render_hunk(struct add_p_state *s, struct hunk *hunk, - struct strbuf *out) + int colored, struct strbuf *out) { - strbuf_add(out, s->plain.buf + hunk->start, - hunk->end - hunk->start); + if (colored) + strbuf_add(out, s->colored.buf + hunk->colored_start, + hunk->colored_end - hunk->colored_start); + else + strbuf_add(out, s->plain.buf + hunk->start, + hunk->end - hunk->start); } static void reassemble_patch(struct add_p_state *s, struct strbuf *out) @@ -103,12 +145,12 @@ static void reassemble_patch(struct add_p_state *s, struct strbuf *out) struct hunk *hunk; size_t i; - render_hunk(s, &s->head, out); + render_hunk(s, &s->head, 0, out); for (i = 0; i < s->hunk_nr; i++) { hunk = s->hunk + i; if (hunk->use == USE_HUNK) - render_hunk(s, hunk, out); + render_hunk(s, hunk, 0, out); } } @@ -130,12 +172,13 @@ static int patch_update_file(struct add_p_state *s) struct hunk *hunk; char ch; struct child_process cp = CHILD_PROCESS_INIT; + int colored = !!s->colored.len; if (!s->hunk_nr) return 0; strbuf_reset(&s->buf); - render_hunk(s, &s->head, &s->buf); + render_hunk(s, &s->head, colored, &s->buf); fputs(s->buf.buf, stdout); for (;;) { if (hunk_index >= s->hunk_nr) @@ -162,7 +205,7 @@ static int patch_update_file(struct add_p_state *s) break; strbuf_reset(&s->buf); - render_hunk(s, hunk, &s->buf); + render_hunk(s, hunk, colored, &s->buf); fputs(s->buf.buf, stdout); strbuf_reset(&s->buf); @@ -247,6 +290,7 @@ int run_add_p(struct repository *r, const struct pathspec *ps) if (repo_refresh_and_write_index(r, REFRESH_QUIET, 0) < 0 || parse_diff(&s, ps) < 0) { strbuf_release(&s.plain); + strbuf_release(&s.colored); return -1; } @@ -256,5 +300,6 @@ int run_add_p(struct repository *r, const struct pathspec *ps) strbuf_release(&s.answer); strbuf_release(&s.buf); strbuf_release(&s.plain); + strbuf_release(&s.colored); return 0; } From d6b95ad71575da9c5ac313b05a043f6aaa745a18 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 19:36:22 +0100 Subject: [PATCH 32/69] t3701: verify that the diff.algorithm config setting is handled Without this patch, there is actually no test in Git's test suite that covers the diff.algorithm feature. Let's add one. We do this by passing a bogus value and then expecting `git diff-files` to produce the appropriate error message. Signed-off-by: Johannes Schindelin --- t/t3701-add-interactive.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 8e26fb984455ff..de8060dc78232c 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -521,6 +521,16 @@ test_expect_success 'detect bogus diffFilter output' ' printf y | test_must_fail force_color git add -p ' +test_expect_success 'diff.algorithm is passed to `git diff-files`' ' + git reset --hard && + + >file && + git add file && + echo changed >file && + git -c diff.algorithm=bogus add -p 2>err && + test_i18ngrep "error: option diff-algorithm accepts " err +' + test_expect_success 'patch-mode via -i prompts for files' ' git reset --hard && From 23cbcb5a6d1ffe859642728ba32e758e533f4fdb Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 21 Mar 2019 09:40:20 +0100 Subject: [PATCH 33/69] built-in add -p: adjust hunk headers as needed When skipping a hunk that adds a different number of lines than it removes, we need to adjust the subsequent hunk headers of non-skipped hunks: in pathological cases, the context is not enough to determine precisely where the patch should be applied. This problem was identified in 23fea4c240 (t3701: add failing test for pathological context lines, 2018-03-01) and fixed in the Perl version in fecc6f3a68 (add -p: adjust offsets of subsequent hunks when one is skipped, 2018-03-01). And this patch fixes it in the C version of `git add -p`. In contrast to the Perl version, we try to keep the extra text on the hunk header (which typically contains the signature of the function whose code is changed in the hunk) intact. Note: while the C version does not support staging mode changes at this stage, we already prepare for this by simply skipping the hunk header if both old and new offset is 0 (this cannot happen for regular hunks, and we will use this as an indicator that we are looking at a special hunk). Likewise, we already prepare for hunk splitting by handling the absence of extra text in the hunk header gracefully: only the first split hunk will have that text, the others will not (indicated by an empty extra text start/end range). Preparing for hunk splitting already at this stage avoids an indentation change of the entire hunk header-printing block later, and is almost as easy to review as without that handling. Signed-off-by: Johannes Schindelin --- add-interactive.c | 15 ++--- add-interactive.h | 15 +++++ add-patch.c | 139 ++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 148 insertions(+), 21 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index bebe2c8237a1e8..728ceb01765551 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -12,16 +12,6 @@ #include "argv-array.h" #include "run-command.h" -struct add_i_state { - struct repository *r; - int use_color; - char header_color[COLOR_MAXLEN]; - char help_color[COLOR_MAXLEN]; - char prompt_color[COLOR_MAXLEN]; - char error_color[COLOR_MAXLEN]; - char reset_color[COLOR_MAXLEN]; -}; - static void init_color(struct repository *r, struct add_i_state *s, const char *slot_name, char *dst, const char *default_color) @@ -38,7 +28,7 @@ static void init_color(struct repository *r, struct add_i_state *s, free(key); } -static int init_add_i_state(struct repository *r, struct add_i_state *s) +int init_add_i_state(struct repository *r, struct add_i_state *s) { const char *value; @@ -57,6 +47,9 @@ static int init_add_i_state(struct repository *r, struct add_i_state *s) init_color(r, s, "error", s->error_color, GIT_COLOR_BOLD_RED); init_color(r, s, "reset", s->reset_color, GIT_COLOR_RESET); + strlcpy(s->fraginfo_color, + diff_get_color(s->use_color, DIFF_FRAGINFO), COLOR_MAXLEN); + return 0; } diff --git a/add-interactive.h b/add-interactive.h index 0e3d93acc93264..e3c6a8bff03fd4 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -1,6 +1,21 @@ #ifndef ADD_INTERACTIVE_H #define ADD_INTERACTIVE_H +#include "color.h" + +struct add_i_state { + struct repository *r; + int use_color; + char header_color[COLOR_MAXLEN]; + char help_color[COLOR_MAXLEN]; + char prompt_color[COLOR_MAXLEN]; + char error_color[COLOR_MAXLEN]; + char reset_color[COLOR_MAXLEN]; + char fraginfo_color[COLOR_MAXLEN]; +}; + +int init_add_i_state(struct repository *r, struct add_i_state *s); + struct repository; struct pathspec; int run_add_i(struct repository *r, const struct pathspec *ps); diff --git a/add-patch.c b/add-patch.c index a137036a7e3523..88826ebedb2ef8 100644 --- a/add-patch.c +++ b/add-patch.c @@ -5,14 +5,26 @@ #include "argv-array.h" #include "pathspec.h" #include "color.h" +#include "diff.h" + +struct hunk_header { + unsigned long old_offset, old_count, new_offset, new_count; + /* + * Start/end offsets to the extra text after the second `@@` in the + * hunk header, e.g. the function signature. This is expected to + * include the newline. + */ + size_t extra_start, extra_end, colored_extra_start, colored_extra_end; +}; struct hunk { size_t start, end, colored_start, colored_end; enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use; + struct hunk_header header; }; struct add_p_state { - struct repository *r; + struct add_i_state s; struct strbuf answer, buf; /* parsed diff */ @@ -35,7 +47,70 @@ static void setup_child_process(struct child_process *cp, cp->git_cmd = 1; argv_array_pushf(&cp->env_array, - INDEX_ENVIRONMENT "=%s", s->r->index_file); + INDEX_ENVIRONMENT "=%s", s->s.r->index_file); +} + +static int parse_range(const char **p, + unsigned long *offset, unsigned long *count) +{ + char *pend; + + *offset = strtoul(*p, &pend, 10); + if (pend == *p) + return -1; + if (*pend != ',') { + *count = 1; + *p = pend; + return 0; + } + *count = strtoul(pend + 1, (char **)p, 10); + return *p == pend + 1 ? -1 : 0; +} + +static int parse_hunk_header(struct add_p_state *s, struct hunk *hunk) +{ + struct hunk_header *header = &hunk->header; + const char *line = s->plain.buf + hunk->start, *p = line; + char *eol = memchr(p, '\n', s->plain.len - hunk->start); + + if (!eol) + eol = s->plain.buf + s->plain.len; + + if (!skip_prefix(p, "@@ -", &p) || + parse_range(&p, &header->old_offset, &header->old_count) < 0 || + !skip_prefix(p, " +", &p) || + parse_range(&p, &header->new_offset, &header->new_count) < 0 || + !skip_prefix(p, " @@", &p)) + return error(_("could not parse hunk header '%.*s'"), + (int)(eol - line), line); + + hunk->start = eol - s->plain.buf + (*eol == '\n'); + header->extra_start = p - s->plain.buf; + header->extra_end = hunk->start; + + if (!s->colored.len) { + header->colored_extra_start = header->colored_extra_end = 0; + return 0; + } + + /* Now find the extra text in the colored diff */ + line = s->colored.buf + hunk->colored_start; + eol = memchr(line, '\n', s->colored.len - hunk->colored_start); + if (!eol) + eol = s->colored.buf + s->colored.len; + p = memmem(line, eol - line, "@@ -", 4); + if (!p) + return error(_("could not parse colored hunk header '%.*s'"), + (int)(eol - line), line); + p = memmem(p + 4, eol - p - 4, " @@", 3); + if (!p) + return error(_("could not parse colored hunk header '%.*s'"), + (int)(eol - line), line); + hunk->colored_start = eol - s->colored.buf + (*eol == '\n'); + header->colored_extra_start = p + 3 - s->colored.buf; + header->colored_extra_end = hunk->colored_start; + + return 0; } static int parse_diff(struct add_p_state *s, const struct pathspec *ps) @@ -109,6 +184,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) hunk->start = p - plain->buf; if (colored) hunk->colored_start = colored_p - colored->buf; + + if (parse_hunk_header(s, hunk) < 0) + return -1; } p = eol == pend ? pend : eol + 1; @@ -130,8 +208,40 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } static void render_hunk(struct add_p_state *s, struct hunk *hunk, - int colored, struct strbuf *out) + ssize_t delta, int colored, struct strbuf *out) { + struct hunk_header *header = &hunk->header; + + if (hunk->header.old_offset != 0 || hunk->header.new_offset != 0) { + /* + * Generate the hunk header dynamically, except for special + * hunks (such as the diff header). + */ + const char *p; + size_t len; + + if (!colored) { + p = s->plain.buf + header->extra_start; + len = header->extra_end - header->extra_start; + } else { + strbuf_addstr(out, s->s.fraginfo_color); + p = s->colored.buf + header->colored_extra_start; + len = header->colored_extra_end + - header->colored_extra_start; + } + + strbuf_addf(out, "@@ -%lu,%lu +%lu,%lu @@", + header->old_offset, header->old_count, + (unsigned long)(header->new_offset + delta), + header->new_count); + if (len) + strbuf_add(out, p, len); + else if (colored) + strbuf_addf(out, "%s\n", GIT_COLOR_RESET); + else + strbuf_addch(out, '\n'); + } + if (colored) strbuf_add(out, s->colored.buf + hunk->colored_start, hunk->colored_end - hunk->colored_start); @@ -144,13 +254,17 @@ static void reassemble_patch(struct add_p_state *s, struct strbuf *out) { struct hunk *hunk; size_t i; + ssize_t delta = 0; - render_hunk(s, &s->head, 0, out); + render_hunk(s, &s->head, 0, 0, out); for (i = 0; i < s->hunk_nr; i++) { hunk = s->hunk + i; - if (hunk->use == USE_HUNK) - render_hunk(s, hunk, 0, out); + if (hunk->use != USE_HUNK) + delta += hunk->header.old_count + - hunk->header.new_count; + else + render_hunk(s, hunk, delta, 0, out); } } @@ -178,7 +292,7 @@ static int patch_update_file(struct add_p_state *s) return 0; strbuf_reset(&s->buf); - render_hunk(s, &s->head, colored, &s->buf); + render_hunk(s, &s->head, 0, colored, &s->buf); fputs(s->buf.buf, stdout); for (;;) { if (hunk_index >= s->hunk_nr) @@ -205,7 +319,7 @@ static int patch_update_file(struct add_p_state *s) break; strbuf_reset(&s->buf); - render_hunk(s, hunk, colored, &s->buf); + render_hunk(s, hunk, 0, colored, &s->buf); fputs(s->buf.buf, stdout); strbuf_reset(&s->buf); @@ -276,7 +390,7 @@ static int patch_update_file(struct add_p_state *s) if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) error(_("'git apply --cached' failed")); - repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0); + repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0); } putchar('\n'); @@ -285,7 +399,12 @@ static int patch_update_file(struct add_p_state *s) int run_add_p(struct repository *r, const struct pathspec *ps) { - struct add_p_state s = { r, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; + struct add_p_state s = { + { r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT + }; + + if (init_add_i_state(r, &s.s)) + return error("Could not read `add -i` config"); if (repo_refresh_and_write_index(r, REFRESH_QUIET, 0) < 0 || parse_diff(&s, ps) < 0) { From 36aeb305aebe61cb216f99b294eac91774692bc9 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 17:37:45 +0100 Subject: [PATCH 34/69] git add -p: use non-zero exit code when the diff generation failed The first thing `git add -p` does is to generate a diff. If this diff cannot be generated, `git add -p` should not continue as if nothing happened, but instead fail. What we *actually* do here is much broader: we now verify for *every* `run_cmd_pipe()` call that the spawned process actually succeeded. Note that we have to change two callers in this patch, as we need to store the spawned process' output in a local variable, which means that the callers can no longer decide whether to interpret the `return <$fh>` in array or in scalar context. This bug was noticed while writing a test case for the diff.algorithm feature, and we let that test case double as a regression test for this fixed bug, too. Signed-off-by: Johannes Schindelin --- git-add--interactive.perl | 8 +++++--- t/t3701-add-interactive.sh | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 20eb81cc92f947..841a33b630c53a 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -163,7 +163,9 @@ sub run_cmd_pipe { } else { my $fh = undef; open($fh, '-|', @_) or die; - return <$fh>; + my @out = <$fh>; + close $fh || die "Cannot close @_ ($!)"; + return @out; } } @@ -210,7 +212,7 @@ sub list_untracked { sub get_empty_tree { return $empty_tree if defined $empty_tree; - $empty_tree = run_cmd_pipe(qw(git hash-object -t tree /dev/null)); + ($empty_tree) = run_cmd_pipe(qw(git hash-object -t tree /dev/null)); chomp $empty_tree; return $empty_tree; } @@ -1103,7 +1105,7 @@ sub edit_hunk_manually { EOF2 close $fh; - chomp(my $editor = run_cmd_pipe(qw(git var GIT_EDITOR))); + chomp(my ($editor) = run_cmd_pipe(qw(git var GIT_EDITOR))); system('sh', '-c', $editor.' "$@"', $editor, $hunkfile); if ($? != 0) { diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index de8060dc78232c..852a9f85163fad 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -527,7 +527,7 @@ test_expect_success 'diff.algorithm is passed to `git diff-files`' ' >file && git add file && echo changed >file && - git -c diff.algorithm=bogus add -p 2>err && + test_must_fail git -c diff.algorithm=bogus add -p 2>err && test_i18ngrep "error: option diff-algorithm accepts " err ' From bd92652348c146b398ce5f8c6e7a975ec3fef644 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 12 Mar 2019 16:40:46 +0100 Subject: [PATCH 35/69] built-in add -p: color the prompt and the help text ... just like the Perl version ;-) Note that this requires the `get_add_i_color()` function being defined globally, which is the entire reason why we gave it such a descriptive name in the first place. Signed-off-by: Johannes Schindelin --- add-interactive.h | 9 +++++++++ add-patch.c | 7 +++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/add-interactive.h b/add-interactive.h index e3c6a8bff03fd4..eb024066ba55bb 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -16,6 +16,15 @@ struct add_i_state { int init_add_i_state(struct repository *r, struct add_i_state *s); +enum color_add_i { + COLOR_HEADER = 0, + COLOR_HELP, + COLOR_PROMPT, + COLOR_ERROR, + COLOR_RESET, +}; +const char *get_add_i_color(enum color_add_i ix); + struct repository; struct pathspec; int run_add_i(struct repository *r, const struct pathspec *ps); diff --git a/add-patch.c b/add-patch.c index 88826ebedb2ef8..300d4cd1f5fe9e 100644 --- a/add-patch.c +++ b/add-patch.c @@ -331,7 +331,9 @@ static int patch_update_file(struct add_p_state *s) strbuf_addstr(&s->buf, ",j"); if (hunk_index + 1 < s->hunk_nr) strbuf_addstr(&s->buf, ",J"); - printf(_("Stage this hunk [y,n,a,d%s,?]? "), s->buf.buf); + color_fprintf(stdout, s->s.prompt_color, + _("Stage this hunk [y,n,a,d%s,?]? "), + s->buf.buf); fflush(stdout); if (strbuf_getline(&s->answer, stdin) == EOF) break; @@ -373,7 +375,8 @@ static int patch_update_file(struct add_p_state *s) else if (undecided_next >= 0 && s->answer.buf[0] == 'j') hunk_index = undecided_next; else - puts(_(help_patch_text)); + color_fprintf(stdout, s->s.help_color, + _(help_patch_text)); } /* Any hunk to be used? */ From cb1a9e2522d44b6c48790955ec577d31b922ac2d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 20 Mar 2019 12:10:11 +0100 Subject: [PATCH 36/69] apply --allow-overlap: fix a corner case Yes, yes, this is supposed to be only a band-aid option for `git add -p` not Doing The Right Thing. But as long as we carry the `--allow-overlap` option, we might just as well get it right. This fixes the case where one hunk inserts a line before the first one, and a hunk whose context overlaps with the first one's appends a line at the end. Signed-off-by: Johannes Schindelin --- apply.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/apply.c b/apply.c index 892ede5a318f75..27b6fd90f7e233 100644 --- a/apply.c +++ b/apply.c @@ -2676,6 +2676,16 @@ static int find_pos(struct apply_state *state, unsigned long backwards, forwards, current; int backwards_lno, forwards_lno, current_lno; + /* + * When running with --allow-overlap, it is possible that a hunk is + * seen that pretends to start at the beginning (but no longer does), + * and that *still* needs to match the end. So trust `match_end` more + * than `match_beginning`. + */ + if (state->allow_overlap && match_beginning && match_end && + img->nr - preimage->nr != 0) + match_beginning = 0; + /* * If match_beginning or match_end is specified, there is no * point starting from a wrong line that will never match and From 7c75f7465209b1e3dc98b6ea79f10f0cf5eb49e7 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 17 Mar 2019 21:10:47 +0100 Subject: [PATCH 37/69] built-in add -p: offer a helpful error message when hunk navigation failed ... just like the Perl version currently does... Signed-off-by: Johannes Schindelin --- add-patch.c | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/add-patch.c b/add-patch.c index 300d4cd1f5fe9e..f8a7099a018e47 100644 --- a/add-patch.c +++ b/add-patch.c @@ -34,6 +34,17 @@ struct add_p_state { size_t hunk_nr, hunk_alloc; }; +static void err(struct add_p_state *s, const char *fmt, ...) +{ + va_list args; + + va_start(args, fmt); + fputs(s->s.error_color, stderr); + vfprintf(stderr, fmt, args); + fputs(s->s.reset_color, stderr); + fputc('\n', stderr); +} + static void setup_child_process(struct child_process *cp, struct add_p_state *s, ...) { @@ -364,17 +375,27 @@ static int patch_update_file(struct add_p_state *s) if (hunk->use == UNDECIDED_HUNK) hunk->use = SKIP_HUNK; } - } else if (hunk_index && s->answer.buf[0] == 'K') - hunk_index--; - else if (hunk_index + 1 < s->hunk_nr && - s->answer.buf[0] == 'J') - hunk_index++; - else if (undecided_previous >= 0 && - s->answer.buf[0] == 'k') - hunk_index = undecided_previous; - else if (undecided_next >= 0 && s->answer.buf[0] == 'j') - hunk_index = undecided_next; - else + } else if (s->answer.buf[0] == 'K') { + if (hunk_index) + hunk_index--; + else + err(s, _("No previous hunk")); + } else if (s->answer.buf[0] == 'J') { + if (hunk_index + 1 < s->hunk_nr) + hunk_index++; + else + err(s, _("No next hunk")); + } else if (s->answer.buf[0] == 'k') { + if (undecided_previous >= 0) + hunk_index = undecided_previous; + else + err(s, _("No previous hunk")); + } else if (s->answer.buf[0] == 'j') { + if (undecided_next >= 0) + hunk_index = undecided_next; + else + err(s, _("No next hunk")); + } else color_fprintf(stdout, s->s.help_color, _(help_patch_text)); } From 794a9082320ce0c1432f97b815e1260a219215c2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 15 Mar 2019 11:11:43 +0100 Subject: [PATCH 38/69] built-in add -p: support multi-file diffs For simplicity, the initial implementation in C handled only a single modified file. Now it handles an arbitrary number of files. Signed-off-by: Johannes Schindelin --- add-patch.c | 90 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 38 deletions(-) diff --git a/add-patch.c b/add-patch.c index f8a7099a018e47..0c620a67ae64d9 100644 --- a/add-patch.c +++ b/add-patch.c @@ -29,9 +29,12 @@ struct add_p_state { /* parsed diff */ struct strbuf plain, colored; - struct hunk head; - struct hunk *hunk; - size_t hunk_nr, hunk_alloc; + struct file_diff { + struct hunk head; + struct hunk *hunk; + size_t hunk_nr, hunk_alloc; + } *file_diff; + size_t file_diff_nr; }; static void err(struct add_p_state *s, const char *fmt, ...) @@ -130,7 +133,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) struct strbuf *plain = &s->plain, *colored = NULL; struct child_process cp = CHILD_PROCESS_INIT; char *p, *pend, *colored_p = NULL, *colored_pend = NULL; - size_t i, color_arg_index; + size_t file_diff_alloc = 0, i, color_arg_index; + struct file_diff *file_diff = NULL; struct hunk *hunk = NULL; int res; @@ -170,7 +174,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } argv_array_clear(&args); - /* parse hunks */ + /* parse files and hunks */ p = plain->buf; pend = p + plain->len; while (p != pend) { @@ -179,17 +183,23 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) eol = pend; if (starts_with(p, "diff ")) { - if (p != plain->buf) - BUG("multi-file diff not yet handled"); - hunk = &s->head; + s->file_diff_nr++; + ALLOC_GROW(s->file_diff, s->file_diff_nr, + file_diff_alloc); + file_diff = s->file_diff + s->file_diff_nr - 1; + memset(file_diff, 0, sizeof(*file_diff)); + hunk = &file_diff->head; + hunk->start = p - plain->buf; + if (colored_p) + hunk->colored_start = colored_p - colored->buf; } else if (p == plain->buf) BUG("diff starts with unexpected line:\n" "%.*s\n", (int)(eol - p), p); else if (starts_with(p, "@@ ")) { - s->hunk_nr++; - ALLOC_GROW(s->hunk, s->hunk_nr, - s->hunk_alloc); - hunk = s->hunk + s->hunk_nr - 1; + file_diff->hunk_nr++; + ALLOC_GROW(file_diff->hunk, file_diff->hunk_nr, + file_diff->hunk_alloc); + hunk = file_diff->hunk + file_diff->hunk_nr - 1; memset(hunk, 0, sizeof(*hunk)); hunk->start = p - plain->buf; @@ -261,16 +271,17 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, hunk->end - hunk->start); } -static void reassemble_patch(struct add_p_state *s, struct strbuf *out) +static void reassemble_patch(struct add_p_state *s, + struct file_diff *file_diff, struct strbuf *out) { struct hunk *hunk; size_t i; ssize_t delta = 0; - render_hunk(s, &s->head, 0, 0, out); + render_hunk(s, &file_diff->head, 0, 0, out); - for (i = 0; i < s->hunk_nr; i++) { - hunk = s->hunk + i; + for (i = 0; i < file_diff->hunk_nr; i++) { + hunk = file_diff->hunk + i; if (hunk->use != USE_HUNK) delta += hunk->header.old_count - hunk->header.new_count; @@ -290,7 +301,8 @@ N_("y - stage this hunk\n" "K - leave this hunk undecided, see previous hunk\n" "? - print help\n"); -static int patch_update_file(struct add_p_state *s) +static int patch_update_file(struct add_p_state *s, + struct file_diff *file_diff) { size_t hunk_index = 0; ssize_t i, undecided_previous, undecided_next; @@ -299,27 +311,27 @@ static int patch_update_file(struct add_p_state *s) struct child_process cp = CHILD_PROCESS_INIT; int colored = !!s->colored.len; - if (!s->hunk_nr) + if (!file_diff->hunk_nr) return 0; strbuf_reset(&s->buf); - render_hunk(s, &s->head, 0, colored, &s->buf); + render_hunk(s, &file_diff->head, 0, colored, &s->buf); fputs(s->buf.buf, stdout); for (;;) { - if (hunk_index >= s->hunk_nr) + if (hunk_index >= file_diff->hunk_nr) hunk_index = 0; - hunk = s->hunk + hunk_index; + hunk = file_diff->hunk + hunk_index; undecided_previous = -1; for (i = hunk_index - 1; i >= 0; i--) - if (s->hunk[i].use == UNDECIDED_HUNK) { + if (file_diff->hunk[i].use == UNDECIDED_HUNK) { undecided_previous = i; break; } undecided_next = -1; - for (i = hunk_index + 1; i < s->hunk_nr; i++) - if (s->hunk[i].use == UNDECIDED_HUNK) { + for (i = hunk_index + 1; i < file_diff->hunk_nr; i++) + if (file_diff->hunk[i].use == UNDECIDED_HUNK) { undecided_next = i; break; } @@ -340,7 +352,7 @@ static int patch_update_file(struct add_p_state *s) strbuf_addstr(&s->buf, ",K"); if (undecided_next >= 0) strbuf_addstr(&s->buf, ",j"); - if (hunk_index + 1 < s->hunk_nr) + if (hunk_index + 1 < file_diff->hunk_nr) strbuf_addstr(&s->buf, ",J"); color_fprintf(stdout, s->s.prompt_color, _("Stage this hunk [y,n,a,d%s,?]? "), @@ -356,22 +368,22 @@ static int patch_update_file(struct add_p_state *s) if (ch == 'y') { hunk->use = USE_HUNK; soft_increment: - while (++hunk_index < s->hunk_nr && - s->hunk[hunk_index].use + while (++hunk_index < file_diff->hunk_nr && + file_diff->hunk[hunk_index].use != UNDECIDED_HUNK) ; /* continue looking */ } else if (ch == 'n') { hunk->use = SKIP_HUNK; goto soft_increment; } else if (ch == 'a') { - for (; hunk_index < s->hunk_nr; hunk_index++) { - hunk = s->hunk + hunk_index; + for (; hunk_index < file_diff->hunk_nr; hunk_index++) { + hunk = file_diff->hunk + hunk_index; if (hunk->use == UNDECIDED_HUNK) hunk->use = USE_HUNK; } } else if (ch == 'd') { - for (; hunk_index < s->hunk_nr; hunk_index++) { - hunk = s->hunk + hunk_index; + for (; hunk_index < file_diff->hunk_nr; hunk_index++) { + hunk = file_diff->hunk + hunk_index; if (hunk->use == UNDECIDED_HUNK) hunk->use = SKIP_HUNK; } @@ -381,7 +393,7 @@ static int patch_update_file(struct add_p_state *s) else err(s, _("No previous hunk")); } else if (s->answer.buf[0] == 'J') { - if (hunk_index + 1 < s->hunk_nr) + if (hunk_index + 1 < file_diff->hunk_nr) hunk_index++; else err(s, _("No next hunk")); @@ -401,14 +413,14 @@ static int patch_update_file(struct add_p_state *s) } /* Any hunk to be used? */ - for (i = 0; i < s->hunk_nr; i++) - if (s->hunk[i].use == USE_HUNK) + for (i = 0; i < file_diff->hunk_nr; i++) + if (file_diff->hunk[i].use == USE_HUNK) break; - if (i < s->hunk_nr) { + if (i < file_diff->hunk_nr) { /* At least one hunk selected: apply */ strbuf_reset(&s->buf); - reassemble_patch(s, &s->buf); + reassemble_patch(s, file_diff, &s->buf); setup_child_process(&cp, s, "apply", "--cached", NULL); if (pipe_command(&cp, s->buf.buf, s->buf.len, @@ -426,6 +438,7 @@ int run_add_p(struct repository *r, const struct pathspec *ps) struct add_p_state s = { { r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; + size_t i; if (init_add_i_state(r, &s.s)) return error("Could not read `add -i` config"); @@ -437,8 +450,9 @@ int run_add_p(struct repository *r, const struct pathspec *ps) return -1; } - if (s.hunk_nr) - patch_update_file(&s); + for (i = 0; i < s.file_diff_nr; i++) + if (patch_update_file(&s, s.file_diff + i)) + break; strbuf_release(&s.answer); strbuf_release(&s.buf); From 6b873ab74ffea09c1b907a76daf9c87e87c7faef Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 15 Mar 2019 17:32:44 +0100 Subject: [PATCH 39/69] built-in add -p: handle deleted empty files This addresses the same problem as 24ab81ae4d (add-interactive: handle deletion of empty files, 2009-10-27), although in a different way: we not only stick the "deleted file" line into its own pseudo hunk, but also the entire remainder (if any) of the same diff. That way, we do not have to play any funny games with regards to coalescing the diff after the user selected what (possibly pseudo-)hunks to stage. Signed-off-by: Johannes Schindelin --- add-patch.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/add-patch.c b/add-patch.c index 0c620a67ae64d9..c8d41759d1aee7 100644 --- a/add-patch.c +++ b/add-patch.c @@ -33,6 +33,7 @@ struct add_p_state { struct hunk head; struct hunk *hunk; size_t hunk_nr, hunk_alloc; + unsigned deleted:1; } *file_diff; size_t file_diff_nr; }; @@ -179,6 +180,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) pend = p + plain->len; while (p != pend) { char *eol = memchr(p, '\n', pend - p); + const char *deleted = NULL; + if (!eol) eol = pend; @@ -195,7 +198,11 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } else if (p == plain->buf) BUG("diff starts with unexpected line:\n" "%.*s\n", (int)(eol - p), p); - else if (starts_with(p, "@@ ")) { + else if (file_diff->deleted) + ; /* keep the rest of the file in a single "hunk" */ + else if (starts_with(p, "@@ ") || + (hunk == &file_diff->head && + skip_prefix(p, "deleted file", &deleted))) { file_diff->hunk_nr++; ALLOC_GROW(file_diff->hunk, file_diff->hunk_nr, file_diff->hunk_alloc); @@ -206,7 +213,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) if (colored) hunk->colored_start = colored_p - colored->buf; - if (parse_hunk_header(s, hunk) < 0) + if (deleted) + file_diff->deleted = 1; + else if (parse_hunk_header(s, hunk) < 0) return -1; } From 21be30664e043dcfa78c6b9be4c49d3b24ce6a55 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 15 Mar 2019 17:59:11 +0100 Subject: [PATCH 40/69] built-in app -p: allow selecting a mode change as a "hunk" This imitates the way the Perl version treats mode changes: it offers the mode change up for the user to decide, as if it was a diff hunk. In contrast to the Perl version, we make use of the fact that the mode line is the first hunk, and explicitly strip out that line from the diff header if that "hunk" was not selected to be applied, and skipping that hunk while coalescing the diff. The Perl version plays some kind of diff line lego instead. Signed-off-by: Johannes Schindelin --- add-patch.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 75 insertions(+), 5 deletions(-) diff --git a/add-patch.c b/add-patch.c index c8d41759d1aee7..8ed626f9e302a0 100644 --- a/add-patch.c +++ b/add-patch.c @@ -33,7 +33,7 @@ struct add_p_state { struct hunk head; struct hunk *hunk; size_t hunk_nr, hunk_alloc; - unsigned deleted:1; + unsigned deleted:1, mode_change:1; } *file_diff; size_t file_diff_nr; }; @@ -128,6 +128,14 @@ static int parse_hunk_header(struct add_p_state *s, struct hunk *hunk) return 0; } +static int is_octal(const char *p, size_t len) +{ + while (len--) + if (*p < '0' || *(p++) > '7') + return 0; + return 1; +} + static int parse_diff(struct add_p_state *s, const struct pathspec *ps) { struct argv_array args = ARGV_ARRAY_INIT; @@ -180,7 +188,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) pend = p + plain->len; while (p != pend) { char *eol = memchr(p, '\n', pend - p); - const char *deleted = NULL; + const char *deleted = NULL, *mode_change = NULL; if (!eol) eol = pend; @@ -217,8 +225,30 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) file_diff->deleted = 1; else if (parse_hunk_header(s, hunk) < 0) return -1; + } else if (hunk == &file_diff->head && + ((skip_prefix(p, "old mode ", &mode_change) || + skip_prefix(p, "new mode ", &mode_change)) && + is_octal(mode_change, eol - mode_change))) { + if (!file_diff->mode_change) { + if (file_diff->hunk_nr++) + BUG("mode change before first hunk"); + ALLOC_GROW(file_diff->hunk, file_diff->hunk_nr, + file_diff->hunk_alloc); + memset(file_diff->hunk, 0, sizeof(struct hunk)); + file_diff->hunk->start = p - plain->buf; + if (colored_p) + file_diff->hunk->colored_start = + colored_p - colored->buf; + file_diff->mode_change = 1; + } else if (file_diff->hunk_nr != 1) + BUG("mode change after first hunk?"); } + if (file_diff->deleted && file_diff->mode_change) + BUG("diff contains delete *and* a mode change?!?\n%.*s", + (int)(eol - (plain->buf + file_diff->head.start)), + plain->buf + file_diff->head.start); + p = eol == pend ? pend : eol + 1; hunk->end = p - plain->buf; @@ -232,6 +262,13 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) hunk->colored_end = colored_p - colored->buf; } + + if (mode_change) { + file_diff->hunk->end = hunk->end; + if (colored_p) + file_diff->hunk->colored_end = + hunk->colored_end; + } } return 0; @@ -280,6 +317,39 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, hunk->end - hunk->start); } +static void render_diff_header(struct add_p_state *s, + struct file_diff *file_diff, int colored, + struct strbuf *out) +{ + /* + * If there was a mode change, the first hunk is a pseudo hunk that + * corresponds to the mode line in the header. If the user did not want + * to stage that "hunk", we actually have to cut it out from the header. + */ + int skip_mode_change = + file_diff->mode_change && file_diff->hunk->use != USE_HUNK; + struct hunk *head = &file_diff->head, *first = file_diff->hunk; + + if (!skip_mode_change) { + render_hunk(s, head, 0, colored, out); + return; + } + + if (colored) { + const char *p = s->colored.buf; + + strbuf_add(out, p + head->colored_start, + first->colored_start - head->colored_start); + strbuf_add(out, p + first->colored_end, + head->colored_end - first->colored_end); + } else { + const char *p = s->plain.buf; + + strbuf_add(out, p + head->start, first->start - head->start); + strbuf_add(out, p + first->end, head->end - first->end); + } +} + static void reassemble_patch(struct add_p_state *s, struct file_diff *file_diff, struct strbuf *out) { @@ -287,9 +357,9 @@ static void reassemble_patch(struct add_p_state *s, size_t i; ssize_t delta = 0; - render_hunk(s, &file_diff->head, 0, 0, out); + render_diff_header(s, file_diff, 0, out); - for (i = 0; i < file_diff->hunk_nr; i++) { + for (i = file_diff->mode_change; i < file_diff->hunk_nr; i++) { hunk = file_diff->hunk + i; if (hunk->use != USE_HUNK) delta += hunk->header.old_count @@ -324,7 +394,7 @@ static int patch_update_file(struct add_p_state *s, return 0; strbuf_reset(&s->buf); - render_hunk(s, &file_diff->head, 0, colored, &s->buf); + render_diff_header(s, file_diff, colored, &s->buf); fputs(s->buf.buf, stdout); for (;;) { if (hunk_index >= file_diff->hunk_nr) From 590a2419278a417fe6f2f152e107ee0a3a1bc57d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 21:33:55 +0100 Subject: [PATCH 41/69] built-in add -p: show different prompts for mode changes and deletions Just like the Perl version, we now helpfully ask the user whether they want to stage a mode change, or a deletion. Note that we define the prompts in an array, in preparation for a later patch that changes those prompts to yet different versions for `git reset -p`, `git stash -p` and `git checkout -p` (which all call the `git add -p` machinery to do the actual work). Signed-off-by: Johannes Schindelin --- add-patch.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/add-patch.c b/add-patch.c index 8ed626f9e302a0..957b605d3d67b6 100644 --- a/add-patch.c +++ b/add-patch.c @@ -7,6 +7,16 @@ #include "color.h" #include "diff.h" +enum prompt_mode_type { + PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK +}; + +static const char *prompt_mode[] = { + N_("Stage mode change [y,n,a,d%s,?]? "), + N_("Stage deletion [y,n,a,d%s,?]? "), + N_("Stage this hunk [y,n,a,d%s,?]? ") +}; + struct hunk_header { unsigned long old_offset, old_count, new_offset, new_count; /* @@ -389,6 +399,7 @@ static int patch_update_file(struct add_p_state *s, char ch; struct child_process cp = CHILD_PROCESS_INIT; int colored = !!s->colored.len; + enum prompt_mode_type prompt_mode_type; if (!file_diff->hunk_nr) return 0; @@ -433,9 +444,16 @@ static int patch_update_file(struct add_p_state *s, strbuf_addstr(&s->buf, ",j"); if (hunk_index + 1 < file_diff->hunk_nr) strbuf_addstr(&s->buf, ",J"); + + if (file_diff->deleted) + prompt_mode_type = PROMPT_DELETION; + else if (file_diff->mode_change && !hunk_index) + prompt_mode_type = PROMPT_MODE_CHANGE; + else + prompt_mode_type = PROMPT_HUNK; + color_fprintf(stdout, s->s.prompt_color, - _("Stage this hunk [y,n,a,d%s,?]? "), - s->buf.buf); + _(prompt_mode[prompt_mode_type]), s->buf.buf); fflush(stdout); if (strbuf_getline(&s->answer, stdin) == EOF) break; From 5988354d2a41591cacbe867562e8cdf8cdc21fb6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 17 Mar 2019 21:12:41 +0100 Subject: [PATCH 42/69] built-in add -p: implement the hunk splitting feature If this developer's workflow is any indication, then this is *the* most useful feature of Git's interactive `add `command. Note: once again, this is not a verbatim conversion from the Perl code to C: the `hunk_splittable()` function, for example, essentially did all the work of splitting the hunk, just to find out whether more than one hunk would have been the result (and then tossed that result into the trash). In C we instead count the number of resulting hunks (without actually doing the work of splitting, but just counting the transitions from non-context lines to context lines), and store that information with the hunk, and we do that *while* parsing the diff in the first place. Another deviation: the built-in `git add -p` was designed with a single strbuf holding the diff (and another one holding the colored diff, if that one was asked for) in mind, and hunks essentially store just the start and end offsets pointing into that strbuf. As a consequence, when we split hunks, we now use a special mode where the hunk header is generated dynamically, and only the rest of the hunk is stored using such start/end offsets. This way, we also avoid the frequent formatting/re-parsing of the hunk header of the Perl version. Signed-off-by: Johannes Schindelin --- add-patch.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 182 insertions(+), 2 deletions(-) diff --git a/add-patch.c b/add-patch.c index 957b605d3d67b6..04050217c7fdd1 100644 --- a/add-patch.c +++ b/add-patch.c @@ -28,7 +28,7 @@ struct hunk_header { }; struct hunk { - size_t start, end, colored_start, colored_end; + size_t start, end, colored_start, colored_end, splittable_into; enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use; struct hunk_header header; }; @@ -151,7 +151,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) struct argv_array args = ARGV_ARRAY_INIT; struct strbuf *plain = &s->plain, *colored = NULL; struct child_process cp = CHILD_PROCESS_INIT; - char *p, *pend, *colored_p = NULL, *colored_pend = NULL; + char *p, *pend, *colored_p = NULL, *colored_pend = NULL, marker = '\0'; size_t file_diff_alloc = 0, i, color_arg_index; struct file_diff *file_diff = NULL; struct hunk *hunk = NULL; @@ -221,6 +221,13 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) else if (starts_with(p, "@@ ") || (hunk == &file_diff->head && skip_prefix(p, "deleted file", &deleted))) { + if (marker == '-' || marker == '+') + /* + * Should not happen; previous hunk did not end + * in a context line? Handle it anyway. + */ + hunk->splittable_into++; + file_diff->hunk_nr++; ALLOC_GROW(file_diff->hunk, file_diff->hunk_nr, file_diff->hunk_alloc); @@ -235,6 +242,12 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) file_diff->deleted = 1; else if (parse_hunk_header(s, hunk) < 0) return -1; + + /* + * Start counting into how many hunks this one can be + * split + */ + marker = *p; } else if (hunk == &file_diff->head && ((skip_prefix(p, "old mode ", &mode_change) || skip_prefix(p, "new mode ", &mode_change)) && @@ -259,6 +272,12 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) (int)(eol - (plain->buf + file_diff->head.start)), plain->buf + file_diff->head.start); + if ((marker == '-' || marker == '+') && + (*p == ' ' || *p == '\\')) + hunk->splittable_into++; + if (marker) + marker = *p; + p = eol == pend ? pend : eol + 1; hunk->end = p - plain->buf; @@ -281,9 +300,25 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } } + if (marker == '-' || marker == '+') + /* + * Last hunk ended in non-context line (i.e. it appended lines + * to the file, so there are no trailing context lines). + */ + hunk->splittable_into++; + return 0; } +static size_t find_next_line(struct strbuf *sb, size_t offset) +{ + char *eol = memchr(sb->buf + offset, '\n', sb->len - offset); + + if (!eol) + return sb->len; + return eol - sb->buf + 1; +} + static void render_hunk(struct add_p_state *s, struct hunk *hunk, ssize_t delta, int colored, struct strbuf *out) { @@ -379,6 +414,139 @@ static void reassemble_patch(struct add_p_state *s, } } +static int split_hunk(struct add_p_state *s, struct file_diff *file_diff, + size_t hunk_index) +{ + int colored = !!s->colored.len, first = 1; + struct hunk *hunk = file_diff->hunk + hunk_index; + size_t splittable_into; + size_t end, colored_end, current, colored_current = 0, context_line_count; + struct hunk_header remaining, *header; + char marker, ch; + + if (hunk_index >= file_diff->hunk_nr) + BUG("invalid hunk index: %d (must be >= 0 and < %d)", + (int)hunk_index, (int)file_diff->hunk_nr); + + if (hunk->splittable_into < 2) + return 0; + splittable_into = hunk->splittable_into; + + end = hunk->end; + colored_end = hunk->colored_end; + + memcpy(&remaining, &hunk->header, sizeof(remaining)); + + file_diff->hunk_nr += splittable_into - 1; + ALLOC_GROW(file_diff->hunk, file_diff->hunk_nr, file_diff->hunk_alloc); + if (hunk_index + splittable_into < file_diff->hunk_nr) + memmove(file_diff->hunk + hunk_index + splittable_into, + file_diff->hunk + hunk_index + 1, + (file_diff->hunk_nr - hunk_index - splittable_into) + * sizeof(*hunk)); + hunk = file_diff->hunk + hunk_index; + hunk->splittable_into = 1; + memset(hunk + 1, 0, (splittable_into - 1) * sizeof(*hunk)); + + header = &hunk->header; + header->old_count = header->new_count = 0; + + current = hunk->start; + if (colored) + colored_current = hunk->colored_start; + marker = '\0'; + context_line_count = 0; + + while (splittable_into > 1) { + ch = s->plain.buf[current]; + if ((marker == '-' || marker == '+') && ch == ' ') { + first = 0; + hunk[1].start = current; + if (colored) + hunk[1].colored_start = colored_current; + context_line_count = 0; + } + + if (marker != ' ' || (ch != '-' && ch != '+')) { +next_hunk_line: + /* current hunk not done yet */ + if (ch == ' ') + context_line_count++; + else if (ch == '-') + header->old_count++; + else if (ch == '+') + header->new_count++; + else + BUG("unhandled diff marker: '%c'", ch); + marker = ch; + current = find_next_line(&s->plain, current); + if (colored) + colored_current = + find_next_line(&s->colored, + colored_current); + continue; + } + + if (first) { + if (header->old_count || header->new_count) + BUG("counts are off: %d/%d", + (int)header->old_count, + (int)header->new_count); + + header->old_count = context_line_count; + header->new_count = context_line_count; + context_line_count = 0; + first = 0; + goto next_hunk_line; + } + + remaining.old_offset += header->old_count; + remaining.old_count -= header->old_count; + remaining.new_offset += header->new_count; + remaining.new_count -= header->new_count; + + /* initialize next hunk header's offsets */ + hunk[1].header.old_offset = + header->old_offset + header->old_count; + hunk[1].header.new_offset = + header->new_offset + header->new_count; + + /* add one split hunk */ + header->old_count += context_line_count; + header->new_count += context_line_count; + + hunk->end = current; + if (colored) + hunk->colored_end = colored_current; + + hunk++; + hunk->splittable_into = 1; + hunk->use = hunk[-1].use; + header = &hunk->header; + + header->old_count = header->new_count = context_line_count; + context_line_count = 0; + + splittable_into--; + marker = ch; + } + + /* last hunk simply gets the rest */ + if (header->old_offset != remaining.old_offset) + BUG("miscounted old_offset: %lu != %lu", + header->old_offset, remaining.old_offset); + if (header->new_offset != remaining.new_offset) + BUG("miscounted new_offset: %lu != %lu", + header->new_offset, remaining.new_offset); + header->old_count = remaining.old_count; + header->new_count = remaining.new_count; + hunk->end = end; + if (colored) + hunk->colored_end = colored_end; + + return 0; +} + static const char help_patch_text[] = N_("y - stage this hunk\n" "n - do not stage this hunk\n" @@ -388,6 +556,7 @@ N_("y - stage this hunk\n" "J - leave this hunk undecided, see next hunk\n" "k - leave this hunk undecided, see previous undecided hunk\n" "K - leave this hunk undecided, see previous hunk\n" + "s - split the current hunk into smaller hunks\n" "? - print help\n"); static int patch_update_file(struct add_p_state *s, @@ -444,6 +613,8 @@ static int patch_update_file(struct add_p_state *s, strbuf_addstr(&s->buf, ",j"); if (hunk_index + 1 < file_diff->hunk_nr) strbuf_addstr(&s->buf, ",J"); + if (hunk->splittable_into > 1) + strbuf_addstr(&s->buf, ",s"); if (file_diff->deleted) prompt_mode_type = PROMPT_DELETION; @@ -504,6 +675,15 @@ static int patch_update_file(struct add_p_state *s, hunk_index = undecided_next; else err(s, _("No next hunk")); + } else if (s->answer.buf[0] == 's') { + size_t splittable_into = hunk->splittable_into; + if (splittable_into < 2) + err(s, _("Sorry, cannot split this hunk")); + else if (!split_hunk(s, file_diff, + hunk - file_diff->hunk)) + color_fprintf_ln(stdout, s->s.header_color, + _("Split into %d hunks."), + (int)splittable_into); } else color_fprintf(stdout, s->s.help_color, _(help_patch_text)); From 73bcd8ead58c39d73f5dec97c7aa71e9ab159555 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 19 Mar 2019 20:50:14 +0100 Subject: [PATCH 43/69] built-in add -p: coalesce hunks after splitting them This is considered "the right thing to do", according to 933e44d3a0 ("add -p": work-around an old laziness that does not coalesce hunks, 2011-04-06). Note: we cannot simply modify the hunks while merging them; Once we implement hunk editing, we will call `reassemble_patch()` whenever a hunk is edited, therefore we must not modify the hunks (because the user might e.g. hit `K` and change their mind whether to stage the previous hunk). Signed-off-by: Johannes Schindelin --- add-patch.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/add-patch.c b/add-patch.c index 04050217c7fdd1..f3a113982ad885 100644 --- a/add-patch.c +++ b/add-patch.c @@ -395,6 +395,48 @@ static void render_diff_header(struct add_p_state *s, } } +/* Coalesce hunks again that were split */ +static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff, + size_t *hunk_index, struct hunk *temp) +{ + size_t i = *hunk_index; + struct hunk *hunk = file_diff->hunk + i; + struct hunk_header *header = &temp->header, *next; + + if (hunk->use != USE_HUNK) + return 0; + + memcpy(temp, hunk, sizeof(*temp)); + /* We simply skip the colored part (if any) when merging hunks */ + temp->colored_start = temp->colored_end = 0; + + for (; i + 1 < file_diff->hunk_nr; i++) { + hunk++; + next = &hunk->header; + + if (hunk->use != USE_HUNK || + header->new_offset >= next->new_offset || + header->new_offset + header->new_count < next->new_offset || + temp->start >= hunk->start || + temp->end < hunk->start) + break; + + temp->end = hunk->end; + temp->colored_end = hunk->colored_end; + + header->old_count = next->old_offset + next->old_count + - header->old_offset; + header->new_count = next->new_offset + next->new_count + - header->new_offset; + } + + if (i == *hunk_index) + return 0; + + *hunk_index = i; + return 1; +} + static void reassemble_patch(struct add_p_state *s, struct file_diff *file_diff, struct strbuf *out) { @@ -405,12 +447,19 @@ static void reassemble_patch(struct add_p_state *s, render_diff_header(s, file_diff, 0, out); for (i = file_diff->mode_change; i < file_diff->hunk_nr; i++) { + struct hunk temp = { 0 }; + hunk = file_diff->hunk + i; if (hunk->use != USE_HUNK) delta += hunk->header.old_count - hunk->header.new_count; - else + else { + /* merge overlapping hunks into a temporary hunk */ + if (merge_hunks(s, file_diff, &i, &temp)) + hunk = &temp; + render_hunk(s, hunk, delta, 0, out); + } } } From 30a2c35eba901df12431fa7e4ceafe55c554bd5b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 19 Mar 2019 22:27:51 +0100 Subject: [PATCH 44/69] built-in add -p: implement hunk editing Just like `git add --edit` allows the user to edit the diff before it is being applied to the index, this feature allows the user to edit the diff *hunk*. Naturally, it gets a bit more complicated here because the result has to play well with the remaining hunks of the overall diff. Therefore, we have to do a loop in which we let the user edit the hunk, then test whether the result would work, and if not, drop the edits and let the user decide whether to try editing the hunk again. Note: in contrast to the Perl version, we use the same diff "coalescing" (i.e. merging overlapping hunks into a single one) also for the check after editing, and we introduce a new flag for that purpose that asks the `reassemble_patch()` function to pretend that all hunks were selected for use. This allows us to continue to run `git apply` *without* the `--allow-overlap` option (unlike the Perl version), and it also fixes two known breakages in `t3701-add-interactive.sh` (which we cannot mark as resolved so far because the Perl script version is still the default and continues to have those breakages). Signed-off-by: Johannes Schindelin --- add-interactive.c | 7 + add-interactive.h | 3 + add-patch.c | 351 +++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 344 insertions(+), 17 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 728ceb01765551..71e159c2d2cf73 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -49,6 +49,13 @@ int init_add_i_state(struct repository *r, struct add_i_state *s) strlcpy(s->fraginfo_color, diff_get_color(s->use_color, DIFF_FRAGINFO), COLOR_MAXLEN); + strlcpy(s->context_color, + diff_get_color(s->use_color, DIFF_CONTEXT), COLOR_MAXLEN); + strlcpy(s->file_old_color, + diff_get_color(s->use_color, DIFF_FILE_OLD), COLOR_MAXLEN); + strlcpy(s->file_new_color, + diff_get_color(s->use_color, DIFF_FILE_NEW), COLOR_MAXLEN); + return 0; } diff --git a/add-interactive.h b/add-interactive.h index eb024066ba55bb..cc16b9436b3034 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -12,6 +12,9 @@ struct add_i_state { char error_color[COLOR_MAXLEN]; char reset_color[COLOR_MAXLEN]; char fraginfo_color[COLOR_MAXLEN]; + char context_color[COLOR_MAXLEN]; + char file_old_color[COLOR_MAXLEN]; + char file_new_color[COLOR_MAXLEN]; }; int init_add_i_state(struct repository *r, struct add_i_state *s); diff --git a/add-patch.c b/add-patch.c index f3a113982ad885..864f233dc6348f 100644 --- a/add-patch.c +++ b/add-patch.c @@ -29,6 +29,7 @@ struct hunk_header { struct hunk { size_t start, end, colored_start, colored_end, splittable_into; + ssize_t delta; enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use; struct hunk_header header; }; @@ -397,13 +398,13 @@ static void render_diff_header(struct add_p_state *s, /* Coalesce hunks again that were split */ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff, - size_t *hunk_index, struct hunk *temp) + size_t *hunk_index, int use_all, struct hunk *temp) { - size_t i = *hunk_index; + size_t i = *hunk_index, delta; struct hunk *hunk = file_diff->hunk + i; struct hunk_header *header = &temp->header, *next; - if (hunk->use != USE_HUNK) + if (!use_all && hunk->use != USE_HUNK) return 0; memcpy(temp, hunk, sizeof(*temp)); @@ -414,20 +415,94 @@ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff, hunk++; next = &hunk->header; - if (hunk->use != USE_HUNK || - header->new_offset >= next->new_offset || - header->new_offset + header->new_count < next->new_offset || - temp->start >= hunk->start || - temp->end < hunk->start) + if ((!use_all && hunk->use != USE_HUNK) || + header->new_offset >= next->new_offset + temp->delta || + header->new_offset + header->new_count + < next->new_offset + temp->delta) break; - temp->end = hunk->end; - temp->colored_end = hunk->colored_end; + if (temp->start < hunk->start && temp->end > hunk->start) { + temp->end = hunk->end; + temp->colored_end = hunk->colored_end; + delta = 0; + } else { + const char *plain = s->plain.buf; + size_t overlapping_line_count = header->new_offset + + header->new_count - temp->delta + - next->new_offset; + size_t overlap_end = hunk->start; + size_t overlap_start = overlap_end; + size_t overlap_next, len, i; + + /* + * One of the hunks was edited; let's ensure that at + * least the last context line of the first hunk + * overlaps with the corresponding line of the second + * hunk, and then merge. + */ + + for (i = 0; i < overlapping_line_count; i++) { + overlap_next = find_next_line(&s->plain, + overlap_end); + + if (overlap_next > hunk->end) + BUG("failed to find %d context lines " + "in:\n%.*s", + (int)overlapping_line_count, + (int)(hunk->end - hunk->start), + plain + hunk->start); + + if (plain[overlap_end] != ' ') + return error(_("expected context line " + "#%d in\n%.*s"), + (int)(i + 1), + (int)(hunk->end + - hunk->start), + plain + hunk->start); + + overlap_start = overlap_end; + overlap_end = overlap_next; + } + len = overlap_end - overlap_start; + + if (len > temp->end - temp->start || + memcmp(plain + temp->end - len, + plain + overlap_start, len)) + return error(_("hunks do not overlap:\n%.*s\n" + "\tdoes not end with:\n%.*s"), + (int)(temp->end - temp->start), + plain + temp->start, + (int)len, plain + overlap_start); + + /* + * Since the start-end ranges are not adjacent, we + * cannot simply take the union of the ranges. To + * address that, we temporarily append the union of the + * lines to the `plain` strbuf. + */ + if (temp->end != s->plain.len) { + size_t start = s->plain.len; + + strbuf_add(&s->plain, plain + temp->start, + temp->end - temp->start); + plain = s->plain.buf; + temp->start = start; + temp->end = s->plain.len; + } + + strbuf_add(&s->plain, + plain + overlap_end, + hunk->end - overlap_end); + temp->end = s->plain.len; + temp->splittable_into += hunk->splittable_into; + delta = temp->delta; + temp->delta += hunk->delta; + } header->old_count = next->old_offset + next->old_count - header->old_offset; - header->new_count = next->new_offset + next->new_count - - header->new_offset; + header->new_count = next->new_offset + delta + + next->new_count - header->new_offset; } if (i == *hunk_index) @@ -438,10 +513,11 @@ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff, } static void reassemble_patch(struct add_p_state *s, - struct file_diff *file_diff, struct strbuf *out) + struct file_diff *file_diff, int use_all, + struct strbuf *out) { struct hunk *hunk; - size_t i; + size_t save_len = s->plain.len, i; ssize_t delta = 0; render_diff_header(s, file_diff, 0, out); @@ -450,15 +526,24 @@ static void reassemble_patch(struct add_p_state *s, struct hunk temp = { 0 }; hunk = file_diff->hunk + i; - if (hunk->use != USE_HUNK) + if (!use_all && hunk->use != USE_HUNK) delta += hunk->header.old_count - hunk->header.new_count; else { /* merge overlapping hunks into a temporary hunk */ - if (merge_hunks(s, file_diff, &i, &temp)) + if (merge_hunks(s, file_diff, &i, use_all, &temp)) hunk = &temp; render_hunk(s, hunk, delta, 0, out); + + /* + * In case `merge_hunks()` used `plain` as a scratch + * pad (this happens when an edited hunk had to be + * coalesced with another hunk). + */ + strbuf_setlen(&s->plain, save_len); + + delta += hunk->delta; } } } @@ -596,6 +681,227 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff, return 0; } +static void recolor_hunk(struct add_p_state *s, struct hunk *hunk) +{ + const char *plain = s->plain.buf; + size_t current, eol, next; + + if (!s->colored.len) + return; + + hunk->colored_start = s->colored.len; + for (current = hunk->start; current < hunk->end; ) { + for (eol = current; eol < hunk->end; eol++) + if (plain[eol] == '\n') + break; + next = eol + (eol < hunk->end); + if (eol > current && plain[eol - 1] == '\r') + eol--; + + strbuf_addstr(&s->colored, + plain[current] == '-' ? + s->s.file_old_color : + plain[current] == '+' ? + s->s.file_new_color : + s->s.context_color); + strbuf_add(&s->colored, plain + current, eol - current); + strbuf_addstr(&s->colored, GIT_COLOR_RESET); + if (next > eol) + strbuf_add(&s->colored, plain + eol, next - eol); + current = next; + } + hunk->colored_end = s->colored.len; +} + +static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk) +{ + char *path = xstrdup(git_path("addp-hunk-edit.diff")); + int fd = xopen(path, O_WRONLY | O_CREAT | O_TRUNC, 0666); + struct strbuf buf = STRBUF_INIT; + size_t i, j; + int res, copy; + + if (fd < 0) { + res = error_errno(_("could not open '%s' for writing"), path); + goto edit_hunk_manually_finish; + } + + strbuf_commented_addf(&buf, _("Manual hunk edit mode -- see bottom for " + "a quick guide.\n")); + render_hunk(s, hunk, 0, 0, &buf); + strbuf_commented_addf(&buf, + _("---\n" + "To remove '%c' lines, make them ' ' lines " + "(context).\n" + "To remove '%c' lines, delete them.\n" + "Lines starting with %c will be removed.\n"), + '-', '+', comment_line_char); + strbuf_commented_addf(&buf, + _("If the patch applies cleanly, the edited hunk " + "will immediately be\n" + "marked for staging.\n")); + /* + * TRANSLATORS: 'it' refers to the patch mentioned in the previous + * messages. + */ + strbuf_commented_addf(&buf, + _("If it does not apply cleanly, you will be " + "given an opportunity to\n" + "edit again. If all lines of the hunk are " + "removed, then the edit is\n" + "aborted and the hunk is left unchanged.\n")); + if (write_in_full(fd, buf.buf, buf.len) < 0) { + res = error_errno(_("could not write to '%s'"), path); + goto edit_hunk_manually_finish; + } + + res = close(fd); + fd = -1; + if (res < 0) + goto edit_hunk_manually_finish; + + hunk->start = s->plain.len; + if (launch_editor(path, &s->plain, NULL) < 0) { + res = error_errno(_("could not edit '%s'"), path); + goto edit_hunk_manually_finish; + } + unlink(path); + + /* strip out commented lines */ + copy = s->plain.buf[hunk->start] != comment_line_char; + for (i = j = hunk->start; i < s->plain.len; ) { + if (copy) + s->plain.buf[j++] = s->plain.buf[i]; + if (s->plain.buf[i++] == '\n') + copy = s->plain.buf[i] != comment_line_char; + } + + if (j == hunk->start) + /* User aborted by deleting everything */ + goto edit_hunk_manually_finish; + + res = 1; + strbuf_setlen(&s->plain, j); + hunk->end = j; + recolor_hunk(s, hunk); + if (s->plain.buf[hunk->start] == '@' && + /* If the hunk header was deleted, simply use the original one. */ + parse_hunk_header(s, hunk) < 0) + res = -1; + +edit_hunk_manually_finish: + if (fd >= 0) + close(fd); + free(path); + strbuf_release(&buf); + + return res; +} + +static ssize_t recount_edited_hunk(struct add_p_state *s, struct hunk *hunk, + size_t orig_old_count, size_t orig_new_count) +{ + struct hunk_header *header = &hunk->header; + size_t i; + + header->old_count = header->new_count = 0; + for (i = hunk->start; i < hunk->end; ) { + switch (s->plain.buf[i]) { + case '-': + header->old_count++; + break; + case '+': + header->new_count++; + break; + case ' ': case '\r': case '\n': + header->old_count++; + header->new_count++; + break; + } + + i = find_next_line(&s->plain, i); + } + + return orig_old_count - orig_new_count + - header->old_count + header->new_count; +} + +static int run_apply_check(struct add_p_state *s, + struct file_diff *file_diff) +{ + struct child_process cp = CHILD_PROCESS_INIT; + + strbuf_reset(&s->buf); + reassemble_patch(s, file_diff, 1, &s->buf); + + setup_child_process(&cp, s, + "apply", "--cached", "--check", NULL); + if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) + return error(_("'git apply --cached' failed")); + + return 0; +} + +static int prompt_yesno(struct add_p_state *s, const char *prompt) +{ + for (;;) { + color_fprintf(stdout, s->s.prompt_color, "%s", _(prompt)); + fflush(stdout); + if (strbuf_getline(&s->answer, stdin) == EOF) + return -1; + strbuf_trim_trailing_newline(&s->answer); + switch (tolower(s->answer.buf[0])) { + case 'n': return 0; + case 'y': return 1; + } + } +} + +static int edit_hunk_loop(struct add_p_state *s, + struct file_diff *file_diff, struct hunk *hunk) +{ + size_t plain_len = s->plain.len, colored_len = s->colored.len; + struct hunk backup; + + memcpy(&backup, hunk, sizeof(backup)); + + for (;;) { + int res = edit_hunk_manually(s, hunk); + if (res == 0) { + /* abandonded */ + memcpy(hunk, &backup, sizeof(backup)); + return -1; + } + + if (res > 0) { + hunk->delta += + recount_edited_hunk(s, hunk, + backup.header.old_count, + backup.header.new_count); + if (!run_apply_check(s, file_diff)) + return 0; + } + + /* Drop edits (they were appended to s->plain) */ + strbuf_setlen(&s->plain, plain_len); + strbuf_setlen(&s->colored, colored_len); + memcpy(hunk, &backup, sizeof(backup)); + + /* + * TRANSLATORS: do not translate [y/n] + * The program will only accept that input at this point. + * Consider translating (saying "no" discards!) as + * (saying "n" for "no" discards!) if the translation + * of the word "no" does not start with n. + */ + res = prompt_yesno(s, _("Your edited hunk does not apply. " + "Edit again (saying \"no\" discards!) " + "[y/n]? ")); + if (res < 1) + return -1; + } +} + static const char help_patch_text[] = N_("y - stage this hunk\n" "n - do not stage this hunk\n" @@ -606,6 +912,7 @@ N_("y - stage this hunk\n" "k - leave this hunk undecided, see previous undecided hunk\n" "K - leave this hunk undecided, see previous hunk\n" "s - split the current hunk into smaller hunks\n" + "e - manually edit the current hunk\n" "? - print help\n"); static int patch_update_file(struct add_p_state *s, @@ -664,6 +971,9 @@ static int patch_update_file(struct add_p_state *s, strbuf_addstr(&s->buf, ",J"); if (hunk->splittable_into > 1) strbuf_addstr(&s->buf, ",s"); + if (hunk_index + 1 > file_diff->mode_change && + !file_diff->deleted) + strbuf_addstr(&s->buf, ",e"); if (file_diff->deleted) prompt_mode_type = PROMPT_DELETION; @@ -733,6 +1043,13 @@ static int patch_update_file(struct add_p_state *s, color_fprintf_ln(stdout, s->s.header_color, _("Split into %d hunks."), (int)splittable_into); + } else if (s->answer.buf[0] == 'e') { + if (hunk_index + 1 == file_diff->mode_change) + err(s, _("Sorry, cannot edit this hunk")); + else if (edit_hunk_loop(s, file_diff, hunk) >= 0) { + hunk->use = USE_HUNK; + goto soft_increment; + } } else color_fprintf(stdout, s->s.help_color, _(help_patch_text)); @@ -746,7 +1063,7 @@ static int patch_update_file(struct add_p_state *s, if (i < file_diff->hunk_nr) { /* At least one hunk selected: apply */ strbuf_reset(&s->buf); - reassemble_patch(s, file_diff, &s->buf); + reassemble_patch(s, file_diff, 0, &s->buf); setup_child_process(&cp, s, "apply", "--cached", NULL); if (pipe_command(&cp, s->buf.buf, s->buf.len, From fb601aebc3d98675364d66ee854a060c88f527de Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 21 Mar 2019 23:50:53 +0100 Subject: [PATCH 45/69] built-in add -p: implement the 'g' ("goto") command With this patch, it is now possible to see a summary of the available hunks and to navigate between them (by number). A test is added to verify that this behavior matches the one of the Perl version of `git add -p`. Signed-off-by: Johannes Schindelin --- add-patch.c | 86 ++++++++++++++++++++++++++++++++++++++ t/t3701-add-interactive.sh | 16 +++++++ 2 files changed, 102 insertions(+) diff --git a/add-patch.c b/add-patch.c index 864f233dc6348f..cd3d58891ff866 100644 --- a/add-patch.c +++ b/add-patch.c @@ -902,6 +902,54 @@ static int edit_hunk_loop(struct add_p_state *s, } } +#define SUMMARY_HEADER_WIDTH 20 +#define SUMMARY_LINE_WIDTH 80 +static void summarize_hunk(struct add_p_state *s, struct hunk *hunk, + struct strbuf *out) +{ + struct hunk_header *header = &hunk->header; + struct strbuf *plain = &s->plain; + size_t len = out->len, i; + + strbuf_addf(out, " -%lu,%lu +%lu,%lu ", + header->old_offset, header->old_count, + header->new_offset, header->new_count); + if (out->len - len < SUMMARY_HEADER_WIDTH) + strbuf_addchars(out, ' ', + SUMMARY_HEADER_WIDTH + len - out->len); + for (i = hunk->start; i < hunk->end; i = find_next_line(plain, i)) + if (plain->buf[i] != ' ') + break; + if (i < hunk->end) + strbuf_add(out, plain->buf + i, find_next_line(plain, i) - i); + if (out->len - len > SUMMARY_LINE_WIDTH) + strbuf_setlen(out, len + SUMMARY_LINE_WIDTH); + strbuf_complete_line(out); +} + +#define DISPLAY_HUNKS_LINES 20 +static size_t display_hunks(struct add_p_state *s, + struct file_diff *file_diff, size_t start_index) +{ + size_t end_index = start_index + DISPLAY_HUNKS_LINES; + + if (end_index > file_diff->hunk_nr) + end_index = file_diff->hunk_nr; + + while (start_index < end_index) { + struct hunk *hunk = file_diff->hunk + start_index++; + + strbuf_reset(&s->buf); + strbuf_addf(&s->buf, "%c%2d: ", hunk->use == USE_HUNK ? '+' + : hunk->use == SKIP_HUNK ? '-' : ' ', + (int)start_index); + summarize_hunk(s, hunk, &s->buf); + fputs(s->buf.buf, stdout); + } + + return end_index; +} + static const char help_patch_text[] = N_("y - stage this hunk\n" "n - do not stage this hunk\n" @@ -911,6 +959,7 @@ N_("y - stage this hunk\n" "J - leave this hunk undecided, see next hunk\n" "k - leave this hunk undecided, see previous undecided hunk\n" "K - leave this hunk undecided, see previous hunk\n" + "g - select a hunk to go to\n" "s - split the current hunk into smaller hunks\n" "e - manually edit the current hunk\n" "? - print help\n"); @@ -969,6 +1018,8 @@ static int patch_update_file(struct add_p_state *s, strbuf_addstr(&s->buf, ",j"); if (hunk_index + 1 < file_diff->hunk_nr) strbuf_addstr(&s->buf, ",J"); + if (file_diff->hunk_nr > 1) + strbuf_addstr(&s->buf, ",g"); if (hunk->splittable_into > 1) strbuf_addstr(&s->buf, ",s"); if (hunk_index + 1 > file_diff->mode_change && @@ -1034,6 +1085,41 @@ static int patch_update_file(struct add_p_state *s, hunk_index = undecided_next; else err(s, _("No next hunk")); + } else if (s->answer.buf[0] == 'g') { + char *pend; + unsigned long response; + + if (file_diff->hunk_nr < 2) { + err(s, _("No other hunks to goto")); + continue; + } + strbuf_remove(&s->answer, 0, 1); + strbuf_trim(&s->answer); + i = hunk_index > 10 ? hunk_index - 10 : 0; + while (s->answer.len == 0) { + i = display_hunks(s, file_diff, i); + printf("%s", i < file_diff->hunk_nr ? + _("go to which hunk ( to see " + "more)? ") : _("go to which hunk? ")); + fflush(stdout); + if (strbuf_getline(&s->answer, + stdin) == EOF) + break; + strbuf_trim_trailing_newline(&s->answer); + } + + strbuf_trim(&s->answer); + response = strtoul(s->answer.buf, &pend, 10); + if (*pend || pend == s->answer.buf) + err(s, _("Invalid number: '%s'"), + s->answer.buf); + else if (0 < response && response <= file_diff->hunk_nr) + hunk_index = response - 1; + else + err(s, Q_("Sorry, only %d hunk available.", + "Sorry, only %d hunks available.", + file_diff->hunk_nr), + (int)file_diff->hunk_nr); } else if (s->answer.buf[0] == 's') { size_t splittable_into = hunk->splittable_into; if (splittable_into < 2) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 852a9f85163fad..32095d1cf9e43c 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -413,6 +413,22 @@ test_expect_success 'split hunk setup' ' test_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test ' +test_expect_success 'goto hunk' ' + test_when_finished "git reset" && + tr _ " " >expect <<-EOF && + Stage this hunk [y,n,q,a,d,K,g,/,e,?]? + 1: -1,2 +1,3 +15 + _ 2: -2,4 +3,8 +21 + go to which hunk? @@ -1,2 +1,3 @@ + _10 + +15 + _20 + Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?_ + EOF + test_write_lines s y g 1 | git add -p >actual && + tail -n 7 actual.trimmed && + test_cmp expect actual.trimmed +' + test_expect_success 'split hunk "add -p (edit)"' ' # Split, say Edit and do nothing. Then: # From 7a8fea2f280dd611bb306a713a7e4dbd0ed8b856 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 21 Mar 2019 23:50:53 +0100 Subject: [PATCH 46/69] built-in add -p: implement the '/' ("search regex") command This patch implements the hunk searching feature in the C version of `git add -p`. A test is added to verify that this behavior matches the one of the Perl version of `git add -p`. Note that this involves a change of behavior: the Perl version uses (of course) the Perl flavor of regular expressions, while this patch uses the regcomp()/regexec(), i.e. POSIX extended regular expressions. In practice, this behavior change is unlikely to matter. Signed-off-by: Johannes Schindelin --- add-patch.c | 50 +++++++++++++++++++++++++++++++++++++- t/t3701-add-interactive.sh | 14 +++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/add-patch.c b/add-patch.c index cd3d58891ff866..26f35e7b2e4cc1 100644 --- a/add-patch.c +++ b/add-patch.c @@ -960,6 +960,7 @@ N_("y - stage this hunk\n" "k - leave this hunk undecided, see previous undecided hunk\n" "K - leave this hunk undecided, see previous hunk\n" "g - select a hunk to go to\n" + "/ - search for a hunk matching the given regex\n" "s - split the current hunk into smaller hunks\n" "e - manually edit the current hunk\n" "? - print help\n"); @@ -1019,7 +1020,7 @@ static int patch_update_file(struct add_p_state *s, if (hunk_index + 1 < file_diff->hunk_nr) strbuf_addstr(&s->buf, ",J"); if (file_diff->hunk_nr > 1) - strbuf_addstr(&s->buf, ",g"); + strbuf_addstr(&s->buf, ",g,/"); if (hunk->splittable_into > 1) strbuf_addstr(&s->buf, ",s"); if (hunk_index + 1 > file_diff->mode_change && @@ -1120,6 +1121,53 @@ static int patch_update_file(struct add_p_state *s, "Sorry, only %d hunks available.", file_diff->hunk_nr), (int)file_diff->hunk_nr); + } else if (s->answer.buf[0] == '/') { + regex_t regex; + int ret; + + if (file_diff->hunk_nr < 2) { + err(s, _("No other hunks to search")); + continue; + } + strbuf_remove(&s->answer, 0, 1); + strbuf_trim_trailing_newline(&s->answer); + if (s->answer.len == 0) { + printf("%s", _("search for regex? ")); + fflush(stdout); + if (strbuf_getline(&s->answer, + stdin) == EOF) + break; + strbuf_trim_trailing_newline(&s->answer); + if (s->answer.len == 0) + continue; + } + ret = regcomp(®ex, s->answer.buf, + REG_EXTENDED | REG_NOSUB | REG_NEWLINE); + if (ret) { + char errbuf[1024]; + + regerror(ret, ®ex, errbuf, sizeof(errbuf)); + err(s, _("Malformed search regexp %s: %s"), + s->answer.buf, errbuf); + continue; + } + i = hunk_index; + for (;;) { + /* render the hunk into a scratch buffer */ + render_hunk(s, file_diff->hunk + i, 0, 0, + &s->buf); + if (regexec(®ex, s->buf.buf, 0, NULL, 0) + != REG_NOMATCH) + break; + i++; + if (i == file_diff->hunk_nr) + i = 0; + if (i != hunk_index) + continue; + err(s, _("No hunk matches the given pattern")); + break; + } + hunk_index = i; } else if (s->answer.buf[0] == 's') { size_t splittable_into = hunk->splittable_into; if (splittable_into < 2) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 32095d1cf9e43c..5f23d71aed5733 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -429,6 +429,20 @@ test_expect_success 'goto hunk' ' test_cmp expect actual.trimmed ' +test_expect_success 'navigate to hunk via regex' ' + test_when_finished "git reset" && + tr _ " " >expect <<-EOF && + Stage this hunk [y,n,q,a,d,K,g,/,e,?]? @@ -1,2 +1,3 @@ + _10 + +15 + _20 + Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?_ + EOF + test_write_lines s y /1,2 | git add -p >actual && + tail -n 5 actual.trimmed && + test_cmp expect actual.trimmed +' + test_expect_success 'split hunk "add -p (edit)"' ' # Split, say Edit and do nothing. Then: # From fdae5e5c7c4d653e20ada8f6cc81e090530838c9 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 22 Mar 2019 01:24:47 +0100 Subject: [PATCH 47/69] built-in add -p: implement the 'q' ("quit") command This command is actually very similar to the 'd' ("do not stage this hunk or any of the later hunks in the file") command: it just does something on top, namely leave the loop and return a value indicating that we're quittin'. Signed-off-by: Johannes Schindelin --- add-patch.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/add-patch.c b/add-patch.c index 26f35e7b2e4cc1..90ea5d451ff776 100644 --- a/add-patch.c +++ b/add-patch.c @@ -12,9 +12,9 @@ enum prompt_mode_type { }; static const char *prompt_mode[] = { - N_("Stage mode change [y,n,a,d%s,?]? "), - N_("Stage deletion [y,n,a,d%s,?]? "), - N_("Stage this hunk [y,n,a,d%s,?]? ") + N_("Stage mode change [y,n,a,q,d%s,?]? "), + N_("Stage deletion [y,n,a,q,d%s,?]? "), + N_("Stage this hunk [y,n,a,q,d%s,?]? ") }; struct hunk_header { @@ -953,6 +953,7 @@ static size_t display_hunks(struct add_p_state *s, static const char help_patch_text[] = N_("y - stage this hunk\n" "n - do not stage this hunk\n" + "q - quit; do not stage this hunk or any of the remaining ones\n" "a - stage this and all the remaining hunks\n" "d - do not stage this hunk nor any of the remaining hunks\n" "j - leave this hunk undecided, see next undecided hunk\n" @@ -973,7 +974,7 @@ static int patch_update_file(struct add_p_state *s, struct hunk *hunk; char ch; struct child_process cp = CHILD_PROCESS_INIT; - int colored = !!s->colored.len; + int colored = !!s->colored.len, quit = 0; enum prompt_mode_type prompt_mode_type; if (!file_diff->hunk_nr) @@ -1060,12 +1061,16 @@ static int patch_update_file(struct add_p_state *s, if (hunk->use == UNDECIDED_HUNK) hunk->use = USE_HUNK; } - } else if (ch == 'd') { + } else if (ch == 'd' || ch == 'q') { for (; hunk_index < file_diff->hunk_nr; hunk_index++) { hunk = file_diff->hunk + hunk_index; if (hunk->use == UNDECIDED_HUNK) hunk->use = SKIP_HUNK; } + if (ch == 'q') { + quit = 1; + break; + } } else if (s->answer.buf[0] == 'K') { if (hunk_index) hunk_index--; @@ -1207,7 +1212,7 @@ static int patch_update_file(struct add_p_state *s, } putchar('\n'); - return 0; + return quit; } int run_add_p(struct repository *r, const struct pathspec *ps) From 5ac62a9275be01fe4629551ae0218eb98a4799ba Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 22:33:49 +0100 Subject: [PATCH 48/69] built-in add -p: prepare for patch modes other than "stage" The Perl script backing `git add -p` is used not only for that command, but also for `git stash -p`, `git reset -p` and `git checkout -p`. In preparation for teaching the C version of `git add -p` to support also the latter commands, let's abstract away what is "stage" specific into a dedicated data structure describing the differences between the patch modes. As we prepare for calling the built-in `git add -p` in `run_add_interactive()` via code paths that have not let `add_config()` do its work, we have to make sure to re-parse the config using that function in those cases. Finally, please note that the Perl version tries to make sure that the diffs are only generated for the modified files. This is not actually necessary, as the calls to Git's diff machinery already perform that work, and perform it well. This makes it unnecessary to port the `FILTER` field of the `%patch_modes` struct, as well as the `get_diff_reference()` function. Signed-off-by: Johannes Schindelin --- add-interactive.c | 2 +- add-interactive.h | 8 ++++- add-patch.c | 83 +++++++++++++++++++++++++++++++++-------------- builtin/add.c | 12 +++++-- 4 files changed, 77 insertions(+), 28 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 71e159c2d2cf73..55a02fe3c653a5 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -839,7 +839,7 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, parse_pathspec(&ps_selected, PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL, PATHSPEC_LITERAL_PATH, "", args.argv); - res = run_add_p(s->r, &ps_selected); + res = run_add_p(s->r, ADD_P_STAGE, NULL, &ps_selected); argv_array_clear(&args); clear_pathspec(&ps_selected); } diff --git a/add-interactive.h b/add-interactive.h index cc16b9436b3034..77c47b7ad1fcff 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -31,6 +31,12 @@ const char *get_add_i_color(enum color_add_i ix); struct repository; struct pathspec; int run_add_i(struct repository *r, const struct pathspec *ps); -int run_add_p(struct repository *r, const struct pathspec *ps); + +enum add_p_mode { + ADD_P_STAGE, +}; + +int run_add_p(struct repository *r, enum add_p_mode mode, + const char *revision, const struct pathspec *ps); #endif diff --git a/add-patch.c b/add-patch.c index ed856b5433761b..5e78ba6831d504 100644 --- a/add-patch.c +++ b/add-patch.c @@ -11,10 +11,33 @@ enum prompt_mode_type { PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK }; -static const char *prompt_mode[] = { - N_("Stage mode change [y,n,a,q,d%s,?]? "), - N_("Stage deletion [y,n,a,q,d%s,?]? "), - N_("Stage this hunk [y,n,a,q,d%s,?]? ") +struct patch_mode { + const char *diff[4], *apply[4], *apply_check[4]; + unsigned is_reverse:1, apply_for_checkout:1; + const char *prompt_mode[PROMPT_HUNK + 1]; + const char *edit_hunk_hint, *help_patch_text; +}; + +static struct patch_mode patch_mode_stage = { + .diff = { "diff-files", NULL }, + .apply = { "--cached", NULL }, + .apply_check = { "--cached", NULL }, + .is_reverse = 0, + .prompt_mode = { + N_("Stage mode change [y,n,q,a,d%s,?]? "), + N_("Stage deletion [y,n,q,a,d%s,?]? "), + N_("Stage this hunk [y,n,q,a,d%s,?]? ") + }, + .edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk " + "will immediately be marked for staging."), + .help_patch_text = + N_("y - stage this hunk\n" + "n - do not stage this hunk\n" + "q - quit; do not stage this hunk or any of the remaining " + "ones\n" + "a - stage this hunk and all later hunks in the file\n" + "d - do not stage this hunk or any of the later hunks in " + "the file\n") }; struct hunk_header { @@ -47,6 +70,10 @@ struct add_p_state { unsigned deleted:1, mode_change:1,binary:1; } *file_diff; size_t file_diff_nr; + + /* patch mode */ + struct patch_mode *mode; + const char *revision; }; static void err(struct add_p_state *s, const char *fmt, ...) @@ -158,9 +185,18 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) struct hunk *hunk = NULL; int res; + argv_array_pushv(&args, s->mode->diff); + if (s->revision) { + struct object_id oid; + argv_array_push(&args, + /* could be on an unborn branch */ + !strcmp("HEAD", s->revision) && + get_oid("HEAD", &oid) ? + empty_tree_oid_hex() : s->revision); + } + color_arg_index = args.argc; /* Use `--no-color` explicitly, just in case `diff.color = always`. */ - argv_array_pushl(&args, "diff-files", "-p", "--no-color", "--", NULL); - color_arg_index = args.argc - 2; + argv_array_pushl(&args, "--no-color", "-p", "--", NULL); for (i = 0; i < ps->nr; i++) argv_array_push(&args, ps->items[i].original); @@ -737,11 +773,10 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk) "(context).\n" "To remove '%c' lines, delete them.\n" "Lines starting with %c will be removed.\n"), - '-', '+', comment_line_char); - strbuf_commented_addf(&buf, - _("If the patch applies cleanly, the edited hunk " - "will immediately be\n" - "marked for staging.\n")); + s->mode->is_reverse ? '+' : '-', + s->mode->is_reverse ? '-' : '+', + comment_line_char); + strbuf_commented_addf(&buf, "%s", _(s->mode->edit_hunk_hint)); /* * TRANSLATORS: 'it' refers to the patch mentioned in the previous * messages. @@ -837,7 +872,8 @@ static int run_apply_check(struct add_p_state *s, reassemble_patch(s, file_diff, 1, &s->buf); setup_child_process(&cp, s, - "apply", "--cached", "--check", NULL); + "apply", "--check", NULL); + argv_array_pushv(&cp.args, s->mode->apply_check); if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) return error(_("'git apply --cached' failed")); @@ -952,13 +988,6 @@ static size_t display_hunks(struct add_p_state *s, return end_index; } -static const char help_patch_text[] = -N_("y - stage this hunk\n" - "n - do not stage this hunk\n" - "q - quit; do not stage this hunk or any of the remaining ones\n" - "a - stage this and all the remaining hunks\n" - "d - do not stage this hunk nor any of the remaining hunks\n"); - static const char help_patch_remainder[] = N_("j - leave this hunk undecided, see next undecided hunk\n" "J - leave this hunk undecided, see next hunk\n" @@ -1040,7 +1069,8 @@ static int patch_update_file(struct add_p_state *s, prompt_mode_type = PROMPT_HUNK; color_fprintf(stdout, s->s.prompt_color, - _(prompt_mode[prompt_mode_type]), s->buf.buf); + _(s->mode->prompt_mode[prompt_mode_type]), + s->buf.buf); fflush(stdout); if (strbuf_getline(&s->answer, stdin) == EOF) break; @@ -1197,7 +1227,7 @@ static int patch_update_file(struct add_p_state *s, const char *p = _(help_patch_remainder), *eol = p; color_fprintf(stdout, s->s.help_color, "%s", - _(help_patch_text)); + _(s->mode->help_patch_text)); /* * Show only those lines of the remainder that are @@ -1230,10 +1260,11 @@ static int patch_update_file(struct add_p_state *s, strbuf_reset(&s->buf); reassemble_patch(s, file_diff, 0, &s->buf); - setup_child_process(&cp, s, "apply", "--cached", NULL); + setup_child_process(&cp, s, "apply", NULL); + argv_array_pushv(&cp.args, s->mode->apply); if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) - error(_("'git apply --cached' failed")); + error(_("'git apply' failed")); repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0); } @@ -1241,7 +1272,8 @@ static int patch_update_file(struct add_p_state *s, return quit; } -int run_add_p(struct repository *r, const struct pathspec *ps) +int run_add_p(struct repository *r, enum add_p_mode mode, + const char *revision, const struct pathspec *ps) { struct add_p_state s = { { r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT @@ -1251,6 +1283,9 @@ int run_add_p(struct repository *r, const struct pathspec *ps) if (init_add_i_state(r, &s.s)) return error("Could not read `add -i` config"); + s.mode = &patch_mode_stage; + s.revision = revision; + if (repo_refresh_and_write_index(r, REFRESH_QUIET, 0) < 0 || parse_diff(&s, ps) < 0) { strbuf_release(&s.plain); diff --git a/builtin/add.c b/builtin/add.c index 79edf17bf2bf95..bf17bffd374153 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -181,6 +181,8 @@ static void refresh(int verbose, const struct pathspec *pathspec) free(seen); } +static int add_config(const char *var, const char *value, void *cb); + int run_add_interactive(const char *revision, const char *patch_mode, const struct pathspec *pathspec) { @@ -193,12 +195,18 @@ int run_add_interactive(const char *revision, const char *patch_mode, &use_builtin_add_i); if (use_builtin_add_i == 1) { + enum add_p_mode mode; + if (!patch_mode) return !!run_add_i(the_repository, pathspec); - if (strcmp(patch_mode, "--patch")) + + if (!strcmp(patch_mode, "--patch")) + mode = ADD_P_STAGE; + else die("'%s' not yet supported in the built-in add -p", patch_mode); - return !!run_add_p(the_repository, pathspec); + + return !!run_add_p(the_repository, mode, revision, pathspec); } argv_array_push(&argv, "add--interactive"); From b6dcc0ac34518659b76fbef0de93523ba8fc2674 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 24 Mar 2019 19:55:08 +0100 Subject: [PATCH 49/69] stash -p: respect the add.interactive.usebuiltin setting As `git add` traditionally did not expose the `--patch=` modes via command-line options, `git stash` had to call `git add--interactive` directly. But this prevents the built-in `add -p` from kicking in, as `add--interactive` is the Perl script. So let's introduce support for an optional `` argument in `git add --patch[=]`, and use that in `git stash -p`, so that the built-in interactive add can do its job if configured. Signed-off-by: Johannes Schindelin --- builtin/add.c | 21 +++++++++++++++------ builtin/commit.c | 3 ++- commit.h | 3 ++- git-stash.sh | 2 +- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index bf17bffd374153..7885294ad12238 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -26,7 +26,8 @@ static const char * const builtin_add_usage[] = { N_("git add [] [--] ..."), NULL }; -static int patch_interactive, add_interactive, edit_interactive; +static const char *patch_interactive; +static int add_interactive, edit_interactive; static int take_worktree_changes; static int add_renormalize; @@ -224,9 +225,11 @@ int run_add_interactive(const char *revision, const char *patch_mode, return status; } -int interactive_add(int argc, const char **argv, const char *prefix, int patch) +int interactive_add(int argc, const char **argv, const char *prefix, + const char *patch_mode) { struct pathspec pathspec; + char buffer[64]; parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_FULL | @@ -234,9 +237,13 @@ int interactive_add(int argc, const char **argv, const char *prefix, int patch) PATHSPEC_PREFIX_ORIGIN, prefix, argv); - return run_add_interactive(NULL, - patch ? "--patch" : NULL, - &pathspec); + if (patch_mode) { + xsnprintf(buffer, sizeof(buffer), "--patch%s%s", + *patch_mode ? "=" : "", patch_mode); + patch_mode = buffer; + } + + return run_add_interactive(NULL, patch_mode, &pathspec); } static int edit_patch(int argc, const char **argv, const char *prefix) @@ -314,7 +321,9 @@ static struct option builtin_add_options[] = { OPT__VERBOSE(&verbose, N_("be verbose")), OPT_GROUP(""), OPT_BOOL('i', "interactive", &add_interactive, N_("interactive picking")), - OPT_BOOL('p', "patch", &patch_interactive, N_("select hunks interactively")), + { OPTION_STRING, 'p', "patch", &patch_interactive, N_("patch-mode"), + N_("select hunks interactively"), PARSE_OPT_OPTARG, NULL, + (intptr_t) "" }, OPT_BOOL('e', "edit", &edit_interactive, N_("edit current diff and apply")), OPT__FORCE(&ignored_too, N_("allow adding otherwise ignored files"), 0), OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked files")), diff --git a/builtin/commit.c b/builtin/commit.c index 2986553d5ffb97..711b10aa900a0c 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -355,7 +355,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT)); setenv(INDEX_ENVIRONMENT, get_lock_file_path(&index_lock), 1); - if (interactive_add(argc, argv, prefix, patch_interactive) != 0) + if (interactive_add(argc, argv, prefix, + patch_interactive ? "" : NULL) != 0) die(_("interactive add failed")); if (old_index_env && *old_index_env) diff --git a/commit.h b/commit.h index 42728c2906608a..3d624adb9a31c7 100644 --- a/commit.h +++ b/commit.h @@ -284,7 +284,8 @@ extern int delayed_reachability_test(struct shallow_info *si, int c); extern void prune_shallow(unsigned options); extern struct trace_key trace_shallow; -extern int interactive_add(int argc, const char **argv, const char *prefix, int patch); +extern int interactive_add(int argc, const char **argv, const char *prefix, + const char *patch_mode); extern int run_add_interactive(const char *revision, const char *patch_mode, const struct pathspec *pathspec); diff --git a/git-stash.sh b/git-stash.sh index 789ce2f41d4a3c..c0163f77fca12d 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -181,7 +181,7 @@ create_stash () { # find out what the user wants GIT_INDEX_FILE="$TMP-index" \ - git add--interactive --patch=stash -- "$@" && + git add --patch=stash -- "$@" && # state of the working tree w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) || From 9efe35e970783a103aad641639d3641e236bf9e3 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 22:38:02 +0100 Subject: [PATCH 50/69] built-in add -p: implement the "stash" and "reset" patch modes The `git stash` and `git reset` commands support a `--patch` option, and both simply hand off to `git add -p` to perform that work. Let's teach the built-in version of `git add -p` do perform that work, too. Signed-off-by: Johannes Schindelin --- add-interactive.h | 2 ++ add-patch.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++- builtin/add.c | 4 +++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/add-interactive.h b/add-interactive.h index 77c47b7ad1fcff..fc182c72eb5995 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -34,6 +34,8 @@ int run_add_i(struct repository *r, const struct pathspec *ps); enum add_p_mode { ADD_P_STAGE, + ADD_P_STASH, + ADD_P_RESET, }; int run_add_p(struct repository *r, enum add_p_mode mode, diff --git a/add-patch.c b/add-patch.c index 5e78ba6831d504..c705d931b48fc1 100644 --- a/add-patch.c +++ b/add-patch.c @@ -40,6 +40,72 @@ static struct patch_mode patch_mode_stage = { "the file\n") }; +static struct patch_mode patch_mode_stash = { + .diff = { "diff-index", "HEAD", NULL }, + .apply = { "--cached", NULL }, + .apply_check = { "--cached", NULL }, + .is_reverse = 0, + .prompt_mode = { + N_("Stash mode change [y,n,q,a,d%s,?]? "), + N_("Stash deletion [y,n,q,a,d%s,?]? "), + N_("Stash this hunk [y,n,q,a,d%s,?]? "), + }, + .edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk " + "will immediately be marked for stashing."), + .help_patch_text = + N_("y - stash this hunk\n" + "n - do not stash this hunk\n" + "q - quit; do not stash this hunk or any of the remaining " + "ones\n" + "a - stash this hunk and all later hunks in the file\n" + "d - do not stash this hunk or any of the later hunks in " + "the file\n"), +}; + +static struct patch_mode patch_mode_reset_head = { + .diff = { "diff-index", "--cached", NULL }, + .apply = { "-R", "--cached", NULL }, + .apply_check = { "-R", "--cached", NULL }, + .is_reverse = 1, + .prompt_mode = { + N_("Unstage mode change [y,n,q,a,d%s,?]? "), + N_("Unstage deletion [y,n,q,a,d%s,?]? "), + N_("Unstage this hunk [y,n,q,a,d%s,?]? "), + }, + .edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk " + "will immediately be marked for unstaging."), + .help_patch_text = + N_("y - unstage this hunk\n" + "n - do not unstage this hunk\n" + "q - quit; do not unstage this hunk or any of the remaining " + "ones\n" + "a - unstage this hunk and all later hunks in the file\n" + "d - do not unstage this hunk or any of the later hunks in " + "the file\n"), +}; + +static struct patch_mode patch_mode_reset_nothead = { + .diff = { "diff-index", "-R", "--cached", NULL }, + .apply = { "--cached", NULL }, + .apply_check = { "--cached", NULL }, + .is_reverse = 0, + .prompt_mode = { + N_("Apply mode change to index [y,n,q,a,d%s,?]? "), + N_("Apply deletion to index [y,n,q,a,d%s,?]? "), + N_("Apply this hunk to index [y,n,q,a,d%s,?]? "), + }, + .edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk " + "will immediately be marked for applying."), + .help_patch_text = + N_("y - apply this hunk to index\n" + "n - do not apply this hunk to index\n" + "q - quit; do not apply this hunk or any of the remaining " + "ones\n" + "a - apply this hunk and all later hunks in the file\n" + "d - do not apply this hunk or any of the later hunks in " + "the file\n"), +}; + struct hunk_header { unsigned long old_offset, old_count, new_offset, new_count; /* @@ -1283,7 +1349,15 @@ int run_add_p(struct repository *r, enum add_p_mode mode, if (init_add_i_state(r, &s.s)) return error("Could not read `add -i` config"); - s.mode = &patch_mode_stage; + if (mode == ADD_P_STASH) + s.mode = &patch_mode_stash; + else if (mode == ADD_P_RESET) { + if (!revision || !strcmp(revision, "HEAD")) + s.mode = &patch_mode_reset_head; + else + s.mode = &patch_mode_reset_nothead; + } else + s.mode = &patch_mode_stage; s.revision = revision; if (repo_refresh_and_write_index(r, REFRESH_QUIET, 0) < 0 || diff --git a/builtin/add.c b/builtin/add.c index 7885294ad12238..745fb0b4b6589f 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -203,6 +203,10 @@ int run_add_interactive(const char *revision, const char *patch_mode, if (!strcmp(patch_mode, "--patch")) mode = ADD_P_STAGE; + else if (!strcmp(patch_mode, "--patch=stash")) + mode = ADD_P_STASH; + else if (!strcmp(patch_mode, "--patch=reset")) + mode = ADD_P_RESET; else die("'%s' not yet supported in the built-in add -p", patch_mode); From e9bd93ae53faedbe399786682093351e859fbfcc Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 12 Mar 2019 15:45:03 +0100 Subject: [PATCH 51/69] built-in add -p: support interactive.diffFilter The Perl version supports post-processing the colored diff (that is generated in addition to the uncolored diff, intended to offer a prettier user experience) by a command configured via that config setting, and now the built-in version does that, too. Signed-off-by: Johannes Schindelin --- add-interactive.c | 4 ++++ add-interactive.h | 3 +++ add-patch.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/add-interactive.c b/add-interactive.c index 55a02fe3c653a5..27dfc9feea2868 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -56,6 +56,10 @@ int init_add_i_state(struct repository *r, struct add_i_state *s) strlcpy(s->file_new_color, diff_get_color(s->use_color, DIFF_FILE_NEW), COLOR_MAXLEN); + free(s->interactive_diff_filter); + if (git_config_get_string("interactive.difffilter", + &s->interactive_diff_filter)) + s->interactive_diff_filter = NULL; return 0; } diff --git a/add-interactive.h b/add-interactive.h index 2c04ada1a3d356..013c8b5b993259 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -15,6 +15,8 @@ struct add_i_state { char context_color[COLOR_MAXLEN]; char file_old_color[COLOR_MAXLEN]; char file_new_color[COLOR_MAXLEN]; + + char *interactive_diff_filter; }; int init_add_i_state(struct repository *r, struct add_i_state *s); @@ -27,6 +29,7 @@ enum color_add_i { COLOR_RESET, }; const char *get_add_i_color(enum color_add_i ix); +const char *get_interactive_diff_filter(void); struct repository; struct pathspec; diff --git a/add-patch.c b/add-patch.c index ae7a31e0c3d66a..540f8cd1be0da0 100644 --- a/add-patch.c +++ b/add-patch.c @@ -347,6 +347,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) if (want_color_fd(1, -1)) { struct child_process colored_cp = CHILD_PROCESS_INIT; + const char *diff_filter = s->s.interactive_diff_filter; setup_child_process(&colored_cp, s, NULL); xsnprintf((char *)args.argv[color_arg_index], 8, "--color"); @@ -356,6 +357,24 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) argv_array_clear(&args); if (res) return error(_("could not parse colored diff")); + + if (diff_filter) { + struct child_process filter_cp = CHILD_PROCESS_INIT; + + setup_child_process(&filter_cp, s, + diff_filter, NULL); + filter_cp.git_cmd = 0; + filter_cp.use_shell = 1; + strbuf_reset(&s->buf); + if (pipe_command(&filter_cp, + colored->buf, colored->len, + &s->buf, colored->len, + NULL, 0) < 0) + return error(_("failed to run '%s'"), + diff_filter); + strbuf_swap(colored, &s->buf); + } + strbuf_complete_line(colored); colored_p = colored->buf; colored_pend = colored_p + colored->len; @@ -457,6 +476,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) colored_pend - colored_p); if (colored_eol) colored_p = colored_eol + 1; + else if (p != pend) + /* colored shorter than non-colored? */ + goto mismatched_output; else colored_p = colored_pend; @@ -478,6 +500,15 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) */ hunk->splittable_into++; + /* non-colored shorter than colored? */ + if (colored_p != colored_pend) { +mismatched_output: + error(_("mismatched output from interactive.diffFilter")); + advise(_("Your filter must maintain a one-to-one correspondence\n" + "between its input and output lines.")); + return -1; + } + return 0; } From 1bd8c117ec6ab416254538f8f2af25b1da83e82b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 24 Mar 2019 23:26:58 +0100 Subject: [PATCH 52/69] built-in add -p: only show the applicable parts of the help text When displaying the only hunk in a file's diff, the prompt already excludes the commands to navigate to the previous/next hunk. Let's also let the `?` command show only the help lines corresponding to the commands that are displayed in the prompt. Signed-off-by: Johannes Schindelin --- add-patch.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/add-patch.c b/add-patch.c index 90ea5d451ff776..17e5d39a989243 100644 --- a/add-patch.c +++ b/add-patch.c @@ -955,8 +955,10 @@ N_("y - stage this hunk\n" "n - do not stage this hunk\n" "q - quit; do not stage this hunk or any of the remaining ones\n" "a - stage this and all the remaining hunks\n" - "d - do not stage this hunk nor any of the remaining hunks\n" - "j - leave this hunk undecided, see next undecided hunk\n" + "d - do not stage this hunk nor any of the remaining hunks\n"); + +static const char help_patch_remainder[] = +N_("j - leave this hunk undecided, see next undecided hunk\n" "J - leave this hunk undecided, see next hunk\n" "k - leave this hunk undecided, see previous undecided hunk\n" "K - leave this hunk undecided, see previous hunk\n" @@ -1189,9 +1191,31 @@ static int patch_update_file(struct add_p_state *s, hunk->use = USE_HUNK; goto soft_increment; } - } else - color_fprintf(stdout, s->s.help_color, + } else { + const char *p = _(help_patch_remainder), *eol = p; + + color_fprintf(stdout, s->s.help_color, "%s", _(help_patch_text)); + + /* + * Show only those lines of the remainder that are + * actually applicable with the current hunk. + */ + for (; *p; p = eol + (*eol == '\n')) { + eol = strchrnul(p, '\n'); + + /* + * `s->buf` still contains the part of the + * commands shown in the prompt that are not + * always available. + */ + if (*p != '?' && !strchr(s->buf.buf, *p)) + continue; + + color_fprintf_ln(stdout, s->s.help_color, + "%.*s", (int)(eol - p), p); + } + } } /* Any hunk to be used? */ From 125dfcfa0861983f3a822014d9f50c500d1cb974 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 22:38:02 +0100 Subject: [PATCH 53/69] built-in add -p: implement the "checkout" patch modes This patch teaches the built-in `git add -p` machinery all the tricks it needs to know in order to act as the work horse for `git checkout -p`. Apart from the minor changes (slightly reworded messages, different `diff` and `apply --check` invocations), it requires a new function to actually apply the changes, as `git checkout -p` is a bit special in that respect: when the desired changes do not apply to the index, but apply to the work tree, Git does not fail straight away, but asks the user whether to apply the changes to the worktree at least. Signed-off-by: Johannes Schindelin --- add-interactive.h | 1 + add-patch.c | 139 ++++++++++++++++++++++++++++++++++++++++++++-- builtin/add.c | 5 +- 3 files changed, 138 insertions(+), 7 deletions(-) diff --git a/add-interactive.h b/add-interactive.h index fc182c72eb5995..2c04ada1a3d356 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -36,6 +36,7 @@ enum add_p_mode { ADD_P_STAGE, ADD_P_STASH, ADD_P_RESET, + ADD_P_CHECKOUT, }; int run_add_p(struct repository *r, enum add_p_mode mode, diff --git a/add-patch.c b/add-patch.c index c705d931b48fc1..ae7a31e0c3d66a 100644 --- a/add-patch.c +++ b/add-patch.c @@ -106,6 +106,72 @@ static struct patch_mode patch_mode_reset_nothead = { "the file\n"), }; +static struct patch_mode patch_mode_checkout_index = { + .diff = { "diff-files", NULL }, + .apply = { "-R", NULL }, + .apply_check = { "-R", NULL }, + .is_reverse = 1, + .prompt_mode = { + N_("Discard mode change from worktree [y,n,q,a,d%s,?]? "), + N_("Discard deletion from worktree [y,n,q,a,d%s,?]? "), + N_("Discard this hunk from worktree [y,n,q,a,d%s,?]? "), + }, + .edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk " + "will immediately be marked for discarding."), + .help_patch_text = + N_("y - discard this hunk from worktree\n" + "n - do not discard this hunk from worktree\n" + "q - quit; do not discard this hunk or any of the remaining " + "ones\n" + "a - discard this hunk and all later hunks in the file\n" + "d - do not discard this hunk or any of the later hunks in " + "the file\n"), +}; + +static struct patch_mode patch_mode_checkout_head = { + .diff = { "diff-index", NULL }, + .apply_for_checkout = 1, + .apply_check = { "-R", NULL }, + .is_reverse = 1, + .prompt_mode = { + N_("Discard mode change from index and worktree [y,n,q,a,d%s,?]? "), + N_("Discard deletion from index and worktree [y,n,q,a,d%s,?]? "), + N_("Discard this hunk from index and worktree [y,n,q,a,d%s,?]? "), + }, + .edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk " + "will immediately be marked for discarding."), + .help_patch_text = + N_("y - discard this hunk from index and worktree\n" + "n - do not discard this hunk from index and worktree\n" + "q - quit; do not discard this hunk or any of the remaining " + "ones\n" + "a - discard this hunk and all later hunks in the file\n" + "d - do not discard this hunk or any of the later hunks in " + "the file\n"), +}; + +static struct patch_mode patch_mode_checkout_nothead = { + .diff = { "diff-index", "-R", NULL }, + .apply_for_checkout = 1, + .apply_check = { NULL }, + .is_reverse = 0, + .prompt_mode = { + N_("Apply mode change to index and worktree [y,n,q,a,d%s,?]? "), + N_("Apply deletion to index and worktree [y,n,q,a,d%s,?]? "), + N_("Apply this hunk to index and worktree [y,n,q,a,d%s,?]? "), + }, + .edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk " + "will immediately be marked for applying."), + .help_patch_text = + N_("y - apply this hunk to index and worktree\n" + "n - do not apply this hunk to index and worktree\n" + "q - quit; do not apply this hunk or any of the remaining " + "ones\n" + "a - apply this hunk and all later hunks in the file\n" + "d - do not apply this hunk or any of the later hunks in " + "the file\n"), +}; + struct hunk_header { unsigned long old_offset, old_count, new_offset, new_count; /* @@ -1006,6 +1072,57 @@ static int edit_hunk_loop(struct add_p_state *s, } } +static int apply_for_checkout(struct add_p_state *s, struct strbuf *diff, + int is_reverse) +{ + const char *reverse = is_reverse ? "-R" : NULL; + struct child_process check_index = CHILD_PROCESS_INIT; + struct child_process check_worktree = CHILD_PROCESS_INIT; + struct child_process apply_index = CHILD_PROCESS_INIT; + struct child_process apply_worktree = CHILD_PROCESS_INIT; + int applies_index, applies_worktree; + + setup_child_process(&check_index, s, + "apply", "--cached", "--check", reverse, NULL); + applies_index = !pipe_command(&check_index, diff->buf, diff->len, + NULL, 0, NULL, 0); + + setup_child_process(&check_worktree, s, + "apply", "--check", reverse, NULL); + applies_worktree = !pipe_command(&check_worktree, diff->buf, diff->len, + NULL, 0, NULL, 0); + + if (applies_worktree && applies_index) { + setup_child_process(&apply_index, s, + "apply", "--cached", reverse, NULL); + pipe_command(&apply_index, diff->buf, diff->len, + NULL, 0, NULL, 0); + + setup_child_process(&apply_worktree, s, + "apply", reverse, NULL); + pipe_command(&apply_worktree, diff->buf, diff->len, + NULL, 0, NULL, 0); + + return 1; + } + + if (!applies_index) { + err(s, _("The selected hunks do not apply to the index!")); + if (prompt_yesno(s, _("Apply them to the worktree " + "anyway? ")) > 0) { + setup_child_process(&apply_worktree, s, + "apply", reverse, NULL); + return pipe_command(&apply_worktree, diff->buf, + diff->len, NULL, 0, NULL, 0); + } + err(s, _("Nothing was applied.\n")); + } else + /* As a last resort, show the diff to the user */ + fwrite(diff->buf, diff->len, 1, stderr); + + return 0; +} + #define SUMMARY_HEADER_WIDTH 20 #define SUMMARY_LINE_WIDTH 80 static void summarize_hunk(struct add_p_state *s, struct hunk *hunk, @@ -1326,11 +1443,16 @@ static int patch_update_file(struct add_p_state *s, strbuf_reset(&s->buf); reassemble_patch(s, file_diff, 0, &s->buf); - setup_child_process(&cp, s, "apply", NULL); - argv_array_pushv(&cp.args, s->mode->apply); - if (pipe_command(&cp, s->buf.buf, s->buf.len, - NULL, 0, NULL, 0)) - error(_("'git apply' failed")); + if (s->mode->apply_for_checkout) + apply_for_checkout(s, &s->buf, + s->mode->is_reverse); + else { + setup_child_process(&cp, s, "apply", NULL); + argv_array_pushv(&cp.args, s->mode->apply); + if (pipe_command(&cp, s->buf.buf, s->buf.len, + NULL, 0, NULL, 0)) + error(_("'git apply' failed")); + } repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0); } @@ -1356,6 +1478,13 @@ int run_add_p(struct repository *r, enum add_p_mode mode, s.mode = &patch_mode_reset_head; else s.mode = &patch_mode_reset_nothead; + } else if (mode == ADD_P_CHECKOUT) { + if (!revision) + s.mode = &patch_mode_checkout_index; + else if (!strcmp(revision, "HEAD")) + s.mode = &patch_mode_checkout_head; + else + s.mode = &patch_mode_checkout_nothead; } else s.mode = &patch_mode_stage; s.revision = revision; diff --git a/builtin/add.c b/builtin/add.c index 745fb0b4b6589f..5f983ab1d3abd5 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -207,9 +207,10 @@ int run_add_interactive(const char *revision, const char *patch_mode, mode = ADD_P_STASH; else if (!strcmp(patch_mode, "--patch=reset")) mode = ADD_P_RESET; + else if (!strcmp(patch_mode, "--patch=checkout")) + mode = ADD_P_CHECKOUT; else - die("'%s' not yet supported in the built-in add -p", - patch_mode); + die("'%s' not supported", patch_mode); return !!run_add_p(the_repository, mode, revision, pathspec); } From b47efd8bddbf0bcbcf531a8115d23a2601958488 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 15:42:52 +0100 Subject: [PATCH 54/69] built-in add -p: handle diff.algorithm The Perl version of `git add -p` reads the config setting `diff.algorithm` and if set, uses it to generate the diff using the specified algorithm. This patch ports that functionality to the C version. To make sure that this works as intended, we add a regression test case that tries to specify a bogus diff algorithm and then verifies that `git diff-files` produced the expected error message. Note: In that new test case, we actually ignore the exit code of `git add -p`. The reason is that the C version exits with failure (as one might expect), but the Perl version does not. In fact, the Perl version continues happily after the uncolored diff failed, trying to generate the colored diff, still not catching the problem, and then it pretends to have succeeded (with exit code 0). This is arguably a bug in the Perl version, and fixing it is safely outside the scope of this patch. Signed-off-by: Johannes Schindelin --- add-interactive.c | 5 +++++ add-interactive.h | 3 ++- add-patch.c | 3 +++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/add-interactive.c b/add-interactive.c index 27dfc9feea2868..a2d126e0cc7be6 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -61,6 +61,11 @@ int init_add_i_state(struct repository *r, struct add_i_state *s) &s->interactive_diff_filter)) s->interactive_diff_filter = NULL; + free(s->interactive_diff_algorithm); + if (git_config_get_string("diff.algorithm", + &s->interactive_diff_algorithm)) + s->interactive_diff_algorithm = NULL; + return 0; } diff --git a/add-interactive.h b/add-interactive.h index 013c8b5b993259..25e300b4a0e338 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -16,7 +16,7 @@ struct add_i_state { char file_old_color[COLOR_MAXLEN]; char file_new_color[COLOR_MAXLEN]; - char *interactive_diff_filter; + char *interactive_diff_filter, *interactive_diff_algorithm; }; int init_add_i_state(struct repository *r, struct add_i_state *s); @@ -30,6 +30,7 @@ enum color_add_i { }; const char *get_add_i_color(enum color_add_i ix); const char *get_interactive_diff_filter(void); +const char *get_interactive_diff_algorithm(void); struct repository; struct pathspec; diff --git a/add-patch.c b/add-patch.c index 540f8cd1be0da0..1137ae185402d8 100644 --- a/add-patch.c +++ b/add-patch.c @@ -309,6 +309,7 @@ static int is_octal(const char *p, size_t len) static int parse_diff(struct add_p_state *s, const struct pathspec *ps) { struct argv_array args = ARGV_ARRAY_INIT; + const char *diff_algorithm = s->s.interactive_diff_algorithm; struct strbuf *plain = &s->plain, *colored = NULL; struct child_process cp = CHILD_PROCESS_INIT; char *p, *pend, *colored_p = NULL, *colored_pend = NULL, marker = '\0'; @@ -318,6 +319,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) int res; argv_array_pushv(&args, s->mode->diff); + if (diff_algorithm) + argv_array_pushf(&args, "--diff-algorithm=%s", diff_algorithm); if (s->revision) { struct object_id oid; argv_array_push(&args, From e6b1b1884973f82fe746cb89ba428cb609c44242 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 24 Mar 2019 22:54:01 +0100 Subject: [PATCH 55/69] built-in add -p: show helpful hint when nothing can be staged This patch will make `git add -p` show "No changes." or "Only binary files changed." in that case. Signed-off-by: Johannes Schindelin --- add-patch.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/add-patch.c b/add-patch.c index 17e5d39a989243..ed856b5433761b 100644 --- a/add-patch.c +++ b/add-patch.c @@ -44,7 +44,7 @@ struct add_p_state { struct hunk head; struct hunk *hunk; size_t hunk_nr, hunk_alloc; - unsigned deleted:1, mode_change:1; + unsigned deleted:1, mode_change:1,binary:1; } *file_diff; size_t file_diff_nr; }; @@ -266,7 +266,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) file_diff->mode_change = 1; } else if (file_diff->hunk_nr != 1) BUG("mode change after first hunk?"); - } + } else if (hunk == &file_diff->head && + starts_with(p, "Binary files ")) + file_diff->binary = 1; if (file_diff->deleted && file_diff->mode_change) BUG("diff contains delete *and* a mode change?!?\n%.*s", @@ -1244,7 +1246,7 @@ int run_add_p(struct repository *r, const struct pathspec *ps) struct add_p_state s = { { r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; - size_t i; + size_t i, binary_count = 0; if (init_add_i_state(r, &s.s)) return error("Could not read `add -i` config"); @@ -1257,9 +1259,16 @@ int run_add_p(struct repository *r, const struct pathspec *ps) } for (i = 0; i < s.file_diff_nr; i++) - if (patch_update_file(&s, s.file_diff + i)) + if (s.file_diff[i].binary && !s.file_diff[i].hunk_nr) + binary_count++; + else if (patch_update_file(&s, s.file_diff + i)) break; + if (s.file_diff_nr == 0) + fprintf(stderr, _("No changes.\n")); + else if (binary_count == s.file_diff_nr) + fprintf(stderr, _("Only binary files changed.\n")); + strbuf_release(&s.answer); strbuf_release(&s.buf); strbuf_release(&s.plain); From d858409298a8ea12066a0b620f5106fef8f887ab Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 29 Mar 2019 15:03:09 +0100 Subject: [PATCH 56/69] commit --interactive: make it work with the built-in `add -i` The built-in `git add -i` machinery obviously has its `the_repository` structure initialized at the point where `cmd_commit()` calls it, and therefore does not look at the environment variable `GIT_INDEX_FILE`. But it has to, because the index was already locked, and we want to ask the interactive add machinery to work on the `index.lock` file instead of the `index` file. Technically, we could teach `run_add_i()` (and `run_add_p()`) to look specifically at that environment variable, but the entire idea of passing in a parameter of type `struct repository *` is to allow working on multiple repositories (and their index files) independently. So let's instead override the `index_file` field of that structure temporarily. Signed-off-by: Johannes Schindelin --- builtin/commit.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 711b10aa900a0c..0c6e97edcd08c8 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -344,7 +344,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix die(_("index file corrupt")); if (interactive) { - char *old_index_env = NULL; + char *old_index_env = NULL, *old_repo_index_file; hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR); refresh_cache_or_die(refresh_flags); @@ -352,13 +352,17 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix if (write_locked_index(&the_index, &index_lock, 0)) die(_("unable to create temporary index")); + old_repo_index_file = the_repository->index_file; + the_repository->index_file = + (char *)get_lock_file_path(&index_lock); old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT)); - setenv(INDEX_ENVIRONMENT, get_lock_file_path(&index_lock), 1); + setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1); if (interactive_add(argc, argv, prefix, patch_interactive ? "" : NULL) != 0) die(_("interactive add failed")); + the_repository->index_file = old_repo_index_file; if (old_index_env && *old_index_env) setenv(INDEX_ENVIRONMENT, old_index_env, 1); else From 40410bae2b1e4c15c71af0b053f3efc027cf196c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 4 Apr 2019 22:17:07 +0200 Subject: [PATCH 57/69] terminal: make the code of disable_echo() reusable We are about to introduce the function `enable_non_canonical()`, which shares almost the complete code with `disable_echo()`. Let's prepare for that, by refactoring out that shared code. Signed-off-by: Johannes Schindelin --- compat/terminal.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/compat/terminal.c b/compat/terminal.c index fa13ee672db33e..1fb40b3a0a9950 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -32,7 +32,7 @@ static void restore_term(void) term_fd = -1; } -static int disable_echo(void) +static int disable_bits(tcflag_t bits) { struct termios t; @@ -43,7 +43,7 @@ static int disable_echo(void) old_term = t; sigchain_push_common(restore_term_on_signal); - t.c_lflag &= ~ECHO; + t.c_lflag &= ~bits; if (!tcsetattr(term_fd, TCSAFLUSH, &t)) return 0; @@ -53,6 +53,11 @@ static int disable_echo(void) return -1; } +static int disable_echo(void) +{ + return disable_bits(ECHO); +} + #elif defined(GIT_WINDOWS_NATIVE) #define INPUT_PATH "CONIN$" @@ -72,7 +77,7 @@ static void restore_term(void) hconin = INVALID_HANDLE_VALUE; } -static int disable_echo(void) +static int disable_bits(DWORD bits) { hconin = CreateFile("CONIN$", GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ, NULL, OPEN_EXISTING, @@ -82,7 +87,7 @@ static int disable_echo(void) GetConsoleMode(hconin, &cmode); sigchain_push_common(restore_term_on_signal); - if (!SetConsoleMode(hconin, cmode & (~ENABLE_ECHO_INPUT))) { + if (!SetConsoleMode(hconin, cmode & ~bits)) { CloseHandle(hconin); hconin = INVALID_HANDLE_VALUE; return -1; @@ -91,6 +96,12 @@ static int disable_echo(void) return 0; } +static int disable_echo(void) +{ + return disable_bits(ENABLE_ECHO_INPUT); +} + + #endif #ifndef FORCE_TEXT From 8d5239f5fc8294a3cd8f11ceffcb045421d9f69b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 4 Apr 2019 22:21:20 +0200 Subject: [PATCH 58/69] terminal: accommodate Git for Windows' default terminal Git for Windows' Git Bash runs in MinTTY by default, which does not have a Win32 Console instance, but uses MSYS2 pseudo terminals instead. This is a problem, as Git for Windows does not want to use the MSYS2 emulation layer for Git itself, and therefore has no direct way to interact with that pseudo terminal. As a workaround, use the `stty` utility (which is included in Git for Windows, and which *is* an MSYS2 program, so it knows how to deal with the pseudo terminal). Note: If Git runs in a regular CMD or PowerShell window, there *is* a regular Win32 Console to work with. This is not a problem for the MSYS2 `stty`: it copes with this scenario just fine. Also note that we introduce support for more bits than would be necessary for a mere `disable_echo()` here, in preparation for the upcoming `enable_non_canonical()` function. Signed-off-by: Johannes Schindelin --- compat/terminal.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/compat/terminal.c b/compat/terminal.c index 1fb40b3a0a9950..16e9949da10e5d 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -2,6 +2,8 @@ #include "compat/terminal.h" #include "sigchain.h" #include "strbuf.h" +#include "run-command.h" +#include "string-list.h" #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE) @@ -64,11 +66,28 @@ static int disable_echo(void) #define OUTPUT_PATH "CONOUT$" #define FORCE_TEXT "t" +static int use_stty = 1; +static struct string_list stty_restore = STRING_LIST_INIT_DUP; static HANDLE hconin = INVALID_HANDLE_VALUE; static DWORD cmode; static void restore_term(void) { + if (use_stty) { + int i; + struct child_process cp = CHILD_PROCESS_INIT; + + if (stty_restore.nr == 0) + return; + + argv_array_push(&cp.args, "stty"); + for (i = 0; i < stty_restore.nr; i++) + argv_array_push(&cp.args, stty_restore.items[i].string); + run_command(&cp); + string_list_clear(&stty_restore, 0); + return; + } + if (hconin == INVALID_HANDLE_VALUE) return; @@ -79,6 +98,37 @@ static void restore_term(void) static int disable_bits(DWORD bits) { + if (use_stty) { + struct child_process cp = CHILD_PROCESS_INIT; + + argv_array_push(&cp.args, "stty"); + + if (bits & ENABLE_LINE_INPUT) { + string_list_append(&stty_restore, "icanon"); + argv_array_push(&cp.args, "-icanon"); + } + + if (bits & ENABLE_ECHO_INPUT) { + string_list_append(&stty_restore, "echo"); + argv_array_push(&cp.args, "-echo"); + } + + if (bits & ENABLE_PROCESSED_INPUT) { + string_list_append(&stty_restore, "-ignbrk"); + string_list_append(&stty_restore, "intr"); + string_list_append(&stty_restore, "^c"); + argv_array_push(&cp.args, "ignbrk"); + argv_array_push(&cp.args, "intr"); + argv_array_push(&cp.args, ""); + } + + if (run_command(&cp) == 0) + return 0; + + /* `stty` could not be executed; access the Console directly */ + use_stty = 0; + } + hconin = CreateFile("CONIN$", GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); From 09af4d86f78d78a788b89bb8cea8243ec3ef0e55 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 26 Mar 2019 21:28:10 +0100 Subject: [PATCH 59/69] terminal: add a new function to read a single keystroke Typically, input on the command-line is line-based. It is actually not really easy to get single characters (or better put: keystrokes). We provide two implementations here: - One that handles `/dev/tty` based systems as well as native Windows. The former uses the `tcsetattr()` function to put the terminal into "raw mode", which allows us to read individual keystrokes, one by one. The latter uses `stty.exe` to do the same, falling back to direct Win32 Console access. Thanks to the refactoring leading up to this commit, this is a single function, with the platform-specific details hidden away in conditionally-compiled code blocks. - A fall-back which simply punts and reads back an entire line. Note that the function writes the keystroke into an `strbuf` rather than a `char`, in preparation for reading Escape sequences (e.g. when the user hit an arrow key). This is also required for UTF-8 sequences in case the keystroke corresponds to a non-ASCII letter. Signed-off-by: Johannes Schindelin --- compat/terminal.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++ compat/terminal.h | 3 +++ 2 files changed, 58 insertions(+) diff --git a/compat/terminal.c b/compat/terminal.c index 16e9949da10e5d..1b2564042ac60c 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -60,6 +60,11 @@ static int disable_echo(void) return disable_bits(ECHO); } +static int enable_non_canonical(void) +{ + return disable_bits(ICANON | ECHO); +} + #elif defined(GIT_WINDOWS_NATIVE) #define INPUT_PATH "CONIN$" @@ -151,6 +156,10 @@ static int disable_echo(void) return disable_bits(ENABLE_ECHO_INPUT); } +static int enable_non_canonical(void) +{ + return disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT); +} #endif @@ -198,6 +207,33 @@ char *git_terminal_prompt(const char *prompt, int echo) return buf.buf; } +int read_key_without_echo(struct strbuf *buf) +{ + static int warning_displayed; + int ch; + + if (warning_displayed || enable_non_canonical() < 0) { + if (!warning_displayed) { + warning("reading single keystrokes not supported on " + "this platform; reading line instead"); + warning_displayed = 1; + } + + return strbuf_getline(buf, stdin); + } + + strbuf_reset(buf); + ch = getchar(); + if (ch == EOF) { + restore_term(); + return EOF; + } + + strbuf_addch(buf, ch); + restore_term(); + return 0; +} + #else char *git_terminal_prompt(const char *prompt, int echo) @@ -205,4 +241,23 @@ char *git_terminal_prompt(const char *prompt, int echo) return getpass(prompt); } +int read_key_without_echo(struct strbuf *buf) +{ + static int warning_displayed; + const char *res; + + if (!warning_displayed) { + warning("reading single keystrokes not supported on this " + "platform; reading line instead"); + warning_displayed = 1; + } + + res = getpass(""); + strbuf_reset(buf); + if (!res) + return EOF; + strbuf_addstr(buf, res); + return 0; +} + #endif diff --git a/compat/terminal.h b/compat/terminal.h index 97db7cd69d65fc..a9d52b8464e2f6 100644 --- a/compat/terminal.h +++ b/compat/terminal.h @@ -3,4 +3,7 @@ char *git_terminal_prompt(const char *prompt, int echo); +/* Read a single keystroke, without echoing it to the terminal */ +int read_key_without_echo(struct strbuf *buf); + #endif /* COMPAT_TERMINAL_H */ From 597ac6904de58cbd7e109c6598e0a1dd26b52915 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 26 Mar 2019 21:37:27 +0100 Subject: [PATCH 60/69] built-in add -p: respect the `interactive.singlekey` config setting The Perl version of `git add -p` supports this config setting to allow users to input commands via single characters (as opposed to having to press the key afterwards). This is an opt-in feature because it requires Perl packages (Term::ReadKey and Term::Cap, where it tries to handle an absence of the latter package gracefully) to work. Note that at least on Ubuntu, that Perl package is not installed by default (it needs to be installed via `sudo apt-get install libterm-readkey-perl`), so this feature is probably not used a whole lot. In C, we obviously do not have these packages available, but we just introduced `read_single_keystroke()` that is similar to what Term::ReadKey provides, and we use that here. Signed-off-by: Johannes Schindelin --- add-interactive.c | 4 ++++ add-interactive.h | 2 ++ add-patch.c | 21 +++++++++++++++++---- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index a2d126e0cc7be6..b455c07d2fa7d9 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -66,6 +66,10 @@ int init_add_i_state(struct repository *r, struct add_i_state *s) &s->interactive_diff_algorithm)) s->interactive_diff_algorithm = NULL; + if (git_config_get_bool("interactive.singlekey", + &s->use_single_key)) + s->use_single_key = 0; + return 0; } diff --git a/add-interactive.h b/add-interactive.h index 25e300b4a0e338..41b9b2fa384384 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -16,6 +16,7 @@ struct add_i_state { char file_old_color[COLOR_MAXLEN]; char file_new_color[COLOR_MAXLEN]; + int use_single_key; char *interactive_diff_filter, *interactive_diff_algorithm; }; @@ -31,6 +32,7 @@ enum color_add_i { const char *get_add_i_color(enum color_add_i ix); const char *get_interactive_diff_filter(void); const char *get_interactive_diff_algorithm(void); +int get_interactive_use_single_key(void); struct repository; struct pathspec; diff --git a/add-patch.c b/add-patch.c index 1137ae185402d8..5bacbb7bc455ac 100644 --- a/add-patch.c +++ b/add-patch.c @@ -6,6 +6,7 @@ #include "pathspec.h" #include "color.h" #include "diff.h" +#include "compat/terminal.h" enum prompt_mode_type { PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK @@ -1046,14 +1047,27 @@ static int run_apply_check(struct add_p_state *s, return 0; } +static int read_single_character(struct add_p_state *s) +{ + if (s->s.use_single_key) { + int res = read_key_without_echo(&s->answer); + printf("%s\n", res == EOF ? "" : s->answer.buf); + return res; + } + + if (strbuf_getline(&s->answer, stdin) == EOF) + return EOF; + strbuf_trim_trailing_newline(&s->answer); + return 0; +} + static int prompt_yesno(struct add_p_state *s, const char *prompt) { for (;;) { color_fprintf(stdout, s->s.prompt_color, "%s", _(prompt)); fflush(stdout); - if (strbuf_getline(&s->answer, stdin) == EOF) + if (read_single_character(s) == EOF) return -1; - strbuf_trim_trailing_newline(&s->answer); switch (tolower(s->answer.buf[0])) { case 'n': return 0; case 'y': return 1; @@ -1289,9 +1303,8 @@ static int patch_update_file(struct add_p_state *s, _(s->mode->prompt_mode[prompt_mode_type]), s->buf.buf); fflush(stdout); - if (strbuf_getline(&s->answer, stdin) == EOF) + if (read_single_character(s) == EOF) break; - strbuf_trim_trailing_newline(&s->answer); if (!s->answer.len) continue; From 8cf5fd62d881e808c77303ccbd960ea1d6267f38 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 27 Mar 2019 17:14:02 +0100 Subject: [PATCH 61/69] built-in add -p: handle Escape sequences in interactive.singlekey mode This recapitulates part of b5cc003253c8 (add -i: ignore terminal escape sequences, 2011-05-17): add -i: ignore terminal escape sequences On the author's terminal, the up-arrow input sequence is ^[[A, and thus fat-fingering an up-arrow into 'git checkout -p' is quite dangerous: git-add--interactive.perl will ignore the ^[ and [ characters and happily treat A as "discard everything". As a band-aid fix, use Term::Cap to get all terminal capabilities. Then use the heuristic that any capability value that starts with ^[ (i.e., \e in perl) must be a key input sequence. Finally, given an input that starts with ^[, read more characters until we have read a full escape sequence, then return that to the caller. We use a timeout of 0.5 seconds on the subsequent reads to avoid getting stuck if the user actually input a lone ^[. Since none of the currently recognized keys start with ^[, the net result is that the sequence as a whole will be ignored and the help displayed. Note that we leave part for later which uses "Term::Cap to get all terminal capabilities", for several reasons: 1. it is actually not really necessary, as the timeout of 0.5 seconds should be plenty sufficient to catch Escape sequences, 2. it is cleaner to keep the change to special-case Escape sequences separate from the change that reads all terminal capabilities to speed things up, and 3. in practice, relying on the terminal capabilities is a bit overrated, as the information could be incomplete, or plain wrong. For example, in this developer's tmux sessions, the terminal capabilities claim that the "cursor up" sequence is ^[M, but the actual sequence produced by the "cursor up" key is ^[[A. Signed-off-by: Johannes Schindelin --- compat/terminal.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/compat/terminal.c b/compat/terminal.c index 1b2564042ac60c..ee7dff079c4bf9 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -4,6 +4,7 @@ #include "strbuf.h" #include "run-command.h" #include "string-list.h" +#include "argv-array.h" #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE) @@ -161,6 +162,37 @@ static int enable_non_canonical(void) return disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT); } +/* + * Override `getchar()`, as the default implementation does not use + * `ReadFile()`. + * + * This poses a problem when we want to see whether the standard + * input has more characters, as the default of Git for Windows is to start the + * Bash in a MinTTY, which uses a named pipe to emulate a pty, in which case + * our `poll()` emulation calls `PeekNamedPipe()`, which seems to require + * `ReadFile()` to be called first to work properly (it only reports 0 + * available bytes, otherwise). + * + * So let's just override `getchar()` with a version backed by `ReadFile()` and + * go our merry ways from here. + */ +static int mingw_getchar(void) +{ + DWORD read = 0; + unsigned char ch; + + if (!ReadFile(GetStdHandle(STD_INPUT_HANDLE), &ch, 1, &read, NULL)) + return EOF; + + if (!read) { + error("Unexpected 0 read"); + return EOF; + } + + return ch; +} +#define getchar mingw_getchar + #endif #ifndef FORCE_TEXT @@ -228,8 +260,31 @@ int read_key_without_echo(struct strbuf *buf) restore_term(); return EOF; } - strbuf_addch(buf, ch); + + if (ch == '\033' /* ESC */) { + /* + * We are most likely looking at an Escape sequence. Let's try + * to read more bytes, waiting at most half a second, assuming + * that the sequence is complete if we did not receive any byte + * within that time. + * + * Start by replacing the Escape byte with ^[ */ + strbuf_splice(buf, buf->len - 1, 1, "^[", 2); + + for (;;) { + struct pollfd pfd = { .fd = 0, .events = POLLIN }; + + if (poll(&pfd, 1, 500) < 1) + break; + + ch = getchar(); + if (ch == EOF) + return 0; + strbuf_addch(buf, ch); + } + } + restore_term(); return 0; } From 89951d7fbd69e9474c77e723475cd82a682d6c43 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 27 Mar 2019 17:14:02 +0100 Subject: [PATCH 62/69] built-in add -p: handle Escape sequences more efficiently When `interactive.singlekey = true`, we react immediately to keystrokes, even to Escape sequences (e.g. when pressing a cursor key). The problem with Escape sequences is that we do not really know when they are done, and as a heuristic we poll standard input for half a second to make sure that we got all of it. While waiting half a second is not asking for a whole lot, it can become quite annoying over time, therefore with this patch, we read the terminal capabilities (if available) and extract known Escape sequences from there, then stop polling immediately when we detected that the user pressed a key that generated such a known sequence. This recapitulates the remaining part of b5cc003253c8 (add -i: ignore terminal escape sequences, 2011-05-17). Note: We do *not* query the terminal capabilities directly. That would either require a lot of platform-specific code, or it would require linking to a library such as ncurses. Linking to a library in the built-ins is something we try very hard to avoid (we even kicked the libcurl dependency to a non-built-in remote helper, just to shave off a tiny fraction of a second from Git's startup time). And the platform-specific code would be a maintenance nightmare. Even worse: in Git for Windows' case, we would need to query MSYS2 pseudo terminals, which `git.exe` simply cannot do (because it is intentionally *not* an MSYS2 program). To address this, we simply spawn `infocmp -L -1` and parse its output (which works even in Git for Windows, because that helper is included in the end-user facing installations). This is done only once, as in the Perl version, but it is done only when the first Escape sequence is encountered, not upon startup of `git add -i`; This saves on startup time, yet makes reacting to the first Escape sequence slightly more sluggish. But it allows us to keep the terminal-related code encapsulated in the `compat/terminal.c` file. Signed-off-by: Johannes Schindelin --- compat/terminal.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/compat/terminal.c b/compat/terminal.c index ee7dff079c4bf9..cb08d7a7fcbacc 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -5,6 +5,7 @@ #include "run-command.h" #include "string-list.h" #include "argv-array.h" +#include "hashmap.h" #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE) @@ -239,6 +240,70 @@ char *git_terminal_prompt(const char *prompt, int echo) return buf.buf; } +/* + * The `is_known_escape_sequence()` function returns 1 if the passed string + * corresponds to an Escape sequence that the terminal capabilities contains. + * + * To avoid depending on ncurses or other platform-specific libraries, we rely + * on the presence of the `infocmp` executable to do the job for us (failing + * silently if the program is not available or refused to run). + */ +struct escape_sequence_entry { + struct hashmap_entry entry; + char sequence[FLEX_ARRAY]; +}; + +static int sequence_entry_cmp(const void *hashmap_cmp_fn_data, + const struct escape_sequence_entry *e1, + const struct escape_sequence_entry *e2, + const void *keydata) +{ + return strcmp(e1->sequence, keydata ? keydata : e2->sequence); +} + +static int is_known_escape_sequence(const char *sequence) +{ + static struct hashmap sequences; + static int initialized; + + if (!initialized) { + struct child_process cp = CHILD_PROCESS_INIT; + struct strbuf buf = STRBUF_INIT; + char *p, *eol; + + hashmap_init(&sequences, (hashmap_cmp_fn)sequence_entry_cmp, + NULL, 0); + + argv_array_pushl(&cp.args, "infocmp", "-L", "-1", NULL); + if (pipe_command(&cp, NULL, 0, &buf, 0, NULL, 0)) + strbuf_setlen(&buf, 0); + + for (eol = p = buf.buf; *p; p = eol + 1) { + p = strchr(p, '='); + if (!p) + break; + p++; + eol = strchrnul(p, '\n'); + + if (starts_with(p, "\\E")) { + char *comma = memchr(p, ',', eol - p); + struct escape_sequence_entry *e; + + p[0] = '^'; + p[1] = '['; + FLEX_ALLOC_MEM(e, sequence, p, comma - p); + hashmap_entry_init(e, strhash(e->sequence)); + hashmap_add(&sequences, e); + } + if (!*eol) + break; + } + initialized = 1; + } + + return !!hashmap_get_from_hash(&sequences, strhash(sequence), sequence); +} + int read_key_without_echo(struct strbuf *buf) { static int warning_displayed; @@ -272,7 +337,12 @@ int read_key_without_echo(struct strbuf *buf) * Start by replacing the Escape byte with ^[ */ strbuf_splice(buf, buf->len - 1, 1, "^[", 2); - for (;;) { + /* + * Query the terminal capabilities once about all the Escape + * sequences it knows about, so that we can avoid waiting for + * half a second when we know that the sequence is complete. + */ + while (!is_known_escape_sequence(buf->buf)) { struct pollfd pfd = { .fd = 0, .events = POLLIN }; if (poll(&pfd, 1, 500) < 1) From 779d4ddf4b391adee934e03253bca438d8be5c62 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 28 Mar 2019 20:06:37 +0100 Subject: [PATCH 63/69] ci: include the built-in `git add -i` in the `linux-gcc` job This job runs the test suite twice, once in regular mode, and once with a whole slew of `GIT_TEST_*` variables set. Now that the built-in version of `git add --interactive` is feature-complete, let's also throw `GIT_TEST_MULTI_PACK_INDEX` into that fray. Signed-off-by: Johannes Schindelin --- ci/run-build-and-tests.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh index cdd29134404739..044a142742168f 100755 --- a/ci/run-build-and-tests.sh +++ b/ci/run-build-and-tests.sh @@ -20,6 +20,7 @@ then export GIT_TEST_OE_DELTA_SIZE=5 export GIT_TEST_COMMIT_GRAPH=1 export GIT_TEST_MULTI_PACK_INDEX=1 + export GIT_TEST_ADD_I_USE_BUILTIN=1 make test fi From b6d244806d752de1f28f7523bf0403af5b6345ee Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 12 Sep 2015 12:25:47 +0200 Subject: [PATCH 64/69] t3701: verify that we can add *lots* of files interactively Signed-off-by: Johannes Schindelin --- t/t3701-add-interactive.sh | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 5f23d71aed5733..0a7bc67745b015 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -740,6 +740,27 @@ test_expect_success 'add -p patch editing works with pathological context lines' test_cmp expected-2 actual ' +test_expect_success EXPENSIVE 'add -i with a lot of files' ' + git reset --hard && + x160=0123456789012345678901234567890123456789 && + x160=$x160$x160$x160$x160 && + y= && + i=0 && + while test $i -le 200 + do + name=$(printf "%s%03d" $x160 $i) && + echo $name >$name && + git add -N $name && + y="${y}y$LF" && + i=$(($i+1)) || + break + done && + echo "$y" | git add -p -- . && + git diff --cached >staged && + test_line_count = 1407 staged && + git reset --hard +' + test_expect_success 'show help from add--helper' ' git reset --hard && cat >expect <<-EOF && From 36fd328ea7863bce0805c4ddabd7b6af2bfb5874 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 20 Dec 2018 21:44:42 +0200 Subject: [PATCH 65/69] re-revert tests: add a special setup where stash.useBuiltin is off We temporarily reverted part of this commit to allow merging `add-p` without conflicts. Now it is time to re-apply that part. Signed-off-by: Johannes Schindelin --- t/README | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/t/README b/t/README index 1ae941f88ea865..b1a598ae39ae50 100644 --- a/t/README +++ b/t/README @@ -383,6 +383,10 @@ GIT_TEST_REBASE_USE_BUILTIN=, when false, disables the builtin version of git-rebase. See 'rebase.useBuiltin' in git-config(1). +GIT_TEST_STASH_USE_BUILTIN=, when false, disables the +built-in version of git-stash. See 'stash.useBuiltin' in +git-config(1). + GIT_TEST_ADD_I_USE_BUILTIN=, when true, enables the builtin version of git add -i. See 'add.interactive.useBuiltin' in git-config(1). From 954b915e0b04a68d135c2738e4bd0cf7da2300fb Mon Sep 17 00:00:00 2001 From: Karsten Blees Date: Thu, 19 Mar 2015 16:33:44 +0100 Subject: [PATCH 66/69] mingw: Support `git_terminal_prompt` with more terminals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `git_terminal_prompt()` function expects the terminal window to be attached to a Win32 Console. However, this is not the case with terminal windows other than `cmd.exe`'s, e.g. with MSys2's own `mintty`. Non-cmd terminals such as `mintty` still have to have a Win32 Console to be proper console programs, but have to hide the Win32 Console to be able to provide more flexibility (such as being resizeable not only vertically but also horizontally). By writing to that Win32 Console, `git_terminal_prompt()` manages only to send the prompt to nowhere and to wait for input from a Console to which the user has no access. This commit introduces a function specifically to support `mintty` -- or other terminals that are compatible with MSys2's `/dev/tty` emulation. The most prominent user of `git_terminal_prompt()` is certainly `git-remote-https.exe`. It is an interesting use case because both `stdin` and `stdout` are redirected when Git calls said executable, yet it still wants to access the terminal. When running inside a `mintty`, the terminal is not accessible to the `git-remote-https.exe` program, though, because it is a MinGW program and the `mintty` terminal is not backed by a Win32 console. To solve that problem, we simply call out to the shell -- which is an *MSys2* program and can therefore access `/dev/tty`. Helped-by: 마누엘 Signed-off-by: Karsten Blees Signed-off-by: Johannes Schindelin --- compat/terminal.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/compat/terminal.c b/compat/terminal.c index cb08d7a7fcbacc..57c512ac045ab2 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -1,4 +1,4 @@ -#include "git-compat-util.h" +#include "cache.h" #include "compat/terminal.h" #include "sigchain.h" #include "strbuf.h" @@ -194,6 +194,55 @@ static int mingw_getchar(void) } #define getchar mingw_getchar +static char *shell_prompt(const char *prompt, int echo) +{ + const char *read_input[] = { + /* Note: call 'bash' explicitly, as 'read -s' is bash-specific */ + "bash", "-c", echo ? + "cat >/dev/tty && read -r line /dev/tty && read -r -s line /dev/tty", + NULL + }; + struct child_process child = CHILD_PROCESS_INIT; + static struct strbuf buffer = STRBUF_INIT; + int prompt_len = strlen(prompt), len = -1, code; + + child.argv = read_input; + child.in = -1; + child.out = -1; + child.silent_exec_failure = 1; + + if (start_command(&child)) + return NULL; + + if (write_in_full(child.in, prompt, prompt_len) != prompt_len) { + error("could not write to prompt script"); + close(child.in); + goto ret; + } + close(child.in); + + strbuf_reset(&buffer); + len = strbuf_read(&buffer, child.out, 1024); + if (len < 0) { + error("could not read from prompt script"); + goto ret; + } + + strbuf_strip_suffix(&buffer, "\n"); + strbuf_strip_suffix(&buffer, "\r"); + +ret: + close(child.out); + code = finish_command(&child); + if (code) { + error("failed to execute prompt script (exit code %d)", code); + return NULL; + } + + return len < 0 ? NULL : buffer.buf; +} + #endif #ifndef FORCE_TEXT @@ -206,6 +255,15 @@ char *git_terminal_prompt(const char *prompt, int echo) int r; FILE *input_fh, *output_fh; +#ifdef GIT_WINDOWS_NATIVE + + /* try shell_prompt first, fall back to CONIN/OUT if bash is missing */ + char *result = shell_prompt(prompt, echo); + if (result) + return result; + +#endif + input_fh = fopen(INPUT_PATH, "r" FORCE_TEXT); if (!input_fh) return NULL; From 212565694dbaa5097988f3aa6954812e6cd1fa90 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 4 Apr 2019 15:33:03 +0200 Subject: [PATCH 67/69] built-in stash: use the built-in `git add -p` if so configured The scripted version of `git stash` called directly into the Perl script `git-add--interactive.perl`, and this was faithfully converted to C. However, we have a much better way to do this now: call `git add --patch=`, which incidentally also respects the config setting `add.interactive.useBuiltin`. Let's do this. Signed-off-by: Johannes Schindelin --- builtin/stash.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/builtin/stash.c b/builtin/stash.c index f7428b08f5ac75..b066e70a1f04ab 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -981,9 +981,9 @@ static int stash_patch(struct stash_info *info, struct pathspec ps, { int ret = 0; struct child_process cp_read_tree = CHILD_PROCESS_INIT; - struct child_process cp_add_i = CHILD_PROCESS_INIT; struct child_process cp_diff_tree = CHILD_PROCESS_INIT; struct index_state istate = { NULL }; + char *old_index_env = NULL, *old_repo_index_file; remove_path(stash_index_path.buf); @@ -997,16 +997,19 @@ static int stash_patch(struct stash_info *info, struct pathspec ps, } /* Find out what the user wants. */ - cp_add_i.git_cmd = 1; - argv_array_pushl(&cp_add_i.args, "add--interactive", "--patch=stash", - "--", NULL); - add_pathspecs(&cp_add_i.args, ps); - argv_array_pushf(&cp_add_i.env_array, "GIT_INDEX_FILE=%s", - stash_index_path.buf); - if (run_command(&cp_add_i)) { - ret = -1; - goto done; - } + old_repo_index_file = the_repository->index_file; + the_repository->index_file = stash_index_path.buf; + old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT)); + setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1); + + ret = run_add_interactive(NULL, "--patch=stash", &ps); + + the_repository->index_file = old_repo_index_file; + if (old_index_env && *old_index_env) + setenv(INDEX_ENVIRONMENT, old_index_env, 1); + else + unsetenv(INDEX_ENVIRONMENT); + FREE_AND_NULL(old_index_env); /* State of the working tree. */ if (write_index_as_tree(&info->w_tree, &istate, stash_index_path.buf, 0, From 9c4ed70bebf9303e85793fcb65fda151545e48b8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 6 Apr 2019 22:31:40 +0200 Subject: [PATCH 68/69] t3904: fix incorrect demonstration of a bug In 7e9e048661 (stash -p: demonstrate failure of split with mixed y/n, 2015-04-16), a regression test for a known breakage that was added to the test script `t3904-stash-patch.sh` that demonstrated that splitting a hunk and trying to stash only part of that split hunk fails (but shouldn't). As expected, it still fails, but for the wrong reason: once the bug is fixed, we would expect stderr to show nothing, yet the regression test expects stderr to show something. Let's fix that by telling that regression test case to expect nothing to be printed to stderr. While at it, also drop the obvious left-over from debugging where the regression test did not mind `git stash -p` to return a non-zero exit status. Of course, the regression test still fails, but this time for the correct reason. Signed-off-by: Johannes Schindelin --- t/t3904-stash-patch.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh index 9546b6f8a4e2fd..ab7d7aa6de1893 100755 --- a/t/t3904-stash-patch.sh +++ b/t/t3904-stash-patch.sh @@ -106,8 +106,8 @@ test_expect_failure 'stash -p with split hunk' ' ccc EOF printf "%s\n" s n y q | - test_might_fail git stash -p 2>error && - ! test_must_be_empty error && + git stash -p 2>error && + test_must_be_empty error && grep "added line 1" test && ! grep "added line 2" test ' From b85a6f800902a86b86c69ac8bc8828b3f6e69e1a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 6 Apr 2019 22:46:09 +0200 Subject: [PATCH 69/69] stash -p: (partially) fix bug concerning split hunks When trying to stash part of the worktree changes by splitting a hunk and then only partially accepting the split bits and pieces, the user is presented with a rather cryptic error: error: patch failed: : error: test: patch does not apply Cannot remove worktree changes and the command would fail to stash the desired parts of the worktree changes (even if the `stash` ref was actually updated correctly). We even have a test case demonstrating that failure, carrying it for four years already. The explanation: when splitting a hunk, the changed lines are no longer separated by more than 3 lines (which is the amount of context lines Git's diffs use by default), but less than that. So when staging only part of the diff hunk for stashing, the resulting diff that we want to apply to the worktree in reverse will contain those changes to be dropped surrounded by three context lines, but since the diff is relative to HEAD rather than to the worktree, these context lines will not match. Example time. Let's assume that the file README contains these lines: We the people and the worktree added some lines so that it contains these lines instead: We are the kind people and the user tries to stash the line containing "are", then the command will internally stage this line to a temporary index file and try to revert the diff between HEAD and that index file. The diff hunk that `git stash` tries to revert will look somewhat like this: @@ -1776,3 +1776,4 We +are the people It is obvious, now, that the trailing context lines overlap with the part of the original diff hunk that the user did *not* want to stash. Keeping in mind that context lines in diffs serve the primary purpose of finding the exact location when the diff does not apply precisely (but when the exact line number in the file to be patched differs from the line number indicated in the diff), we work around this by reducing the amount of context lines: the diff was just generated. Note: this is not a *full* fix for the issue. Just as demonstrated in t3701's 'add -p works with pathological context lines' test case, there are ambiguities in the diff format. It is very rare in practice, of course, to encounter such repeated lines. The full solution for such cases would be to replace the approach of generating a diff from the stash and then applying it in reverse by emulating `git revert` (i.e. doing a 3-way merge). However, in `git stash -p` it would not apply to `HEAD` but instead to the worktree, which makes this non-trivial to implement as long as we also maintain a scripted version of `add -i`. Signed-off-by: Johannes Schindelin --- builtin/stash.c | 2 +- git-legacy-stash.sh | 2 +- t/t3904-stash-patch.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/stash.c b/builtin/stash.c index b066e70a1f04ab..0b65934fb3701b 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1019,7 +1019,7 @@ static int stash_patch(struct stash_info *info, struct pathspec ps, } cp_diff_tree.git_cmd = 1; - argv_array_pushl(&cp_diff_tree.args, "diff-tree", "-p", "HEAD", + argv_array_pushl(&cp_diff_tree.args, "diff-tree", "-p", "-U1", "HEAD", oid_to_hex(&info->w_tree), "--", NULL); if (pipe_command(&cp_diff_tree, NULL, 0, out_patch, 0, NULL, 0)) { ret = -1; diff --git a/git-legacy-stash.sh b/git-legacy-stash.sh index 5f019108184bd4..cc85114dd6bb06 100755 --- a/git-legacy-stash.sh +++ b/git-legacy-stash.sh @@ -212,7 +212,7 @@ create_stash () { w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) || die "$(gettext "Cannot save the current worktree state")" - git diff-tree -p HEAD $w_tree -- >"$TMP-patch" && + git diff-tree -p -U1 HEAD $w_tree -- >"$TMP-patch" && test -s "$TMP-patch" || die "$(gettext "No changes selected")" diff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh index ab7d7aa6de1893..accfe3845c418e 100755 --- a/t/t3904-stash-patch.sh +++ b/t/t3904-stash-patch.sh @@ -89,7 +89,7 @@ test_expect_success 'none of this moved HEAD' ' verify_saved_head ' -test_expect_failure 'stash -p with split hunk' ' +test_expect_success 'stash -p with split hunk' ' git reset --hard && cat >test <<-\EOF && aaa