| Message ID | 20190623200338.17144-3-rohit.ashiwal265@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Teach cherry-pick/revert to skip commits | expand |
Hi Rohit On 23/06/2019 21:03, Rohit Ashiwal wrote: > In the case of merge conflicts, while performing a revert, we are > currently advised to use `git cherry-pick --<sequencer-options>` > of which --continue is incompatible for continuing the revert. I'm not sure why this says we cannot use --continue with revert, I think it should work fine. Also below your new advice recommends `revert --continue` as an option which contradicts this part of the commit message. Oh do you mean `cherry-pick --abort` and `cherry-pick --quit` work with a revert but `cherry-pick --continue` does not? If so that's a quirk of the implementation that should be fixed. If that's the case I'm think mentioning it just confuses the commit message. Just say the advice for revert should say revert. I'd also squash the first patch into this one so the new variable is used when it is introduced Best Wishes Phillip > Introduce a separate advice message for `git revert`. Also change > the signature of `create_seq_dir` to handle which advice to display > selectively. > > Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> > --- > sequencer.c | 35 +++++++++++++++++++++++++++++------ > 1 file changed, 29 insertions(+), 6 deletions(-) > > diff --git a/sequencer.c b/sequencer.c > index f88a97fb10..0ef2622a69 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -2650,15 +2650,38 @@ static int walk_revs_populate_todo(struct todo_list *todo_list, > return 0; > } > > -static int create_seq_dir(void) > +static int create_seq_dir(struct repository *r) > { > - if (file_exists(git_path_seq_dir())) { > - error(_("a cherry-pick or revert is already in progress")); > - advise(_("try \"git cherry-pick (--continue | --quit | --abort)\"")); > + enum replay_action action; > + const char *in_progress_error = NULL; > + const char *in_progress_advice = NULL; > + > + if (!sequencer_get_last_command(r, &action)) { > + switch (action) { > + case REPLAY_REVERT: > + in_progress_error = _("revert is already in progress"); > + in_progress_advice = > + _("try \"git revert (--continue | --abort | --quit)\""); > + break; > + case REPLAY_PICK: > + in_progress_error = _("cherry-pick is already in progress"); > + in_progress_advice = > + _("try \"git cherry-pick (--continue | --abort | --quit)\""); > + break; > + default: > + BUG("unexpected action in create_seq_dir"); > + } > + } > + if (in_progress_error) { > + error("%s", in_progress_error); > + if (advice_sequencer_in_use) > + advise("%s", in_progress_advice); > return -1; > - } else if (mkdir(git_path_seq_dir(), 0777) < 0) > + } > + if (mkdir(git_path_seq_dir(), 0777) < 0) > return error_errno(_("could not create sequencer directory '%s'"), > git_path_seq_dir()); > + > return 0; > } > > @@ -4237,7 +4260,7 @@ int sequencer_pick_revisions(struct repository *r, > */ > > if (walk_revs_populate_todo(&todo_list, opts) || > - create_seq_dir() < 0) > + create_seq_dir(r) < 0) > return -1; > if (get_oid("HEAD", &oid) && (opts->action == REPLAY_REVERT)) > return error(_("can't revert as initial commit")); >
diff --git a/sequencer.c b/sequencer.c index f88a97fb10..0ef2622a69 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2650,15 +2650,38 @@ static int walk_revs_populate_todo(struct todo_list *todo_list, return 0; } -static int create_seq_dir(void) +static int create_seq_dir(struct repository *r) { - if (file_exists(git_path_seq_dir())) { - error(_("a cherry-pick or revert is already in progress")); - advise(_("try \"git cherry-pick (--continue | --quit | --abort)\"")); + enum replay_action action; + const char *in_progress_error = NULL; + const char *in_progress_advice = NULL; + + if (!sequencer_get_last_command(r, &action)) { + switch (action) { + case REPLAY_REVERT: + in_progress_error = _("revert is already in progress"); + in_progress_advice = + _("try \"git revert (--continue | --abort | --quit)\""); + break; + case REPLAY_PICK: + in_progress_error = _("cherry-pick is already in progress"); + in_progress_advice = + _("try \"git cherry-pick (--continue | --abort | --quit)\""); + break; + default: + BUG("unexpected action in create_seq_dir"); + } + } + if (in_progress_error) { + error("%s", in_progress_error); + if (advice_sequencer_in_use) + advise("%s", in_progress_advice); return -1; - } else if (mkdir(git_path_seq_dir(), 0777) < 0) + } + if (mkdir(git_path_seq_dir(), 0777) < 0) return error_errno(_("could not create sequencer directory '%s'"), git_path_seq_dir()); + return 0; } @@ -4237,7 +4260,7 @@ int sequencer_pick_revisions(struct repository *r, */ if (walk_revs_populate_todo(&todo_list, opts) || - create_seq_dir() < 0) + create_seq_dir(r) < 0) return -1; if (get_oid("HEAD", &oid) && (opts->action == REPLAY_REVERT)) return error(_("can't revert as initial commit"));
In the case of merge conflicts, while performing a revert, we are currently advised to use `git cherry-pick --<sequencer-options>` of which --continue is incompatible for continuing the revert. Introduce a separate advice message for `git revert`. Also change the signature of `create_seq_dir` to handle which advice to display selectively. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> --- sequencer.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-)