git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>, git@vger.kernel.org
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [PATCH v2] format-patch: respect --stat in cover letter's diffstat
Date: Mon, 12 Nov 2018 11:24:38 +0100	[thread overview]
Message-ID: <b89707b3-515c-7275-8e56-d901a0c64736@redhat.com> (raw)
In-Reply-To: <20181110054646.10149-2-pclouds@gmail.com>

On 11/10/18 06:46, 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(-)

* This submission should have been posted as v3, not v2. V2 was posted at

https://public-inbox.org/git/20181107164953.24965-1-pclouds@gmail.com/

* Comparing the patch emails, the only difference is that this version
renames "expect.40" to "expect.60". This should have been mentioned in a
cover letter, or in the Notes section of the current submission.

* In my response to the (original) v2 posting, at

https://public-inbox.org/git/f0f95dd0-1a9e-01d0-70f4-3c6d5450df70@redhat.com/

I stated that I didn't try to run the test suite, and gave my T-b (under
the circumstances described there). Given that the change in v3 (= this
submission) is limited to the test case, I think my T-b should have been
carried forward.

Thanks
Laszlo



> 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),
> diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
> index 6e2cf933f7..28c053849a 100755
> --- a/t/t4052-stat-output.sh
> +++ b/t/t4052-stat-output.sh
> @@ -44,42 +44,50 @@ show --stat
>  log -1 --stat
>  EOF
>  
> -while read cmd args
> +cat >expect.60 <<-'EOF'
> + ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
> +EOF
> +cat >expect.6030 <<-'EOF'
> + ...aaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
> +EOF
> +cat >expect2.60 <<-'EOF'
> + ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
> + ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
> +EOF
> +cat >expect2.6030 <<-'EOF'
> + ...aaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
> + ...aaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
> +EOF
> +while read expect cmd args
>  do
> -	cat >expect <<-'EOF'
> -	 ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
> -	EOF
>  	test_expect_success "$cmd --stat=width: a long name is given more room when the bar is short" '
>  		git $cmd $args --stat=40 >output &&
>  		grep " | " output >actual &&
> -		test_cmp expect actual
> +		test_cmp $expect.60 actual
>  	'
>  
>  	test_expect_success "$cmd --stat-width=width with long name" '
>  		git $cmd $args --stat-width=40 >output &&
>  		grep " | " output >actual &&
> -		test_cmp expect actual
> +		test_cmp $expect.60 actual
>  	'
>  
> -	cat >expect <<-'EOF'
> -	 ...aaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
> -	EOF
>  	test_expect_success "$cmd --stat=...,name-width with long name" '
>  		git $cmd $args --stat=60,30 >output &&
>  		grep " | " output >actual &&
> -		test_cmp expect actual
> +		test_cmp $expect.6030 actual
>  	'
>  
>  	test_expect_success "$cmd --stat-name-width with long name" '
>  		git $cmd $args --stat-name-width=30 >output &&
>  		grep " | " output >actual &&
> -		test_cmp expect actual
> +		test_cmp $expect.6030 actual
>  	'
>  done <<\EOF
> -format-patch -1 --stdout
> -diff HEAD^ HEAD --stat
> -show --stat
> -log -1 --stat
> +expect2 format-patch --cover-letter -1 --stdout
> +expect diff HEAD^ HEAD --stat
> +expect show --stat
> +expect log -1 --stat
>  EOF
>  
>  
> @@ -95,6 +103,16 @@ test_expect_success 'preparation for big change tests' '
>  	git commit -m message abcd
>  '
>  
> +cat >expect72 <<'EOF'
> + abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> + abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +EOF
> +test_expect_success "format-patch --cover-letter ignores COLUMNS (big change)" '
> +	COLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&
> +	grep " | " output >actual &&
> +	test_cmp expect72 actual
> +'
> +
>  cat >expect72 <<'EOF'
>   abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  EOF
> 


  reply	other threads:[~2018-11-12 10:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-10  5:46 [PATCH 00/22] Kill the_index part 5 Nguyễn Thái Ngọc Duy
2018-11-10  5:46 ` [PATCH v2] format-patch: respect --stat in cover letter's diffstat Nguyễn Thái Ngọc Duy
2018-11-12 10:24   ` Laszlo Ersek [this message]
2018-11-10  5:46 ` [PATCH 01/22] wt-status.c: remove implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-11-10  5:46 ` [PATCH 02/22] wt-status.c: remove implicit dependency the_repository Nguyễn Thái Ngọc Duy
2018-11-10  5:46 ` [PATCH 03/22] list-objects-filter.c: remove implicit dependency on the_index Nguyễn Thái Ngọc Duy
  -- strict thread matches above, loose matches on Subject: below --
2018-11-06 10:48 [PATCH] format-patch: respect --stat when explicitly specified Leif Lindholm
2018-11-07 16:49 ` [PATCH v2] format-patch: respect --stat in cover letter's diffstat Nguyễn Thái Ngọc Duy
2018-11-07 22:21   ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b89707b3-515c-7275-8e56-d901a0c64736@redhat.com \
    --to=lersek@redhat.com \
    --cc=git@vger.kernel.org \
    --cc=leif.lindholm@linaro.org \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).