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

[Elijah Newren <newren@gmail.com>]

Hi Phillip,

On Mon, Nov 12, 2018 at 10:21 AM Phillip Wood <phillip.wood@talktalk.net> wrote:
> >> -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

:-)

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

> >> +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.