Re: [PATCH v3 1/7] rebase: fix incompatible options error message

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 28, 2018 at 8:12 AM Duy Nguyen <pclouds@xxxxxxxxx> wrote:
>
> On Thu, Nov 22, 2018 at 7:32 PM Elijah Newren <newren@xxxxxxxxx> wrote:
> >
> > 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.
>
> Can we simplify it further and remove the "error: " prefix? "fatal:
> error: " looks redundant.

Sure, I can do that.  Looks like there are a few other cases that need
fixing as well:
$ git grep error: builtin/rebase.c
builtin/rebase.c:                       die(_("error: cannot combine
interactive options "
builtin/rebase.c:                       die(_("error: cannot combine
merge options (--merge, "
builtin/rebase.c:                       die(_("error: cannot combine
'--preserve-merges' with "
builtin/rebase.c:                       die(_("error: cannot combine
'--rebase-merges' with "
builtin/rebase.c:                       die(_("error: cannot combine
'--rebase-merges' with "

Perhaps, for consistency, I should also change the error message in
the git-legacy-rebase.sh script to use 'fatal' instead of 'error'?:

$ git grep error: *rebase*.sh
git-legacy-rebase.sh:                   die "$(gettext "error: cannot
combine interactive options (--interactive, --exec, --rebase-merges,
--preserve-merges, --keep-empty, --root + --onto) with am options
($incompatible_opts)")"
git-legacy-rebase.sh:                   die "$(gettext "error: cannot
combine merge options (--merge, --strategy, --strategy-option) with am
options ($incompatible_opts)")"
git-legacy-rebase.sh:           die "$(gettext "error: cannot combine
'--signoff' with '--preserve-merges'")"
git-legacy-rebase.sh:           die "$(gettext "error: cannot combine
'--preserve-merges' with '--rebase-merges'")"
git-legacy-rebase.sh:           die "$(gettext "error: cannot combine
'--rebase-merges' with '--strategy-option'")"
git-legacy-rebase.sh:           die "$(gettext "error: cannot combine
'--rebase-merges' with '--strategy'")"



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux