Re: [PATCH v2 2/2] rebase: Implement --merge via git-rebase--interactive

Hi Phillip,

On Mon, Nov 12, 2018 at 10:21 AM Phillip Wood [off-list ref] wrote:

quoted

quoted

-Flags only understood by the am backend:
+The following options:

  * --committer-date-is-author-date
  * --ignore-date

@@ -520,15 +512,12 @@ Flags only understood by the am backend:
  * --ignore-whitespace
  * -C

-Flags understood by both merge and interactive backends:
+are incompatible with the following options:

  * --merge
  * --strategy
  * --strategy-option
  * --allow-empty-message
-
-Flags only understood by the interactive backend:
-

It's nice to see this being simplified

:-)

quoted

quoted

-if test -n "$git_am_opt"; then
-    incompatible_opts=$(echo " $git_am_opt " | \
-                        sed -e 's/ -q / /g' -e 's/^ \(.*\) $/\1/')
-    if test -n "$interactive_rebase"
+incompatible_opts=$(echo " $git_am_opt " | \
+                sed -e 's/ -q / /g' -e 's/^ \(.*\) $/\1/')

Why are we no longer guarding this behind the condition that the user
specified *any* option intended for the `am` backend?

I was confused by this as well, what if the user asks for 'rebase
--exec=<cmd> --ignore-whitespace'?

They'd still get an error message about incompatible options; see my
email to Dscho.  However, since it tripped you both up, I'll make the
clean up here a separate commit with some comments.

quoted

quoted

+if test -n "$incompatible_opts"
+then
+    if test -n "$actually_interactive" || test "$do_merge"
     then
-            if test -n "$incompatible_opts"
-            then
-                    die "$(gettext "error: cannot combine interactive options (--interactive, --exec, --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with am options ($incompatible_opts)")"
-            fi
-    fi
-    if test -n "$do_merge"; then
-            if test -n "$incompatible_opts"
-            then
-                    die "$(gettext "error: cannot combine merge options (--merge, --strategy, --strategy-option) with am options ($incompatible_opts)")"
-            fi
+            die "$(gettext "error: cannot combine am options ($incompatible_opts) with either interactive or merge options")"
     fi
 fi

If you want to change the error message here, I think you need to change
the corresponding message in builtin/rebase.c

Indeed, will fix.