From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 27 Nov 2009 21:11:31 GMT
Subject: Re: [PATCH] git-am: don't ignore --keep (-k) option
Message-ID: <7vfx7zk70c.fsf@alter.siamese.dyndns.org>
In-Reply-To: <87y6lr203s.fsf@meyering.net>

Jim Meyering <jim@meyering.net> writes:

>> At the level of "what does each line of the code do", this is a fix, but
>> as we do a lot more than just stripping "[PATCH] " from the beginning of
>> the Subject: line these days, I think we are better off declaring defeat
>> in this particular codepath and not doing anything here.
>
> Sounds fine to me.
> Glad you're keeping everything in perspective.

What the case statement tries to do is _wrong_; "mailinfo -k" keeps the
original prefix and all the case statement does is to add an extra [PATCH]
that did not exist anywhere in the original on top of that.

What is funny is that the case statement has been trying to do a wrong
thing from day-one, ever since the script was introduced in d1c5f2a (Add
git-am, applymbox replacement., 2005-10-07).  That version uses $keep to
hold -k or empty, gives that to mailinfo for producing $dotest/info, and
it has the same case statement that switches on $keep_subject nobody sets
to add an extra "[PATCH]" in front.  Luckily, due to the typo you found,
nobody was bitten by the bug, and your patch will break things for people
by enabling it ;-).

Thanks for noticing this one.  It began an innocent bug nobody noticed,
but it is embarrassing that we carefully _maintained_ that code nobody
triggers for four years.