On 11/06/18 16:19, Elijah Newren wrote:
Hi Phillip
On Sun, Jun 10, 2018 at 12:40 PM, Phillip Wood
<phillip.wood@xxxxxxxxxxxx> wrote:
Documentation/git-rebase.txt | 15 +++++++++++++--
git-rebase.sh | 17 +++++++++++++++++
t/t3422-rebase-incompatible-options.sh | 10 +++++-----
3 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 0e20a66e73..451252c173 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -243,6 +243,10 @@ leave out at most one of A and B, in which case it
defaults to HEAD.
--keep-empty::
Keep the commits that do not change anything from its
parents in the result.
++
+This uses the `--interactive` machinery internally, and as such,
+anything that is incompatible with --interactive is incompatible
+with this option.
--allow-empty-message::
By default, rebasing commits with an empty message will fail.
@@ -324,6 +328,8 @@ which makes little sense.
and after each change. When fewer lines of surrounding
context exist they all must match. By default no context is
ever ignored.
+ Incompatible with the --merge and --interactive options, or
+ anything that implies those options or their machinery.
struct replay_opts has an allow_empty_message member so I'm not sure that's
true.
I think you were confused by the way the patch broke up. The jump to
line 328 means that this comment is about the -C option, not the
--allow-empty-message option.
Ah you're right, I missed the hunk header
However, I probably should add a comment next to the
--allow-empty-message option, to not the reverse is true, i.e. that
it's incompatible with am-based rebases. (git-rebase--am.sh ignores
the allow_empty_message variable set in git-rebase.sh, unlike
git-rebase--interactive.sh and git-rebase--merge.sh)
That sounds like a good idea
-f::
--force-rebase::
@@ -355,13 +361,15 @@ default is `--no-fork-point`, otherwise the default
is `--fork-point`.
--whitespace=<option>::
These flag are passed to the 'git apply' program
(see linkgit:git-apply[1]) that applies the patch.
- Incompatible with the --interactive option.
+ Incompatible with the --merge and --interactive options, or
+ anything that implies those options or their machinery.
I wonder if it is better just to list the incompatible options it might be a
bit long but it would be nicer for the user than them having to work out
which options imply --interactive.
That could work. Would this be done at the end of the 'OPTIONS'
section of the manpage? Should I create an 'INCOMPATIBLE OPTIONS'
section that follows the 'OPTIONS' section?
I think that would be the best way of doing it, maybe with a note in the
description of the am options to check in that section to see what they
can be safely combined with.
--committer-date-is-author-date::
--ignore-date::
These flags are passed to 'git am' to easily change the dates
of the rebased commits (see linkgit:git-am[1]).
- Incompatible with the --interactive option.
+ Incompatible with the --merge and --interactive options, or
+ anything that implies those options or their machinery.
--signoff::
Add a Signed-off-by: trailer to all the rebased commits. Note
@@ -400,6 +408,9 @@ The `--rebase-merges` mode is similar in spirit to
`--preserve-merges`, but
in contrast to that option works well in interactive rebases: commits
can be
reordered, inserted and dropped at will.
+
+This uses the `--interactive` machinery internally, but it can be run
+without an explicit `--interactive`.
++
Without more context it's hard to judge but I'm not sure this adds anything
useful
Hmm, yeah. I noted that --exec had similar wording, noted that
--preserve-merges had something along the same lines but as a warning,
and didn't see the similar wording for --rebase-merges -- I somehow
missed the paragraph right above where I added these lines. Oops.
Anyway, I'll pull it out.
If we can get a good description of which options are compatible with
what then hopefully we can remove the existing references to implicit
interactive and am, the user should only have to worry about which
options are compatible.
It is currently only possible to recreate the merge commits using the
`recursive` merge strategy; Different merge strategies can be used only
via
explicit `exec git merge -s <strategy> [...]` commands.
diff --git a/git-rebase.sh b/git-rebase.sh
index 40be59ecc4..f1dbecba18 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -503,6 +503,23 @@ then
git_format_patch_opt="$git_format_patch_opt --progress"
fi
+if test -n "$git_am_opt"; then
+ incompatible_opts=`echo "$git_am_opt" | sed -e 's/ -q//'`
I think the style guide recommends $() over ``
Will fix.
Thanks for taking a look!
Thanks for working on this, it should make things simpler for user's to
understand
Best Wishes
Phillip
Elijah
