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.mor...@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

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

Reply via email to