From mboxrd@z Thu Jan 1 00:00:00 1970 From: Junio C Hamano Subject: Re: [PATCH bg/format-patch-p-noop] log-tree: always add --- marker when options are patch and a stat Date: Tue, 03 Nov 2009 21:49:46 -0800 Message-ID: <7vy6mmltz9.fsf@alter.siamese.dyndns.org> References: <1257283456-7007-1-git-send-email-bebarino@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: git@vger.kernel.org, =?utf-8?Q?Bj=C3=B6rn?= Gustavsson To: Stephen Boyd X-From: git-owner@vger.kernel.org Wed Nov 04 06:50:05 2009 Return-path: Envelope-to: gcvg-git-2@lo.gmane.org Received: from vger.kernel.org ([209.132.176.167]) by lo.gmane.org with esmtp (Exim 4.50) id 1N5Yku-0003Ri-TY for gcvg-git-2@lo.gmane.org; Wed, 04 Nov 2009 06:50:05 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751372AbZKDFtw (ORCPT ); Wed, 4 Nov 2009 00:49:52 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751281AbZKDFtw (ORCPT ); Wed, 4 Nov 2009 00:49:52 -0500 Received: from a-pb-sasl-sd.pobox.com ([64.74.157.62]:61789 "EHLO sasl.smtp.pobox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751087AbZKDFtw (ORCPT ); Wed, 4 Nov 2009 00:49:52 -0500 Received: from sasl.smtp.pobox.com (unknown [127.0.0.1]) by a-pb-sasl-sd.pobox.com (Postfix) with ESMTP id BA2C291824; Wed, 4 Nov 2009 00:49:55 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=pobox.com; h=to:cc:subject :references:from:date:message-id:mime-version:content-type; s= sasl; bh=Qfx5al1PQhiXJwvgmy1KOi4N5Gs=; b=aQA4heT/c9u3WWMuHQUvzy3 tOYzCYyscugcbwXO4/x64w/xi2TPUQ34vTNbOmKcBny/M8n2jZaa9/rXmgbHDNdG wMsnBooUmoic+1Poh5yO79G83dStDhUQGrRabHxDnwJ8/x2qijnXNIQZNzAFhNpW OUCxsadSHjNuGzObYddM= DomainKey-Signature: a=rsa-sha1; c=nofws; d=pobox.com; h=to:cc:subject :references:from:date:message-id:mime-version:content-type; q= dns; s=sasl; b=Oqxh5VUgw4eeKbS2IbJUjx3SEXMPD6O7lbE1e8+lunomL1EeA obaWDrg2r1qwnoEOyMXRY7N06V+MwyI85zvMD6w1HohPQWvcx3mWfSRY+7hXNd4T 5a/uT+NybSKXp0JVx4I4Wntmyykc5IKplXEItdeFp4PY0CGPRffe7fqsno= Received: from a-pb-sasl-sd.pobox.com (unknown [127.0.0.1]) by a-pb-sasl-sd.pobox.com (Postfix) with ESMTP id 935B391823; Wed, 4 Nov 2009 00:49:52 -0500 (EST) Received: from pobox.com (unknown [68.225.240.211]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by a-pb-sasl-sd.pobox.com (Postfix) with ESMTPSA id 2D77191821; Wed, 4 Nov 2009 00:49:47 -0500 (EST) User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux) X-Pobox-Relay-ID: DF459712-C905-11DE-900D-A67CBBB5EC2E-77302942!a-pb-sasl-sd.pobox.com Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: Stephen Boyd writes: > Previously, the three dash marker (---) would only be added if the diff > output format was a patch and diffstat (usually -p and --stat). Now that > patches are always generated by format-patch regardless of the stat > format being used (--stat, --raw, --numstat, etc.), always add the three > dash marker when a patch is being generated and a stat option is used. > > This allows users to choose the stat format they want and unifies the > format of patches with stats. It also make patches easier to apply when > generated by format-patch with non-standard stat options as the stat is > no longer considered part of the commit message. > > Signed-off-by: Stephen Boyd > --- I actually am more worried about 68daa64 from 14 months ago, as I vaguely recall seeing an explicit user request that in some community the diffstat information is unwanted on their mailing list, and I am reasonably sure that "-p suppresses --stat" was done deliberately to satisfy them (even though it may have been a suboptimal UI and --no-stat might have been a lot more straightforward). Even though I personally find the stat information very useful, I would be happier if somebody reverts the bg/format-patch-p-noop series and instead fixes the regression caused by 68daa64, and does so without touching any output from the low-level plumbing like diff-tree that may be used by scripts. With older (say 1.6.0) git, format-patch with the -p option does not give these three-dash lines, and it does look funny. Even though the same funniness appears only when you use --raw or --numstat with the current code, if we fix "-p" to suppress the default "--stat", this will become an issue again. > I'm not sure this is wanted though and I guess this could break people's > scripts. Are people actually using --numstat or --raw to put the stat into > the commit message? I am not worried so much about "format-patch --any-option" output; I think it is sane to have three-dash line in it and that is what people expect to see. I however think people used "diff-tree --pretty --raw" as a mechanism to obtain statistics. A script can easily see where the header is and where messages are (they are four-space indented), and what remains after stripping them give you the list of paths each commit touches. --numstat was invented to help this kind of application gather line-level statistics more easily, and I am a bit reluctant to suddenly start giving three-dash in their output. It will upset what is reading from the pipe downstream. In an ideal world, I would probably say: * format-patch should have three-dash after the commit message, no matter what format the patch is asked for, and it always will give patch text. * format-patch -p should be reinstated as a way to ask for "just patch text, no diffstat". Introducing a new option --no-stat _in addition_ to improve the UI is Ok. * format-patch -U should not be mistaken as a request to suppress diffstat; what 68daa64 _tried_ to do was worthy. * Other commands of "log" family that understand -p/-U to produce patch text should also give three-dash after the log message, and no three-dash when they don't produce patch text.