[PATCH 1/2] git-rebase--interactive.sh: rework skip_unnecessary_picks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Brandon Casey <drafnel@xxxxxxxxx>

Commit cd035b1c introduced the exec command to interactive rebase.  In
doing so, it modified the way that skip_unnecessary_picks iterates through
the list of rebase commands so that it avoided collapsing multiple spaces
into a single space.  This is necessary for example if the argument to the
exec command contains a path with multiple spaces in it.

The way it did this was by reading each line of rebase commands into a
single variable, and then breaking the individual components out using
echo, sed, and cut.  It used the individual broken-out components for
decision making, and was still able to write the original line to the
output file from the variable it had saved it in.  But, since we only
really need to look at anything other than the first element of the line
when a 'pick' command is encountered, and even that is only necessary when
we are still searching for "unnecessary" picks, and since newer rebase
commands like 'exec' may not even require a sha1 field, let's make our read
statement parse its input into a "command" variable, and a "rest" variable,
and then only break out the sha1 from $rest, and call git-rev-parse, when
absolutely necessary.

I think this future proofs this subroutine, avoids calling git-rev-parse
unnecessarily, and possibly with bogus arguments, and still accomplishes
the goal of not mangling the $rest of the rebase command.

Signed-off-by: Brandon Casey <casey@xxxxxxxxxxxxxxx>
---


Brian Gernhardt wrote:
> On Aug 13, 2010, at 12:37 PM, Ævar Arnfjörð Bjarmason wrote:
> 
>> 39e388728 (merging git rebase -i exec support) broke the funny names
>> test in t3404-rebase-interactive.sh:
> 
> Just noticed this myself when your e-mail hit the list.
> 
>> This one breaks under bash too, does it work for you Matthieu? If so
>> what sort of environment are you executing it in?
> 
> Also broken here: OS X 10.6.4 using bash 3.2.48 and dash 0.5.6-20-ga92255d

The conversion to use printf instead of echo, introduced by 938791cd, was
lost in the merge.  This first patch is just a cleanup that I think
simplifies and improves the code.  The second patch should fix the breakage.

-Brandon


 git-rebase--interactive.sh |   23 ++++++++++++++---------
 1 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bf49b5b..2e5bed0 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -619,25 +619,30 @@ do_rest () {
 # skip picking commits whose parents are unchanged
 skip_unnecessary_picks () {
 	fd=3
-	while read -r line
+	while read -r command rest
 	do
-		command=$(echo "$line" | sed 's/  */ /' | cut -d ' ' -f 1)
-		sha1=$(echo "$line"    | sed 's/  */ /' | cut -d ' ' -f 2)
-		rest=$(echo "$line"    | sed 's/  */ /' | cut -d ' ' -f 3-)
 		# fd=3 means we skip the command
-		case "$fd,$command,$(git rev-parse --verify --quiet "$sha1"^)" in
-		3,pick,"$ONTO"*|3,p,"$ONTO"*)
+		case "$fd,$command" in
+		3,pick|3,p)
 			# pick a commit whose parent is current $ONTO -> skip
-			ONTO=$sha1
+			sha1=$(echo "$rest" | cut -d ' ' -f 1)
+			case "$(git rev-parse --verify --quiet "$sha1"^)" in
+			"$ONTO"*)
+				ONTO=$sha1
+				;;
+			*)
+				fd=1
+				;;
+			esac
 			;;
-		3,#*|3,,*)
+		3,#*|3,)
 			# copy comments
 			;;
 		*)
 			fd=1
 			;;
 		esac
-		echo "$line" >&$fd
+		echo "$command${rest:+ }$rest" >&$fd
 	done <"$TODO" >"$TODO.new" 3>>"$DONE" &&
 	mv -f "$TODO".new "$TODO" &&
 	case "$(peek_next_command)" in
-- 
1.7.2.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]