Re: [PATCH v2] format-patch: respect --stat in cover letter's diffstat

[Laszlo Ersek <lersek@redhat.com>]

On 11/07/18 17:49, Nguyễn Thái Ngọc Duy wrote:
> Commit 43662b23ab (format-patch: keep cover-letter diffstat wrapped in
> 72 columns - 2018-01-24) uncondtionally sets stat width to 72 when
> generating diffstat for the cover letter, ignoring --stat from command
> line. But it should only do so when stat width is still default
> (i.e. stat_width == 0).
> 
> In order to fix this, we should only set stat_width if stat_width is
> zero. But it will never be. Commit 071dd0ba43 (format-patch: reduce
> patch diffstat width to 72 - 2018-02-01) makes sure that default stat
> width will be 72 (ignoring $COLUMNS, but could still be overriden by
> --stat). So all we need to do here is drop the assignment.
> 
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Helped-by: Leif Lindholm <leif.lindholm@linaro.org>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/log.c          |  2 --
>  t/t4052-stat-output.sh | 48 +++++++++++++++++++++++++++++-------------
>  2 files changed, 33 insertions(+), 17 deletions(-)
> 
> diff --git a/builtin/log.c b/builtin/log.c
> index 061d4fd864..1a39c6e52a 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1009,8 +1009,6 @@ static void show_diffstat(struct rev_info *rev,
>  
>  	memcpy(&opts, &rev->diffopt, sizeof(opts));
>  	opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
> -	opts.stat_width = MAIL_DEFAULT_WRAP;
> -
>  	diff_setup_done(&opts);
>  
>  	diff_tree_oid(get_commit_tree_oid(origin),

Because I plan to use the patch on top of v2.19.1 (until the next major
release, v2.20, is made), that's also where I applied and tested the patch.

With master @ a4b8ab5363a3, this patch targets show_diffstat(). At
v2.19.1, commit fa5b7ea670f4 ("format-patch: allow additional generated
content in make_cover_letter()", 2018-07-23) had not occurred yet, so
there the subject code still lived in make_cover_letter(). On my end,
git-am has applied the hunk to make_cover_letter() seamlessly.

I tested the patch with "--stat=1000 --stat-graph-width=20", formatting
an edk2 series that contained commit 1ed6498c4a02
("UefiCpuPkg/CommonFeature: Skip locking when the feature is disabled",
2018-11-07). The long pathname
"UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c" is no longer
truncated in the cumulative diffstat, in the cover letter.

Tested-by: Laszlo Ersek <lersek@redhat.com>

> diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
> [...]

I didn't try to run the test suite (I wasn't conscious of it anyway); I
built & installed git with

  nice make -j4 prefix=... all doc info
  nice make prefix=... install install-doc install-html install-info

I also wasn't watching the make log. So if those make targets don't
include the test suite, then I didn't exercise the new test case.

Thank you!
Laszlo