Re: git cherry-pick conflict error message is deceptive when cherry-picking multiple commits

[Christian Couder <christian.couder@gmail.com>]

Hi Stephen,

On Wed, Aug 10, 2016 at 9:21 PM, Stephen Morton
<stephen.morton@nokia.com> wrote:
>
> Formatting on previous email was terrible, plus the diff wasn't performed
> against origin. Re-sending.

Thanks for working on this...

> (Finally getting back to this.)
>
> Something like the diff below, then Johannes?

...but please try to send a real patch.

There is https://github.com/git/git/blob/master/Documentation/SubmittingPatches
and also SubmitGit that can help you do that.

> (I intentionally print the '--continue' hint even in the case whereit's last
> of n commits that fails.)
>
>
> Stephen
>
>
> ~/ws/extern/git (maint *%>) > git diff @{u}
> diff --git a/sequencer.c b/sequencer.c
> index c6362d6..e0071aa 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -154,7 +154,7 @@ static void free_message(struct commit *commit, struct
> commit_message *msg)
>         unuse_commit_buffer(commit, msg->message);
>  }
>
> -static void print_advice(int show_hint, struct replay_opts *opts)
> +static void print_advice(int show_hint, int multiple_commits, struct
> replay_opts *opts)

Here multiple_commits is not the last argument...

>  {
>         char *msg = getenv("GIT_CHERRY_PICK_HELP");
>
> @@ -174,9 +174,14 @@ static void print_advice(int show_hint, struct
> replay_opts *opts)
>                         advise(_("after resolving the conflicts, mark the
> corrected paths\n"
>                                  "with 'git add <paths>' or 'git rm
> <paths>'"));
>                 else
> -                       advise(_("after resolving the conflicts, mark the
> corrected paths\n"
> -                                "with 'git add <paths>' or 'git rm
> <paths>'\n"
> -                                "and commit the result with 'git
> commit'"));
> +                        if  (multiple_commits)
> +                               advise(_("after resolving the conflicts,
> mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\n"
> +                                        "then continue with 'git %s
> --continue'\n"
> +                                        "or cancel with 'git %s --abort'"
> ), action_name(opts), action_name(opts));
> +                        else
> +                                advise(_("after resolving the conflicts,
> mark the corrected paths\n"
> +                                        "with 'git add <paths>' or 'git rm
> <paths>'\n"
> +                                        "and commit the result with 'git
> commit'"));
>         }
>  }
>
> @@ -440,7 +445,7 @@ static int allow_empty(struct replay_opts *opts, struct
> commit *commit)
>                 return 1;
>  }
>
> -static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
> +static int do_pick_commit(struct commit *commit, struct replay_opts *opts,
> int multiple_commits)

... but here multiple_commits is the last argument.
It would be better if it was more consistent.

>  {
>         unsigned char head[20];
>         struct commit *base, *next, *parent;
> @@ -595,7 +600,7 @@ static int do_pick_commit(struct commit *commit, struct
> replay_opts *opts)
>                       : _("could not apply %s... %s"),
>                       find_unique_abbrev(commit->object.oid.hash,
> DEFAULT_ABBREV),
>                       msg.subject);
> -               print_advice(res == 1, opts);
> +               print_advice(res == 1, multiple_commits, opts);
>                 rerere(opts->allow_rerere_auto);
>                 goto leave;
>         }
> @@ -959,6 +964,7 @@ static int pick_commits(struct commit_list *todo_list,
> struct replay_opts *opts)
>  {
>         struct commit_list *cur;
>         int res;
> +    int multiple_commits = (todo_list->next) != NULL;

Why not "last_commit" instead of "multiple_commits"?

Thanks,
Christian.