Re: [PATCH v8 3/3] rebase: add a config option for --rebase-merges

>
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
> >> @@ -1514,13 +1542,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> >>                              break;
> >>              if (i >= 0 || options.type == REBASE_APPLY) {
> >> -                    if (is_merge(&options))
> >> -                            die(_("apply options and merge options "
> >> -                                      "cannot be used together"));
> >
> > This isn't a new change but having thought about it I'm not sure
> > moving this code is a good idea. If the user runs
> >
> >       git -c rebase.updateRefs=true rebase --whitespace=fix --exec "make test"
> >
> > then instead of getting an message saying that they cannot use apply
> > and merge options together they'll get a message suggesting they pass
> > --no-update-refs which wont fix the problem for them.
>
> Hmph.  Instead of dying here, ...
>
> >> +                    if (options.autosquash == -1 && options.config_autosquash == 1)
> >>                              die(_("apply options are incompatible with rebase.autoSquash.  Consider adding --no-autosquash"));
> >> +                    else if (options.rebase_merges == -1 && options.config_rebase_merges == 1)
> >> +                            die(_("apply options are incompatible with rebase.rebaseMerges.  Consider adding --no-rebase-merges"));
> >>                      else if (options.update_refs == -1 && options.config_update_refs == 1)
> >>                              die(_("apply options are incompatible with rebase.updateRefs.  Consider adding --no-update-refs"));
>
> ... we get caught here, and the next one is not triggered.
>
> >> +                    else if (is_merge(&options))
> >> +                            die(_("apply options and merge options "
> >> +                                      "cannot be used together"));
> >>                      else
> >>                              options.type = REBASE_APPLY;
>
> What's the reason why "cannot be used together" is moved to the last
> in the chain?
>
> The first two new conditions in this chain try to catch an
> invocation with some apply-specific command line option
> (e.g. "--whitespace=fix") when used with configuration variables
> specific to the merge-backend (e.g. "rebase.merges") and suggest
> overriding the configuration from the command line, and I suspect
> that the motivation behind this change is that their error messages
> are more specific than the generic "apply and merge do not mix".