Enabling scissors by default?

Enabling scissors by default?

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I was wondering why am's scissors option is not enabled by default.
It seems a very handy feature, but I'm reluctant to use it when
sending patches because the recipient has to notice the scissors and
remember to pass --scissors to git am.

Could this be made the default?

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQEcBAEBAgAGBQJQ7JLGAAoJEJrBOlT6nu75iDYIANFiiH50RlL9WKEfaoybeA5K
ZLodBze1TcAYIx2/ad6qY+XCoq98+nVXTkv2IAleDiNlfeIhKD24UTWNCysT8p1J
5KeFfR4paxLJLJKkmSL5s3DJbyjLlJWcxD7vGku6F4k35NmY3VYR4rJ/CVv0YRrs
p4nNG/EXWBo3/ngiL9QS4E65N0CfcOOjn48RQUmk1DGXSFNHP4L1KuJ4dA9cs9BC
5KmNwh5X6OOal0Lf+ezbxzvoGMwQmhBAxx3t8JQR3E22dLQlUq7stlPl5LDd+Cis
XWfNk3B4NuFTum9LqWnM5TN89WCCFh4/pskdRd5ONF51G0jbuF/hBFbwU05qL/4=
=Qd94
-----END PGP SIGNATURE-----

Re: Enabling scissors by default?

[Junio C Hamano]

Phillip Susi <psusi@ubuntu.com> writes:

> I was wondering why am's scissors option is not enabled by default.

It is very easy to miss misidentification of scissors line; as a
dangerous, potentially information losing option, I do not think it
should be on by default.

Another reason (and this is the original one) why it is not enabled
is to discourage the contributors from overusing scissors -- >8 --
line.  If you always have to write too much stuff before the proper
explanation of your patch, so that the integrator has to use -c
option all the time, you are explaining your patches wrong.

Re: Enabling scissors by default?

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/08/2013 05:42 PM, Junio C Hamano wrote:
> It is very easy to miss misidentification of scissors line; as a 
> dangerous, potentially information losing option, I do not think
> it should be on by default.

I suppose if it only requires one instance of >8 or <8 and one -, it
might be *slightly* dangerous, but if it required a slightly longer
minimum line length, it would be pretty darn unlikely to get triggered
by accident, and of course, is easily disabled.

> Another reason (and this is the original one) why it is not
> enabled is to discourage the contributors from overusing scissors
> -- >8 -- line.  If you always have to write too much stuff before
> the proper explanation of your patch, so that the integrator has to
> use -c option all the time, you are explaining your patches wrong.

I often see patches being tweaked in response to feedback and
resubmitted, usually with a description of what has changed since the
previous version.  Such descriptions don't need to be in the change
log when it is finally applied and seem a perfect use of scissors.

Usually such version to version descriptions are put in a cover
letter, but if you are only submitting a single patch instead of an
entire series, using a cover letter seems silly when you could just
put the comments in one email and clearly mark them as not needing to
go into the final changelog.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQEcBAEBAgAGBQJQ7KriAAoJEJrBOlT6nu755UkIALIT3T5yHH5i+0HOrXLlXzQR
+S2jJfFZ8Kcc+kleiEJ3uLFVGTLMpRyjJFKceOuB4/TdJFUivrYJHWJxcKmW8WzK
BJKZOjt/jv9r8Qt/AB7KA45S7awfQnOWkg6KQlJa1IM0nUPbo4upgMlWar9l7vjz
Hkr7geuHY4fsVUJ7R0rYPcT3pue8ywsT4a9o/ocstfXmC05IrLKQtzO4TuvfiaTb
yBG+rAPKz36zfxCN5NyKExZO6v/LnCKym/PH4a6wYIeTUz1EvuaPy5lQOo6ORQ4h
xbSyBRDPN4yiVgNXfSQmGKwd9XPqs6h8Z0q3X5mGZyOXurw0JFRJlJ3v8hHIvqg=
=Rn7z
-----END PGP SIGNATURE-----

Re: Enabling scissors by default?

[Junio C Hamano]

Phillip Susi <psusi@ubuntu.com> writes:

> On 01/08/2013 05:42 PM, Junio C Hamano wrote:
>> It is very easy to miss misidentification of scissors line; as a 
>> dangerous, potentially information losing option, I do not think
>> it should be on by default.
>
> I suppose if it only requires one instance of >8 or <8 and one -, it
> might be *slightly* dangerous, but if it required a slightly longer
> minimum line length, it would be pretty darn unlikely to get triggered
> by accident, and of course, is easily disabled.

"Easily disabled" is never a good enough reason to change the long
established default of not doing anything funky unless the user
explicitly asks it to do things differently.

You could introduce a new configuration variable "am.scissors" and
personally turn it on, though.  Setting that variable *does* count
as the user explicitly asking for it.

> I often see patches being tweaked in response to feedback and
> resubmitted, usually with a description of what has changed since the
> previous version.  Such descriptions don't need to be in the change
> log when it is finally applied and seem a perfect use of scissors.

Putting such small logs under "---" line is the accepted practice.

Re: Enabling scissors by default?

[Jeff King]

On Tue, Jan 08, 2013 at 03:36:09PM -0800, Junio C Hamano wrote:

> You could introduce a new configuration variable "am.scissors" and
> personally turn it on, though.  Setting that variable *does* count
> as the user explicitly asking for it.

I think we have mailinfo.scissors already.

> > I often see patches being tweaked in response to feedback and
> > resubmitted, usually with a description of what has changed since the
> > previous version.  Such descriptions don't need to be in the change
> > log when it is finally applied and seem a perfect use of scissors.
> 
> Putting such small logs under "---" line is the accepted practice.

Maybe it is just me, but I find the scissors form more readable, because
the "cover letter" material often serves to introduce and give context
to the patch (e.g., "Thanks for your feedback. I've tried to do X, and
it came out well. Here's the patch." serves as an introduction, and
logically comes before the commit message itself).

That does not say anything one way or another about how dangerous or not
it might be to enable scissors by default. Just my two cents that I like
the scissors style for patches that come as part of a discussion (and I
prefer the "---" style when making comments on the contents of a patch;
i.e., when the comments make more sense to be read after reading the
commit message to understand what the patch does).

-Peff

Re: Enabling scissors by default?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Jan 08, 2013 at 03:36:09PM -0800, Junio C Hamano wrote:
>
>> You could introduce a new configuration variable "am.scissors" and
>> personally turn it on, though.  Setting that variable *does* count
>> as the user explicitly asking for it.
>
> I think we have mailinfo.scissors already.

Oh, spoiler.  I was hoping that I could entice new people into doing
a little digging on their own X-<.


>> > I often see patches being tweaked in response to feedback and
>> > resubmitted, usually with a description of what has changed since the
>> > previous version.  Such descriptions don't need to be in the change
>> > log when it is finally applied and seem a perfect use of scissors.
>> 
>> Putting such small logs under "---" line is the accepted practice.
>
> Maybe it is just me, but I find the scissors form more readable, because
> the "cover letter" material often serves to introduce and give context
> to the patch (e.g., "Thanks for your feedback. I've tried to do X, and
> it came out well. Here's the patch." serves as an introduction, and
> logically comes before the commit message itself).
>
> That does not say anything one way or another about how dangerous or not
> it might be to enable scissors by default. Just my two cents that I like
> the scissors style for patches that come as part of a discussion (and I
> prefer the "---" style when making comments on the contents of a patch;
> i.e., when the comments make more sense to be read after reading the
> commit message to understand what the patch does).

Yes, scissors have their uses, namely when presenting a patch in a
discussion context.  Otherwise we wouldn't have introduced it in the
first place.

But the "desription of what has changed since the previous version"
use case I was responding to is where the space below "---" is meant
to be used from very early days of Git (the convention established
on the kernel list).