* git cherry-pick --continue? @ 2010-02-10 20:37 Sverre Rabbelier 2010-02-10 21:04 ` Jeff King 0 siblings, 1 reply; 22+ messages in thread From: Sverre Rabbelier @ 2010-02-10 20:37 UTC (permalink / raw) To: Git List 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-10 20:37 git cherry-pick --continue? Sverre Rabbelier @ 2010-02-10 21:04 ` Jeff King 2010-02-10 21:24 ` Jeff King 0 siblings, 1 reply; 22+ messages in thread From: Jeff King @ 2010-02-10 21:04 UTC (permalink / raw) To: Sverre Rabbelier; +Cc: Git List 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-10 21:04 ` Jeff King @ 2010-02-10 21:24 ` Jeff King 2010-02-10 21:26 ` Sverre Rabbelier 2010-02-10 22:01 ` Junio C Hamano 0 siblings, 2 replies; 22+ messages in thread From: Jeff King @ 2010-02-10 21:24 UTC (permalink / raw) To: Sverre Rabbelier; +Cc: Junio C Hamano, Git List 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-10 21:24 ` Jeff King @ 2010-02-10 21:26 ` Sverre Rabbelier 2010-02-10 22:01 ` Junio C Hamano 1 sibling, 0 replies; 22+ messages in thread From: Sverre Rabbelier @ 2010-02-10 21:26 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Git List 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-10 21:24 ` Jeff King 2010-02-10 21:26 ` Sverre Rabbelier @ 2010-02-10 22:01 ` Junio C Hamano 2010-02-10 22:21 ` Junio C Hamano 1 sibling, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2010-02-10 22:01 UTC (permalink / raw) To: Jeff King; +Cc: Sverre Rabbelier, Junio C Hamano, Git List 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 ;-). ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-10 22:01 ` Junio C Hamano @ 2010-02-10 22:21 ` Junio C Hamano 2010-02-10 22:23 ` Sverre Rabbelier 2010-02-11 19:32 ` Jeff King 0 siblings, 2 replies; 22+ messages in thread From: Junio C Hamano @ 2010-02-10 22:21 UTC (permalink / raw) To: Jeff King; +Cc: Sverre Rabbelier, Git List 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-10 22:21 ` Junio C Hamano @ 2010-02-10 22:23 ` Sverre Rabbelier 2010-02-10 22:34 ` Junio C Hamano 2010-02-11 19:32 ` Jeff King 1 sibling, 1 reply; 22+ messages in thread From: Sverre Rabbelier @ 2010-02-10 22:23 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, Git List 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-10 22:23 ` Sverre Rabbelier @ 2010-02-10 22:34 ` Junio C Hamano 2010-02-10 22:38 ` Sverre Rabbelier 2010-02-11 21:04 ` Jeff King 0 siblings, 2 replies; 22+ messages in thread From: Junio C Hamano @ 2010-02-10 22:34 UTC (permalink / raw) To: Sverre Rabbelier; +Cc: Jeff King, Git List 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-10 22:34 ` Junio C Hamano @ 2010-02-10 22:38 ` Sverre Rabbelier 2010-02-11 21:04 ` Jeff King 1 sibling, 0 replies; 22+ messages in thread From: Sverre Rabbelier @ 2010-02-10 22:38 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, Git List 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-10 22:34 ` Junio C Hamano 2010-02-10 22:38 ` Sverre Rabbelier @ 2010-02-11 21:04 ` Jeff King 2010-02-11 21:06 ` [PATCH 1/4] cherry-pick: rewrap advice message Jeff King ` (4 more replies) 1 sibling, 5 replies; 22+ messages in thread From: Jeff King @ 2010-02-11 21:04 UTC (permalink / raw) To: Junio C Hamano; +Cc: Sverre Rabbelier, Git List 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/4] cherry-pick: rewrap advice message 2010-02-11 21:04 ` Jeff King @ 2010-02-11 21:06 ` Jeff King 2010-02-11 21:06 ` [PATCH 2/4] cherry-pick: refactor commit parsing code Jeff King ` (3 subsequent siblings) 4 siblings, 0 replies; 22+ messages in thread From: Jeff King @ 2010-02-11 21:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: Sverre Rabbelier, git 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/4] cherry-pick: refactor commit parsing code 2010-02-11 21:04 ` Jeff King 2010-02-11 21:06 ` [PATCH 1/4] cherry-pick: rewrap advice message Jeff King @ 2010-02-11 21:06 ` Jeff King 2010-02-11 21:07 ` [PATCH 3/4] cherry-pick: format help message as strbuf Jeff King ` (2 subsequent siblings) 4 siblings, 0 replies; 22+ messages in thread From: Jeff King @ 2010-02-11 21:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: Sverre Rabbelier, git 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/4] cherry-pick: format help message as strbuf 2010-02-11 21:04 ` Jeff King 2010-02-11 21:06 ` [PATCH 1/4] cherry-pick: rewrap advice message Jeff King 2010-02-11 21:06 ` [PATCH 2/4] cherry-pick: refactor commit parsing code Jeff King @ 2010-02-11 21:07 ` Jeff King 2010-02-11 21:08 ` [PATCH 4/4] cherry-pick: show commit name instead of sha1 Jeff King 2010-02-11 21:19 ` git cherry-pick --continue? Jeff King 4 siblings, 0 replies; 22+ messages in thread From: Jeff King @ 2010-02-11 21:07 UTC (permalink / raw) To: Junio C Hamano; +Cc: Sverre Rabbelier, git 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/4] cherry-pick: show commit name instead of sha1 2010-02-11 21:04 ` Jeff King ` (2 preceding siblings ...) 2010-02-11 21:07 ` [PATCH 3/4] cherry-pick: format help message as strbuf Jeff King @ 2010-02-11 21:08 ` Jeff King 2010-02-11 21:19 ` git cherry-pick --continue? Jeff King 4 siblings, 0 replies; 22+ messages in thread From: Jeff King @ 2010-02-11 21:08 UTC (permalink / raw) To: Junio C Hamano; +Cc: Sverre Rabbelier, git 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-11 21:04 ` Jeff King ` (3 preceding siblings ...) 2010-02-11 21:08 ` [PATCH 4/4] cherry-pick: show commit name instead of sha1 Jeff King @ 2010-02-11 21:19 ` Jeff King 2010-02-11 23:05 ` Jay Soffian 4 siblings, 1 reply; 22+ messages in thread From: Jeff King @ 2010-02-11 21:19 UTC (permalink / raw) To: Junio C Hamano; +Cc: Sverre Rabbelier, Git List 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-11 21:19 ` git cherry-pick --continue? Jeff King @ 2010-02-11 23:05 ` Jay Soffian 2010-02-11 23:13 ` Junio C Hamano 2010-02-11 23:57 ` Jeff King 0 siblings, 2 replies; 22+ messages in thread From: Jay Soffian @ 2010-02-11 23:05 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Sverre Rabbelier, Git List 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-11 23:05 ` Jay Soffian @ 2010-02-11 23:13 ` Junio C Hamano 2010-02-11 23:57 ` Jeff King 1 sibling, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2010-02-11 23:13 UTC (permalink / raw) To: Jay Soffian; +Cc: Jeff King, Junio C Hamano, Sverre Rabbelier, Git List 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-11 23:05 ` Jay Soffian 2010-02-11 23:13 ` Junio C Hamano @ 2010-02-11 23:57 ` Jeff King 1 sibling, 0 replies; 22+ messages in thread From: Jeff King @ 2010-02-11 23:57 UTC (permalink / raw) To: Jay Soffian; +Cc: Junio C Hamano, Sverre Rabbelier, Git List 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-10 22:21 ` Junio C Hamano 2010-02-10 22:23 ` Sverre Rabbelier @ 2010-02-11 19:32 ` Jeff King 2010-02-11 20:36 ` Junio C Hamano 1 sibling, 1 reply; 22+ messages in thread From: Jeff King @ 2010-02-11 19:32 UTC (permalink / raw) To: Junio C Hamano; +Cc: Sverre Rabbelier, Git List 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-11 19:32 ` Jeff King @ 2010-02-11 20:36 ` Junio C Hamano 2010-02-11 22:27 ` Jeff King 0 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2010-02-11 20:36 UTC (permalink / raw) To: Jeff King; +Cc: Sverre Rabbelier, Git List 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-11 20:36 ` Junio C Hamano @ 2010-02-11 22:27 ` Jeff King 2010-02-12 14:11 ` SZEDER Gábor 0 siblings, 1 reply; 22+ messages in thread From: Jeff King @ 2010-02-11 22:27 UTC (permalink / raw) To: Junio C Hamano; +Cc: Sverre Rabbelier, Git List 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: git cherry-pick --continue? 2010-02-11 22:27 ` Jeff King @ 2010-02-12 14:11 ` SZEDER Gábor 0 siblings, 0 replies; 22+ messages in thread From: SZEDER Gábor @ 2010-02-12 14:11 UTC (permalink / raw) To: Junio C Hamano, Shawn O. Pearce; +Cc: Jeff King, Sverre Rabbelier, Git List 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
end of thread, other threads:[~2010-02-12 14:11 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-02-10 20:37 git cherry-pick --continue? Sverre Rabbelier 2010-02-10 21:04 ` Jeff King 2010-02-10 21:24 ` Jeff King 2010-02-10 21:26 ` Sverre Rabbelier 2010-02-10 22:01 ` Junio C Hamano 2010-02-10 22:21 ` Junio C Hamano 2010-02-10 22:23 ` Sverre Rabbelier 2010-02-10 22:34 ` Junio C Hamano 2010-02-10 22:38 ` Sverre Rabbelier 2010-02-11 21:04 ` Jeff King 2010-02-11 21:06 ` [PATCH 1/4] cherry-pick: rewrap advice message Jeff King 2010-02-11 21:06 ` [PATCH 2/4] cherry-pick: refactor commit parsing code Jeff King 2010-02-11 21:07 ` [PATCH 3/4] cherry-pick: format help message as strbuf Jeff King 2010-02-11 21:08 ` [PATCH 4/4] cherry-pick: show commit name instead of sha1 Jeff King 2010-02-11 21:19 ` git cherry-pick --continue? Jeff King 2010-02-11 23:05 ` Jay Soffian 2010-02-11 23:13 ` Junio C Hamano 2010-02-11 23:57 ` Jeff King 2010-02-11 19:32 ` Jeff King 2010-02-11 20:36 ` Junio C Hamano 2010-02-11 22:27 ` Jeff King 2010-02-12 14:11 ` SZEDER Gábor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).