Re: [PATCH v2 2/2] format-patch: reduce patch diffstat width to 72

[Duy Nguyen]

On Sat, Jan 27, 2018 at 11:47 PM, Jeff King  wrote:
> On Thu, Jan 25, 2018 at 06:59:27PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
>> diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
>> index 9f563db20a..1e62333b46 100755
>> --- a/t/t4052-stat-output.sh
>> +++ b/t/t4052-stat-output.sh
>> @@ -60,7 +60,7 @@ do
>>   test_cmp expect actual
>>   '
>>  done <<\EOF
>> -format-patch -1 --stdout
>> +format-patch --stat=80 -1 --stdout
>>  diff HEAD^ HEAD --stat
>>  show --stat
>>  log -1 --stat
>
> This hunk confused me. I think what is going on is this:
>
>   - we have a loop that runs the same test on several commands
>
>   - that loop expects format-patch, diff, etc, to have the same output
>
>   - now that format-patch differs from the other commands in its default
> length, we need to use a manual --stat-width to get identical output
>
> It seems like that kind of nullifies the point of some of the tests in
> the loop, though, since they are meant to check the behavior without
> --stat.
>
> OTOH, I think that case is tested later (in the other tests you
> adjusted). So I guess these tests are just covering the "name vs bar
> length" part?

My bad. My thought was.. "Hmm.. I would need to take format-patch out
out the loop since it won't match exactly 80 columns anymore. That's a
lot of work. Hey how about using this opportunity to test that --stat=
can still override default settings?" I didn't realize the follow
tests in that loop set stat width. I'll take format-patch out of the
loop and deal with it separately.
-- 
Duy

Re: [PATCH v2 2/2] format-patch: reduce patch diffstat width to 72

[Jeff King]

On Thu, Jan 25, 2018 at 06:59:27PM +0700, Nguyễn Thái Ngọc Duy wrote:

> diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
> index 9f563db20a..1e62333b46 100755
> --- a/t/t4052-stat-output.sh
> +++ b/t/t4052-stat-output.sh
> @@ -60,7 +60,7 @@ do
>   test_cmp expect actual
>   '
>  done <<\EOF
> -format-patch -1 --stdout
> +format-patch --stat=80 -1 --stdout
>  diff HEAD^ HEAD --stat
>  show --stat
>  log -1 --stat

This hunk confused me. I think what is going on is this:

  - we have a loop that runs the same test on several commands

  - that loop expects format-patch, diff, etc, to have the same output

  - now that format-patch differs from the other commands in its default
length, we need to use a manual --stat-width to get identical output

It seems like that kind of nullifies the point of some of the tests in
the loop, though, since they are meant to check the behavior without
--stat.

OTOH, I think that case is tested later (in the other tests you
adjusted). So I guess these tests are just covering the "name vs bar
length" part?

-Peff

[PATCH v2 2/2] format-patch: reduce patch diffstat width to 72

[Nguyễn Thái Ngọc Duy]

Patches generated by format-patch are meant to be exchanged as emails,
most of the time. And since it's generally agreed that text in mails
should be wrapped around 70 columns or so, make sure these diffstat
follow the convention (especially when used with --cover-letter since we
already defaults to wrapping 72 columns). The default can still be
overriden with command line options.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/log.c  |  2 ++
 t/t4052-stat-output.sh | 28 ++--
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 96af897403..94ee177d56 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1617,6 +1617,8 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
(!rev.diffopt.output_format ||
 rev.diffopt.output_format == DIFF_FORMAT_PATCH))
rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | 
DIFF_FORMAT_SUMMARY;
+   if (!rev.diffopt.stat_width)
+   rev.diffopt.stat_width = MAIL_DEFAULT_WRAP;
 
/* Always generate a patch */
rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index 9f563db20a..1e62333b46 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -60,7 +60,7 @@ do
test_cmp expect actual
'
 done <<\EOF
-format-patch -1 --stdout
+format-patch --stat=80 -1 --stdout
 diff HEAD^ HEAD --stat
 show --stat
 log -1 --stat
@@ -79,11 +79,11 @@ test_expect_success 'preparation for big change tests' '
git commit -m message abcd
 '
 
-cat >expect80 <<'EOF'
- abcd | 1000 ++
+cat >expect72 <<'EOF'
+ abcd | 1000 ++
 EOF
-cat >expect80-graph <<'EOF'
-|  abcd | 1000 
++
+cat >expect72-graph <<'EOF'
+|  abcd | 1000 ++
 EOF
 cat >expect200 <<'EOF'
  abcd | 1000 
++
@@ -107,7 +107,7 @@ do
test_cmp "$expect-graph" actual
'
 done <<\EOF
-ignores expect80 format-patch -1 --stdout
+ignores expect72 format-patch -1 --stdout
 respects expect200 diff HEAD^ HEAD --stat
 respects expect200 show --stat
 respects expect200 log -1 --stat
@@ -135,7 +135,7 @@ do
test_cmp "$expect-graph" actual
'
 done <<\EOF
-ignores expect80 format-patch -1 --stdout
+ignores expect72 format-patch -1 --stdout
 respects expect40 diff HEAD^ HEAD --stat
 respects expect40 show --stat
 respects expect40 log -1 --stat
@@ -163,7 +163,7 @@ do
test_cmp "$expect-graph" actual
'
 done <<\EOF
-ignores expect80 format-patch -1 --stdout
+ignores expect72 format-patch -1 --stdout
 respects expect40 diff HEAD^ HEAD --stat
 respects expect40 show --stat
 respects expect40 log -1 --stat
@@ -250,11 +250,11 @@ show --stat
 log -1 --stat
 EOF
 
-cat >expect80 <<'EOF'
- ...aaa | 1000 
+cat >expect72 <<'EOF'
+ ...aa | 1000 +
 EOF
-cat >expect80-graph <<'EOF'
-|  ...aaa | 1000 

+cat >expect72-graph <<'EOF'
+|  ...aa | 1000 +
 EOF
 cat >expect200 <<'EOF'
  aaa | 1000 
+++
@@ -278,7 +278,7 @@ do
test_cmp "$expect-graph" actual
'
 done <<\EOF
-ignores expect80 format-patch -1 --stdout
+ignores expect72 format-patch -1 --stdout
 respects expect200 diff HEAD^ HEAD --stat
 respects expect200 show --stat
 respects expect200 log -1 --stat
@@ -308,7 +308,7 @@ do
test_cmp "$expect-graph" actual
'
 done <<\EOF
-ignores expect80 format-patch -1 --stdout
+ignores expect72 format-patch -1 --stdout
 respects expect1 diff HEAD^ HEAD --stat
 respects expect1 show --stat
 respects expect1 log -1 --stat
-- 
2.16.1.200.g71ee9f6994