{"thread":{"id":"22605","subject":"git cherry-pick --continue?","startedAt":"2010-02-10T20:37:10Z","lastAt":"2010-02-12T14:11:47Z","messageCount":22,"participants":["Sverre Rabbelier","Jeff King","Junio C Hamano","Jay Soffian","SZEDER Gábor"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"134181","messageId":"fabb9a1e1002101237i60a0b2c5j6d1e52b33dacbaa2@mail.gmail.com","threadId":"22605","inReplyTo":null,"subject":"git cherry-pick --continue?","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-02-10T20:37:10Z","receivedAt":"2010-02-10T20:37:10Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nAt the moment git cherry-pick stands out from the sequencer tools in\nthat it doesn't support --continue but requires you to manually supply\nthe '-c ...' argument to 'git commit' when there's a conflict instead.\nIs it desirable that we add such an option? And if so, how would one\ngo about implementing it?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"134183","messageId":"20100210210419.GA7728@coredump.intra.peff.net","threadId":"22605","inReplyTo":"fabb9a1e1002101237i60a0b2c5j6d1e52b33dacbaa2@mail.gmail.com","subject":"Re: git cherry-pick --continue?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-10T21:04:19Z","receivedAt":"2010-02-10T21:04:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 10, 2010 at 09:37:10PM +0100, Sverre Rabbelier wrote:\n\n> At the moment git cherry-pick stands out from the sequencer tools in\n> that it doesn't support --continue but requires you to manually supply\n> the '-c ...' argument to 'git commit' when there's a conflict instead.\n> Is it desirable that we add such an option? And if so, how would one\n> go about implementing it?\n\nI think it makes sense. It is perhaps a little iffy to use \"continue\"\nwhen you are not really continuing on to further cherry-picks (and in a\nrebase, continue is not just about resolving conflicts, but about\ncontinuing to the next item in the rebase). Cherry-picks are more like\n\"am --resolved\"[1].\n\nBut semantic nitpicks aside, I think \"continue\" is a good enough name.\nThe differences between what it means for each command are fairly\nobvious based on the command, and there is real value in a user just\nhaving to remember one verb.\n\n-Peff\n\n[1] On the other hand, I usually mistype that as \"git am --continue\",\nwhich _does_ make sense, since you are applying a sequence of patches.\nMaybe \"am\" should support both.\n"},{"id":"134185","messageId":"20100210212408.GB7728@coredump.intra.peff.net","threadId":"22605","inReplyTo":"20100210210419.GA7728@coredump.intra.peff.net","subject":"Re: git cherry-pick --continue?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-10T21:24:08Z","receivedAt":"2010-02-10T21:24:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 10, 2010 at 04:04:19PM -0500, Jeff King wrote:\n\n> [1] On the other hand, I usually mistype that as \"git am --continue\",\n> which _does_ make sense, since you are applying a sequence of patches.\n> Maybe \"am\" should support both.\n\nHmm. I was thinking \"am\" was the odd man out, but really there are only\ntwo sequencer commands that I noted: rebase and am. So you could perhaps\nargue that rebase should also learn \"--resolved\". Or am I forgetting\none?\n\nI find the patch below convenient. I dunno if anybody else actually\ncares.\n\n-- >8 --\nSubject: [PATCH] am: allow --continue as a synonym for --resolved\n\nRebase calls this same function \"--continue\", which means\nusers may be trained to type it. There is no reason to\ndeprecate --resolved (or -r), but adding this synonym is\nfriendly to users.\n\nSigned-off-by: Jeff King \n---\n git-am.sh | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex c8b9cbb..88cef39 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -26,6 +26,7 @@ patch-format= format the patch(es) are in\n reject pass it through git-apply\n resolvemsg= override error message when patch failure occurs\n r,resolved to be used after a patch failure\n+continue synonym for --resolved\n skip skip the current patch\n abort restore the original branch and abort the patching operation.\n committer-date-is-author-date lie about committer date\n@@ -318,7 +319,7 @@ do\n \t\tscissors=t ;;\n \t--no-scissors)\n \t\tscissors=f ;;\n-\t-r|--resolved)\n+\t-r|--resolved|--continue)\n \t\tresolved=t ;;\n \t--skip)\n \t\tskip=t ;;\n-- \n1.7.0.rc2.22.g0716c.dirty\n"},{"id":"134186","messageId":"fabb9a1e1002101326h332508abo1a9da1185120799a@mail.gmail.com","threadId":"22605","inReplyTo":"20100210212408.GB7728@coredump.intra.peff.net","subject":"Re: git cherry-pick --continue?","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-02-10T21:26:33Z","receivedAt":"2010-02-10T21:26:33Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Feb 10, 2010 at 22:24, Jeff King wrote:\n> I find the patch below convenient. I dunno if anybody else actually\n> cares.\n\nI do, it's useful when applying a bunch of patches and conflicts arise\n:). Typing 'git am --continue' is what I'm used to from 'git rebase'.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"134188","messageId":"7v63644uoq.fsf@alter.siamese.dyndns.org","threadId":"22605","inReplyTo":"20100210212408.GB7728@coredump.intra.peff.net","subject":"Re: git cherry-pick --continue?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-10T22:01:09Z","receivedAt":"2010-02-10T22:01:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Hmm. I was thinking \"am\" was the odd man out, but really there are only\n> two sequencer commands that I noted: rebase and am. So you could perhaps\n> argue that rebase should also learn \"--resolved\". Or am I forgetting\n> one?\n\nI don't think you are forgetting anything, except that \"am\" came first with\n\"resolved\".\n\nThe focus of the verb is \"I declare I am finished marking the resolution\".\nTaking that declaration and continuing is ultimately \"am\"'s decision. IOW\nthe user is not telling \"am\" to continue---the difference is subtle, but\nit was a conscious design decision.\n\n\"rebase --continue\" came later, and I think its focus is placed more\nheavily on the instruction side (\"Please continue\"), and not on the\ndeclaration side (\"I now have marked the resolution for all paths\").\n\nThis causes people sometimes to want to see it \"continue\", even when then\nhaven't marked the resolved paths as resolved. I personally think the\nfocus is misplaced.\n\nBut that is just a philosophical difference ;-).\n"},{"id":"134192","messageId":"7vbpfw3f6t.fsf@alter.siamese.dyndns.org","threadId":"22605","inReplyTo":"7v63644uoq.fsf@alter.siamese.dyndns.org","subject":"Re: git cherry-pick --continue?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-10T22:21:14Z","receivedAt":"2010-02-10T22:21:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> Hmm. I was thinking \"am\" was the odd man out, but really there are only\n>> two sequencer commands that I noted: rebase and am. So you could perhaps\n>> argue that rebase should also learn \"--resolved\". Or am I forgetting\n>> one?\n\nHaving said all I did in the previous message, I think \"am --continue\"\nwould be a good addition.\n\nAnd \"rebase --resolved\" would not make any sense if the reason the control\nis given back to you was because you ran \"rebase -i\" and marked a commit\nto be \"edit\"ed. Of course, we could add \"--resolved\" and \"--edited\" (or\nperhaps \"--amended\") to \"rebase\", and have it make sure that the correct\none is given. For example, when it stopped for \"edit\", it would reject\n\"rebase --resolved\". But I do not think it is worth the hassle.\n"},{"id":"134194","messageId":"fabb9a1e1002101423y79460afdn2bc31b117195ef42@mail.gmail.com","threadId":"22605","inReplyTo":"7vbpfw3f6t.fsf@alter.siamese.dyndns.org","subject":"Re: git cherry-pick --continue?","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-02-10T22:23:29Z","receivedAt":"2010-02-10T22:23:29Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Feb 10, 2010 at 23:21, Junio C Hamano wrote:\n> Having said all I did in the previous message, I think \"am --continue\"\n> would be a good addition.\n\nHow about 'cherry-pick --resolved' though ;).\n\n> And \"rebase --resolved\" would not make any sense if the reason the control\n> is given back to you was because you ran \"rebase -i\" and marked a commit\n> to be \"edit\"ed.  Of course, we could add \"--resolved\" and \"--edited\" (or\n> perhaps \"--amended\") to \"rebase\", and have it make sure that the correct\n> one is given.  For example, when it stopped for \"edit\", it would reject\n> \"rebase --resolved\".  But I do not think it is worth the hassle.\n\nI don't see any benefit to that, in fact, I'd recommend against it.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"134197","messageId":"7vpr4c200i.fsf@alter.siamese.dyndns.org","threadId":"22605","inReplyTo":"fabb9a1e1002101423y79460afdn2bc31b117195ef42@mail.gmail.com","subject":"Re: git cherry-pick --continue?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-10T22:34:21Z","receivedAt":"2010-02-10T22:34:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n> On Wed, Feb 10, 2010 at 23:21, Junio C Hamano wrote:\n>> Having said all I did in the previous message, I think \"am --continue\"\n>> would be a good addition.\n>\n> How about 'cherry-pick --resolved' though ;).\n\nChanging the insn to suggest using \"-C topic\" when the original command\nline was \"git cherry-pick topic\" would be a good addition, too. Currently\nwe suggest \"-c\" and abbreviated object name, neither of which is sensible.\n\nWhile I think \"--resolved\" makes sense, I do not see much benefit, and it\nlargely depends on what you do. If you are suggesting to commit with what\nis kept in $GIT_DIR/MERGE_MSG, I would actually recommend against it. The\nmessage will have \"Conflicts:\" information but that is meaningless unless\nyou are recording from what context the commit was cherry-picked from at\nthe same time.\n"},{"id":"134249","messageId":"fabb9a1e1002101438v74aea1b9v482ff49fa4ddd15d@mail.gmail.com","threadId":"22605","inReplyTo":"7vpr4c200i.fsf@alter.siamese.dyndns.org","subject":"Re: git cherry-pick --continue?","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-02-10T22:38:28Z","receivedAt":"2010-02-10T22:38:28Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Feb 10, 2010 at 23:34, Junio C Hamano wrote:\n> Changing the insn to suggest using \"-C topic\" when the original command\n> line was \"git cherry-pick topic\" would be a good addition, too.  Currently\n> we suggest \"-c\" and abbreviated object name, neither of which is sensible.\n\nI think it's sensible to use '-c' instead of '-C', just like 'git\nrebase -i' lets you change the commit message after you resolve a\nconflict.\n\n> While I think \"--resolved\" makes sense, I do not see much benefit, and it\n> largely depends on what you do.  If you are suggesting to commit with what\n> is kept in $GIT_DIR/MERGE_MSG, I would actually recommend against it.  The\n> message will have \"Conflicts:\" information but that is meaningless unless\n> you are recording from what context the commit was cherry-picked from at\n> the same time.\n\nI'm not sure how to implement it, but I was thinking to just automate\nthe 'git commit -c ...' part. I don't like having to copy/paste some\ninstruction, so maybe we can record the original commit somewhere and\nhave 'git cherry-pick --resolve' be equivalent to 'git commit -c `cat\n.git/CHERRY_PICK_CMT`', or somesuch?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"134252","messageId":"20100211193248.GA27072@coredump.intra.peff.net","threadId":"22605","inReplyTo":"7vbpfw3f6t.fsf@alter.siamese.dyndns.org","subject":"Re: git cherry-pick --continue?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-11T19:32:48Z","receivedAt":"2010-02-11T19:32:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 10, 2010 at 02:21:14PM -0800, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > Jeff King writes:\n> >\n> >> Hmm. I was thinking \"am\" was the odd man out, but really there are only\n> >> two sequencer commands that I noted: rebase and am. So you could perhaps\n> >> argue that rebase should also learn \"--resolved\". Or am I forgetting\n> >> one?\n> \n> Having said all I did in the previous message, I think \"am --continue\"\n> would be a good addition.\n\nOK. I agree with your philosophical ramblings in the previous message,\nbut I also think there is some value in making it simple for the user to\nremember.\n\nDo you just want to pick up my patch from earlier in the thread, or do\nyou have further comments? The only thing I could think to change would\nbe that we may not want to even bother advertising --continue in the\nusage message (conversely, we could go a step further and actually\nadvertise it in the manpage).\n\n> And \"rebase --resolved\" would not make any sense if the reason the control\n> is given back to you was because you ran \"rebase -i\" and marked a commit\n> to be \"edit\"ed. Of course, we could add \"--resolved\" and \"--edited\" (or\n> perhaps \"--amended\") to \"rebase\", and have it make sure that the correct\n> one is given. For example, when it stopped for \"edit\", it would reject\n> \"rebase --resolved\". But I do not think it is worth the hassle.\n\nAgreed.\n\n-Peff\n"},{"id":"134255","messageId":"7vfx5733x7.fsf@alter.siamese.dyndns.org","threadId":"22605","inReplyTo":"20100211193248.GA27072@coredump.intra.peff.net","subject":"Re: git cherry-pick --continue?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-11T20:36:52Z","receivedAt":"2010-02-11T20:36:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> Having said all I did in the previous message, I think \"am --continue\"\n>> would be a good addition.\n>\n> OK. I agree with your philosophical ramblings in the previous message,\n> but I also think there is some value in making it simple for the user to\n> remember.\n\nCertainly I am in agreement and that is why I think \"am --continue\" is a\ngood thing.\n\n> Do you just want to pick up my patch from earlier in the thread, or do\n> you have further comments? The only thing I could think to change would\n> be that we may not want to even bother advertising --continue in the\n> usage message (conversely, we could go a step further and actually\n> advertise it in the manpage).\n\nI would say our eventual goal should be to make \"--continue\" the primary\nword the end users would see. It would bring us closer to that goal to\nstart advertising --continue early.\n"},{"id":"134257","messageId":"20100211210445.GA8819@coredump.intra.peff.net","threadId":"22605","inReplyTo":"7vpr4c200i.fsf@alter.siamese.dyndns.org","subject":"Re: git cherry-pick --continue?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-11T21:04:45Z","receivedAt":"2010-02-11T21:04:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 10, 2010 at 02:34:21PM -0800, Junio C Hamano wrote:\n\n> Changing the insn to suggest using \"-C topic\" when the original command\n> line was \"git cherry-pick topic\" would be a good addition, too. Currently\n> we suggest \"-c\" and abbreviated object name, neither of which is sensible.\n\nI think using the actual name instead of the abbreviated sha1 is\nsensible. But I think \"-c\" makes sense, as it gives the user the chance\nto look over the commit message to see if they need to tweak it to match\nthe conflict fixups. Savvy users who don't want to do that will know to\nuse \"-C\".\n\nSeries to follow:\n\n [1/4]: cherry-pick: rewrap advice message\n [2/4]: cherry-pick: refactor commit parsing code\n [3/4]: cherry-pick: format help message as strbuf\n [4/4]: cherry-pick: show commit name instead of sha1\n\n-Peff\n"},{"id":"134258","messageId":"20100211210601.GA9066@coredump.intra.peff.net","threadId":"22605","inReplyTo":"20100211210445.GA8819@coredump.intra.peff.net","subject":"[PATCH 1/4] cherry-pick: rewrap advice message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-11T21:06:01Z","receivedAt":"2010-02-11T21:06:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The current message overflows on an 80-character terminal.\nWhile we're at it, fix the spelling of 'committing'.\n\nSigned-off-by: Jeff King \n---\n builtin-revert.c | 8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 8ac86f0..83e5c0a 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -213,13 +213,13 @@ static char *help_msg(const unsigned char *sha1)\n \t\treturn msg;\n \n \tstrcpy(helpbuf, \" After resolving the conflicts,\\n\"\n-\t \"mark the corrected paths with 'git add ' \"\n-\t \"or 'git rm ' and commit the result.\");\n+\t\t\"mark the corrected paths with 'git add ' or 'git rm '\\n\"\n+\t\t\"and commit the result.\");\n \n \tif (action == CHERRY_PICK) {\n \t\tsprintf(helpbuf + strlen(helpbuf),\n-\t\t\t\"\\nWhen commiting, use the option \"\n-\t\t\t\"'-c %s' to retain authorship and message.\",\n+\t\t\t\" When committing, use the option '-c %s'\\n\"\n+\t\t\t\"to retain authorship and message.\",\n \t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n \t}\n \treturn helpbuf;\n-- \n1.7.0.rc2.40.g33ed2.dirty\n"},{"id":"134259","messageId":"20100211210642.GB9066@coredump.intra.peff.net","threadId":"22605","inReplyTo":"20100211210445.GA8819@coredump.intra.peff.net","subject":"[PATCH 2/4] cherry-pick: refactor commit parsing code","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-11T21:06:43Z","receivedAt":"2010-02-11T21:06:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These lines are really just lookup_commit_reference\nre-implemented.\n\nSigned-off-by: Jeff King \n---\nObviously this changes the exact text of the error messages, but I doubt\nanybody cares too much.\n\n builtin-revert.c | 10 ++--------\n 1 files changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 83e5c0a..012c646 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -68,15 +68,9 @@ static void parse_args(int argc, const char **argv)\n \n \tif (get_sha1(arg, sha1))\n \t\tdie (\"Cannot find '%s'\", arg);\n-\tcommit = (struct commit *)parse_object(sha1);\n+\tcommit = lookup_commit_reference(sha1);\n \tif (!commit)\n-\t\tdie (\"Could not find %s\", sha1_to_hex(sha1));\n-\tif (commit->object.type == OBJ_TAG) {\n-\t\tcommit = (struct commit *)\n-\t\t\tderef_tag((struct object *)commit, arg, strlen(arg));\n-\t}\n-\tif (commit->object.type != OBJ_COMMIT)\n-\t\tdie (\"'%s' does not point to a commit\", arg);\n+\t\texit(1);\n }\n \n static char *get_oneline(const char *message)\n-- \n1.7.0.rc2.40.g33ed2.dirty\n"},{"id":"134260","messageId":"20100211210706.GC9066@coredump.intra.peff.net","threadId":"22605","inReplyTo":"20100211210445.GA8819@coredump.intra.peff.net","subject":"[PATCH 3/4] cherry-pick: format help message as strbuf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-11T21:07:06Z","receivedAt":"2010-02-11T21:07:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This gets rid of the fixed-size buffer and an unchecked\nsprintf. That sprintf is actually OK as the only\nvariable-sized thing put in it is an abbreviated sha1, which\nis bounded at 40 characters. However, the next patch will\nchange that to something unbounded.\n\nNote that this function now returns an allocated buffer\ninstead of a static one; however, it doesn't matter as the\nonly caller exits immediately.\n\nSigned-off-by: Jeff King \n---\n builtin-revert.c | 8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 012c646..77e4f4e 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -200,23 +200,23 @@ static void set_author_ident_env(const char *message)\n \n static char *help_msg(const unsigned char *sha1)\n {\n-\tstatic char helpbuf[1024];\n+\tstruct strbuf helpbuf = STRBUF_INIT;\n \tchar *msg = getenv(\"GIT_CHERRY_PICK_HELP\");\n \n \tif (msg)\n \t\treturn msg;\n \n-\tstrcpy(helpbuf, \" After resolving the conflicts,\\n\"\n+\tstrbuf_addstr(&helpbuf, \" After resolving the conflicts,\\n\"\n \t\t\"mark the corrected paths with 'git add ' or 'git rm '\\n\"\n \t\t\"and commit the result.\");\n \n \tif (action == CHERRY_PICK) {\n-\t\tsprintf(helpbuf + strlen(helpbuf),\n+\t\tstrbuf_addf(&helpbuf,\n \t\t\t\" When committing, use the option '-c %s'\\n\"\n \t\t\t\"to retain authorship and message.\",\n \t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n \t}\n-\treturn helpbuf;\n+\treturn strbuf_detach(&helpbuf, NULL);\n }\n \n static struct tree *empty_tree(void)\n-- \n1.7.0.rc2.40.g33ed2.dirty\n"},{"id":"134261","messageId":"20100211210815.GD9066@coredump.intra.peff.net","threadId":"22605","inReplyTo":"20100211210445.GA8819@coredump.intra.peff.net","subject":"[PATCH 4/4] cherry-pick: show commit name instead of sha1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-11T21:08:15Z","receivedAt":"2010-02-11T21:08:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we have a conflict, we advise the user to do:\n\n git commit -c $sha1\n\nThis works fine, but is unnecessarily confusing and annoying\nfor the user to type, when:\n\n git commit -c $the_thing_you_called_cherry_pick_with\n\nworks just as well.\n\nSigned-off-by: Jeff King \n---\nI am pretty sure of the \"works just as well\". I couldn't think of a\nreason why the commit name they gave would have changed between the\ncherry-pick and the time they commit the resolved state.\n\n builtin-revert.c | 14 +++++++-------\n 1 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 77e4f4e..ad61249 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -38,6 +38,7 @@ static const char * const cherry_pick_usage[] = {\n static int edit, no_replay, no_commit, mainline, signoff;\n static enum { REVERT, CHERRY_PICK } action;\n static struct commit *commit;\n+static const char *commit_name;\n static int allow_rerere_auto;\n \n static const char *me;\n@@ -49,7 +50,6 @@ static void parse_args(int argc, const char **argv)\n \tconst char * const * usage_str =\n \t\taction == REVERT ? revert_usage : cherry_pick_usage;\n \tunsigned char sha1[20];\n-\tconst char *arg;\n \tint noop;\n \tstruct option options[] = {\n \t\tOPT_BOOLEAN('n', \"no-commit\", &no_commit, \"don't automatically commit\"),\n@@ -64,10 +64,10 @@ static void parse_args(int argc, const char **argv)\n \n \tif (parse_options(argc, argv, NULL, options, usage_str, 0) != 1)\n \t\tusage_with_options(usage_str, options);\n-\targ = argv[0];\n \n-\tif (get_sha1(arg, sha1))\n-\t\tdie (\"Cannot find '%s'\", arg);\n+\tcommit_name = argv[0];\n+\tif (get_sha1(commit_name, sha1))\n+\t\tdie (\"Cannot find '%s'\", commit_name);\n \tcommit = lookup_commit_reference(sha1);\n \tif (!commit)\n \t\texit(1);\n@@ -198,7 +198,7 @@ static void set_author_ident_env(const char *message)\n \t\t\tsha1_to_hex(commit->object.sha1));\n }\n \n-static char *help_msg(const unsigned char *sha1)\n+static char *help_msg(const char *name)\n {\n \tstruct strbuf helpbuf = STRBUF_INIT;\n \tchar *msg = getenv(\"GIT_CHERRY_PICK_HELP\");\n@@ -214,7 +214,7 @@ static char *help_msg(const unsigned char *sha1)\n \t\tstrbuf_addf(&helpbuf,\n \t\t\t\" When committing, use the option '-c %s'\\n\"\n \t\t\t\"to retain authorship and message.\",\n-\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n+\t\t\tname);\n \t}\n \treturn strbuf_detach(&helpbuf, NULL);\n }\n@@ -403,7 +403,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\tif (commit_lock_file(&msg_file) < 0)\n \t\t\tdie (\"Error wrapping up %s\", defmsg);\n \t\tfprintf(stderr, \"Automatic %s failed.%s\\n\",\n-\t\t\tme, help_msg(commit->object.sha1));\n+\t\t\tme, help_msg(commit_name));\n \t\trerere(allow_rerere_auto);\n \t\texit(1);\n \t}\n-- \n1.7.0.rc2.40.g33ed2.dirty\n"},{"id":"134262","messageId":"20100211211937.GA12752@coredump.intra.peff.net","threadId":"22605","inReplyTo":"20100211210445.GA8819@coredump.intra.peff.net","subject":"Re: git cherry-pick --continue?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-11T21:19:37Z","receivedAt":"2010-02-11T21:19:37Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 11, 2010 at 04:04:45PM -0500, Jeff King wrote:\n\n> Series to follow:\n> \n> [1/4]: cherry-pick: rewrap advice message\n> [2/4]: cherry-pick: refactor commit parsing code\n> [3/4]: cherry-pick: format help message as strbuf\n> [4/4]: cherry-pick: show commit name instead of sha1\n\nActually, I think the message is still a bit ugly after this, so perhaps\nthis 5/4 would help:\n\n-- >8 --\nSubject: [PATCH] cherry-pick: prettify the advice message\n\nIt's hard to see the \"how to commit\" part of this message,\nwhich users may want to cut and paste. On top of that,\nhaving it in paragraph form means that a really long commit\nname may cause ugly wrapping. Let's make it prettier, like:\n\n Automatic cherry-pick failed. After resolving the conflicts,\n mark the corrected paths with 'git add ' or 'git rm '\n and commit the result with:\n\n git commit -c HEAD~23\n\nSigned-off-by: Jeff King \n---\n builtin-revert.c | 10 ++++++----\n 1 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex ad61249..eff5268 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -208,14 +208,16 @@ static char *help_msg(const char *name)\n \n \tstrbuf_addstr(&helpbuf, \" After resolving the conflicts,\\n\"\n \t\t\"mark the corrected paths with 'git add ' or 'git rm '\\n\"\n-\t\t\"and commit the result.\");\n+\t\t\"and commit the result\");\n \n \tif (action == CHERRY_PICK) {\n-\t\tstrbuf_addf(&helpbuf,\n-\t\t\t\" When committing, use the option '-c %s'\\n\"\n-\t\t\t\"to retain authorship and message.\",\n+\t\tstrbuf_addf(&helpbuf, \" with: \\n\"\n+\t\t\t\"\\n\"\n+\t\t\t\" git commit -c %s\\n\",\n \t\t\tname);\n \t}\n+\telse\n+\t\tstrbuf_addch(&helpbuf, '.');\n \treturn strbuf_detach(&helpbuf, NULL);\n }\n \n-- \n1.7.0.rc2.32.g190cd.dirty\n"},{"id":"134269","messageId":"20100211222714.GA12805@coredump.intra.peff.net","threadId":"22605","inReplyTo":"7vfx5733x7.fsf@alter.siamese.dyndns.org","subject":"Re: git cherry-pick --continue?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-11T22:27:14Z","receivedAt":"2010-02-11T22:27:14Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 11, 2010 at 12:36:52PM -0800, Junio C Hamano wrote:\n\n> > Do you just want to pick up my patch from earlier in the thread, or do\n> > you have further comments? The only thing I could think to change would\n> > be that we may not want to even bother advertising --continue in the\n> > usage message (conversely, we could go a step further and actually\n> > advertise it in the manpage).\n> \n> I would say our eventual goal should be to make \"--continue\" the primary\n> word the end users would see. It would bring us closer to that goal to\n> start advertising --continue early.\n\nOK. Then I think my patch is fine. But we could also do this if we\nwanted to push it further now:\n\n-- >8 --\nSubject: [PATCH] am: switch --resolved to --continue\n\nRebase calls this same function \"--continue\", which means\nusers may be trained to type it. There is no reason to\ndeprecate --resolved (or -r), so we will keep it as a\nsynonym.\n\nSigned-off-by: Jeff King \n---\nBetween this and the previous patch, I don't have a strong preference.\nYou can decide.\n\n Documentation/git-am.txt | 3 ++-\n git-am.sh | 5 +++--\n 2 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex c3e4f12..c66c565 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -15,7 +15,7 @@ SYNOPSIS\n \t [--whitespace=