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

[Stephen Morton <stephen.morton@nokia.com>]

Responding to a few comments...

On 2016-08-14 7:44 AM, Christian Couder wrote:
> multiple_commits)
> ... but here multiple_commits is the last argument.
> It would be better if it was more consistent.
(Johannes made the same comment.)
Yes. Will do.


>
> multiple_commits = (todo_list->next) != NULL;
> Why not "last_commit" instead of "multiple_commits"?
>

Because it *isn't*. You can see that in pick_commits(), I set 
multiple_commits outside of the `for todo_list` loop. It is not 
re-evaluated at every iteration of the loop. As per my comment when 
emailing the patch "I intentionally print the '--continue' hint even in 
the case where it's last of n commits that fails. " I think it makes 
much more sense that "this is the message you always get when 
cherry-picking multiple commits as opposed to "this is the message you 
sometimes get, except when it's the last one". (Yes, the careful 
observer will realize that if when cherry-picking multiple commits, 
there are conflicts in the second-last and last then the --continue  
from the second-last will result in multiple_commits being set to 0. I 
can live with that.)


On 2016-08-16 4:44 AM, Remi Galan Alfonso wrote:
> Hi Stephen,
>
> Stephen Morton <stephen.morton@nokia.com> writes:
>> +                        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'"));
> In both cases (multiple_commits or not), the beginning of the advise
> is nearly the same, with only a '\n' in the middle being the
> difference:
>
> multiple_commits:
>   "after resolving the conflicts, mark the corrected paths with 'git
>   add <paths>' or 'git rm <paths>'\n"
>
> !multiple_commits:
>   "after resolving the conflicts, mark the corrected paths\n with 'git
>   add <paths>' or 'git rm <paths>'\n"
>                                                    ~~~~~~~^
>
> In 'multiple_commits' case the advise is more than 80 characters long,
> did you forget the '\n' in that case?
A previous comment had indicated that having 4 lines was too many. And I 
tend to agree. So I tried to squash it into 3. Back in xterm days, 80 
characters was sacrosanct, but is it really a big deal to exceed it now?


On 2016-08-14 7:44 AM, Christian Couder wrote:
> ...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.
Agreed. I just want to send a patch that stands a reasonable chance of 
getting accepted.

Stephen


-- 
Stephen Morton, 7750 SR Product Group, SW Development Tools/DevOps
w: +1-613-784-6026 (int: 2-825-6026) m: +1-613-302-2589 | EST Time Zone