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

On Wed, Nov 28, 2018 at 12:28 AM Johannes Schindelin
[off-list ref] wrote:

Hi Elijah,

On Wed, 21 Nov 2018, Elijah Newren wrote:

quoted

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

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.