threads / patch / 24468

patchrevert: only suggest to commit if not passing -n

Subject: [PATCH] revert: only suggest to commit if not passing -n

## tl;dr

4 messages between Jul 22, 2010 and Aug 13, 2010. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Carlo Marcelo Arenas Belon· Jul 22, 2010, 13:51 UTC · lore

while doing revert or cherry-pick, if the automatic merge fails and the user specifically suggested he didn't want to commit, then don't suggest to do that as part of the conflict resolution.

Signed-off-by: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>
---
 builtin/revert.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

Show changes to builtin/revert.c +11 −7

diff --git a/builtin/revert.c b/builtin/revert.c
index 8b9d829..72d0753 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -250,14 +250,18 @@ static char *help_msg(void)
 		return msg;
 
 	strbuf_addstr(&helpbuf, "  After resolving the conflicts,\n"
-		"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\n"
-		"and commit the result");
+		"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'");
+	if (!no_commit) {
+		strbuf_addf(&helpbuf, "\nand commit the result");
 
-	if (action == CHERRY_PICK) {
-		strbuf_addf(&helpbuf, " with: \n"
-			"\n"
-			"        git commit -c %s\n",
-			    sha1_to_hex(commit->object.sha1));
+		if (action == CHERRY_PICK) {
+			strbuf_addf(&helpbuf, " with: \n"
+				"\n"
+				"        git commit -c %s\n",
+				    sha1_to_hex(commit->object.sha1));
+		}
+		else
+			strbuf_addch(&helpbuf, '.');
 	}
 	else
 		strbuf_addch(&helpbuf, '.');

-- 
1.7.1.1

Jared Hance· Jul 23, 2010, 16:42 UTC · re: Carlo Marcelo Arenas Belon · lore

Re: [PATCH] revert: only suggest to commit if not passing -n

On Thu, Jul 22, 2010 at 06:51:32AM -0700, Carlo Marcelo Arenas Belon wrote:

Show 6 quoted lines

>  	strbuf_addstr(&helpbuf, "  After resolving the conflicts,\n"
> -		"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\n"
> -		"and commit the result");
> +		"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'");
> +	if (!no_commit) {
> +		strbuf_addf(&helpbuf, "\nand commit the result");

Shouldn't we use strbuf_addstr here? We aren't using the formatting part of strbuf_addf.

Carlo Marcelo Arenas Belon· Jul 25, 2010, 12:54 UTC · re: Jared Hance · lore

[PATCH v2] revert: only suggest to commit if not passing -n

while doing revert or cherry-pick, if the automatic merge fails and the user specifically suggested he didn't want to commit, then don't suggest to do that as part of the conflict resolution.

Signed-off-by: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>
---
 builtin/revert.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

Show changes to builtin/revert.c +11 −7

diff --git a/builtin/revert.c b/builtin/revert.c
index 8b9d829..b7cb69b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -250,14 +250,18 @@ static char *help_msg(void)
 		return msg;
 
 	strbuf_addstr(&helpbuf, "  After resolving the conflicts,\n"
-		"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\n"
-		"and commit the result");
+		"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'");
+	if (!no_commit) {
+		strbuf_addstr(&helpbuf, "\nand commit the result");
 
-	if (action == CHERRY_PICK) {
-		strbuf_addf(&helpbuf, " with: \n"
-			"\n"
-			"        git commit -c %s\n",
-			    sha1_to_hex(commit->object.sha1));
+		if (action == CHERRY_PICK) {
+			strbuf_addf(&helpbuf, " with: \n"
+				"\n"
+				"        git commit -c %s\n",
+				    sha1_to_hex(commit->object.sha1));
+		}
+		else
+			strbuf_addch(&helpbuf, '.');
 	}
 	else
 		strbuf_addch(&helpbuf, '.');

-- 
1.7.2

Junio C Hamano· Aug 13, 2010, 04:54 UTC · re: Carlo Marcelo Arenas Belon · lore

Re: [PATCH v2] revert: only suggest to commit if not passing -n

Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:

> while doing revert or cherry-pick, if the automatic merge fails
> and the user specifically suggested he didn't want to commit,
> then don't suggest to do that as part of the conflict resolution.

I agree that the suggestion does not make sense, but realistically, when the user said

    git cherry-pick --no-commit $something

we have no idea if the user wants to add or remove once the conflict has been resolved. More often than not, "cherry-pick --no-commit" is followed by further edit, at least in the use cases I've seen, so "git add/rm" is not the first command the user will run after resolving the conflicts.

So it _might_ make sense not to even suggest "add/rm" in that case.

After all, this is a help/advice message, and "cherry-pick --no-commit" is sort of an advanced feature anyway, so...

Show 37 quoted lines

> Signed-off-by: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>
> ---
>  builtin/revert.c |   20 ++++++++++++--------
>  1 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 8b9d829..b7cb69b 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -250,14 +250,18 @@ static char *help_msg(void)
>  		return msg;
>  
>  	strbuf_addstr(&helpbuf, "  After resolving the conflicts,\n"
> -		"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\n"
> -		"and commit the result");
> +		"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'");
> +	if (!no_commit) {
> +		strbuf_addstr(&helpbuf, "\nand commit the result");
>  
> -	if (action == CHERRY_PICK) {
> -		strbuf_addf(&helpbuf, " with: \n"
> -			"\n"
> -			"        git commit -c %s\n",
> -			    sha1_to_hex(commit->object.sha1));
> +		if (action == CHERRY_PICK) {
> +			strbuf_addf(&helpbuf, " with: \n"
> +				"\n"
> +				"        git commit -c %s\n",
> +				    sha1_to_hex(commit->object.sha1));
> +		}
> +		else
> +			strbuf_addch(&helpbuf, '.');
>  	}
>  	else
>  		strbuf_addch(&helpbuf, '.');
> -- 
> 1.7.2