{"thread":{"id":"33326","subject":"[PATCH] cherry-pick -x: improve handling of one-liner commit messages","startedAt":"2013-03-29T15:38:18Z","lastAt":"2013-03-29T20:49:29Z","messageCount":5,"participants":["Miklos Vajna","Junio C Hamano","Brandon Casey"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"212623","messageId":"20130329153818.GB27251@suse.cz","threadId":"33326","inReplyTo":null,"subject":"[PATCH] cherry-pick -x: improve handling of one-liner commit messages","fromName":"Miklos Vajna","fromEmail":"vmiklos@suse.cz","sentAt":"2013-03-29T15:38:18Z","receivedAt":"2013-03-29T15:38:18Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"git cherry-pick -x normally just appends the \"cherry picked from commit\"\nline at the end of the message, which is fine. However, in case the\noriginal commit message had only one line, first append a newline,\notherwise the second line won't be empty, which is against\nrecommendations.\n---\n sequencer.c | 10 ++++++++++\n t/t3501-revert-cherry-pick.sh | 8 ++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex aef5e8a..1ae0e43 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -496,6 +496,16 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\t}\n \n \t\tif (opts->record_origin) {\n+\n+\t\t\t/*\n+\t\t\t * If this the message is a one-liner, append a\n+\t\t\t * newline, so the second line will be empty, as\n+\t\t\t * recommended.\n+\t\t\t */\n+\t\t\tp = strstr(msgbuf.buf, \"\\n\\n\");\n+\t\t\tif (!p)\n+\t\t\t\tstrbuf_addch(&msgbuf, '\\n');\n+\n \t\t\tstrbuf_addstr(&msgbuf, \"(cherry picked from commit \");\n \t\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));\n \t\t\tstrbuf_addstr(&msgbuf, \")\\n\");\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 6f489e2..858c744 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -70,6 +70,14 @@ test_expect_success 'cherry-pick after renaming branch' '\n \n '\n \n+test_expect_success 'cherry-pick -x of one-liner commit message' '\n+\n+\tgit checkout rename2 &&\n+\tgit cherry-pick -x added &&\n+\tgit show -s --pretty=format:%s | test_must_fail grep \"cherry picked\"\n+\n+'\n+\n test_expect_success 'revert after renaming branch' '\n \n \tgit checkout rename1 &&\n-- \n1.8.1.4\n"},{"id":"212632","messageId":"7v8v56p1bl.fsf@alter.siamese.dyndns.org","threadId":"33326","inReplyTo":"20130329153818.GB27251@suse.cz","subject":"Re: [PATCH] cherry-pick -x: improve handling of one-liner commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-29T17:23:26Z","receivedAt":"2013-03-29T17:23:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna writes:\n\n> git cherry-pick -x normally just appends the \"cherry picked from commit\"\n> line at the end of the message, which is fine. However, in case the\n> original commit message had only one line, first append a newline,\n> otherwise the second line won't be empty, which is against\n> recommendations.\n> ---\n\nSign-off?\n\nI think this is part of the bc/append-signed-off-by topic that is\nabout to graduate to 'master'; more specifically, b971e04f54e7\n(sequencer.c: always separate \"(cherry picked from\" from commit\nbody, 2013-02-12) does the equivalent, no?\n\n> sequencer.c | 10 ++++++++++\n> t/t3501-revert-cherry-pick.sh | 8 ++++++++\n> 2 files changed, 18 insertions(+)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index aef5e8a..1ae0e43 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -496,6 +496,16 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n> \t\t}\n> \n> \t\tif (opts->record_origin) {\n> +\n> +\t\t\t/*\n> +\t\t\t * If this the message is a one-liner, append a\n> +\t\t\t * newline, so the second line will be empty, as\n> +\t\t\t * recommended.\n> +\t\t\t */\n> +\t\t\tp = strstr(msgbuf.buf, \"\\n\\n\");\n> +\t\t\tif (!p)\n> +\t\t\t\tstrbuf_addch(&msgbuf, '\\n');\n> +\n> \t\t\tstrbuf_addstr(&msgbuf, \"(cherry picked from commit \");\n> \t\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));\n> \t\t\tstrbuf_addstr(&msgbuf, \")\\n\");\n> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\n> index 6f489e2..858c744 100755\n> --- a/t/t3501-revert-cherry-pick.sh\n> +++ b/t/t3501-revert-cherry-pick.sh\n> @@ -70,6 +70,14 @@ test_expect_success 'cherry-pick after renaming branch' '\n> \n> '\n> \n> +test_expect_success 'cherry-pick -x of one-liner commit message' '\n> +\n> +\tgit checkout rename2 &&\n> +\tgit cherry-pick -x added &&\n> +\tgit show -s --pretty=format:%s | test_must_fail grep \"cherry picked\"\n> +\n> +'\n> +\n> test_expect_success 'revert after renaming branch' '\n> \n> \tgit checkout rename1 &&\n"},{"id":"212635","messageId":"CA+sFfMet0578rLwAKBBsd8DM3thsw=8+joEAAf44TzwqUZgXzw@mail.gmail.com","threadId":"33326","inReplyTo":"7v8v56p1bl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] cherry-pick -x: improve handling of one-liner commit messages","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2013-03-29T17:41:17Z","receivedAt":"2013-03-29T17:41:17Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On Fri, Mar 29, 2013 at 10:23 AM, Junio C Hamano wrote:\n> Miklos Vajna writes:\n>\n>> git cherry-pick -x normally just appends the \"cherry picked from commit\"\n>> line at the end of the message, which is fine. However, in case the\n>> original commit message had only one line, first append a newline,\n>> otherwise the second line won't be empty, which is against\n>> recommendations.\n>> ---\n>\n> Sign-off?\n>\n> I think this is part of the bc/append-signed-off-by topic that is\n> about to graduate to 'master'; more specifically, b971e04f54e7\n> (sequencer.c: always separate \"(cherry picked from\" from commit\n> body, 2013-02-12) does the equivalent, no?\n\nYeah, I think this case is already handled.\n\nMiklos, can you check out next and see if your problem case is handled?\n\n-Brandon\n"},{"id":"212649","messageId":"20130329203915.GA5157@suse.cz","threadId":"33326","inReplyTo":"CA+sFfMet0578rLwAKBBsd8DM3thsw=8+joEAAf44TzwqUZgXzw@mail.gmail.com","subject":"Re: [PATCH] cherry-pick -x: improve handling of one-liner commit messages","fromName":"Miklos Vajna","fromEmail":"vmiklos@suse.cz","sentAt":"2013-03-29T20:39:16Z","receivedAt":"2013-03-29T20:39:16Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Hi,\n\nOn Fri, Mar 29, 2013 at 10:41:17AM -0700, Brandon Casey wrote:\n> > Sign-off?\n\nIndeed, I forgot about it, my bad.\n\n> > I think this is part of the bc/append-signed-off-by topic that is\n> > about to graduate to 'master'; more specifically, b971e04f54e7\n> > (sequencer.c: always separate \"(cherry picked from\" from commit\n> > body, 2013-02-12) does the equivalent, no?\n> \n> Yeah, I think this case is already handled.\n> \n> Miklos, can you check out next and see if your problem case is handled?\n\nI just checked next and right, that solves the problem I was fixing.\n\nSo -- sorry for the noise. :-)\n\nMiklos\n"},{"id":"212650","messageId":"7vr4iylyna.fsf@alter.siamese.dyndns.org","threadId":"33326","inReplyTo":"20130329203915.GA5157@suse.cz","subject":"Re: [PATCH] cherry-pick -x: improve handling of one-liner commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-29T20:49:29Z","receivedAt":"2013-03-29T20:49:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna writes:\n\n> On Fri, Mar 29, 2013 at 10:41:17AM -0700, Brandon Casey wrote:\n>\n>> > I think this is part of the bc/append-signed-off-by topic that is\n>> > about to graduate to 'master'; more specifically, b971e04f54e7\n>> > (sequencer.c: always separate \"(cherry picked from\" from commit\n>> > body, 2013-02-12) does the equivalent, no?\n>> \n>> Yeah, I think this case is already handled.\n>> \n>> Miklos, can you check out next and see if your problem case is handled?\n>\n> I just checked next and right, that solves the problem I was fixing.\n>\n> So -- sorry for the noise. :-)\n\nDon't be sorry.\n\nPeople noticing an issue, trying 'master' and 'next' to see that it\nhas already been resolved, is the only way for us to know we are\nhelping people other than those who had the original itch.\n\nThe ideal would be for more people to run 'next' and report both\nproblems and successes ;-)\n"}]}