Re: [PATCH/RFC] diff: add --compact-summary option to complement --stat

[Duy Nguyen]

On Sun, Jan 14, 2018 at 4:37 PM, Simon Ruderich  wrote:
> On Sat, Jan 13, 2018 at 08:22:11PM +0700, Nguyễn Thái Ngọc Duy wrote:
>> [snip]
>>
>> For mode changes, executable bit is denoted as "(+x)" or "(-x)" when
>> it's added or removed respectively. The same for when a regular file is
>> replaced with a symlink "(+l)" or the other way "(-l)". This also
>> applies to new files. New regulare files are "A", while new executable
>> files or symlinks are "A+x" or "A+l".
>
> I like the short summary. However I find the use of parentheses
> inconsistent.

I agree. I put them in parentheses because somehow to me plain "+x"
looks weird to me.

> Why not use them either always (also for "(A+l)")
> or never? Was there a specific reason why you added them just in
> one place?

Actually shortly after I sent the mail, I realized I could do better.
Since this is a mode _modification_, we could denote it with "M" (most
files in diffstat are "M" for obvious reasons, we just don't print it
because it adds no value), so here we could print "M+x" or "M-x". This
aligns well with "A+l" or "A+x" for example and is one character
shorter than my old way.
-- 
Duy

Re: [PATCH/RFC] diff: add --compact-summary option to complement --stat

[Simon Ruderich]

On Sat, Jan 13, 2018 at 08:22:11PM +0700, Nguyễn Thái Ngọc Duy wrote:
> [snip]
>
> For mode changes, executable bit is denoted as "(+x)" or "(-x)" when
> it's added or removed respectively. The same for when a regular file is
> replaced with a symlink "(+l)" or the other way "(-l)". This also
> applies to new files. New regulare files are "A", while new executable
> files or symlinks are "A+x" or "A+l".

I like the short summary. However I find the use of parentheses
inconsistent. Why not use them either always (also for "(A+l)")
or never? Was there a specific reason why you added them just in
one place?

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

Re: [PATCH/RFC] diff: add --compact-summary option to complement --stat

[Philip Oakley]

(one spelling spotted)..
From: "Nguyễn Thái Ngọc Duy" 

This is partly inspired by gerrit web interface which shows diffstat
like this, e.g. with commit 0433d533f1 (notice the "A" column on the
third line):

Documentation/merge-config.txt |  4 +
builtin/merge.c|  2 +
  A t/t5573-pull-verify-signatures.sh  | 81 ++
t/t7612-merge-verify-signatures.sh | 45 ++
  4 files changed, 132 insertions(+)

In other words, certain information currently shown with --summary is
embedded in the diffstat. This helps reading (all information of the
same file in the same line instead of two) and can reduce the number of
lines if you add/delete a lot of files.

The new option --compact-summary implements this with a tweak to support
mode change, which is shown in --summary too.

For mode changes, executable bit is denoted as "(+x)" or "(-x)" when
it's added or removed respectively. The same for when a regular file is
replaced with a symlink "(+l)" or the other way "(-l)". This also
applies to new files. New regulare files are "A", while new executable
files or symlinks are "A+x" or "A+l".

Note, there is still one piece of information missing from --summary,
the rename/copy percentage. That could probably be added later. It's not
as useful as the others anyway.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
I have had something similar for years but the data is shown after
the path name instead (it's incidentally shown in the diffstat right
below). I was going to clean it up and submit it again, but my recent
experience with Gerrit changed my mind a bit about the output.

Documentation/diff-options.txt | 11 
diff.c | 64 
+-

diff.h |  1 +
t/t4013-diff-various.sh|  5 ++
...y_--root_--stat_--compact-summary_initial (new) | 12 
...R_--root_--stat_--compact-summary_initial (new) | 12 
...ree_--stat_--compact-summary_initial_mode (new) |  4 ++
..._-R_--stat_--compact-summary_initial_mode (new) |  4 ++
8 files changed, 110 insertions(+), 3 deletions(-)
create mode 100644 
t/t4013/diff.diff-tree_--pretty_--root_--stat_--compact-summary_initial
create mode 100644 
t/t4013/diff.diff-tree_--pretty_-R_--root_--stat_--compact-summary_initial
create mode 100644 
t/t4013/diff.diff-tree_--stat_--compact-summary_initial_mode
create mode 100644 
t/t4013/diff.diff-tree_-R_--stat_--compact-summary_initial_mode


diff --git a/Documentation/diff-options.txt 
b/Documentation/diff-options.txt

index 9d1586b956..ff93ff74d0 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -188,6 +188,17 @@ and accumulating child directory counts in the parent 
directories:

 Output a condensed summary of extended header information
 such as creations, renames and mode changes.

+--compact-summary::
+ Output a condensed summary of extended header information in
+ front of the file name part of diffstat. This option is
+ ignored if --stat is not specified.
++
+Fle creations or deletions are denoted with "A" or "D" respectively,


s/Fle/File/ ?


+optionally "+l" if it's a symlink, or "+x" if it's executable.
+Mode changes are put in brackets, e.g. "+x" or "-x" for adding or
+removing executable bit respectively, "+l" or "-l" for becoming a
+symlink or a regular file.
+
ifndef::git-format-patch[]
--patch-with-stat::
 Synonym for `-p --stat`.
diff --git a/diff.c b/diff.c
index fb22b19f09..3f676d 100644
--- a/diff.c
+++ b/diff.c
@@ -2131,6 +2131,7 @@ struct diffstat_t {
 char *from_name;
 char *name;
 char *print_name;
+ const char *status_code;
 unsigned is_unmerged:1;
 unsigned is_binary:1;
 unsigned is_renamed:1;
@@ -2271,6 +2272,7 @@ static void show_stats(struct diffstat_t *data, 
struct diff_options *options)

{
 int i, len, add, del, adds = 0, dels = 0;
 uintmax_t max_change = 0, max_len = 0;
+ int max_status_len = 0;
 int total_files = data->nr, count;
 int width, name_width, graph_width, number_width = 0, bin_width = 0;
 const char *reset, *add_c, *del_c;
@@ -2287,6 +2289,18 @@ static void show_stats(struct diffstat_t *data, 
struct diff_options *options)

 add_c = diff_get_color_opt(options, DIFF_FILE_NEW);
 del_c = diff_get_color_opt(options, DIFF_FILE_OLD);

+ for (i = 0; (i < count) && (i < data->nr); i++) {
+ const struct diffstat_file *file = data->files[i];
+ int len;
+
+ if (!file->status_code)
+ continue;
+ len = strlen(file->status_code) + 1;
+
+ if (len > max_status_len)
+ max_status_len = len;
+ }
+
 /*
 * Find the longest filename and max number of changes
 */
@@ -2383,6 +2397,8 @@ static void show_stats(struct diffstat_t *data, 
struct diff_options *options)

   options->stat_name_width < max_len) ?
 options->stat_name_width : max_len;

+ name_width += max_status_len;
+
 /*
 * Adjust adjustable widths not to exceed maximum width
 */
@@ -2402,6 +2418,8 

[PATCH/RFC] diff: add --compact-summary option to complement --stat

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

This is partly inspired by gerrit web interface which shows diffstat
like this, e.g. with commit 0433d533f1 (notice the "A" column on the
third line):

 Documentation/merge-config.txt |  4 +
 builtin/merge.c|  2 +
   A t/t5573-pull-verify-signatures.sh  | 81 ++
 t/t7612-merge-verify-signatures.sh | 45 ++
   4 files changed, 132 insertions(+)

In other words, certain information currently shown with --summary is
embedded in the diffstat. This helps reading (all information of the
same file in the same line instead of two) and can reduce the number of
lines if you add/delete a lot of files.

The new option --compact-summary implements this with a tweak to support
mode change, which is shown in --summary too.

For mode changes, executable bit is denoted as "(+x)" or "(-x)" when
it's added or removed respectively. The same for when a regular file is
replaced with a symlink "(+l)" or the other way "(-l)". This also
applies to new files. New regulare files are "A", while new executable
files or symlinks are "A+x" or "A+l".

Note, there is still one piece of information missing from --summary,
the rename/copy percentage. That could probably be added later. It's not
as useful as the others anyway.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 I have had something similar for years but the data is shown after
 the path name instead (it's incidentally shown in the diffstat right
 below). I was going to clean it up and submit it again, but my recent
 experience with Gerrit changed my mind a bit about the output.

 Documentation/diff-options.txt | 11 
 diff.c | 64 +-
 diff.h |  1 +
 t/t4013-diff-various.sh|  5 ++
 ...y_--root_--stat_--compact-summary_initial (new) | 12 
 ...R_--root_--stat_--compact-summary_initial (new) | 12 
 ...ree_--stat_--compact-summary_initial_mode (new) |  4 ++
 ..._-R_--stat_--compact-summary_initial_mode (new) |  4 ++
 8 files changed, 110 insertions(+), 3 deletions(-)
 create mode 100644 
t/t4013/diff.diff-tree_--pretty_--root_--stat_--compact-summary_initial
 create mode 100644 
t/t4013/diff.diff-tree_--pretty_-R_--root_--stat_--compact-summary_initial
 create mode 100644 t/t4013/diff.diff-tree_--stat_--compact-summary_initial_mode
 create mode 100644 
t/t4013/diff.diff-tree_-R_--stat_--compact-summary_initial_mode

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9d1586b956..ff93ff74d0 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -188,6 +188,17 @@ and accumulating child directory counts in the parent 
directories:
Output a condensed summary of extended header information
such as creations, renames and mode changes.
 
+--compact-summary::
+   Output a condensed summary of extended header information in
+   front of the file name part of diffstat. This option is
+   ignored if --stat is not specified.
++
+Fle creations or deletions are denoted with "A" or "D" respectively,
+optionally "+l" if it's a symlink, or "+x" if it's executable.
+Mode changes are put in brackets, e.g. "+x" or "-x" for adding or
+removing executable bit respectively, "+l" or "-l" for becoming a
+symlink or a regular file.
+
 ifndef::git-format-patch[]
 --patch-with-stat::
Synonym for `-p --stat`.
diff --git a/diff.c b/diff.c
index fb22b19f09..3f676d 100644
--- a/diff.c
+++ b/diff.c
@@ -2131,6 +2131,7 @@ struct diffstat_t {
char *from_name;
char *name;
char *print_name;
+   const char *status_code;
unsigned is_unmerged:1;
unsigned is_binary:1;
unsigned is_renamed:1;
@@ -2271,6 +2272,7 @@ static void show_stats(struct diffstat_t *data, struct 
diff_options *options)
 {
int i, len, add, del, adds = 0, dels = 0;
uintmax_t max_change = 0, max_len = 0;
+   int max_status_len = 0;
int total_files = data->nr, count;
int width, name_width, graph_width, number_width = 0, bin_width = 0;
const char *reset, *add_c, *del_c;
@@ -2287,6 +2289,18 @@ static void show_stats(struct diffstat_t *data, struct 
diff_options *options)
add_c = diff_get_color_opt(options, DIFF_FILE_NEW);
del_c = diff_get_color_opt(options, DIFF_FILE_OLD);
 
+   for (i = 0; (i < count) && (i < data->nr); i++) {
+   const struct diffstat_file *file = data->files[i];
+   int len;
+
+   if (!file->status_code)
+   continue;
+   len = strlen(file->status_code) + 1;
+
+   if (len > max_status_len)
+   max_status_len = len;
+   }
+
/*
 * Find the longest filename and max number of changes
 */
@@ -2383,6 +2397,8 @@ static