Re: Rebase/cherry-picking idea

From: Wincent Colaiuta <win@wincent.com>
Date: Nov 26, 2007, 12:34 UTC
Message-ID: <CDF48716-F198-4B33-A5F5-8A2DE1F177EB@wincent.com>
In-Reply-To: <D21294CA-6FD0-40F5-B0D6-5155865DA69A@wincent.com>

El 26/11/2007, a las 12:27, Wincent Colaiuta escribió:

Show 42 quoted lines

> El 26/11/2007, a las 10:32, Benoit Sigoure escribió:
>
>> On Nov 26, 2007, at 10:02 AM, Wincent Colaiuta wrote:
>>
>>> In using "git-rebase --interactive" to re-order commits you  
>>> occasionally get conflicts and will see a message like this:
>>>
>>> 	When commiting, use the option '-c %s' to retain authorship and  
>>> message
>>>
>>> I was thinking that it might be nice to stash away this commit id  
>>> somewhere in GIT_DIR so that the user didn't have to explicitly  
>>> remember it, and add a new switch to git-commit that could be used  
>>> to automatically use that stashed commit id, something like:
>>>
>>> 	git commit --retain
>>>
>>> Although I most often see this kind of message in interactive  
>>> rebasing, the message is generated in builtin-revert.c when cherry- 
>>> picking, so you can also see it in any other situation where  
>>> you're cherry picking and there's a conflict.
>>>
>>> What do people think? Would this be a nice usability improvement?  
>>> Or is it adding clutter?
>>
>>
>> I'm not sure but I think this message is just some unwanted  
>> (misleading) noise, since when you rebase, once you solve the  
>> conflicts, you git-rebase --continue, you don't git-commit.
>
> Looks like you're right. I just did a simple test and it turns out  
> that after a conflict, this:
>
> 	git commit -c ...
> 	git rebase --continue
>
> Produces exactly the same history as this:
>
> 	git rebase --continue
>
> So I think that misleading noise needs to be suppressed or reworded  
> when rebasing. Will look into it.

How about something like this? It would obviously be nice if we could avoid adding another option to builtin-revert; perhaps when/if git- rebase becomes a builtin we can avoid that. The other alternative, and probably one I like I bit more, would be to auto-detect that a rebase is in progress by looking inside the GIT_DIR, although that would also alter the behaviour of manual invocations of git-revert and git-cherry- pick during an interactive rebase (do people actually do that?). What do you think?

diff --git a/builtin-revert.c b/builtin-revert.c
index a0586f9..36e36c3 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -30,7 +30,7 @@ static const char * const cherry_pick_usage[] = {
  	NULL
  };

-static int edit, no_replay, no_commit, mainline;
+static int edit, no_replay, no_commit, rebasing, mainline;
  static enum { REVERT, CHERRY_PICK } action;
  static struct commit *commit;

@@ -50,6 +50,7 @@ static void parse_args(int argc, const char **argv)
  		OPT_BOOLEAN('e', "edit", &edit, "edit the commit message"),
  		OPT_BOOLEAN('x', NULL, &no_replay, "append commit name when cherry- 
picking"),
  		OPT_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
+		OPT_BOOLEAN(0, "rebasing", &rebasing, "use rebase mode"),
  		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
  		OPT_END(),
  	};
@@ -352,11 +353,16 @@ static int revert_or_cherry_pick(int argc, const  
char **argv)
  		}
  		if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
  			die ("Error wrapping up %s", defmsg);
+		if (rebasing)
+			message = "run 'git rebase --continue' "
+			    "or 'git rebase --abort'";
+		else
+			message = "commit the result";
  		fprintf(stderr, "Automatic %s failed.  "
  			"After resolving the conflicts,\n"
  			"mark the corrected paths with 'git add <paths>' "
-			"and commit the result.\n", me);
-		if (action == CHERRY_PICK) {
+			"and %s.\n", me, message);
+		if (action == CHERRY_PICK && !rebasing) {
  			fprintf(stderr, "When commiting, use the option "
  				"'-c %s' to retain authorship and message.\n",
  				find_unique_abbrev(commit->object.sha1,
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bf44b6a..5afb843 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -117,7 +117,7 @@ pick_one () {
  		sha1=$(git rev-parse --short $sha1)
  		output warn Fast forward to $sha1
  	else
-		output git cherry-pick "$@"
+		output git cherry-pick --rebasing "$@"
  	fi
  }

@@ -187,7 +187,7 @@ pick_one_preserving_merges () {
  			fi
  			;;
  		*)
-			output git cherry-pick "$@" ||
+			output git cherry-pick --rebasing "$@" ||
  				die_with_patch $sha1 "Could not pick $sha1"
  			;;
  		esac