diff options
| -rw-r--r-- | builtin/revert.c | 4 | ||||
| -rw-r--r-- | sequencer.c | 52 | ||||
| -rw-r--r-- | sequencer.h | 6 | ||||
| -rwxr-xr-x | t/t3510-cherry-pick-sequence.sh | 32 |
4 files changed, 77 insertions, 17 deletions
diff --git a/builtin/revert.c b/builtin/revert.c index 314a86c562..237f2f18d4 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -182,7 +182,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts) "--signoff", opts->signoff, "--no-commit", opts->no_commit, "-x", opts->record_origin, - "--edit", opts->edit, + "--edit", opts->edit > 0, NULL); if (cmd) { @@ -230,8 +230,6 @@ int cmd_revert(int argc, const char **argv, const char *prefix) struct replay_opts opts = REPLAY_OPTS_INIT; int res; - if (isatty(0)) - opts.edit = 1; opts.action = REPLAY_REVERT; sequencer_init_config(&opts); res = run_sequencer(argc, argv, &opts); diff --git a/sequencer.c b/sequencer.c index b4c63ae207..fd183b5593 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2032,13 +2032,24 @@ static void record_in_rewritten(struct object_id *oid, flush_rewritten_pending(); } +static int should_edit(struct replay_opts *opts) { + if (opts->edit < 0) + /* + * Note that we only handle the case of non-conflicted + * commits; continue_single_pick() handles the conflicted + * commits itself instead of calling this function. + */ + return (opts->action == REPLAY_REVERT && isatty(0)) ? 1 : 0; + return opts->edit; +} + static int do_pick_commit(struct repository *r, struct todo_item *item, struct replay_opts *opts, int final_fixup, int *check_todo) { - unsigned int flags = opts->edit ? EDIT_MSG : 0; - const char *msg_file = opts->edit ? NULL : git_path_merge_msg(r); + unsigned int flags = should_edit(opts) ? EDIT_MSG : 0; + const char *msg_file = should_edit(opts) ? NULL : git_path_merge_msg(r); struct object_id head; struct commit *base, *next, *parent; const char *base_label, *next_label; @@ -3283,9 +3294,9 @@ static int save_opts(struct replay_opts *opts) if (opts->no_commit) res |= git_config_set_in_file_gently(opts_file, "options.no-commit", "true"); - if (opts->edit) - res |= git_config_set_in_file_gently(opts_file, - "options.edit", "true"); + if (opts->edit >= 0) + res |= git_config_set_in_file_gently(opts_file, "options.edit", + opts->edit ? "true" : "false"); if (opts->allow_empty) res |= git_config_set_in_file_gently(opts_file, "options.allow-empty", "true"); @@ -4259,7 +4270,7 @@ static int pick_commits(struct repository *r, prev_reflog_action = xstrdup(getenv(GIT_REFLOG_ACTION)); if (opts->allow_ff) assert(!(opts->signoff || opts->no_commit || - opts->record_origin || opts->edit || + opts->record_origin || should_edit(opts) || opts->committer_date_is_author_date || opts->ignore_date)); if (read_and_refresh_cache(r, opts)) @@ -4552,14 +4563,33 @@ cleanup_head_ref: return sequencer_remove_state(opts); } -static int continue_single_pick(struct repository *r) +static int continue_single_pick(struct repository *r, struct replay_opts *opts) { - const char *argv[] = { "commit", NULL }; + struct strvec argv = STRVEC_INIT; + int ret; if (!refs_ref_exists(get_main_ref_store(r), "CHERRY_PICK_HEAD") && !refs_ref_exists(get_main_ref_store(r), "REVERT_HEAD")) return error(_("no cherry-pick or revert in progress")); - return run_command_v_opt(argv, RUN_GIT_CMD); + + strvec_push(&argv, "commit"); + + /* + * continue_single_pick() handles the case of recovering from a + * conflict. should_edit() doesn't handle that case; for a conflict, + * we want to edit if the user asked for it, or if they didn't specify + * and stdin is a tty. + */ + if (!opts->edit || (opts->edit < 0 && !isatty(0))) + /* + * Include --cleanup=strip as well because we don't want the + * "# Conflicts:" messages. + */ + strvec_pushl(&argv, "--no-edit", "--cleanup=strip", NULL); + + ret = run_command_v_opt(argv.v, RUN_GIT_CMD); + strvec_clear(&argv); + return ret; } static int commit_staged_changes(struct repository *r, @@ -4729,7 +4759,7 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts) goto release_todo_list; } } else if (!file_exists(get_todo_path(opts))) - return continue_single_pick(r); + return continue_single_pick(r, opts); else if ((res = read_populate_todo(r, &todo_list, opts))) goto release_todo_list; @@ -4738,7 +4768,7 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts) if (refs_ref_exists(get_main_ref_store(r), "CHERRY_PICK_HEAD") || refs_ref_exists(get_main_ref_store(r), "REVERT_HEAD")) { - res = continue_single_pick(r); + res = continue_single_pick(r, opts); if (res) goto release_todo_list; } diff --git a/sequencer.h b/sequencer.h index f8b2e4ab85..d57d8ea23d 100644 --- a/sequencer.h +++ b/sequencer.h @@ -31,8 +31,10 @@ enum commit_msg_cleanup_mode { struct replay_opts { enum replay_action action; - /* Boolean options */ + /* Tri-state options: unspecified, false, or true */ int edit; + + /* Boolean options */ int record_origin; int no_commit; int signoff; @@ -71,7 +73,7 @@ struct replay_opts { /* Only used by REPLAY_NONE */ struct rev_info *revs; }; -#define REPLAY_OPTS_INIT { .action = -1, .current_fixups = STRBUF_INIT } +#define REPLAY_OPTS_INIT { .edit = -1, .action = -1, .current_fixups = STRBUF_INIT } /* * Note that ordering matters in this enum. Not only must it match the mapping diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh index b76cb6de91..49010aa946 100755 --- a/t/t3510-cherry-pick-sequence.sh +++ b/t/t3510-cherry-pick-sequence.sh @@ -65,7 +65,7 @@ test_expect_success 'cherry-pick persists opts correctly' ' # gets interrupted, use a high-enough number that is larger # than the number of parents of any commit we have created mainline=4 && - test_expect_code 128 git cherry-pick -s -m $mainline --strategy=recursive -X patience -X ours initial..anotherpick && + test_expect_code 128 git cherry-pick -s -m $mainline --strategy=recursive -X patience -X ours --edit initial..anotherpick && test_path_is_dir .git/sequencer && test_path_is_file .git/sequencer/head && test_path_is_file .git/sequencer/todo && @@ -84,6 +84,36 @@ test_expect_success 'cherry-pick persists opts correctly' ' ours EOF git config --file=.git/sequencer/opts --get-all options.strategy-option >actual && + test_cmp expect actual && + echo "true" >expect && + git config --file=.git/sequencer/opts --get-all options.edit >actual && + test_cmp expect actual +' + +test_expect_success 'revert persists opts correctly' ' + pristine_detach initial && + # to make sure that the session to revert a sequence + # gets interrupted, revert commits that are not in the history + # of HEAD. + test_expect_code 1 git revert -s --strategy=recursive -X patience -X ours --no-edit picked yetanotherpick && + test_path_is_dir .git/sequencer && + test_path_is_file .git/sequencer/head && + test_path_is_file .git/sequencer/todo && + test_path_is_file .git/sequencer/opts && + echo "true" >expect && + git config --file=.git/sequencer/opts --get-all options.signoff >actual && + test_cmp expect actual && + echo "recursive" >expect && + git config --file=.git/sequencer/opts --get-all options.strategy >actual && + test_cmp expect actual && + cat >expect <<-\EOF && + patience + ours + EOF + git config --file=.git/sequencer/opts --get-all options.strategy-option >actual && + test_cmp expect actual && + echo "false" >expect && + git config --file=.git/sequencer/opts --get-all options.edit >actual && test_cmp expect actual ' |