Thread (4 messages) flat view 4 messages, 2 authors, 2016-06-15

[PATCHV2 0/2] Decouple rebase --exec from --interactive

From: Stefan Beller <hidden>
Date: 2016-06-15 23:08:47

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help