Re: [PATCH] format-patch: use --compact-summary instead of --summary

2018-03-17 Thread Duy Nguyen
On Fri, Mar 16, 2018 at 10:41 PM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>> For reviewers, a couple fewer lines from diffstat (either cover letter
>> or patches) is a good thing because there's less to read.
>
> I can certainly accept that there may be some reviewers who share
> that view, but me personally, I would rather have them outside the
> main diffstat section, so addition and removal of files that are
> rarer events do stand out.
>
> I guess people would not mind an option to use it, but they can
> already say "git format-patch --compact-summary $upstream..", so...

Good point. I can't negate --summary though. But that calls for another patch.
-- 
Duy


Re: [PATCH] format-patch: use --compact-summary instead of --summary

2018-03-16 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> For reviewers, a couple fewer lines from diffstat (either cover letter
> or patches) is a good thing because there's less to read.

I can certainly accept that there may be some reviewers who share
that view, but me personally, I would rather have them outside the
main diffstat section, so addition and removal of files that are
rarer events do stand out.

I guess people would not mind an option to use it, but they can
already say "git format-patch --compact-summary $upstream..", so...




[PATCH] format-patch: use --compact-summary instead of --summary

2018-03-16 Thread Nguyễn Thái Ngọc Duy
For reviewers, a couple fewer lines from diffstat (either cover letter
or patches) is a good thing because there's less to read.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 I realize that rename percentage is lost with this change. I just
 hope that nobody cares about that, or I'll have to implement rename
 percentage in --compact-summary too in order to move this patch
 forward.

 builtin/log.c |  9 +---
 ...tach_--stdout_--suffix=.diff_initial..side |  7 +++---
 ...at-patch_--attach_--stdout_initial..master | 19 +++-
 ...t-patch_--attach_--stdout_initial..master^ | 12 +-
 ...rmat-patch_--attach_--stdout_initial..side |  7 +++---
 ..._--stdout_--numbered-files_initial..master | 19 +++-
 ..._--subject-prefix=TESTCASE_initial..master | 19 +++-
 ...at-patch_--inline_--stdout_initial..master | 19 +++-
 ...t-patch_--inline_--stdout_initial..master^ | 12 +-
 ...-patch_--inline_--stdout_initial..master^^ |  7 +++---
 ...rmat-patch_--inline_--stdout_initial..side |  7 +++---
 ...-stdout_--cover-letter_-n_initial..master^ | 22 ---
 ...tch_--stdout_--no-numbered_initial..master | 19 +++-
 ...-patch_--stdout_--numbered_initial..master | 19 +++-
 ...diff.format-patch_--stdout_initial..master | 19 +++-
 ...iff.format-patch_--stdout_initial..master^ | 12 +-
 .../diff.format-patch_--stdout_initial..side  |  7 +++---
 t/t4052-stat-output.sh|  4 ++--
 18 files changed, 103 insertions(+), 136 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 94ee177d56..c8e3be5de7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1062,7 +1062,8 @@ static void make_cover_letter(struct rev_info *rev, int 
use_stdout,
return;
 
memcpy(&opts, &rev->diffopt, sizeof(opts));
-   opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
+   opts.output_format = DIFF_FORMAT_DIFFSTAT;
+   opts.flags.stat_with_summary = 1;
opts.stat_width = MAIL_DEFAULT_WRAP;
 
diff_setup_done(&opts);
@@ -1615,8 +1616,10 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
 
if (!use_patch_format &&
(!rev.diffopt.output_format ||
-rev.diffopt.output_format == DIFF_FORMAT_PATCH))
-   rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | 
DIFF_FORMAT_SUMMARY;
+rev.diffopt.output_format == DIFF_FORMAT_PATCH)) {
+   rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT;
+   rev.diffopt.flags.stat_with_summary = 1;
+   }
if (!rev.diffopt.stat_width)
rev.diffopt.stat_width = MAIL_DEFAULT_WRAP;
 
diff --git 
a/t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side 
b/t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side
index 547ca065a5..89ab87fb4e 100644
--- a/t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side
+++ b/t/t4013/diff.format-patch_--attach_--stdout_--suffix=.diff_initial..side
@@ -12,11 +12,10 @@ Content-Type: text/plain; charset=UTF-8; format=fixed
 Content-Transfer-Encoding: 8bit
 
 ---
- dir/sub | 2 ++
- file0   | 3 +++
- file3   | 4 
+ dir/sub | 2 ++
+ file0   | 3 +++
+ file3 (new) | 4 
  3 files changed, 9 insertions(+)
- create mode 100644 file3
 
 
 --g-i-t--v-e-r-s-i-o-n
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_initial..master 
b/t/t4013/diff.format-patch_--attach_--stdout_initial..master
index 52fedc179e..48f284f1d9 100644
--- a/t/t4013/diff.format-patch_--attach_--stdout_initial..master
+++ b/t/t4013/diff.format-patch_--attach_--stdout_initial..master
@@ -14,11 +14,10 @@ Content-Transfer-Encoding: 8bit
 
 This is the second commit.
 ---
- dir/sub | 2 ++
- file0   | 3 +++
- file2   | 3 ---
+ dir/sub  | 2 ++
+ file0| 3 +++
+ file2 (gone) | 3 ---
  3 files changed, 5 insertions(+), 3 deletions(-)
- delete mode 100644 file2
 
 
 --g-i-t--v-e-r-s-i-o-n
@@ -73,10 +72,9 @@ Content-Type: text/plain; charset=UTF-8; format=fixed
 Content-Transfer-Encoding: 8bit
 
 ---
- dir/sub | 2 ++
- file1   | 3 +++
+ dir/sub | 2 ++
+ file1 (new) | 3 +++
  2 files changed, 5 insertions(+)
- create mode 100644 file1
 
 
 --g-i-t--v-e-r-s-i-o-n
@@ -121,11 +119,10 @@ Content-Type: text/plain; charset=UTF-8; format=fixed
 Content-Transfer-Encoding: 8bit
 
 ---
- dir/sub | 2 ++
- file0   | 3 +++
- file3   | 4 
+ dir/sub | 2 ++
+ file0   | 3 +++
+ file3 (new) | 4 
  3 files changed, 9 insertions(+)
- create mode 100644 file3
 
 
 --g-i-t--v-e-r-s-i-o-n
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_initial..master^ 
b/t/t4013/diff.format-patch_--attach_--stdout_initial..master^
index 1c3cde251b..be6de1290a 100644
--- a/t/t4013/diff.format-patch_--attach_--stdout_initial..master^
+++ b/t/t4013/diff.format-patch_--attach_--s