From: Stefan Beller <sbeller@google.com>
To: git@vger.kernel.org
Cc: j6t@kdbg.org, johannes.schindelin@gmail.com, gitster@pobox.com,
Matthieu.Moy@grenoble-inp.fr, Stefan Beller <sbeller@google.com>
Subject: [PATCHV2 0/2] Decouple rebase --exec from --interactive
Date: Thu, 17 Mar 2016 14:44:18 -0700 [thread overview]
Message-ID: <1458251060-311-1-git-send-email-sbeller@google.com> (raw)
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
next reply other threads:[~2016-03-17 21:44 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-17 21:44 Stefan Beller [this message]
2016-03-17 21:44 ` [PATCHV2 1/2] rebase -x: do not die without -i Stefan Beller
2016-03-17 21:58 ` Junio C Hamano
2016-03-18 21:26 ` [PATCHv3 1/2] rebase: decouple --exec from --interactive Stefan Beller
2016-03-18 21:33 ` Junio C Hamano
2016-03-18 21:44 ` Junio C Hamano
2016-03-18 21:46 ` Stefan Beller
2016-03-18 6:46 ` [PATCHV2 1/2] rebase -x: do not die without -i Johannes Schindelin
2016-03-17 21:44 ` [PATCHV2 2/2] t3404: cleanup double empty lines between tests Stefan Beller
2016-03-19 10:43 ` [PATCHV2 0/2] Decouple rebase --exec from --interactive Matthieu Moy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1458251060-311-1-git-send-email-sbeller@google.com \
--to=sbeller@google.com \
--cc=Matthieu.Moy@grenoble-inp.fr \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=johannes.schindelin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).