Re: [PATCH] rebase [-i --exec | -ix] <CMD>...

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] rebase [-i --exec | -ix] <CMD>...

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:59

Kong Lucien [off-list ref] writes:
quoted hunk
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 147fa1a..96dbf26 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -210,6 +210,17 @@ rebase.autosquash::
 
 OPTIONS
 -------
+<cmd>::
+	Shell command executed between each commit applications. The
+	--exec option has to be specified.
++
+You may execute several commands between each commit applications.
+Therefore, you can use one instance of exec:
+	git rebase -i --exec "cmd1; cmd2; ...".
+You can also insert several instances of exec, if you wish to
+only have one command per line for example:
+	git rebase -i --exec "cmd1" --exec "cmd2" ...
+
The description of this and exisitng <newbase> look very odd.  The
usual way to describe an option that take a parameter in the OPTIONS
section is to instead do:

	--onto <newbase>::
		Description of what --onto option does and how <newbase>
		is used in what it does.

	--exec <cmd>::
		Description of what --exec option does, and how	<cmd> 
                is used in what it does.
quoted hunk
@@ -336,6 +347,13 @@ link:howto/revert-a-faulty-me
 	user edit that list before rebasing.  This mode can also be used to
 	split commits (see SPLITTING COMMITS below).
 
+-x::
+--exec::
+	Automatically add "exec" followed by <cmd> between each commit
+	applications (see INTERACTIVE MODE below).
++
+This has to be used along with the `--interactive` option explicitly.
+
OK.
quoted hunk
@@ -521,6 +539,27 @@ in `$SHELL`, or the default shell if
...
+If the option '-i' is missing, The command will return the usage page
+of "git rebase". Same if there is no <cmd> specified behind --exec.
OK.
quoted hunk
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c19b7c..7444160 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -876,6 +876,19 @@ cat >> "$todo" << EOF
 #
 EOF
 
+if test -n "$cmd"
+then
+	OIFS=$IFS
+	IFS=','
+	for i in $cmd
+	do
+		sed "/^pick .*/aexec $i" "$todo" >tmp
+		cat tmp >"$todo"
+	done
+	rm tmp
+	IFS=$OIFS
+fi
Where does this "we split at comma", implying "the user is forbidden
to have a comma in the executed command and its arguments" come
from?

A more conventional way that would be less troublesome in the real
life is to use LF instead in a case like this (and git-rebase.sh
already defines it for exactly a case like this).

I am not sure if that "aexec" is portable outside GNUism, though.
Traditionally the a(ppend) command is spelled like this:

	[1addr]a\
        text

to write one line (cf. http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html#tag_20_116_13_03).

Why is "pick" special?  Don't you want to run the commands after
"edit", "reword", "squash", "fixup"?

This should come immediately after autosquash rearranged the insns
in the $todo file, before the help text is appended to it.
quoted hunk
diff --git a/git-rebase.sh b/git-rebase.sh
index 24a2840..a8b1793 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -76,6 +78,7 @@ If you would prefer to skip this patch, instead run \"git rebase --skip\".
 To check out the original branch and stop rebasing run \"git rebase --abort\".
 "
 unset onto
+unset cmd
 strategy=
 strategy_opts=
 do_merge=
You copied badness from the existing "unset onto"; I do not think
the rest of the code (and especially the new code you add) treats a
variable that is not set and set to an empty string any different,
and setting it to empty is the right thing to do in such a case.
quoted hunk
@@ -219,6 +222,24 @@ do
 		onto="$2"
 		shift
 		;;
+	-x)
+		exec_flag=true
exec_flag is not cleared at the beginning of the script, so if the
command is run without "-x" on the command line, it can have
whatever garbage value that was in the environment.  You know that
$cmd is empty if "-x" was not given, and you can make sure $cmd
becomes non-empty when "-x" is given, so remove use of this variable
and replace any 'test -n "$exec_flag"' with 'test -n "$cmd"'.
+		test 3 -le "$#" || usage
Why 3?  You only care about the presense of $2 you are about to
read, no?
+		if orig_head=$(git rev-parse -q --verify "$2") ||
+		   test `expr substr "$2" 1 1` = -
+		then
+			echo "You must specify a command after --exec option\n"
+			usage
+		else
+			if test -n "$cmd"
+			then
+				cmd="$2,$cmd"
+			else
+				cmd="$2"
+			fi
+		fi
+		shift
+		;;
Are you telling me that I cannot have a branch named "make"?  Drop
this nonsense, and just write it in a more straightforward way, e.g.

	-x)
		test 2 -le "$#" || usage
                cmd="${cmd:+"$cmd$LF"} $2"
                shift
                ;;
                
quoted hunk
 	-i)
 		interactive_rebase=explicit
 		;;
@@ -304,6 +325,13 @@ do
 done
 test $# -gt 2 && usage
 
+if test -n "$exec_flag" &&
+   test -z "$interactive_rebase"
+then
I think this should be

	if test -n "$cmd" && test "$interactive_rebase" != explicit
        then
quoted hunk
@@ -348,7 +376,6 @@ abort)
 	exit
 	;;
 esac
-
 # Make sure no rebase is in progress
 if test -n "$in_progress"
 then
Why?

Re: [PATCH] rebase [-i --exec | -ix] <CMD>...

From: <hidden>
Date: 2016-06-15 22:53:59

Junio C Hamano [off-list ref] a écrit :

quoted
+		if orig_head=$(git rev-parse -q --verify "$2") ||
+		   test `expr substr "$2" 1 1` = -
+		then
+			echo "You must specify a command after --exec option\n"
+			usage
+		else
+			if test -n "$cmd"
+			then
+				cmd="$2,$cmd"
+			else
+				cmd="$2"
+			fi
+		fi
+		shift
+		;;
Are you telling me that I cannot have a branch named "make"?  Drop
this nonsense, and just write it in a more straightforward way, e.g.

	-x)
		test 2 -le "$#" || usage
                cmd="${cmd:+"$cmd$LF"} $2"
                shift
                ;;
I agree with the fact that the code must not restrain you in naming branches.
But shouldn't we keep the "test `expr substr "$2" 1 1` = -" part so that
options can't be right after the "--exec" ?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help