Re: git cherry-pick --continue?

Junio C Hamano [off-list ref] writes:

Jeff King [off-list ref] writes:

quoted

Hmm. I was thinking "am" was the odd man out, but really there are only
two sequencer commands that I noted: rebase and am. So you could perhaps
argue that rebase should also learn "--resolved". Or am I forgetting
one?

Having said all I did in the previous message, I think "am --continue"
would be a good addition.

And "rebase --resolved" would not make any sense if the reason the control
is given back to you was because you ran "rebase -i" and marked a commit
to be "edit"ed.  Of course, we could add "--resolved" and "--edited" (or
perhaps "--amended") to "rebase", and have it make sure that the correct
one is given.  For example, when it stopped for "edit", it would reject
"rebase --resolved".  But I do not think it is worth the hassle.

Heya,

On Wed, Feb 10, 2010 at 23:21, Junio C Hamano [off-list ref] wrote:

Having said all I did in the previous message, I think "am --continue"
would be a good addition.

How about 'cherry-pick --resolved' though ;).

And "rebase --resolved" would not make any sense if the reason the control
is given back to you was because you ran "rebase -i" and marked a commit
to be "edit"ed.  Of course, we could add "--resolved" and "--edited" (or
perhaps "--amended") to "rebase", and have it make sure that the correct
one is given.  For example, when it stopped for "edit", it would reject
"rebase --resolved".  But I do not think it is worth the hassle.

I don't see any benefit to that, in fact, I'd recommend against it.

-- 
Cheers,

Sverre Rabbelier

On Wed, Feb 10, 2010 at 02:21:14PM -0800, Junio C Hamano wrote:

Junio C Hamano [off-list ref] writes:

quoted

Jeff King [off-list ref] writes:

quoted

Hmm. I was thinking "am" was the odd man out, but really there are only
two sequencer commands that I noted: rebase and am. So you could perhaps
argue that rebase should also learn "--resolved". Or am I forgetting
one?

Having said all I did in the previous message, I think "am --continue"
would be a good addition.

OK. I agree with your philosophical ramblings in the previous message,
but I also think there is some value in making it simple for the user to
remember.

Do you just want to pick up my patch from earlier in the thread, or do
you have further comments? The only thing I could think to change would
be that we may not want to even bother advertising --continue in the
usage message (conversely, we could go a step further and actually
advertise it in the manpage).

And "rebase --resolved" would not make any sense if the reason the control
is given back to you was because you ran "rebase -i" and marked a commit
to be "edit"ed.  Of course, we could add "--resolved" and "--edited" (or
perhaps "--amended") to "rebase", and have it make sure that the correct
one is given.  For example, when it stopped for "edit", it would reject
"rebase --resolved".  But I do not think it is worth the hassle.

Agreed.

-Peff

Jeff King [off-list ref] writes:

quoted

Having said all I did in the previous message, I think "am --continue"
would be a good addition.

OK. I agree with your philosophical ramblings in the previous message,
but I also think there is some value in making it simple for the user to
remember.

Certainly I am in agreement and that is why I think "am --continue" is a
good thing.

Do you just want to pick up my patch from earlier in the thread, or do
you have further comments? The only thing I could think to change would
be that we may not want to even bother advertising --continue in the
usage message (conversely, we could go a step further and actually
advertise it in the manpage).

I would say our eventual goal should be to make "--continue" the primary
word the end users would see.  It would bring us closer to that goal to
start advertising --continue early.

On Thu, Feb 11, 2010 at 12:36:52PM -0800, Junio C Hamano wrote:

quoted

Do you just want to pick up my patch from earlier in the thread, or do
you have further comments? The only thing I could think to change would
be that we may not want to even bother advertising --continue in the
usage message (conversely, we could go a step further and actually
advertise it in the manpage).

I would say our eventual goal should be to make "--continue" the primary
word the end users would see.  It would bring us closer to that goal to
start advertising --continue early.

OK. Then I think my patch is fine. But we could also do this if we
wanted to push it further now:

-- >8 --
Subject: [PATCH] am: switch --resolved to --continue

Rebase calls this same function "--continue", which means
users may be trained to type it. There is no reason to
deprecate --resolved (or -r), so we will keep it as a
synonym.

Signed-off-by: Jeff King <redacted>
---
Between this and the previous patch, I don't have a strong preference.
You can decide.

 Documentation/git-am.txt |    3 ++-
 git-am.sh                |    5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index c3e4f12..c66c565 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt

@@ -15,7 +15,7 @@ SYNOPSIS
 	 [--whitespace=<option>] [-C<n>] [-p<n>] [--directory=<dir>]
 	 [--reject] [-q | --quiet] [--scissors | --no-scissors]
 	 [<mbox> | <Maildir>...]
-'git am' (--skip | --resolved | --abort)
+'git am' (--continue | --skip | --abort)
 
 DESCRIPTION
 -----------

@@ -107,6 +107,7 @@ default.   You can use `--no-utf8` to override this.
 	Skip the current patch.  This is only meaningful when
 	restarting an aborted patch.
 
+--continue::
 -r::
 --resolved::
 	After a patch failure (e.g. attempting to apply

diff --git a/git-am.sh b/git-am.sh
index c8b9cbb..3c08d53 100755
--- a/git-am.sh
+++ b/git-am.sh

@@ -25,7 +25,8 @@ p=              pass it through git-apply
 patch-format=   format the patch(es) are in
 reject          pass it through git-apply
 resolvemsg=     override error message when patch failure occurs
-r,resolved      to be used after a patch failure
+continue        continue applying patches after resolving a conflict
+r,resolved      synonyms for --continue
 skip            skip the current patch
 abort           restore the original branch and abort the patching operation.
 committer-date-is-author-date    lie about committer date

@@ -318,7 +319,7 @@ do
 		scissors=t ;;
 	--no-scissors)
 		scissors=f ;;
-	-r|--resolved)
+	-r|--resolved|--continue)
 		resolved=t ;;
 	--skip)
 		skip=t ;;

-- 
1.7.0.rc2.37.g157e8.dirty

Hi,

On Thu, Feb 11, 2010 at 05:27:14PM -0500, Jeff King wrote:

On Thu, Feb 11, 2010 at 12:36:52PM -0800, Junio C Hamano wrote:

quoted

quoted

Do you just want to pick up my patch from earlier in the thread, or do
you have further comments? The only thing I could think to change would
be that we may not want to even bother advertising --continue in the
usage message (conversely, we could go a step further and actually
advertise it in the manpage).

I would say our eventual goal should be to make "--continue" the primary
word the end users would see.  It would bring us closer to that goal to
start advertising --continue early.

OK. Then I think my patch is fine. But we could also do this if we
wanted to push it further now:

-- >8 --
Subject: [PATCH] am: switch --resolved to --continue

Rebase calls this same function "--continue", which means
users may be trained to type it. There is no reason to
deprecate --resolved (or -r), so we will keep it as a
synonym.

Signed-off-by: Jeff King <redacted>

Then maybe we should have this, too.

Best,
Gábor


 -- >8 --
Subject: [PATCH] bash: support 'git am's new '--continue' option

Signed-off-by: SZEDER Gábor <redacted>
---
 contrib/completion/git-completion.bash |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 35acad0..fe93747 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -667,7 +667,7 @@ _git_am ()
 {
 	local cur="${COMP_WORDS[COMP_CWORD]}" dir="$(__gitdir)"
 	if [ -d "$dir"/rebase-apply ]; then
-		__gitcomp "--skip --resolved --abort"
+		__gitcomp "--skip --continue --resolved --abort"
 		return
 	fi
 	case "$cur" in

-- 
1.7.0.rc1.84.g9879