Re: [PATCH] Re: Teach mailinfo to ignore everything before -- >8 -- mark

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 26, 2009, 03:03 UTC
Message-ID: <7veiqzjmy7.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7vvdkbl4ul.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> I think --ignore-scissors is a good thing to add, regardless of what the
> definition of scissors should be.  So your patch should definitely be
> separated into two parts.

Having thought about this a bit more, I do not think --ignore-scissors makes much sense, for several reasons.

Traditionally, a few lines of "background material" that accompanies a patch, without being a part of discussion thread that quotes large chunks of original message with ">" (like you see above), are given below the three-dash line, not above "scissors". This is a good practice not only because we did not have "scissors" support in mailinfo, but because it forced the author to be concise and to the point, and also immediately below the three-dash lines is where the diffstat comes, and it is a place designed to be used for memory refreshers (e.g. "I changed this and that from the previous round based on comments by ...").

The scissors feature shouldn't be used as the replacement for this, not from technical but from human efficiency reasons, as you have to first read above scissors and then jump your eyes down to diffstat, before deciding if it is worth your time to read the commit log message and the patch.

When there is a long discussion, a message in the thread, after following the usual discussion style, may give a (counter)proposal as a "how about this" patch. Such a patch is still _primarily_ for discussion, but it sometimes turn out to be a good solution to the problem discussed in the thread. The maintainer (or participant) then deliberately picks that message and feeds it to "am", and it would be nice if things above scissors are removed automatically. This is the _only_ intended use case of the "scissors" line.

I therefore conclude that using the "remove above scissors" should be a conscious decision, and should not be enabled by default. --obey-scissors would be a good option for this reason.

Besides, if you _lost_ information because the scissors that is on by default gave a false positive, you have to reset HEAD^ and re-apply. If on the other hand we mistakenly kept cruft above a scissors, we can edit it away using "rebase -i". So the failure recovery is much nicer if the feature is not on by default.