Re: [PATCH v3 1/7] rebase: fix incompatible options error message
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Nov 28, 2018, 08:28 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1811280000020.41@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20181122044841.20993-2-newren@gmail.com>
Hi Elijah,
On Wed, 21 Nov 2018, Elijah Newren wrote:
Show 24 quoted lines
> In commit f57696802c30 ("rebase: really just passthru the `git am`
> options", 2018-11-14), the handling of `git am` options was simplified
> dramatically (and an option parsing bug was fixed), but it introduced
> a small regression in the error message shown when options only
> understood by separate backends were used:
>
> $ git rebase --keep --ignore-whitespace
> fatal: error: cannot combine interactive options (--interactive, --exec,
> --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with
> am options (.git/rebase-apply/applying)
>
> $ git rebase --merge --ignore-whitespace
> fatal: error: cannot combine merge options (--merge, --strategy,
> --strategy-option) with am options (.git/rebase-apply/applying)
>
> Note that in both cases, the list of "am options" is
> ".git/rebase-apply/applying", which makes no sense. Since the lists of
> backend-specific options is documented pretty thoroughly in the rebase
> man page (in the "Incompatible Options" section, with multiple links
> throughout the document), and since I expect this list to change over
> time, just simplify the error message.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---This patch is obviously good.
Given that you embedded it in the patch series that makes the sequencer the work horse also for the `merge` backend of `git rebase` in addition to the `interactive` one, may I assume that you intend this patch for post v2.20.0?
Ciao, Dscho
Show 51 quoted lines
> builtin/rebase.c | 11 ++++-------
> git-legacy-rebase.sh | 4 ++--
> 2 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 5b3e5baec8..5ece134ae6 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1202,14 +1202,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> break;
>
> if (is_interactive(&options) && i >= 0)
> - die(_("error: cannot combine interactive options "
> - "(--interactive, --exec, --rebase-merges, "
> - "--preserve-merges, --keep-empty, --root + "
> - "--onto) with am options (%s)"), buf.buf);
> + die(_("error: cannot combine am options "
> + "with interactive options"));
> if (options.type == REBASE_MERGE && i >= 0)
> - die(_("error: cannot combine merge options (--merge, "
> - "--strategy, --strategy-option) with am options "
> - "(%s)"), buf.buf);
> + die(_("error: cannot combine am options "
> + "with merge options "));
> }
>
> if (options.signoff) {
> diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
> index b97ffdc9dd..0a747eb76c 100755
> --- a/git-legacy-rebase.sh
> +++ b/git-legacy-rebase.sh
> @@ -508,13 +508,13 @@ if test -n "$git_am_opt"; then
> 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)")"
> + die "$(gettext "error: cannot combine am options with interactive options")"
> 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)")"
> + die "$(gettext "error: cannot combine am options with merge options")"
> fi
> fi
> fi
> --
> 2.20.0.rc1.7.g58371d377a
>
>