Re: [PATCH 0/3] Document and refuse rebase -s ours

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 15, 2009, 21:04 UTC
Message-ID: <7veinzfqj9.fsf@alter.siamese.dyndns.org>
In-Reply-To: <cover.1258309432.git.trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> I also implemented Nicolas's suggestion to reject -s ours outright;
> I'm not really happy with starting a blacklist there, but maybe it
> helps the next unwary user.

I am inclined to agree with you and Nicolas on this, but I'll let the list decide if [3/3] is a good idea.

I'd rewrite [3/3] in the following way to keep it easier to maintain the blacklist, like this.

            case "$1" in
    -       ours)
    +       ours | theirs | octopus | subtree)
                    die "Refusing to rebase with $1; see git help rebase."
            esac

It would also make it easier to turn this into a whitelist if we choose to,

 git-rebase--interactive.sh |    5 +----
 git-rebase.sh              |    5 +----
 git-sh-setup.sh            |    7 +++++++
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 53d35f3..de7448b 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -571,10 +571,7 @@ first and then run 'git rebase --continue' again."
 			STRATEGY="-s $2"
 			shift ;;
 		esac
-		if test "$STRATEGY" = "-s ours"
-		then
-			die "Refusing to rebase with 'ours' strategy; see git help rebase."
-		fi
+		git_check_merge_strategy_used_in_rebase "${STRATEGY#-s }"
 		;;
 	-m)
 		# we use merge anyway
diff --git a/git-rebase.sh b/git-rebase.sh
index 2d7d566..dd9ec63 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -306,10 +306,7 @@ do
 			strategy="$2"
 			shift ;;
 		esac
-		if test $strategy = ours
-		then
-			die "Refusing to rebase with 'ours' strategy; see git help rebase."
-		fi
+		git_check_merge_strategy_used_in_rebase "$strategy"
 		do_merge=t
 		;;
 	-n|--no-stat)
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index c41c2f7..724955f 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -199,3 +199,10 @@ case $(uname -s) in
 	}
 	;;
 esac
+
+git_check_merge_strategy_used_in_rebase () {
+	case "$1" in
+	ours)
+		die "Refusing to rebase with $1; see git help rebase."
+	esac
+}