Re: [PATCH bg/format-patch-p-noop] log-tree: always add --- marker when options are patch and a stat

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Stephen Boyd <bebarino@xxxxxxxxx> 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 <bebarino@xxxxxxxxx>
> ---

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<n> 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<n> to produce
   patch text should also give three-dash after the log message, and no
   three-dash when they don't produce patch text.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]