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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 27, 2016 at 11:03 AM, Johannes Schindelin
<Johannes.Schindelin@xxxxxx> wrote:
> Hi Stephen,
>
> On Wed, 27 Jul 2016, Stephen Morton wrote:
>
>> Here is my patch then. (Personally, I would add some capitalization and
>> punctuation, but I didn't see much of that in the existing code.) I'm
>> not a regular pull-requester, do I do that, or can somebody else handle
>> that for me?
>
> The process of the patch submission is described in
> Documentation/SubmittingPatches (yes, it is a bit involved, and it is
> slightly easier when you use http://submitgit.herokuapp.com/, but please
> note that this process has served us well over one decade).
>
> Please also note that top-posting is highly discouraged on this list:
>
> A: Because it messes up the order in which people normally read text.
>>Q: Why is top-posting such a bad thing?
>>>A: Top-posting.
>>>>Q: What is the most annoying thing in e-mail?
>
> Now to your patch:
>
>> diff --git a/sequencer.c b/sequencer.c
>> index cdfac82..ce06876 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -176,7 +176,8 @@ static void print_advice(int show_hint, struct
>> replay_opts *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'"));
>> +                                "then continue the %s with 'git %s
>> --continue'\n"
>> +                                "or cancel the %s operation with 'git
>> %s --abort'" ),  action_name(opts), action_name(opts),
>> action_name(opts), action_name(opts));
>
> That is an awful lot of repetition right there, with an added
> inconsistency that the action is referred to by its name alone in the
> "--continue" case, but with "operation" added in the "--abort" case.
>
> And additionally, in the most common case (one commit to cherry-pick), the
> advice now suggests a more complicated operation than necessary: a simply
> `git commit` would be enough, then.
>
> Can't we have a test whether this is the last of the commits to be
> cherry-picked, and if so, have the simpler advice again?
>
> Ciao,
> Johannes

Ok, knowing that I'm not on the last element of the sequencer is
beyond my git code knowledge. I see that in do_pick_commit() , we do
not have a copy of the todo_list. I would assume that would be
necessary, but I'm not certain. I could
file_exists(git_path_seq_dir()). This works to determine if one or
many commits are being cherry-picked / reverted, although it will
return true even on the last of n cherry-picks. I think that is still
reasonable.

I was trying to just take the same text as 'git status' already
displays. It could indeed be made more concise.

Happy to use the submission process, I just didn't know it. Thanks for
letting me know.

(Yup, sorry about the top-posting. I just wan't careful.)

Stephen
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]