From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 08 Feb 2021 21:36:43 GMT
Subject: Re: [PATCH v2 04/11] t/lib-rebase: change the implementation of commands with options
Message-ID: <xmqq1rdq5jis.fsf@gitster.c.googlers.com>
In-Reply-To: <20210208192528.21399-5-charvi077@gmail.com>

Charvi Mendiratta <charvi077@gmail.com> writes:

> However, "fixup" is very different from "exec". Its arguments are not
> arbitrary at all, so there isn't a good reason to mirror the choice of
> "_" to represent a space, which leads to rather unsightly tokens such
> as "fixup_-C". Let's replace it with simpler tokens such as "fixup-C"
> and "fixup-c".

Sadly, I have to say that this change may be making the developer
experience worse.

To use the original, test writers only need to remember a single
rule: "when a single command needs to embed a SP, replace it with
underscore" regardless of which insn they are listing in FAKE_LINES.

Now they need to remember that rule only applies to exec, and merge
and fixup uses a different rule, namely, a SP immediately before a
dash must be removed.

So, if I didn't know you folks have invested enough hours in this
patch, I would have said not to do this, but it is such a small
change, its effect isolated to only those who would be writing tests
for "rebase -i", it may be OK to let them endure a bit additional
burden to remember an extra rule with this patch.  I dunno.