git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Pratik Karki <predatoramigo@gmail.com>,
	Phillip Wood <phillip.wood@talktalk.net>
Subject: Re: [PATCH v3 1/7] rebase: fix incompatible options error message
Date: Wed, 28 Nov 2018 07:58:19 -0800	[thread overview]
Message-ID: <CABPp-BFRrLUJDkyOz=UitGr0eKDxbVq1s19LpoOrY1FJKHsOUQ@mail.gmail.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1811280000020.41@tvgsbejvaqbjf.bet>

On Wed, Nov 28, 2018 at 12:28 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> 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@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

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.

  reply	other threads:[~2018-11-28 15:58 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-08  6:01 [PATCH v2 0/2] Reimplement rebase --merge via interactive machinery Elijah Newren
2018-11-08  6:01 ` [PATCH v2 1/2] git-rebase, sequencer: extend --quiet option for the " Elijah Newren
2018-11-12 15:11   ` Johannes Schindelin
2018-11-08  6:01 ` [PATCH v2 2/2] rebase: Implement --merge via git-rebase--interactive Elijah Newren
2018-11-12 16:21   ` Johannes Schindelin
2018-11-12 18:21     ` Phillip Wood
2018-11-13  9:18       ` Johannes Schindelin
2018-11-14 23:06       ` Elijah Newren
2018-11-13 16:06     ` Elijah Newren
2018-11-14 23:03     ` Elijah Newren
2018-11-15 12:27       ` Johannes Schindelin
2018-11-08  6:33 ` [PATCH v2 0/2] Reimplement rebase --merge via interactive machinery Elijah Newren
2018-11-22  4:48 ` [PATCH v3 0/7] " Elijah Newren
2018-11-22  4:48   ` [PATCH v3 1/7] rebase: fix incompatible options error message Elijah Newren
2018-11-28  8:28     ` Johannes Schindelin
2018-11-28 15:58       ` Elijah Newren [this message]
2018-11-28 16:12     ` Duy Nguyen
2018-11-28 16:31       ` Elijah Newren
2018-11-22  4:48   ` [PATCH v3 2/7] t5407: add a test demonstrating how interactive handles --skip differently Elijah Newren
2018-11-22  4:48   ` [PATCH v3 3/7] am, rebase--merge: do not overlook --skip'ed commits with post-rewrite Elijah Newren
2018-11-22  4:48   ` [PATCH v3 4/7] git-rebase, sequencer: extend --quiet option for the interactive machinery Elijah Newren
2018-11-22  4:48   ` [PATCH v3 5/7] git-legacy-rebase: simplify unnecessary triply-nested if Elijah Newren
2018-11-22  4:48   ` [PATCH v3 6/7] rebase: define linearization ordering and enforce it Elijah Newren
2018-11-22  4:48   ` [PATCH v3 7/7] rebase: Implement --merge via the interactive machinery Elijah Newren
2018-12-11 16:11   ` [PATCH v4 0/8] Reimplement rebase --merge via " Elijah Newren
2018-12-11 16:11     ` [PATCH v4 1/8] rebase: make builtin and legacy script error messages the same Elijah Newren
2018-12-11 16:11     ` [PATCH v4 2/8] rebase: fix incompatible options error message Elijah Newren
2018-12-11 16:11     ` [PATCH v4 3/8] t5407: add a test demonstrating how interactive handles --skip differently Elijah Newren
2018-12-11 16:11     ` [PATCH v4 4/8] am, rebase--merge: do not overlook --skip'ed commits with post-rewrite Elijah Newren
2019-01-21 16:07       ` Johannes Schindelin
2019-01-21 17:59         ` Elijah Newren
2019-01-21 18:11           ` Johannes Schindelin
2018-12-11 16:11     ` [PATCH v4 5/8] git-rebase, sequencer: extend --quiet option for the interactive machinery Elijah Newren
2019-01-21 16:10       ` Johannes Schindelin
2019-01-21 17:50         ` Elijah Newren
2019-01-21 18:19           ` Johannes Schindelin
2019-01-21 18:22             ` Johannes Schindelin
2019-01-22 20:39           ` Junio C Hamano
2019-02-20 11:00             ` Phillip Wood
2019-02-21 17:44               ` Elijah Newren
2018-12-11 16:11     ` [PATCH v4 6/8] git-legacy-rebase: simplify unnecessary triply-nested if Elijah Newren
2018-12-11 16:11     ` [PATCH v4 7/8] rebase: define linearization ordering and enforce it Elijah Newren
2018-12-11 16:11     ` [PATCH v4 8/8] rebase: Implement --merge via the interactive machinery Elijah Newren
2019-01-07 17:15     ` [PATCH v4 0/8] Reimplement rebase --merge via " Elijah Newren
2019-01-07 19:46       ` Junio C Hamano
2019-01-07 20:11         ` Junio C Hamano
2019-01-07 20:39           ` Elijah Newren
2019-01-11 18:36             ` Elijah Newren
2019-01-18 13:36             ` Johannes Schindelin
2019-01-18 14:22               ` Johannes Schindelin
2019-01-18 17:55                 ` Junio C Hamano
2019-01-18 18:07                   ` Elijah Newren
2019-01-18 21:03                   ` Johannes Schindelin
2019-01-18 21:21                     ` Junio C Hamano
2019-01-21 21:02                       ` Johannes Schindelin
2019-01-21 16:03     ` Johannes Schindelin
2019-01-21 21:01       ` Johannes Schindelin
2019-01-21 21:04         ` Elijah Newren
2019-01-29  1:39     ` [PATCH v5 " Elijah Newren
2019-01-29  1:39       ` [PATCH v5 1/8] rebase: make builtin and legacy script error messages the same Elijah Newren
2019-01-29  1:39       ` [PATCH v5 2/8] rebase: fix incompatible options error message Elijah Newren
2019-01-29  1:39       ` [PATCH v5 3/8] t5407: add a test demonstrating how interactive handles --skip differently Elijah Newren
2019-01-29  1:39       ` [PATCH v5 4/8] am, rebase--merge: do not overlook --skip'ed commits with post-rewrite Elijah Newren
2019-01-29  1:39       ` [PATCH v5 5/8] git-rebase, sequencer: extend --quiet option for the interactive machinery Elijah Newren
2019-01-29  1:39       ` [PATCH v5 6/8] git-legacy-rebase: simplify unnecessary triply-nested if Elijah Newren
2019-01-29  1:39       ` [PATCH v5 7/8] rebase: define linearization ordering and enforce it Elijah Newren
2019-01-29  1:39       ` [PATCH v5 8/8] rebase: implement --merge via the interactive machinery Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABPp-BFRrLUJDkyOz=UitGr0eKDxbVq1s19LpoOrY1FJKHsOUQ@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@talktalk.net \
    --cc=predatoramigo@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).