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

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

 



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

--
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]