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

Junio C Hamano [off-list ref] 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.

The 25/08/09, Junio C Hamano wrote:

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.

I'm not sure what between --obey or --ignore will help most to write
good commit message.  I (as a maintainer myself or as a contributor)
usually prefer to --amend a commit rather than play with copy/paste or
starting from scratch. So, I tend to agree even if the reasons are not
exactly the same. :-)

That said, I don't bother what is the default that much. The main
purpose is to have the choice.

For people who _really_ want to obey to scissors by default I'll add an
option to git-config. Whithout more comments, I'll add

  scissors.obey

.

-- 
Nicolas Sebrecht

Nicolas Sebrecht wrote:

For people who _really_ want to obey to scissors by default I'll add an
option to git-config. Whithout more comments, I'll add

  scissors.obey

mailsplit.scissors

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Hi,

On Wed, 26 Aug 2009, Jakub Narebski wrote:

Nicolas Sebrecht wrote:

quoted

For people who _really_ want to obey to scissors by default I'll add 
an option to git-config. Whithout more comments, I'll add

  scissors.obey

mailsplit.scissors

Sorry, did not have time to read this thread properly, but has anybody put 
thought into the interaction between this patch and "git rebase" (which 
uses "git am", and therefore mailsplit, internally)?

Ciao,
Dscho

Jakub Narebski [off-list ref] writes:

Nicolas Sebrecht wrote:

quoted

For people who _really_ want to obey to scissors by default I'll add an
option to git-config. Whithout more comments, I'll add

  scissors.obey

mailsplit.scissors

That may be a better name.

It must take lower precedence than the command line --no-scissors option,
and that option must be given to am when rebase internally runs am, so that
we won't pay attention to scissors when rebasing existing commits.