{"thread":{"id":"24468","subject":"[PATCH] revert: only suggest to commit if not passing -n","startedAt":"2010-07-22T13:51:32Z","lastAt":"2010-08-13T04:54:41Z","messageCount":4,"participants":["Carlo Marcelo Arenas Belon","Jared Hance","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"145996","messageId":"1279806692-6762-1-git-send-email-carenas@sajinet.com.pe","threadId":"24468","inReplyTo":null,"subject":"[PATCH] revert: only suggest to commit if not passing -n","fromName":"Carlo Marcelo Arenas Belon","fromEmail":"carenas@sajinet.com.pe","sentAt":"2010-07-22T13:51:32Z","receivedAt":"2010-07-22T13:51:32Z","isPatch":true,"sender":{"key":"carenas@sajinet.com.pe","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"while doing revert or cherry-pick, if the automatic merge fails\nand the user specifically suggested he didn't want to commit,\nthen don't suggest to do that as part of the conflict resolution.\n\nSigned-off-by: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>\n---\n builtin/revert.c |   20 ++++++++++++--------\n 1 files changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 8b9d829..72d0753 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -250,14 +250,18 @@ static char *help_msg(void)\n \t\treturn msg;\n \n \tstrbuf_addstr(&helpbuf, \"  After resolving the conflicts,\\n\"\n-\t\t\"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\\n\"\n-\t\t\"and commit the result\");\n+\t\t\"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\");\n+\tif (!no_commit) {\n+\t\tstrbuf_addf(&helpbuf, \"\\nand commit the result\");\n \n-\tif (action == CHERRY_PICK) {\n-\t\tstrbuf_addf(&helpbuf, \" with: \\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"        git commit -c %s\\n\",\n-\t\t\t    sha1_to_hex(commit->object.sha1));\n+\t\tif (action == CHERRY_PICK) {\n+\t\t\tstrbuf_addf(&helpbuf, \" with: \\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"        git commit -c %s\\n\",\n+\t\t\t\t    sha1_to_hex(commit->object.sha1));\n+\t\t}\n+\t\telse\n+\t\t\tstrbuf_addch(&helpbuf, '.');\n \t}\n \telse\n \t\tstrbuf_addch(&helpbuf, '.');\n-- \n1.7.1.1\n"},{"id":"146085","messageId":"20100723164218.GA2284@localhost.localdomain","threadId":"24468","inReplyTo":"1279806692-6762-1-git-send-email-carenas@sajinet.com.pe","subject":"Re: [PATCH] revert: only suggest to commit if not passing -n","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2010-07-23T16:42:18Z","receivedAt":"2010-07-23T16:42:18Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"On Thu, Jul 22, 2010 at 06:51:32AM -0700, Carlo Marcelo Arenas Belon wrote:\n>  \tstrbuf_addstr(&helpbuf, \"  After resolving the conflicts,\\n\"\n> -\t\t\"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\\n\"\n> -\t\t\"and commit the result\");\n> +\t\t\"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\");\n> +\tif (!no_commit) {\n> +\t\tstrbuf_addf(&helpbuf, \"\\nand commit the result\");\n\nShouldn't we use strbuf_addstr here? We aren't using the formatting\npart of strbuf_addf.\n"},{"id":"146276","messageId":"1280062470-21891-1-git-send-email-carenas@sajinet.com.pe","threadId":"24468","inReplyTo":"20100723164218.GA2284@localhost.localdomain","subject":"[PATCH v2] revert: only suggest to commit if not passing -n","fromName":"Carlo Marcelo Arenas Belon","fromEmail":"carenas@sajinet.com.pe","sentAt":"2010-07-25T12:54:30Z","receivedAt":"2010-07-25T12:54:30Z","isPatch":true,"sender":{"key":"carenas@sajinet.com.pe","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"while doing revert or cherry-pick, if the automatic merge fails\nand the user specifically suggested he didn't want to commit,\nthen don't suggest to do that as part of the conflict resolution.\n\nSigned-off-by: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>\n---\n builtin/revert.c |   20 ++++++++++++--------\n 1 files changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 8b9d829..b7cb69b 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -250,14 +250,18 @@ static char *help_msg(void)\n \t\treturn msg;\n \n \tstrbuf_addstr(&helpbuf, \"  After resolving the conflicts,\\n\"\n-\t\t\"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\\n\"\n-\t\t\"and commit the result\");\n+\t\t\"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\");\n+\tif (!no_commit) {\n+\t\tstrbuf_addstr(&helpbuf, \"\\nand commit the result\");\n \n-\tif (action == CHERRY_PICK) {\n-\t\tstrbuf_addf(&helpbuf, \" with: \\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"        git commit -c %s\\n\",\n-\t\t\t    sha1_to_hex(commit->object.sha1));\n+\t\tif (action == CHERRY_PICK) {\n+\t\t\tstrbuf_addf(&helpbuf, \" with: \\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"        git commit -c %s\\n\",\n+\t\t\t\t    sha1_to_hex(commit->object.sha1));\n+\t\t}\n+\t\telse\n+\t\t\tstrbuf_addch(&helpbuf, '.');\n \t}\n \telse\n \t\tstrbuf_addch(&helpbuf, '.');\n-- \n1.7.2\n"},{"id":"147990","messageId":"7vvd7fkt7y.fsf@alter.siamese.dyndns.org","threadId":"24468","inReplyTo":"1280062470-21891-1-git-send-email-carenas@sajinet.com.pe","subject":"Re: [PATCH v2] revert: only suggest to commit if not passing -n","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-13T04:54:41Z","receivedAt":"2010-08-13T04:54:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:\n\n> while doing revert or cherry-pick, if the automatic merge fails\n> and the user specifically suggested he didn't want to commit,\n> then don't suggest to do that as part of the conflict resolution.\n\nI agree that the suggestion does not make sense, but realistically, when\nthe user said\n\n    git cherry-pick --no-commit $something\n\nwe have no idea if the user wants to add or remove once the conflict has\nbeen resolved.  More often than not, \"cherry-pick --no-commit\" is followed\nby further edit, at least in the use cases I've seen, so \"git add/rm\" is\nnot the first command the user will run after resolving the conflicts.\n\nSo it _might_ make sense not to even suggest \"add/rm\" in that case.\n\nAfter all, this is a help/advice message, and \"cherry-pick --no-commit\"\nis sort of an advanced feature anyway, so...\n\n> Signed-off-by: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>\n> ---\n>  builtin/revert.c |   20 ++++++++++++--------\n>  1 files changed, 12 insertions(+), 8 deletions(-)\n>\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 8b9d829..b7cb69b 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -250,14 +250,18 @@ static char *help_msg(void)\n>  \t\treturn msg;\n>  \n>  \tstrbuf_addstr(&helpbuf, \"  After resolving the conflicts,\\n\"\n> -\t\t\"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\\n\"\n> -\t\t\"and commit the result\");\n> +\t\t\"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\");\n> +\tif (!no_commit) {\n> +\t\tstrbuf_addstr(&helpbuf, \"\\nand commit the result\");\n>  \n> -\tif (action == CHERRY_PICK) {\n> -\t\tstrbuf_addf(&helpbuf, \" with: \\n\"\n> -\t\t\t\"\\n\"\n> -\t\t\t\"        git commit -c %s\\n\",\n> -\t\t\t    sha1_to_hex(commit->object.sha1));\n> +\t\tif (action == CHERRY_PICK) {\n> +\t\t\tstrbuf_addf(&helpbuf, \" with: \\n\"\n> +\t\t\t\t\"\\n\"\n> +\t\t\t\t\"        git commit -c %s\\n\",\n> +\t\t\t\t    sha1_to_hex(commit->object.sha1));\n> +\t\t}\n> +\t\telse\n> +\t\t\tstrbuf_addch(&helpbuf, '.');\n>  \t}\n>  \telse\n>  \t\tstrbuf_addch(&helpbuf, '.');\n> -- \n> 1.7.2\n"}]}