From c00ed828119886c4fccaadc47a333b2e5ddc5f72 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 12 Mar 2019 15:45:03 +0100 Subject: [PATCH 01/18] 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 | 12 ++++++++++++ add-interactive.h | 3 +++ add-patch.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/add-interactive.c b/add-interactive.c index a5bb14f2f4..1786ea29c4 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -52,6 +52,17 @@ void init_add_i_state(struct add_i_state *s, struct repository *r) diff_get_color(s->use_color, DIFF_FILE_OLD)); init_color(r, s, "new", s->file_new_color, diff_get_color(s->use_color, DIFF_FILE_NEW)); + + FREE_AND_NULL(s->interactive_diff_filter); + git_config_get_string("interactive.difffilter", + &s->interactive_diff_filter); +} + +void clear_add_i_state(struct add_i_state *s) +{ + FREE_AND_NULL(s->interactive_diff_filter); + memset(s, 0, sizeof(*s)); + s->use_color = -1; } /* @@ -1149,6 +1160,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps) strbuf_release(&print_file_item_data.worktree); strbuf_release(&header); prefix_item_list_clear(&commands); + clear_add_i_state(&s); return res; } diff --git a/add-interactive.h b/add-interactive.h index b2f23479c5..46c73867ad 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -15,9 +15,12 @@ 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; }; void init_add_i_state(struct add_i_state *s, struct repository *r); +void clear_add_i_state(struct add_i_state *s); struct repository; struct pathspec; diff --git a/add-patch.c b/add-patch.c index 46c6c183d5..78bde41df0 100644 --- a/add-patch.c +++ b/add-patch.c @@ -398,6 +398,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(s, &colored_cp, NULL); xsnprintf((char *)args.argv[color_arg_index], 8, "--color"); @@ -407,6 +408,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(s, &filter_cp, + 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; @@ -531,6 +550,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; @@ -555,6 +577,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; } @@ -1612,6 +1643,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode, parse_diff(&s, ps) < 0) { strbuf_release(&s.plain); strbuf_release(&s.colored); + clear_add_i_state(&s.s); return -1; } @@ -1630,5 +1662,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode, strbuf_release(&s.buf); strbuf_release(&s.plain); strbuf_release(&s.colored); + clear_add_i_state(&s.s); return 0; } From a1c9b9c0c68420009c42e1b62050dfd2cb88c792 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 22:33:49 +0100 Subject: [PATCH 02/18] 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. 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 | 95 ++++++++++++++++++++++++++++++++++------------- builtin/add.c | 10 ++++- 4 files changed, 85 insertions(+), 30 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 6a5048c83e..a5bb14f2f4 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -924,7 +924,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_ADD, NULL, &ps_selected); argv_array_clear(&args); clear_pathspec(&ps_selected); } diff --git a/add-interactive.h b/add-interactive.h index 062dc3646c..e29a769aba 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -22,6 +22,12 @@ void init_add_i_state(struct add_i_state *s, struct repository *r); 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_ADD, +}; + +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 2c46fe5b33..71356fbd9a 100644 --- a/add-patch.c +++ b/add-patch.c @@ -8,13 +8,41 @@ #include "diff.h" enum prompt_mode_type { - PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK + PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK, + PROMPT_MODE_MAX, /* must be last */ }; -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 { + /* + * The magic constant 4 is chosen such that all patch modes + * provide enough space for three command-line arguments followed by a + * trailing `NULL`. + */ + const char *diff_cmd[4], *apply_args[4], *apply_check_args[4]; + unsigned is_reverse:1, apply_for_checkout:1; + const char *prompt_mode[PROMPT_MODE_MAX]; + const char *edit_hunk_hint, *help_patch_text; +}; + +static struct patch_mode patch_mode_add = { + .diff_cmd = { "diff-files", NULL }, + .apply_args = { "--cached", NULL }, + .apply_check_args = { "--cached", NULL }, + .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 +75,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, ...) @@ -162,9 +194,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_cmd); + 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); @@ -382,7 +423,10 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, - header->colored_extra_start; } - new_offset += delta; + if (s->mode->is_reverse) + old_offset -= delta; + else + new_offset += delta; strbuf_addf(out, "@@ -%lu,%lu +%lu,%lu @@", old_offset, header->old_count, @@ -805,11 +849,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(&s->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(&s->buf, "%s", _(s->mode->edit_hunk_hint)); /* * TRANSLATORS: 'it' refers to the patch mentioned in the previous * messages. @@ -890,7 +933,8 @@ static int run_apply_check(struct add_p_state *s, reassemble_patch(s, file_diff, 1, &s->buf); setup_child_process(s, &cp, - "apply", "--cached", "--check", NULL); + "apply", "--check", NULL); + argv_array_pushv(&cp.args, s->mode->apply_check_args); if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) return error(_("'git apply --cached' failed")); @@ -1005,13 +1049,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" @@ -1097,7 +1134,8 @@ static int patch_update_file(struct add_p_state *s, (uintmax_t)hunk_index + 1, (uintmax_t)file_diff->hunk_nr); 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; @@ -1254,7 +1292,7 @@ soft_increment: 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 @@ -1288,10 +1326,11 @@ soft_increment: reassemble_patch(s, file_diff, 0, &s->buf); discard_index(s->s.r->index); - setup_child_process(s, &cp, "apply", "--cached", NULL); + setup_child_process(s, &cp, "apply", NULL); + argv_array_pushv(&cp.args, s->mode->apply_args); if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) - error(_("'git apply --cached' failed")); + error(_("'git apply' failed")); if (!repo_read_index(s->s.r)) repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0, 1, NULL, NULL, NULL); @@ -1301,7 +1340,8 @@ soft_increment: 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 @@ -1310,6 +1350,9 @@ int run_add_p(struct repository *r, const struct pathspec *ps) init_add_i_state(&s.s, r); + s.mode = &patch_mode_add; + s.revision = revision; + if (discard_index(r->index) < 0 || repo_read_index(r) < 0 || repo_refresh_and_write_index(r, REFRESH_QUIET, 0, 1, NULL, NULL, NULL) < 0 || diff --git a/builtin/add.c b/builtin/add.c index 4c38aff419..57c9d70bc9 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -196,12 +196,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_ADD; + 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 5adbabbba65bbcfa8f8feac02268e90f992ba9f4 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 15:42:52 +0100 Subject: [PATCH 03/18] 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 | 2 +- add-patch.c | 3 +++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/add-interactive.c b/add-interactive.c index 1786ea29c4..9e4bcb382c 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -56,11 +56,16 @@ void init_add_i_state(struct add_i_state *s, struct repository *r) FREE_AND_NULL(s->interactive_diff_filter); git_config_get_string("interactive.difffilter", &s->interactive_diff_filter); + + FREE_AND_NULL(s->interactive_diff_algorithm); + git_config_get_string("diff.algorithm", + &s->interactive_diff_algorithm); } void clear_add_i_state(struct add_i_state *s) { FREE_AND_NULL(s->interactive_diff_filter); + FREE_AND_NULL(s->interactive_diff_algorithm); memset(s, 0, sizeof(*s)); s->use_color = -1; } diff --git a/add-interactive.h b/add-interactive.h index 46c73867ad..923efaf527 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; }; void init_add_i_state(struct add_i_state *s, struct repository *r); diff --git a/add-patch.c b/add-patch.c index 78bde41df0..8f2ee8688b 100644 --- a/add-patch.c +++ b/add-patch.c @@ -360,6 +360,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'; @@ -369,6 +370,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) int res; argv_array_pushv(&args, s->mode->diff_cmd); + if (diff_algorithm) + argv_array_pushf(&args, "--diff-algorithm=%s", diff_algorithm); if (s->revision) { struct object_id oid; argv_array_push(&args, From a1c2a5f4bd1c743397d8a32806c7ce16ab3ae524 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 22:38:02 +0100 Subject: [PATCH 04/18] 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 that command to be able to perform that work, too. Signed-off-by: Johannes Schindelin --- add-interactive.h | 2 ++ add-patch.c | 83 ++++++++++++++++++++++++++++++++++++++++++++--- builtin/add.c | 4 +++ 3 files changed, 85 insertions(+), 4 deletions(-) diff --git a/add-interactive.h b/add-interactive.h index e29a769aba..1f6a61326e 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -25,6 +25,8 @@ int run_add_i(struct repository *r, const struct pathspec *ps); enum add_p_mode { ADD_P_ADD, + 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 71356fbd9a..af0a86f0f7 100644 --- a/add-patch.c +++ b/add-patch.c @@ -19,7 +19,7 @@ struct patch_mode { * trailing `NULL`. */ const char *diff_cmd[4], *apply_args[4], *apply_check_args[4]; - unsigned is_reverse:1, apply_for_checkout:1; + unsigned is_reverse:1, index_only:1, apply_for_checkout:1; const char *prompt_mode[PROMPT_MODE_MAX]; const char *edit_hunk_hint, *help_patch_text; }; @@ -45,6 +45,72 @@ static struct patch_mode patch_mode_add = { "the file\n") }; +static struct patch_mode patch_mode_stash = { + .diff_cmd = { "diff-index", "HEAD", NULL }, + .apply_args = { "--cached", NULL }, + .apply_check_args = { "--cached", NULL }, + .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_cmd = { "diff-index", "--cached", NULL }, + .apply_args = { "-R", "--cached", NULL }, + .apply_check_args = { "-R", "--cached", NULL }, + .is_reverse = 1, + .index_only = 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_cmd = { "diff-index", "-R", "--cached", NULL }, + .apply_args = { "--cached", NULL }, + .apply_check_args = { "--cached", NULL }, + .index_only = 1, + .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; /* @@ -1350,12 +1416,21 @@ int run_add_p(struct repository *r, enum add_p_mode mode, init_add_i_state(&s.s, r); - s.mode = &patch_mode_add; + 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_add; s.revision = revision; if (discard_index(r->index) < 0 || repo_read_index(r) < 0 || - repo_refresh_and_write_index(r, REFRESH_QUIET, 0, 1, - NULL, NULL, NULL) < 0 || + (!s.mode->index_only && + repo_refresh_and_write_index(r, REFRESH_QUIET, 0, 1, + NULL, NULL, NULL) < 0) || parse_diff(&s, ps) < 0) { strbuf_release(&s.plain); strbuf_release(&s.colored); diff --git a/builtin/add.c b/builtin/add.c index 57c9d70bc9..3cc341d961 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_ADD; + 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 a3d288974fd59e78df606d6b686e3de6d1440752 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 4 Apr 2019 22:17:07 +0200 Subject: [PATCH 05/18] 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 fa13ee672d..1fb40b3a0a 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 @@ error: 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 64c11221229f07fdb69d15d5c40fd1e55ec7bb7b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 24 Mar 2019 19:55:08 +0100 Subject: [PATCH 06/18] legacy stash -p: respect the add.interactive.usebuiltin setting As `git add` traditionally did not expose the `--patch=` modes via command-line options, the scripted version of `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 scripted version (which does not have a "fall-back" to the built-in version). So let's introduce support for internal switch for `git add` that the scripted `git stash` can use to call the appropriate backend (scripted or built-in, depending on `add.interactive.useBuiltin`). Signed-off-by: Johannes Schindelin --- builtin/add.c | 15 +++++++++++++++ git-legacy-stash.sh | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/builtin/add.c b/builtin/add.c index 3cc341d961..27cd8d0881 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -31,6 +31,7 @@ static int take_worktree_changes; static int add_renormalize; static int pathspec_file_nul; static const char *pathspec_from_file; +static int legacy_stash_p; /* support for the scripted `git stash` */ struct update_callback_data { int flags; @@ -339,6 +340,8 @@ static struct option builtin_add_options[] = { N_("warn when adding an embedded repository")), OPT_PATHSPEC_FROM_FILE(&pathspec_from_file), OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul), + OPT_HIDDEN_BOOL(0, "legacy-stash-p", &legacy_stash_p, + N_("backend for `git stash -p`")), OPT_END(), }; @@ -439,6 +442,18 @@ int cmd_add(int argc, const char **argv, const char *prefix) exit(interactive_add(argc - 1, argv + 1, prefix, patch_interactive)); } + if (legacy_stash_p) { + struct pathspec pathspec; + + parse_pathspec(&pathspec, 0, + PATHSPEC_PREFER_FULL | + PATHSPEC_SYMLINK_LEADING_PATH | + PATHSPEC_PREFIX_ORIGIN, + prefix, argv); + + return run_add_interactive(NULL, "--patch=stash", &pathspec); + } + if (edit_interactive) { if (pathspec_from_file) die(_("--pathspec-from-file is incompatible with --edit")); diff --git a/git-legacy-stash.sh b/git-legacy-stash.sh index 53fa574301..4d4ebb4f2b 100755 --- a/git-legacy-stash.sh +++ b/git-legacy-stash.sh @@ -207,7 +207,7 @@ create_stash () { # find out what the user wants GIT_INDEX_FILE="$TMP-index" \ - git add--interactive --patch=stash -- "$@" && + git add --legacy-stash-p -- "$@" && # state of the working tree w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) || From 91102f20d4d42b83fbbef64487739d6c0f854670 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 4 Apr 2019 22:21:20 +0200 Subject: [PATCH 07/18] 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 1fb40b3a0a..16e9949da1 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 02dfe19f1fa13bca73ec29bcbe6ee109977f5108 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 4 Apr 2019 15:33:03 +0200 Subject: [PATCH 08/18] 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 the internal API directly, which will now incidentally also respect the `add.interactive.useBuiltin` setting. Let's just 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 4ad3adf4ba..879fc5f368 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -998,9 +998,9 @@ static int stash_patch(struct stash_info *info, const 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); @@ -1014,16 +1014,19 @@ static int stash_patch(struct stash_info *info, const 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 2db73f19aed1af4899a2fe60afa4dc09a4dbbac6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 26 Mar 2019 21:28:10 +0100 Subject: [PATCH 09/18] 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 16e9949da1..1b2564042a 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 97db7cd69d..a9d52b8464 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 6c31bdee1ccf60de0682cac78277eee21b085d4d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 22:38:02 +0100 Subject: [PATCH 10/18] 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 | 138 ++++++++++++++++++++++++++++++++++++++++++++-- builtin/add.c | 5 +- 3 files changed, 137 insertions(+), 7 deletions(-) diff --git a/add-interactive.h b/add-interactive.h index 1f6a61326e..77907f6e21 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -27,6 +27,7 @@ enum add_p_mode { ADD_P_ADD, 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 af0a86f0f7..ec5116c187 100644 --- a/add-patch.c +++ b/add-patch.c @@ -111,6 +111,71 @@ static struct patch_mode patch_mode_reset_nothead = { "the file\n"), }; +static struct patch_mode patch_mode_checkout_index = { + .diff_cmd = { "diff-files", NULL }, + .apply_args = { "-R", NULL }, + .apply_check_args = { "-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_cmd = { "diff-index", NULL }, + .apply_for_checkout = 1, + .apply_check_args = { "-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_cmd = { "diff-index", "-R", NULL }, + .apply_for_checkout = 1, + .apply_check_args = { NULL }, + .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; /* @@ -1067,6 +1132,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(s, &check_index, + "apply", "--cached", "--check", reverse, NULL); + applies_index = !pipe_command(&check_index, diff->buf, diff->len, + NULL, 0, NULL, 0); + + setup_child_process(s, &check_worktree, + "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(s, &apply_index, + "apply", "--cached", reverse, NULL); + pipe_command(&apply_index, diff->buf, diff->len, + NULL, 0, NULL, 0); + + setup_child_process(s, &apply_worktree, + "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(s, &apply_worktree, + "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, @@ -1392,11 +1508,16 @@ soft_increment: reassemble_patch(s, file_diff, 0, &s->buf); discard_index(s->s.r->index); - setup_child_process(s, &cp, "apply", NULL); - argv_array_pushv(&cp.args, s->mode->apply_args); - 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(s, &cp, "apply", NULL); + argv_array_pushv(&cp.args, s->mode->apply_args); + if (pipe_command(&cp, s->buf.buf, s->buf.len, + NULL, 0, NULL, 0)) + error(_("'git apply' failed")); + } if (!repo_read_index(s->s.r)) repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0, 1, NULL, NULL, NULL); @@ -1423,6 +1544,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_add; s.revision = revision; diff --git a/builtin/add.c b/builtin/add.c index 27cd8d0881..bd7cc41f5e 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -208,9 +208,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 778182c9cb2df1a4ea085029d620745e1bf5a22c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 26 Mar 2019 21:37:27 +0100 Subject: [PATCH 11/18] 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 | 2 ++ add-interactive.h | 1 + add-patch.c | 21 +++++++++++++++++---- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 9e4bcb382c..39c3896494 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -60,6 +60,8 @@ void init_add_i_state(struct add_i_state *s, struct repository *r) FREE_AND_NULL(s->interactive_diff_algorithm); git_config_get_string("diff.algorithm", &s->interactive_diff_algorithm); + + git_config_get_bool("interactive.singlekey", &s->use_single_key); } void clear_add_i_state(struct add_i_state *s) diff --git a/add-interactive.h b/add-interactive.h index 923efaf527..693f125e8e 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; }; diff --git a/add-patch.c b/add-patch.c index 8f2ee8688b..d8dafa8168 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, @@ -1149,14 +1150,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; @@ -1396,9 +1410,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 8908d4ac4abbccd5041046bef8aa8dde4d763880 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 23 Mar 2019 22:38:02 +0100 Subject: [PATCH 12/18] built-in add -p: implement the "worktree" patch modes This is a straight-forward port of 2f0896ec3ad4 (restore: support --patch, 2019-04-25) which added support for `git restore -p`. Signed-off-by: Johannes Schindelin --- add-interactive.h | 1 + add-patch.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++ builtin/add.c | 2 ++ 3 files changed, 53 insertions(+) diff --git a/add-interactive.h b/add-interactive.h index 77907f6e21..b2f23479c5 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -28,6 +28,7 @@ enum add_p_mode { ADD_P_STASH, ADD_P_RESET, ADD_P_CHECKOUT, + ADD_P_WORKTREE, }; int run_add_p(struct repository *r, enum add_p_mode mode, diff --git a/add-patch.c b/add-patch.c index ec5116c187..46c6c183d5 100644 --- a/add-patch.c +++ b/add-patch.c @@ -176,6 +176,49 @@ static struct patch_mode patch_mode_checkout_nothead = { "the file\n"), }; +static struct patch_mode patch_mode_worktree_head = { + .diff_cmd = { "diff-index", NULL }, + .apply_args = { "-R", NULL }, + .apply_check_args = { "-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 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_worktree_nothead = { + .diff_cmd = { "diff-index", "-R", NULL }, + .apply_args = { NULL }, + .apply_check_args = { NULL }, + .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 worktree\n" + "n - do not apply this hunk to 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; /* @@ -1551,6 +1594,13 @@ int run_add_p(struct repository *r, enum add_p_mode mode, s.mode = &patch_mode_checkout_head; else s.mode = &patch_mode_checkout_nothead; + } else if (mode == ADD_P_WORKTREE) { + if (!revision) + s.mode = &patch_mode_checkout_index; + else if (!strcmp(revision, "HEAD")) + s.mode = &patch_mode_worktree_head; + else + s.mode = &patch_mode_worktree_nothead; } else s.mode = &patch_mode_add; s.revision = revision; diff --git a/builtin/add.c b/builtin/add.c index bd7cc41f5e..83c7c0f250 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -210,6 +210,8 @@ int run_add_interactive(const char *revision, const char *patch_mode, mode = ADD_P_RESET; else if (!strcmp(patch_mode, "--patch=checkout")) mode = ADD_P_CHECKOUT; + else if (!strcmp(patch_mode, "--patch=worktree")) + mode = ADD_P_WORKTREE; else die("'%s' not supported", patch_mode); From 3887aec388cee6ebe7bca4376ad920a7185c7499 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 27 Mar 2019 17:14:02 +0100 Subject: [PATCH 13/18] 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 | 56 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/compat/terminal.c b/compat/terminal.c index 1b2564042a..b7f58d1781 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -161,6 +161,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 +259,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 b0d84930e0ff1115cd48a25548962c3e68e5aa8e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 29 Mar 2019 15:03:09 +0100 Subject: [PATCH 14/18] 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 when being called from `commit --interactive`, it has to, because the index was already locked in that case, 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()`, or for that matter `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 646e84547d..c70ad01cc9 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -367,7 +367,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); @@ -375,12 +375,16 @@ 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) != 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 9802576c3800ccfc76c6ce41533c0bfddf2cf152 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 27 Mar 2019 17:14:02 +0100 Subject: [PATCH 15/18] 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 | 73 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/compat/terminal.c b/compat/terminal.c index b7f58d1781..35bca03d14 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 "hashmap.h" #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE) @@ -238,6 +239,71 @@ 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->entry, + strhash(e->sequence)); + hashmap_add(&sequences, &e->entry); + } + 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; @@ -271,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 f14f271f109f692a2eaa3d601e7419257ceb58c7 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 6 Apr 2019 22:31:40 +0200 Subject: [PATCH 16/18] 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 9546b6f8a4..ab7d7aa6de 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 0630e922a3e6c97594bfb80631302551fa901cbf Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 28 Mar 2019 20:06:37 +0100 Subject: [PATCH 17/18] 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 ff0ef7f08e..4df54c4efe 100755 --- a/ci/run-build-and-tests.sh +++ b/ci/run-build-and-tests.sh @@ -20,6 +20,7 @@ linux-gcc) 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 ;; linux-gcc-4.8) From 897863393fff0cdd0edce6f10ee686db1c42579a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 6 Apr 2019 22:46:09 +0200 Subject: [PATCH 18/18] 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 879fc5f368..daa3c70c18 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1036,7 +1036,7 @@ static int stash_patch(struct stash_info *info, const 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 4d4ebb4f2b..256d9badec 100755 --- a/git-legacy-stash.sh +++ b/git-legacy-stash.sh @@ -213,7 +213,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 ab7d7aa6de..accfe3845c 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