On Wed, Nov 28, 2018 at 12:28 AM Johannes Schindelin
<Johannes.Schindelin@xxxxxx> wrote:
>
> Hi Elijah,
>
> On Wed, 21 Nov 2018, Elijah Newren 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.
> >
> > Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> > ---
>
> 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
I think post v2.20.0 probably makes the most sense. I was unsure what
the policy was around changing strings late in the cycle, but figured
that the worst case with this regression is someone basically
understands what the message is trying to say but thinks it might be
saying more than they understand and reach out with questions. In
contrast, if we decide to change the string and some translators don't
have enough time to translate it before 2.20.0 goes out, then someone
who doesn't understand English gets an English error message, which
seems a little worse.
I at least wanted it to be discussed, though, which is why I
highlighted it in the cover letter.
