Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[Junio C Hamano <gitster@pobox.com>]

ZheNing Hu <adlternative@gmail.com> writes:

> I think this function "check_need_delete_cherry_pick_head()" should be before
> print_advice(), like this:
>
> +               const char *help_msgs = NULL;
> +
>                 error(command == TODO_REVERT
>                       ? _("could not revert %s... %s")
>                       : _("could not apply %s... %s"),
>                       short_commit_name(commit), msg.subject);
> -               print_advice(r, res == 1, opts);
> +               if (((opts->action == REPLAY_PICK &&
> +                     !opts->rebase_preserve_merges_mode) ||
> +                     (help_msgs = check_need_delete_cherry_pick_head(r))) &&
> +                     res == 1)
> +                       print_advice(opts, help_msgs);

Sorry, but I am not sure what problem this separation is trying to
solve.

The root cause of the issue we have, I think, is because the
decision to delete or keep the cherry-pick-head pseudoref is tied to
what message we give to users in the current code, and the suggested
split of concern is to limit print_advice() to only print the advice
message, and a new helper to decide and remove the pseudoref,
without relying on what is in the GIT_CHERRY_PICK_HELP environment.

It is unclear where you are making the decision to keep or remove
the pseudoref in the above arrangement that lets the new
check_need_delete_cherry_pick_head() decide if the advice is printed
or not.