git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
LELaszlo Ersek <lersek@redhat.com>
Date
Nov 7, 2018, 22:21 UTC
Message-ID
<f0f95dd0-1a9e-01d0-70f4-3c6d5450df70@redhat.com>
In-Reply-To
<20181107164953.24965-1-pclouds@gmail.com>
On 11/07/18 17:49, Nguyễn Thái Ngọc Duy wrote:
Show 33 quoted lines
> 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

Previous: Nguyễn Thái Ngọc Duy
Message 10 of 10 in “format-patch: respect --stat when explicitly specified”
  1. format-patch: respect --stat when explicitly specifiedLeif Lindholm, Nov 6, 2018
  2. Duy NguyenNov 6, 2018
  3. Laszlo ErsekNov 6, 2018
  4. Duy NguyenNov 6, 2018
  5. Leif LindholmNov 6, 2018
  6. Duy NguyenNov 6, 2018
  7. Leif LindholmNov 6, 2018
  8. Duy NguyenNov 6, 2018
  9. format-patch: respect --stat in cover letter's diffstatNguyễn Thái Ngọc Duy, Nov 7, 2018
  10. Laszlo ErsekNov 7, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.