format-patch: set diffstat width to 70 instead of default 80

Patches or cover letters 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.

I noticed this when I quoted a diffstat line [1]. Should we do something like this? diffstat is rarely quoted though so perhaps the stat width should be something like 75.

t4052 fails but I don't think it's worth fixing until it's clear if it's worth doing this.

[1] https://public-inbox.org/git/20180122121426.GD5980@ash/T/#u

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 builtin/log.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

builtin/log.c +6 -2

index 14fdf39165..6be79656c5 100644
@@ -1061,6 +1061,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 
 	memcpy(&opts, &rev->diffopt, sizeof(opts));
 	opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
+	opts.diffopt.stat_width = 70;
 
 	diff_setup_done(&opts);
 

@@ -1611,9 +1612,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		die(_("--check does not make sense"));
 
 	if (!use_patch_format &&
-		(!rev.diffopt.output_format ||
-		 rev.diffopt.output_format == DIFF_FORMAT_PATCH))
+	    (!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 = 70;
+	}
 
 	/* Always generate a patch */
 	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;

Patches or cover letters 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.

I noticed this when I quoted a diffstat line [1]. Should we do something like this? diffstat is rarely quoted though so perhaps the stat width should be something like 75.

builtin/log.c static void make_cover_letter(struct rev_info *rev, int use_stdout, · line 1066

Quoted 3 lines

builtin/log.c +1 -0

+	opts.diffopt.stat_width = 70;
 
 	diff_setup_done(&opts);

I wondered how this should interact with any config, but I don't think you can actually configure the stat-width. You _can_ configure diff.statgraphwidth, though, which seems like a funny inconsistency.

On Mon, Jan 22 2018, Jeff King jotted:

Quoted 8 lines · Nguyễn Thái Ngọc Duy

builtin/log.c static void make_cover_letter(struct rev_info *rev, int use_stdout, · line 1066

Quoted 3 lines

builtin/log.c +1 -0

+	opts.diffopt.stat_width = 70;
 
 	diff_setup_done(&opts);

I wondered how this should interact with any config, but I don't think you can actually configure the stat-width. You _can_ configure diff.statgraphwidth, though, which seems like a funny inconsistency.

Isn't the numeric argument to --stat (this works with/without this patch):

    $ git format-patch -10 --stdout --stat=30 -- t|grep -m 5 ' | '
     ...submodule-update.sh | 1 +
     ...ule-update.sh | 14 ++++++
     ...-addresses.sh | 27 ---
     t/t9000/test.pl  | 67 ------
     ...send-email.sh | 19 ++++++
    $ git format-patch -10 --stdout --stat=90 -- t|grep -m 5 ' | '
     t/lib-submodule-update.sh | 1 +
     t/lib-submodule-update.sh | 14 ++++++++++++++
     t/t9000-addresses.sh | 27 -------------------------
     t/t9000/test.pl      | 67 --------------------------------------------------------------
     t/t9001-send-email.sh | 19 +++++++++++++++++++

Quoted 5 lines · Nguyễn Thái Ngọc Duy

Patches or cover letters 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.

Quoted 3 lines

I noticed this when I quoted a diffstat line [1]. Should we do something like this? diffstat is rarely quoted though so perhaps the stat width should be something like 75.

I think the general idea is sensible. Somewhere I picked up "72" as the right size for email lines to accommodate quoting, but I'm pretty sure you could justify any number between 70 and 75. :)

PS I had a funny feeling that this had come up before not due to
quoting, but just due to people with enormous terminals generating too-long lines. But I couldn't find any discussion, and my (admittedly brief) reading of the code is that we'd actually respect the terminal size by default.

Like v1, these changes keep diffstat generated by format-patch in 72 columns. This constant is already used in the code, so it's a bit better than my random "70 or 75" value.

Granted these hard coded values (both 80 and 72) are not really nice. But I would wait for somebody to say "I need or want this" before I add code to make the default configurable.

Nguyễn Thái Ngọc Duy (2):
format-patch: keep cover-letter diffstat wrapped in 72 columns
format-patch: reduce patch diffstat width to 72

t/t4052-stat-output.sh do · line 66

Quoted 13 lines · Nguyễn Thái Ngọc Duy

t/t4052-stat-output.sh +1 -1

index 9f563db20a..1e62333b46 100755
@@ -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?

v3 fixes tests in 2/2 that I overlooked (but Jeff didn't). Interdiff:
...
Nguyễn Thái Ngọc Duy (2):
format-patch: keep cover-letter diffstat wrapped in 72 columns
format-patch: reduce patch diffstat width to 72