Johannes Sixt:
* Using test_i18ngrep instead of test_i18ncmp
* test_line_count on the lines of the execution output instead
of rebase stdout
Johannes Schindelin:
* It's a patch series now (whitespace fixes are in patch 2)
* you proposed '--exec "touch executed", I'd go with
--exec "echo foo >>file" so I can count it was executed twice.
Junio:
* added words to the commit message. :)
(Essentially quoting Johannes that I am not just testing but also
stopping for failure at the right point.)
Matthieu: (who preferred responding "private reply, top posting and probably HTML, sent from a phone" ;)
Your patch makes the user feel like he is using non-interactive
rebase while we internally go through the interactive code path.
Did you make sure that the difference between interactive and non
interactive do not show up by surprise? For example, iirc the interactive
rebase does not accept --preserve-merges. What happens if the user asks
--exec --preserve-merges?
Also, does the doc need update?
I (as a user) did not know there is a non-interactive mode for rebase,
assuming -i would show the editor and that would have been the only
difference.
I think it won't show up as a surprise (in v1) as the changed code is testing for
being non explicit via:
if test -n "$cmd" &&
test "$interactive_rebase" != explicit
then
- die(...)
+ interactive_rebase=implied
fi
and that is the last occurrence where interactive_rebase is touched for writing.
Quoting from the man page for `--preserve-merges`:
This uses the --interactive machinery internally, but combining it with
the --interactive option explicitly is generally not a good idea unless
you know what you are doing (see BUGS below).
The docs have a similar explanation just the other way round [--exec is supposed
to be used with --interactive, but I avoided using the word "suppose" as that
sounds like preaching to me (Thou' shall not use --exec without --interactive);
I did not copy over the "if you know what you're doing" as it doesn't help the
user here. How are they supposed to know if they are proficient enough for using
it without -i? And we don't have a BUGS section to point at.]
What happens if the user asks --exec --preserve-merges?
We're fine. When parsing `--preserve-merges`:
--preserve-merges)
preserve_merges=t
test -z "$interactive_rebase" && interactive_rebase=implied
Essentially we did the same with --exec in v1 just after parsing all options,
now I realize we can do it the same way instead of afterwards, just tossing
in:
test -z "$interactive_rebase" && interactive_rebase=implied
Stefan Beller (2):
rebase -x: do not die without -i
t3404: cleanup double empty lines between tests
Documentation/git-rebase.txt | 6 +++---
git-rebase.sh | 7 +------
t/t3404-rebase-interactive.sh | 18 +++++-------------
3 files changed, 9 insertions(+), 22 deletions(-)
--
2.8.0.rc3.2.ga804a9e
Signed-off-by: Stefan Beller <redacted>
---
t/t3404-rebase-interactive.sh | 6 ------
1 file changed, 6 deletions(-)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index c8cc03d..f932639 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -771,7 +771,6 @@ test_expect_success 'rebase-i history with funny messages' '
test_cmp expect actual
'
-
test_expect_success 'prepare for rebase -i --exec' '
git checkout master &&
git checkout -b execute &&
@@ -780,7 +779,6 @@ test_expect_success 'prepare for rebase -i --exec' '
test_commit three_exec main.txt three_exec
'
-
test_expect_success 'running "git rebase -i --exec git show HEAD"' '
set_fake_editor &&
git rebase -i --exec "git show HEAD" HEAD~2 >actual &&
@@ -793,7 +791,6 @@ test_expect_success 'running "git rebase -i --exec git show HEAD"' '
test_cmp expected actual
'
-
test_expect_success 'running "git rebase --exec git show HEAD -i"' '
git reset --hard execute &&
set_fake_editor &&
@@ -807,7 +804,6 @@ test_expect_success 'running "git rebase --exec git show HEAD -i"' '
test_cmp expected actual
'
-
test_expect_success 'running "git rebase -ix git show HEAD"' '
git reset --hard execute &&
set_fake_editor &&
@@ -835,7 +831,6 @@ test_expect_success 'rebase -ix with several <CMD>' '
test_cmp expected actual
'
-
test_expect_success 'rebase -ix with several instances of --exec' '
git reset --hard execute &&
set_fake_editor &&
@@ -850,7 +845,6 @@ test_expect_success 'rebase -ix with several instances of --exec' '
test_cmp expected actual
'
-
test_expect_success 'rebase -ix with --autosquash' '
git reset --hard execute &&
git checkout -b autosquash &&
--
2.8.0.rc3.2.ga804a9e
In the later steps of preparing a patch series I do not want to edit the
patches any more, but just make sure the test suite passes after each
patch. Currently I would run
EDITOR=true git rebase -i <anchor> -x "make test"
In an ideal world the command would be simpler and just be
git rebase <anchor> -x "make test"
to run the test for each commit I am about to send out for review.
This patch enables the short line. As things can still break, I'd want
to be able to fix those failures directly at the commit, so it is not
sufficient to just use:
git rev-list old...new |
while read rev
do
$command || break
done
as it only tests and does not stop at a breakage to fix it up.
Signed-off-by: Stefan Beller <redacted>
---
Documentation/git-rebase.txt | 6 +++---
git-rebase.sh | 7 +------
t/t3404-rebase-interactive.sh | 12 +++++-------
3 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 6ed610a..0387b40 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -391,9 +391,6 @@ idea unless you know what you are doing (see BUGS below).
final history. <cmd> will be interpreted as one or more shell
commands.
+
-This option can only be used with the `--interactive` option
-(see INTERACTIVE MODE below).
-+
You may execute several commands by either using one instance of `--exec`
with several commands:
+
@@ -406,6 +403,9 @@ or by giving more than one `--exec`:
If `--autosquash` is used, "exec" lines will not be appended for
the intermediate commits, and will only appear at the end of each
squash/fixup series.
++
+This uses the `--interactive` machinery internally, but it can be run
+without an explicit `--interactive`.
--root::
Rebase all commits reachable from <branch>, instead of
diff --git a/git-rebase.sh b/git-rebase.sh
index cf60c43..0bf41ee 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -248,6 +248,7 @@ do
;;
--exec=*)
cmd="${cmd}exec ${1#--exec=}${LF}"
+ test -z "$interactive_rebase" && interactive_rebase=implied
;;
--interactive)
interactive_rebase=explicit@@ -348,12 +349,6 @@ do
done
test $# -gt 2 && usage
-if test -n "$cmd" &&
- test "$interactive_rebase" != explicit
-then
- die "$(gettext "The --exec option must be used with the --interactive option")"
-fi
-
if test -n "$action"
then
test -z "$in_progress" && die "$(gettext "No rebase in progress?")"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 544f9ad..c8cc03d 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -876,16 +876,14 @@ test_expect_success 'rebase -ix with --autosquash' '
test_cmp expected actual
'
-
-test_expect_success 'rebase --exec without -i shows error message' '
+test_expect_success 'rebase --exec works without -i ' '
git reset --hard execute &&
- set_fake_editor &&
- test_must_fail git rebase --exec "git show HEAD" HEAD~2 2>actual &&
- echo "The --exec option must be used with the --interactive option" >expected &&
- test_i18ncmp expected actual
+ rm -rf exec_output &&
+ git rebase --exec "echo a line >>exec_output" HEAD~2 2>actual &&
+ test_i18ngrep "Successfully rebased and updated" actual &&
+ test_line_count = 2 exec_output
'
-
test_expect_success 'rebase -i --exec without <CMD>' '
git reset --hard execute &&
set_fake_editor &&
--
2.8.0.rc3.2.ga804a9e
Hi Stefan,
On Thu, 17 Mar 2016, Stefan Beller wrote:
git rev-list old...new |
while read rev
do
$command || break
done
As Junio pointed out, there should be only two, not three dots.
Ciao,
Dscho