Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> writes:
> Currently, --stat calculates the longest name from all items but then
> drops some (mode changes) from the output later on.
>
> Instead, drop them from the namelen generation and calculation.
>
> Signed-off-by: Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx>
> ---
> This optimizes (tightens) the display potentially, but we never had tests
> which are sensitive to that.
More importantly I think this is a good first step in the right direction
to reduce the duplicated code that need to implement the same logic.
However, you missed another instance in show_shortstats() no?
I am wondering if it would make it easier to both read and maintain if you
add a boolean field "changed" to "struct diffstat_file", and set it at the
end of builtin_diffstat(), and have these places all check that field.
A possible alternative is not to put such an unchanged entry in the struct
diffstat_t in the first place so that nobody has to worry about it. Right
now, show_numstat() does show 0/0 entries (i.e. only mode change), while
shortstat and normal stat do not, but I somehow have a feeling that this
difference is not by design but by accident. Besides, --numstat that only
says 0/0 is not useful in practice without --summary.
$ edit diff.c
$ chmod +x Makefile
$ git diff --stat
diff.c | 26 ++++++++++++++------------
$ git diff --numstat
0 0 Makefile
14 12 diff.c
$ git diff --numstat --summary
0 0 Makefile
14 12 diff.c
mode change 100644 => 100755 Makefile
Getting rid of an unchanged entry from the diffstat array upfront will
change the behaviour of numstat, but I suspect that it change things in a
better way, making the result consistent with the textual version.
The patch below, diff.c shows that approach, while ndiff.c shows the extra
boolean "changed" approach.
What do you think?
diff.c | 7 +++++++
diff.c => ndiff.c | 26 ++++++++++++++------------
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/diff.c b/diff.c
index 8f4815b..e2ee70a 100644
--- a/diff.c
+++ b/diff.c
@@ -2267,6 +2267,13 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
&xpp, &xecfg);
}
+ if (!data->is_renamed && !data->added && !data->deleted) {
+ diffstat->nr--;
+ free(data->name);
+ free(data->from_name);
+ free(data);
+ }
+
diff_free_filespec_data(one);
diff_free_filespec_data(two);
}
diff --git a/diff.c b/ndiff.c
similarity index 99%
copy from diff.c
copy to ndiff.c
index 8f4815b..1620053 100644
--- a/diff.c
+++ b/ndiff.c
@@ -1222,6 +1222,7 @@ struct diffstat_t {
unsigned is_unmerged:1;
unsigned is_binary:1;
unsigned is_renamed:1;
+ unsigned changed:1;
uintmax_t added, deleted;
} **files;
};
@@ -1350,6 +1351,9 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
for (i = 0; i < data->nr; i++) {
struct diffstat_file *file = data->files[i];
uintmax_t change = file->added + file->deleted;
+
+ if (!file->changed)
+ continue;
fill_print_name(file);
len = strlen(file->print_name);
if (max_len < len)
@@ -1381,6 +1385,11 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
uintmax_t deleted = data->files[i]->deleted;
int name_len;
+ if (!data->files[i]->changed) {
+ total_files--;
+ continue;
+ }
+
/*
* "scale" the filename
*/
@@ -1415,11 +1424,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
fprintf(options->file, " Unmerged\n");
continue;
}
- else if (!data->files[i]->is_renamed &&
- (added + deleted == 0)) {
- total_files--;
- continue;
- }
/*
* scale the add/delete
@@ -1457,15 +1461,12 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option
for (i = 0; i < data->nr; i++) {
if (!data->files[i]->is_binary &&
!data->files[i]->is_unmerged) {
- int added = data->files[i]->added;
- int deleted= data->files[i]->deleted;
- if (!data->files[i]->is_renamed &&
- (added + deleted == 0)) {
+ if (!data->files[i]->changed) {
total_files--;
- } else {
- adds += added;
- dels += deleted;
+ continue;
}
+ adds += data->files[i]->added;
+ dels += data->files[i]->deleted;
}
}
if (options->output_prefix) {
@@ -2266,6 +2267,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,
&xpp, &xecfg);
}
+ data->changed = data->is_renamed || data->added || data->deleted;
diff_free_filespec_data(one);
diff_free_filespec_data(two);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html