[PATCH 0/5] Dynamic diff summary for merge/pull

[PATCH 0/5] Dynamic diff summary for merge/pull

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

On Sat, Jan 14, 2012 at 2:49 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> And what I've *really* wanted is to actually see the diff itself if
> it
> is small. So some kind of "dynamic summary": for one-liners (or
> ten-liners), show the whole diff. For medium-sized changes, show the
> whole diffstat. And for really big changes, show an outline and the
> "768 files changed, 179851 lines added, 7630 lines removed" stats.
>
> IOW, whatever fits in, say, 50 lines or less.

Here's an attempt to do that. The main patch is 4/5 to implement
--dynstat. 5/5 just makes merge/pull pass --dynstat to diff machinery.

--dynstat tries various diff output and use one that fits within 50
lines:

 --patch + --shortstat
 --stat (maybe plus --summary)
 --dirstat + --shortstat
 --shortstat

Many hardcoded constants should be more flexible (i.e. 50 lines limit
could be retrieved from terminal settigns), but that's minor for
now imo. I think we should also hint users to do
"git diff --stat --summary HEAD@{1} HEAD" too via advice framework if
we turn --dynstat to default for merge/pull

I have not run "make test" so it may break a few tests because of 2/2.

Nguyễn Thái Ngọc Duy (5):
  diff: add space around "="
  diff: do not duplicate shortstat with "git diff --stat --shortstat"
  diff: buffer dirstat output
  diff: add --dynstat
  merge: support setting merge.[diff]stat to dynstat

 builtin/merge.c |   30 +++++++++++--
 diff.c          |  127 +++++++++++++++++++++++++++++++++++++++++++++++--------
 diff.h          |    1 +
 3 files changed, 135 insertions(+), 23 deletions(-)

-- 
1.7.3.1.256.g2539c.dirty

[PATCH 1/5] diff: add space around "="

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 diff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/diff.c b/diff.c
index 374ecf3..d4223e2 100644
--- a/diff.c
+++ b/diff.c
@@ -1482,7 +1482,7 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option
 		if (!data->files[i]->is_binary &&
 		    !data->files[i]->is_unmerged) {
 			int added = data->files[i]->added;
-			int deleted= data->files[i]->deleted;
+			int deleted = data->files[i]->deleted;
 			if (!data->files[i]->is_renamed &&
 			    (added + deleted == 0)) {
 				total_files--;
-- 
1.7.3.1.256.g2539c.dirty

[PATCH 2/5] diff: do not duplicate shortstat with "git diff --stat --shortstat"

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 diff.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index d4223e2..203b054 100644
--- a/diff.c
+++ b/diff.c
@@ -1465,10 +1465,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 			fprintf(options->file, "%s ...\n", line_prefix);
 		extra_shown = 1;
 	}
-	fprintf(options->file, "%s", line_prefix);
-	fprintf(options->file,
-	       " %d files changed, %d insertions(+), %d deletions(-)\n",
-	       total_files, adds, dels);
 }
 
 static void show_shortstats(struct diffstat_t *data, struct diff_options *options)
@@ -4218,6 +4214,9 @@ void diff_flush(struct diff_options *options)
 	if (output_format & DIFF_FORMAT_DIRSTAT && DIFF_OPT_TST(options, DIRSTAT_BY_LINE))
 		dirstat_by_line = 1;
 
+	if (output_format & DIFF_FORMAT_DIFFSTAT)
+		output_format |= DIFF_FORMAT_SHORTSTAT;
+
 	if (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT) ||
 	    dirstat_by_line) {
 		struct diffstat_t diffstat;
-- 
1.7.3.1.256.g2539c.dirty

[PATCH 3/5] diff: buffer dirstat output

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

This allows us to decide whether to print it later on.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 diff.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index 203b054..8c448b5 100644
--- a/diff.c
+++ b/diff.c
@@ -1548,6 +1548,7 @@ struct dirstat_file {
 struct dirstat_dir {
 	struct dirstat_file *files;
 	int alloc, nr, permille, cumulative;
+	struct strbuf *sb;
 };
 
 static long gather_dirstat(struct diff_options *opt, struct dirstat_dir *dir,
@@ -1597,8 +1598,8 @@ static long gather_dirstat(struct diff_options *opt, struct dirstat_dir *dir,
 		if (this_dir) {
 			int permille = this_dir * 1000 / changed;
 			if (permille >= dir->permille) {
-				fprintf(opt->file, "%s%4d.%01d%% %.*s\n", line_prefix,
-					permille / 10, permille % 10, baselen, base);
+				strbuf_addf(dir->sb, "%s%4d.%01d%% %.*s\n", line_prefix,
+					    permille / 10, permille % 10, baselen, base);
 				if (!dir->cumulative)
 					return 0;
 			}
@@ -1614,18 +1615,20 @@ static int dirstat_compare(const void *_a, const void *_b)
 	return strcmp(a->name, b->name);
 }
 
-static void show_dirstat(struct diff_options *options)
+static void show_dirstat(struct diff_options *options, struct strbuf *output)
 {
 	int i;
 	unsigned long changed;
 	struct dirstat_dir dir;
 	struct diff_queue_struct *q = &diff_queued_diff;
+	struct strbuf sb = STRBUF_INIT;
 
 	dir.files = NULL;
 	dir.alloc = 0;
 	dir.nr = 0;
 	dir.permille = options->dirstat_permille;
 	dir.cumulative = DIFF_OPT_TST(options, DIRSTAT_CUMULATIVE);
+	dir.sb = output ? output : &sb;
 
 	changed = 0;
 	for (i = 0; i < q->nr; i++) {
@@ -1711,6 +1714,9 @@ found_damage:
 	/* Show all directories with more than x% of the changes */
 	qsort(dir.files, dir.nr, sizeof(dir.files[0]), dirstat_compare);
 	gather_dirstat(options, &dir, changed, "", 0);
+	if (!output)
+		fputs(sb.buf, options->file);
+	strbuf_release(&sb);
 }
 
 static void show_dirstat_by_line(struct diffstat_t *data, struct diff_options *options)
@@ -1718,6 +1724,7 @@ static void show_dirstat_by_line(struct diffstat_t *data, struct diff_options *o
 	int i;
 	unsigned long changed;
 	struct dirstat_dir dir;
+	struct strbuf sb = STRBUF_INIT;
 
 	if (data->nr == 0)
 		return;
@@ -1727,6 +1734,7 @@ static void show_dirstat_by_line(struct diffstat_t *data, struct diff_options *o
 	dir.nr = 0;
 	dir.permille = options->dirstat_permille;
 	dir.cumulative = DIFF_OPT_TST(options, DIRSTAT_CUMULATIVE);
+	dir.sb = &sb;
 
 	changed = 0;
 	for (i = 0; i < data->nr; i++) {
@@ -1755,6 +1763,8 @@ static void show_dirstat_by_line(struct diffstat_t *data, struct diff_options *o
 	/* Show all directories with more than x% of the changes */
 	qsort(dir.files, dir.nr, sizeof(dir.files[0]), dirstat_compare);
 	gather_dirstat(options, &dir, changed, "", 0);
+	fputs(sb.buf, options->file);
+	strbuf_release(&sb);
 }
 
 static void free_diffstat_info(struct diffstat_t *diffstat)
@@ -4239,7 +4249,7 @@ void diff_flush(struct diff_options *options)
 		separator++;
 	}
 	if ((output_format & DIFF_FORMAT_DIRSTAT) && !dirstat_by_line)
-		show_dirstat(options);
+		show_dirstat(options, NULL);
 
 	if (output_format & DIFF_FORMAT_SUMMARY && !is_summary_empty(q)) {
 		for (i = 0; i < q->nr; i++) {
-- 
1.7.3.1.256.g2539c.dirty

[PATCH 4/5] diff: add --dynstat

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 diff.c |  100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 diff.h |    1 +
 2 files changed, 92 insertions(+), 9 deletions(-)

diff --git a/diff.c b/diff.c
index 8c448b5..b945ad6 100644
--- a/diff.c
+++ b/diff.c
@@ -1226,6 +1226,7 @@ struct diffstat_t {
 		unsigned is_renamed:1;
 		uintmax_t added, deleted;
 	} **files;
+	struct strbuf dynstat;
 };
 
 static struct diffstat_file *diffstat_add(struct diffstat_t *diffstat,
@@ -1779,6 +1780,7 @@ static void free_diffstat_info(struct diffstat_t *diffstat)
 		free(f);
 	}
 	free(diffstat->files);
+	strbuf_release(&diffstat->dynstat);
 }
 
 struct checkdiff_t {
@@ -3332,6 +3334,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->output_format |= DIFF_FORMAT_NUMSTAT;
 	else if (!strcmp(arg, "--shortstat"))
 		options->output_format |= DIFF_FORMAT_SHORTSTAT;
+	else if (!strcmp(arg, "--dynstat"))
+		options->output_format |= DIFF_FORMAT_DYNSTAT;
 	else if (!strcmp(arg, "-X") || !strcmp(arg, "--dirstat"))
 		return parse_dirstat_opt(options, "");
 	else if (!prefixcmp(arg, "-X"))
@@ -3922,6 +3926,76 @@ static int check_pair_status(struct diff_filepair *p)
 	}
 }
 
+static int diff_flush_dynstat(struct diff_options *o, struct diffstat_t *diffstat)
+{
+	struct diff_queue_struct *q = &diff_queued_diff;
+	int i, adds = 0, dels = 0, summary_lines = 0, printed_lines = 0, total_files;
+
+	memset(diffstat, 0, sizeof(struct diffstat_t));
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+		if (check_pair_status(p))
+			diff_flush_stat(p, o, diffstat);
+	}
+
+	total_files = diffstat->nr;
+	if (!total_files)
+		return o->output_format;
+
+	/* how many lines for --stat ? */
+	for (i = 0; i < diffstat->nr; i++) {
+		struct diffstat_file *f = diffstat->files[i];
+		if (!f->is_binary && !f->is_unmerged) {
+			if (!f->is_renamed && (f->added + f->deleted == 0))
+				total_files--;
+			else {
+				adds += f->added;
+				dels += f->deleted;
+			}
+		}
+	}
+
+	/* how many lines for --summary ? */
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+		if ((p->status & (DIFF_STATUS_DELETED | DIFF_STATUS_ADDED |
+				 DIFF_STATUS_COPIED | DIFF_STATUS_RENAMED)) ||
+		    p->score)
+			summary_lines++;
+	}
+
+	if (printed_lines + total_files < 50) {
+		o->output_format |= DIFF_FORMAT_DIFFSTAT;
+		printed_lines += total_files;
+		if (printed_lines + summary_lines < 50) {
+			printed_lines += summary_lines;
+			o->output_format |= DIFF_FORMAT_SUMMARY;
+		}
+	} else {
+		const char *s;
+		int nr = 0;
+		show_dirstat(o, &diffstat->dynstat);
+		s = diffstat->dynstat.buf;
+
+		/* how many lines for --dirstat ? */
+		while (s && (s = strchr(s, '\n'))) {
+			nr++;
+			s++;
+		}
+		if (nr < 50)
+			printed_lines += nr;
+		else
+			strbuf_release(&diffstat->dynstat);
+	}
+
+	/* how many lines for --patch ? */
+	if (adds + dels < 10 && printed_lines < 10)
+		o->output_format |= DIFF_FORMAT_PATCH;
+
+	o->output_format |= DIFF_FORMAT_SHORTSTAT;
+	return o->output_format;
+}
+
 static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)
 {
 	int fmt = opt->output_format;
@@ -4201,6 +4275,7 @@ void diff_flush(struct diff_options *options)
 	int i, output_format = options->output_format;
 	int separator = 0;
 	int dirstat_by_line = 0;
+	struct diffstat_t diffstat;
 
 	/*
 	 * Order: raw, stat, summary, patch
@@ -4221,6 +4296,19 @@ void diff_flush(struct diff_options *options)
 		separator++;
 	}
 
+	memset(&diffstat, 0, sizeof(struct diffstat_t));
+	strbuf_init(&diffstat.dynstat, 0);
+	if (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT|DIFF_FORMAT_DYNSTAT)) {
+		for (i = 0; i < q->nr; i++) {
+			struct diff_filepair *p = q->queue[i];
+			if (check_pair_status(p))
+				diff_flush_stat(p, options, &diffstat);
+		}
+	}
+
+	if (output_format & DIFF_FORMAT_DYNSTAT)
+		output_format = diff_flush_dynstat(options, &diffstat);
+
 	if (output_format & DIFF_FORMAT_DIRSTAT && DIFF_OPT_TST(options, DIRSTAT_BY_LINE))
 		dirstat_by_line = 1;
 
@@ -4229,25 +4317,19 @@ void diff_flush(struct diff_options *options)
 
 	if (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT) ||
 	    dirstat_by_line) {
-		struct diffstat_t diffstat;
-
-		memset(&diffstat, 0, sizeof(struct diffstat_t));
-		for (i = 0; i < q->nr; i++) {
-			struct diff_filepair *p = q->queue[i];
-			if (check_pair_status(p))
-				diff_flush_stat(p, options, &diffstat);
-		}
 		if (output_format & DIFF_FORMAT_NUMSTAT)
 			show_numstat(&diffstat, options);
 		if (output_format & DIFF_FORMAT_DIFFSTAT)
 			show_stats(&diffstat, options);
+		if ((output_format & DIFF_FORMAT_DYNSTAT) && diffstat.dynstat.len)
+			fputs(diffstat.dynstat.buf, options->file);
 		if (output_format & DIFF_FORMAT_SHORTSTAT)
 			show_shortstats(&diffstat, options);
 		if (output_format & DIFF_FORMAT_DIRSTAT)
 			show_dirstat_by_line(&diffstat, options);
-		free_diffstat_info(&diffstat);
 		separator++;
 	}
+	free_diffstat_info(&diffstat);
 	if ((output_format & DIFF_FORMAT_DIRSTAT) && !dirstat_by_line)
 		show_dirstat(options, NULL);
 
diff --git a/diff.h b/diff.h
index ae71f4c..6bebad8 100644
--- a/diff.h
+++ b/diff.h
@@ -39,6 +39,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 #define DIFF_FORMAT_PATCH	0x0010
 #define DIFF_FORMAT_SHORTSTAT	0x0020
 #define DIFF_FORMAT_DIRSTAT	0x0040
+#define DIFF_FORMAT_DYNSTAT	0x0080
 
 /* These override all above */
 #define DIFF_FORMAT_NAME	0x0100
-- 
1.7.3.1.256.g2539c.dirty

[PATCH 5/5] merge: support setting merge.[diff]stat to dynstat

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/merge.c |   30 +++++++++++++++++++++++++-----
 1 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 3a45172..b8280ce 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -46,7 +46,13 @@ static const char * const builtin_merge_usage[] = {
 	NULL
 };
 
-static int show_diffstat = 1, shortlog_len = -1, squash;
+enum show_diffstat {
+	NO_STAT,
+	DIFF_STAT,
+	DYNAMIC_STAT
+};
+
+static int show_diffstat = NO_STAT, shortlog_len = -1, squash;
 static int option_commit = 1, allow_fast_forward = 1;
 static int fast_forward_only, option_edit;
 static int allow_trivial = 1, have_message;
@@ -399,8 +405,15 @@ static void finish(struct commit *head_commit,
 	if (new_head && show_diffstat) {
 		struct diff_options opts;
 		diff_setup(&opts);
-		opts.output_format |=
-			DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
+		switch (show_diffstat) {
+		case DIFF_STAT:
+			opts.output_format |=
+				DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
+			break;
+		case DYNAMIC_STAT:
+			opts.output_format |= DIFF_FORMAT_DYNSTAT;
+			break;
+		}
 		opts.detect_rename = DIFF_DETECT_RENAME;
 		if (diff_setup_done(&opts) < 0)
 			die(_("diff_setup_done failed"));
@@ -554,8 +567,15 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 		return 0;
 	}
 
-	if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat"))
-		show_diffstat = git_config_bool(k, v);
+	if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat")) {
+		int val = git_config_maybe_bool(k, v);
+		if (val != -1)
+			show_diffstat = val ? DIFF_STAT : NO_STAT;
+		else if (!strcasecmp(v, "dynstat"))
+			show_diffstat = DYNAMIC_STAT;
+		else
+			die(_("unrecognized value \"%s\" for %s"), v, k);
+	}
 	else if (!strcmp(k, "pull.twohead"))
 		return git_config_string(&pull_twohead, k, v);
 	else if (!strcmp(k, "pull.octopus"))
-- 
1.7.3.1.256.g2539c.dirty