git cherry-pick --continue?

git cherry-pick --continue?

[Sverre Rabbelier]

Heya,

At the moment git cherry-pick stands out from the sequencer tools in
that it doesn't support --continue but requires you to manually supply
the '-c ...' argument to 'git commit' when there's a conflict instead.
Is it desirable that we add such an option? And if so, how would one
go about implementing it?

-- 
Cheers,

Sverre Rabbelier

Re: git cherry-pick --continue?

[Jeff King]

On Wed, Feb 10, 2010 at 09:37:10PM +0100, Sverre Rabbelier wrote:

> At the moment git cherry-pick stands out from the sequencer tools in
> that it doesn't support --continue but requires you to manually supply
> the '-c ...' argument to 'git commit' when there's a conflict instead.
> Is it desirable that we add such an option? And if so, how would one
> go about implementing it?

I think it makes sense. It is perhaps a little iffy to use "continue"
when you are not really continuing on to further cherry-picks (and in a
rebase, continue is not just about resolving conflicts, but about
continuing to the next item in the rebase). Cherry-picks are more like
"am --resolved"[1].

But semantic nitpicks aside, I think "continue" is a good enough name.
The differences between what it means for each command are fairly
obvious based on the command, and there is real value in a user just
having to remember one verb.

-Peff

[1] On the other hand, I usually mistype that as "git am --continue",
which _does_ make sense, since you are applying a sequence of patches.
Maybe "am" should support both.

Re: git cherry-pick --continue?

[Jeff King]

On Wed, Feb 10, 2010 at 04:04:19PM -0500, Jeff King wrote:

> [1] On the other hand, I usually mistype that as "git am --continue",
> which _does_ make sense, since you are applying a sequence of patches.
> Maybe "am" should support both.

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?

I find the patch below convenient. I dunno if anybody else actually
cares.

-- >8 --
Subject: [PATCH] am: allow --continue as a synonym for --resolved

Rebase calls this same function "--continue", which means
users may be trained to type it. There is no reason to
deprecate --resolved (or -r), but adding this synonym is
friendly to users.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-am.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index c8b9cbb..88cef39 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -26,6 +26,7 @@ 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        synonym for --resolved
 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.22.g0716c.dirty

Re: git cherry-pick --continue?

[Sverre Rabbelier]

Heya,

On Wed, Feb 10, 2010 at 22:24, Jeff King <peff@peff.net> wrote:
> I find the patch below convenient. I dunno if anybody else actually
> cares.

I do, it's useful when applying a bunch of patches and conflicts arise
:). Typing 'git am --continue' is what I'm used to from 'git rebase'.

-- 
Cheers,

Sverre Rabbelier

Re: git cherry-pick --continue?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> 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?

I don't think you are forgetting anything, except that "am" came first with
"resolved".

The focus of the verb is "I declare I am finished marking the resolution".
Taking that declaration and continuing is ultimately "am"'s decision.  IOW
the user is not telling "am" to continue---the difference is subtle, but
it was a conscious design decision.

"rebase --continue" came later, and I think its focus is placed more
heavily on the instruction side ("Please continue"), and not on the
declaration side ("I now have marked the resolution for all paths").

This causes people sometimes to want to see it "continue", even when then
haven't marked the resolved paths as resolved.  I personally think the
focus is misplaced.

But that is just a philosophical difference ;-).

Re: git cherry-pick --continue?

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> 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.

Re: git cherry-pick --continue?

[Sverre Rabbelier]

Heya,

On Wed, Feb 10, 2010 at 23:21, Junio C Hamano <gitster@pobox.com> 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

Re: git cherry-pick --continue?

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> On Wed, Feb 10, 2010 at 23:21, Junio C Hamano <gitster@pobox.com> 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 ;).

Changing the insn to suggest using "-C topic" when the original command
line was "git cherry-pick topic" would be a good addition, too.  Currently
we suggest "-c" and abbreviated object name, neither of which is sensible.

While I think "--resolved" makes sense, I do not see much benefit, and it
largely depends on what you do.  If you are suggesting to commit with what
is kept in $GIT_DIR/MERGE_MSG, I would actually recommend against it.  The
message will have "Conflicts:" information but that is meaningless unless
you are recording from what context the commit was cherry-picked from at
the same time.

Re: git cherry-pick --continue?

[Sverre Rabbelier]

Heya,

On Wed, Feb 10, 2010 at 23:34, Junio C Hamano <gitster@pobox.com> wrote:
> Changing the insn to suggest using "-C topic" when the original command
> line was "git cherry-pick topic" would be a good addition, too.  Currently
> we suggest "-c" and abbreviated object name, neither of which is sensible.

I think it's sensible to use '-c' instead of '-C', just like 'git
rebase -i' lets you change the commit message after you resolve a
conflict.

> While I think "--resolved" makes sense, I do not see much benefit, and it
> largely depends on what you do.  If you are suggesting to commit with what
> is kept in $GIT_DIR/MERGE_MSG, I would actually recommend against it.  The
> message will have "Conflicts:" information but that is meaningless unless
> you are recording from what context the commit was cherry-picked from at
> the same time.

I'm not sure how to implement it, but I was thinking to just automate
the 'git commit -c ...' part. I don't like having to copy/paste some
instruction, so maybe we can record the original commit somewhere and
have 'git cherry-pick --resolve' be equivalent to 'git commit -c `cat
.git/CHERRY_PICK_CMT`', or somesuch?

-- 
Cheers,

Sverre Rabbelier

Re: git cherry-pick --continue?

[Jeff King]

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

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Jeff King <peff@peff.net> writes:
> >
> >> 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

Re: git cherry-pick --continue?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> 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.

Re: git cherry-pick --continue?

[Jeff King]

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

> Changing the insn to suggest using "-C topic" when the original command
> line was "git cherry-pick topic" would be a good addition, too.  Currently
> we suggest "-c" and abbreviated object name, neither of which is sensible.

I think using the actual name instead of the abbreviated sha1 is
sensible. But I think "-c" makes sense, as it gives the user the chance
to look over the commit message to see if they need to tweak it to match
the conflict fixups. Savvy users who don't want to do that will know to
use "-C".

Series to follow:

  [1/4]: cherry-pick: rewrap advice message
  [2/4]: cherry-pick: refactor commit parsing code
  [3/4]: cherry-pick: format help message as strbuf
  [4/4]: cherry-pick: show commit name instead of sha1

-Peff

[PATCH 1/4] cherry-pick: rewrap advice message

[Jeff King]

The current message overflows on an 80-character terminal.
While we're at it, fix the spelling of 'committing'.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-revert.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin-revert.c b/builtin-revert.c
index 8ac86f0..83e5c0a 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -213,13 +213,13 @@ static char *help_msg(const unsigned char *sha1)
 		return msg;
 
 	strcpy(helpbuf, "  After resolving the conflicts,\n"
-	       "mark the corrected paths with 'git add <paths>' "
-	       "or 'git rm <paths>' and commit the result.");
+		"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\n"
+		"and commit the result.");
 
 	if (action == CHERRY_PICK) {
 		sprintf(helpbuf + strlen(helpbuf),
-			"\nWhen commiting, use the option "
-			"'-c %s' to retain authorship and message.",
+			"  When committing, use the option '-c %s'\n"
+			"to retain authorship and message.",
 			find_unique_abbrev(sha1, DEFAULT_ABBREV));
 	}
 	return helpbuf;
-- 
1.7.0.rc2.40.g33ed2.dirty

[PATCH 2/4] cherry-pick: refactor commit parsing code

[Jeff King]

These lines are really just lookup_commit_reference
re-implemented.

Signed-off-by: Jeff King <peff@peff.net>
---
Obviously this changes the exact text of the error messages, but I doubt
anybody cares too much.

 builtin-revert.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/builtin-revert.c b/builtin-revert.c
index 83e5c0a..012c646 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -68,15 +68,9 @@ static void parse_args(int argc, const char **argv)
 
 	if (get_sha1(arg, sha1))
 		die ("Cannot find '%s'", arg);
-	commit = (struct commit *)parse_object(sha1);
+	commit = lookup_commit_reference(sha1);
 	if (!commit)
-		die ("Could not find %s", sha1_to_hex(sha1));
-	if (commit->object.type == OBJ_TAG) {
-		commit = (struct commit *)
-			deref_tag((struct object *)commit, arg, strlen(arg));
-	}
-	if (commit->object.type != OBJ_COMMIT)
-		die ("'%s' does not point to a commit", arg);
+		exit(1);
 }
 
 static char *get_oneline(const char *message)
-- 
1.7.0.rc2.40.g33ed2.dirty

[PATCH 3/4] cherry-pick: format help message as strbuf

[Jeff King]

This gets rid of the fixed-size buffer and an unchecked
sprintf. That sprintf is actually OK as the only
variable-sized thing put in it is an abbreviated sha1, which
is bounded at 40 characters. However, the next patch will
change that to something unbounded.

Note that this function now returns an allocated buffer
instead of a static one; however, it doesn't matter as the
only caller exits immediately.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-revert.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin-revert.c b/builtin-revert.c
index 012c646..77e4f4e 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -200,23 +200,23 @@ static void set_author_ident_env(const char *message)
 
 static char *help_msg(const unsigned char *sha1)
 {
-	static char helpbuf[1024];
+	struct strbuf helpbuf = STRBUF_INIT;
 	char *msg = getenv("GIT_CHERRY_PICK_HELP");
 
 	if (msg)
 		return msg;
 
-	strcpy(helpbuf, "  After resolving the conflicts,\n"
+	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.");
 
 	if (action == CHERRY_PICK) {
-		sprintf(helpbuf + strlen(helpbuf),
+		strbuf_addf(&helpbuf,
 			"  When committing, use the option '-c %s'\n"
 			"to retain authorship and message.",
 			find_unique_abbrev(sha1, DEFAULT_ABBREV));
 	}
-	return helpbuf;
+	return strbuf_detach(&helpbuf, NULL);
 }
 
 static struct tree *empty_tree(void)
-- 
1.7.0.rc2.40.g33ed2.dirty

[PATCH 4/4] cherry-pick: show commit name instead of sha1

[Jeff King]

When we have a conflict, we advise the user to do:

  git commit -c $sha1

This works fine, but is unnecessarily confusing and annoying
for the user to type, when:

  git commit -c $the_thing_you_called_cherry_pick_with

works just as well.

Signed-off-by: Jeff King <peff@peff.net>
---
I am pretty sure of the "works just as well". I couldn't think of a
reason why the commit name they gave would have changed between the
cherry-pick and the time they commit the resolved state.

 builtin-revert.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin-revert.c b/builtin-revert.c
index 77e4f4e..ad61249 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -38,6 +38,7 @@ static const char * const cherry_pick_usage[] = {
 static int edit, no_replay, no_commit, mainline, signoff;
 static enum { REVERT, CHERRY_PICK } action;
 static struct commit *commit;
+static const char *commit_name;
 static int allow_rerere_auto;
 
 static const char *me;
@@ -49,7 +50,6 @@ static void parse_args(int argc, const char **argv)
 	const char * const * usage_str =
 		action == REVERT ?  revert_usage : cherry_pick_usage;
 	unsigned char sha1[20];
-	const char *arg;
 	int noop;
 	struct option options[] = {
 		OPT_BOOLEAN('n', "no-commit", &no_commit, "don't automatically commit"),
@@ -64,10 +64,10 @@ static void parse_args(int argc, const char **argv)
 
 	if (parse_options(argc, argv, NULL, options, usage_str, 0) != 1)
 		usage_with_options(usage_str, options);
-	arg = argv[0];
 
-	if (get_sha1(arg, sha1))
-		die ("Cannot find '%s'", arg);
+	commit_name = argv[0];
+	if (get_sha1(commit_name, sha1))
+		die ("Cannot find '%s'", commit_name);
 	commit = lookup_commit_reference(sha1);
 	if (!commit)
 		exit(1);
@@ -198,7 +198,7 @@ static void set_author_ident_env(const char *message)
 			sha1_to_hex(commit->object.sha1));
 }
 
-static char *help_msg(const unsigned char *sha1)
+static char *help_msg(const char *name)
 {
 	struct strbuf helpbuf = STRBUF_INIT;
 	char *msg = getenv("GIT_CHERRY_PICK_HELP");
@@ -214,7 +214,7 @@ static char *help_msg(const unsigned char *sha1)
 		strbuf_addf(&helpbuf,
 			"  When committing, use the option '-c %s'\n"
 			"to retain authorship and message.",
-			find_unique_abbrev(sha1, DEFAULT_ABBREV));
+			name);
 	}
 	return strbuf_detach(&helpbuf, NULL);
 }
@@ -403,7 +403,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		if (commit_lock_file(&msg_file) < 0)
 			die ("Error wrapping up %s", defmsg);
 		fprintf(stderr, "Automatic %s failed.%s\n",
-			me, help_msg(commit->object.sha1));
+			me, help_msg(commit_name));
 		rerere(allow_rerere_auto);
 		exit(1);
 	}
-- 
1.7.0.rc2.40.g33ed2.dirty

Re: git cherry-pick --continue?

[Jeff King]

On Thu, Feb 11, 2010 at 04:04:45PM -0500, Jeff King wrote:

> Series to follow:
> 
>   [1/4]: cherry-pick: rewrap advice message
>   [2/4]: cherry-pick: refactor commit parsing code
>   [3/4]: cherry-pick: format help message as strbuf
>   [4/4]: cherry-pick: show commit name instead of sha1

Actually, I think the message is still a bit ugly after this, so perhaps
this 5/4 would help:

-- >8 --
Subject: [PATCH] cherry-pick: prettify the advice message

It's hard to see the "how to commit" part of this message,
which users may want to cut and paste. On top of that,
having it in paragraph form means that a really long commit
name may cause ugly wrapping. Let's make it prettier, like:

  Automatic cherry-pick failed.  After resolving the conflicts,
  mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
  and commit the result with:

          git commit -c HEAD~23

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-revert.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/builtin-revert.c b/builtin-revert.c
index ad61249..eff5268 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -208,14 +208,16 @@ static char *help_msg(const char *name)
 
 	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.");
+		"and commit the result");
 
 	if (action == CHERRY_PICK) {
-		strbuf_addf(&helpbuf,
-			"  When committing, use the option '-c %s'\n"
-			"to retain authorship and message.",
+		strbuf_addf(&helpbuf, " with: \n"
+			"\n"
+			"        git commit -c %s\n",
 			name);
 	}
+	else
+		strbuf_addch(&helpbuf, '.');
 	return strbuf_detach(&helpbuf, NULL);
 }
 
-- 
1.7.0.rc2.32.g190cd.dirty

Re: git cherry-pick --continue?

[Jeff King]

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

> > 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 <peff@peff.net>
---
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

Re: git cherry-pick --continue?

[Jay Soffian]

On Thu, Feb 11, 2010 at 4:19 PM, Jeff King <peff@peff.net> wrote:
>  Automatic cherry-pick failed.  After resolving the conflicts,
>  mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
>  and commit the result with:
>
>          git commit -c HEAD~23
>

Blech, how is this an improvement? Why can't I just say "git
cherry-pick --continue"?

If I've still got the message in my terminal, it's no harder to use
the SHA1. And if I've lost the message in my terminal, HEAD~23 is lost
and I've got to dig the SHA1 out of my shell history anyway.

(This isn't hypothetical; typically I'll cherry-pick, do some crap in
that terminal such that the message is lost, then I've got to go
through my history to see which commit I had been cherry picking. So
changing the message doesn't help with that situation at all.)

j.

Re: git cherry-pick --continue?

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> Blech, how is this an improvement? Why can't I just say "git
> cherry-pick --continue"?
>
> If I've still got the message in my terminal, it's no harder to use
> the SHA1. And if I've lost the message in my terminal, HEAD~23 is lost
> and I've got to dig the SHA1 out of my shell history anyway.

Maybe it doesn't look like an improvement to you, but I usually do my
editing in other terminal and come back to the shell, so this is a huge
improvement.  It depends on the user.

I don't think Peff meant this change as the sole replacement for --cont
anyway, so I don't understand why you are so upset about it.

Re: git cherry-pick --continue?

[Jeff King]

On Thu, Feb 11, 2010 at 06:05:18PM -0500, Jay Soffian wrote:

> On Thu, Feb 11, 2010 at 4:19 PM, Jeff King <peff@peff.net> wrote:
> >  Automatic cherry-pick failed.  After resolving the conflicts,
> >  mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
> >  and commit the result with:
> >
> >          git commit -c HEAD~23
> >
> 
> Blech, how is this an improvement? Why can't I just say "git
> cherry-pick --continue"?

Because nobody has implemented it yet. ;P

The two improvements here are:

  1. We show what you typed as "git cherry-pick $X", so if you want to
     re-type it, you know it is just "git commit -c $X". This is not
     about speed, but about being more sensible for users to read.

  2. The output is now cut-and-pasteable, so you don't have to type it
     if it is still available in your terminal.

These were meant to make the situation better than it was; I agree that
"git cherry-pick --continue" would be better still.

-Peff

Re: git cherry-pick --continue?

[SZEDER Gábor]

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:
> 
> > > 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 <peff@peff.net>

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 <szeder@ira.uka.de>
---
 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