[PATCH v1 1/3] blame: add --aggregate option

[PATCH v1 1/3] blame: add --aggregate option

[Edmundo Carmona Antoranz]

To avoid taking up so much space on normal output printing duplicate
information on consecutive lines that "belong" to the same revision,
this option allows to print a single line with the information about
the revision before the lines of content themselves.

Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
---
 Documentation/blame-options.txt |  4 ++
 Documentation/git-blame.txt     |  4 +-
 builtin/blame.c                 | 85 +++++++++++++++++++++++++++--------------
 3 files changed, 63 insertions(+), 30 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 2669b87c9..be2a327ff 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -49,6 +49,10 @@ include::line-range-format.txt[]
 	Show the result incrementally in a format designed for
 	machine consumption.
 
+--aggregate::
+	Aggregate information about revisions. This option makes no
+	difference on incremental or porcelain format.
+
 --encoding=<encoding>::
 	Specifies the encoding used to output author names
 	and commit summaries. Setting it to `none` makes blame
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index fdc3aea30..385eaf7be 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,8 +10,8 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]
 	    [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
-	    [--progress] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>..<rev>]
-	    [--] <file>
+	    [--progress] [--abbrev=<n>] [--aggregate] 
+	    [<rev> | --contents <file> | --reverse <rev>..<rev>] [--] <file>
 
 DESCRIPTION
 -----------
diff --git a/builtin/blame.c b/builtin/blame.c
index 126b8c9e5..09b3b0c8a 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1884,6 +1884,7 @@ static const char *format_time(unsigned long time, const char *tz_str,
 #define OUTPUT_NO_AUTHOR       0200
 #define OUTPUT_SHOW_EMAIL	0400
 #define OUTPUT_LINE_PORCELAIN 01000
+#define OUTPUT_AGGREGATE      02000
 
 static void emit_porcelain_details(struct origin *suspect, int repeat)
 {
@@ -1931,43 +1932,41 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
 		putchar('\n');
 }
 
-static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
-{
-	int cnt;
-	const char *cp;
-	struct origin *suspect = ent->suspect;
-	struct commit_info ci;
-	char hex[GIT_SHA1_HEXSZ + 1];
-	int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
-
-	get_commit_info(suspect->commit, &ci, 1);
-	sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
-
-	cp = nth_line(sb, ent->lno);
-	for (cnt = 0; cnt < ent->num_lines; cnt++) {
-		char ch;
-		int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
-
-		if (suspect->commit->object.flags & UNINTERESTING) {
+/**
+ * Print information about the revision.
+ * This information can be used in either aggregated output
+ * or prepending each line of the content of the file being blamed
+ * 
+ * if line_number == 0, it's an aggregate line
+ */
+static void print_revision_info(char* revision_hex, int revision_length, struct blame_entry* ent,
+		struct commit_info ci, int opt, int show_raw_time, int line_number)
+{
+	if (!line_number)
+		printf("\t");
+	int length = revision_length;
+	if (!line_number || !(opt & OUTPUT_AGGREGATE)) {
+		if (ent->suspect->commit->object.flags & UNINTERESTING) {
 			if (blank_boundary)
-				memset(hex, ' ', length);
+				memset(revision_hex, ' ', length);
 			else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {
 				length--;
 				putchar('^');
 			}
 		}
 
-		printf("%.*s", length, hex);
+		printf("%.*s", length, revision_hex);
 		if (opt & OUTPUT_ANNOTATE_COMPAT) {
 			const char *name;
 			if (opt & OUTPUT_SHOW_EMAIL)
 				name = ci.author_mail.buf;
 			else
 				name = ci.author.buf;
-			printf("\t(%10s\t%10s\t%d)", name,
+			printf("\t(%10s\t%10s", name,
 			       format_time(ci.author_time, ci.author_tz.buf,
-					   show_raw_time),
-			       ent->lno + 1 + cnt);
+					   show_raw_time));
+			if (line_number)
+				printf("\t%d)", line_number);
 		} else {
 			if (opt & OUTPUT_SHOW_SCORE)
 				printf(" %*d %02d",
@@ -1975,10 +1974,10 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 				       ent->suspect->refcnt);
 			if (opt & OUTPUT_SHOW_NAME)
 				printf(" %-*.*s", longest_file, longest_file,
-				       suspect->path);
-			if (opt & OUTPUT_SHOW_NUMBER)
+				       ent->suspect->path);
+			if ((opt & OUTPUT_SHOW_NUMBER) && line_number)
 				printf(" %*d", max_orig_digits,
-				       ent->s_lno + 1 + cnt);
+				       line_number);
 
 			if (!(opt & OUTPUT_NO_AUTHOR)) {
 				const char *name;
@@ -1994,9 +1993,38 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 						   ci.author_tz.buf,
 						   show_raw_time));
 			}
-			printf(" %*d) ",
-			       max_digits, ent->lno + 1 + cnt);
+			if (line_number)
+				printf(" %*d) ",
+					   max_digits, line_number);
 		}
+	}
+	if (line_number && (opt & OUTPUT_AGGREGATE))
+		printf(opt & OUTPUT_ANNOTATE_COMPAT ? "%*d)" : " %*d) ",
+			   max_digits, line_number);
+	if (!line_number)
+		printf(")\n");
+}
+
+static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
+{
+	int cnt;
+	const char *cp;
+	struct origin *suspect = ent->suspect;
+	struct commit_info ci;
+	char hex[GIT_SHA1_HEXSZ + 1];
+	int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
+	int revision_length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
+
+	get_commit_info(suspect->commit, &ci, 1);
+	sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+
+	if (opt & OUTPUT_AGGREGATE)
+		print_revision_info(hex, revision_length, ent, ci, opt, show_raw_time, 0);
+
+	cp = nth_line(sb, ent->lno);
+	char ch;
+	for (cnt = 0; cnt < ent->num_lines; cnt++) {
+		print_revision_info(hex, revision_length, ent, ci, opt, show_raw_time, ent->lno + 1 + cnt);
 		do {
 			ch = *cp++;
 			putchar(ch);
@@ -2609,6 +2637,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		{ OPTION_CALLBACK, 'C', NULL, &opt, N_("score"), N_("Find line copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback },
 		{ OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
 		OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
+		OPT_BIT(0, "aggregate", &output_option, N_("Aggregate output"), OUTPUT_AGGREGATE),
 		OPT__ABBREV(&abbrev),
 		OPT_END()
 	};
-- 
2.11.0.rc1

[PATCH v1 2/3] blame: add --hint option

[Edmundo Carmona Antoranz]

When printing aggregate information about revisions, this option
allows to add the 1-line summary of the revision to provide the
reader with some additional context about the revision itself.

Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
---
 Documentation/blame-options.txt |  4 ++++
 Documentation/git-blame.txt     |  2 +-
 builtin/blame.c                 | 13 +++++++++++--
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index be2a327ff..64858a631 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -53,6 +53,10 @@ include::line-range-format.txt[]
 	Aggregate information about revisions. This option makes no
 	difference on incremental or porcelain format.
 
+--hint::
+	Show revision hints on aggregated information about the revision.
+	Implies --aggregate.
+
 --encoding=<encoding>::
 	Specifies the encoding used to output author names
 	and commit summaries. Setting it to `none` makes blame
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 385eaf7be..ed8b119fa 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]
 	    [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
-	    [--progress] [--abbrev=<n>] [--aggregate] 
+	    [--progress] [--abbrev=<n>] [--aggregate] [--hint]
 	    [<rev> | --contents <file> | --reverse <rev>..<rev>] [--] <file>
 
 DESCRIPTION
diff --git a/builtin/blame.c b/builtin/blame.c
index 09b3b0c8a..7473b50e9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1885,6 +1885,7 @@ static const char *format_time(unsigned long time, const char *tz_str,
 #define OUTPUT_SHOW_EMAIL	0400
 #define OUTPUT_LINE_PORCELAIN 01000
 #define OUTPUT_AGGREGATE      02000
+#define OUTPUT_HINT           04000
 
 static void emit_porcelain_details(struct origin *suspect, int repeat)
 {
@@ -2001,8 +2002,12 @@ static void print_revision_info(char* revision_hex, int revision_length, struct
 	if (line_number && (opt & OUTPUT_AGGREGATE))
 		printf(opt & OUTPUT_ANNOTATE_COMPAT ? "%*d)" : " %*d) ",
 			   max_digits, line_number);
-	if (!line_number)
-		printf(")\n");
+	if (!line_number) {
+		printf(")");
+		if (opt & OUTPUT_HINT)
+			printf(" %s", ci.summary.buf);
+		printf("\n");
+	}
 }
 
 static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
@@ -2018,6 +2023,9 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 	get_commit_info(suspect->commit, &ci, 1);
 	sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
 
+	if (~(opt & OUTPUT_AGGREGATE) & (opt & OUTPUT_HINT))
+		opt=opt|OUTPUT_AGGREGATE;
+
 	if (opt & OUTPUT_AGGREGATE)
 		print_revision_info(hex, revision_length, ent, ci, opt, show_raw_time, 0);
 
@@ -2638,6 +2646,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		{ OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
 		OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
 		OPT_BIT(0, "aggregate", &output_option, N_("Aggregate output"), OUTPUT_AGGREGATE),
+		OPT_BIT(0, "hint", &output_option, N_("Show revision hints"), OUTPUT_HINT),
 		OPT__ABBREV(&abbrev),
 		OPT_END()
 	};
-- 
2.11.0.rc1

[PATCH v1 3/3] blame: add --color option

[Edmundo Carmona Antoranz]

Revision information will be highlighted so that it's easier
to tell from content of the file being "blamed".

Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
---
 Documentation/blame-options.txt |  5 +++++
 Documentation/git-blame.txt     |  2 +-
 builtin/blame.c                 | 13 +++++++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 64858a631..4abb4eb7e 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -57,6 +57,11 @@ include::line-range-format.txt[]
 	Show revision hints on aggregated information about the revision.
 	Implies --aggregate.
 
+--[no-]color::
+	Revision information will be highlighted on normal output
+	when running git from a terminal. This option allows
+	for color to be forcibly enabled or disabled.
+
 --encoding=<encoding>::
 	Specifies the encoding used to output author names
 	and commit summaries. Setting it to `none` makes blame
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index ed8b119fa..d25cbc5ef 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]
 	    [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
-	    [--progress] [--abbrev=<n>] [--aggregate] [--hint]
+	    [--progress] [--abbrev=<n>] [--aggregate] [--hint] [--color]
 	    [<rev> | --contents <file> | --reverse <rev>..<rev>] [--] <file>
 
 DESCRIPTION
diff --git a/builtin/blame.c b/builtin/blame.c
index 7473b50e9..c661dd538 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1886,6 +1886,7 @@ static const char *format_time(unsigned long time, const char *tz_str,
 #define OUTPUT_LINE_PORCELAIN 01000
 #define OUTPUT_AGGREGATE      02000
 #define OUTPUT_HINT           04000
+#define OUTPUT_COLOR         010000
 
 static void emit_porcelain_details(struct origin *suspect, int repeat)
 {
@@ -1943,6 +1944,8 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
 static void print_revision_info(char* revision_hex, int revision_length, struct blame_entry* ent,
 		struct commit_info ci, int opt, int show_raw_time, int line_number)
 {
+	if (opt & OUTPUT_COLOR)
+		printf("%s", GIT_COLOR_BOLD);
 	if (!line_number)
 		printf("\t");
 	int length = revision_length;
@@ -2008,6 +2011,8 @@ static void print_revision_info(char* revision_hex, int revision_length, struct
 			printf(" %s", ci.summary.buf);
 		printf("\n");
 	}
+	if (opt & OUTPUT_COLOR)
+		printf("%s", GIT_COLOR_RESET);
 }
 
 static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
@@ -2608,6 +2613,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	char *final_commit_name = NULL;
 	enum object_type type;
 	struct commit *final_commit = NULL;
+	int show_color;
 
 	struct string_list range_list = STRING_LIST_INIT_NODUP;
 	int output_option = 0, opt = 0;
@@ -2647,6 +2653,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
 		OPT_BIT(0, "aggregate", &output_option, N_("Aggregate output"), OUTPUT_AGGREGATE),
 		OPT_BIT(0, "hint", &output_option, N_("Show revision hints"), OUTPUT_HINT),
+		OPT_BOOL(0, "color", &show_color, N_("Show colors on revision information")),
 		OPT__ABBREV(&abbrev),
 		OPT_END()
 	};
@@ -2666,6 +2673,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	save_commit_buffer = 0;
 	dashdash_pos = 0;
 	show_progress = -1;
+	show_color = -1;
 
 	parse_options_start(&ctx, argc, argv, prefix, options,
 			    PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0);
@@ -2698,6 +2706,11 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	} else if (show_progress < 0)
 		show_progress = isatty(2);
 
+	if (show_color < 0)
+		show_color = isatty(1);
+	if (show_color)
+		output_option = output_option | OUTPUT_COLOR;
+
 	if (0 < abbrev && abbrev < GIT_SHA1_HEXSZ)
 		/* one more abbrev length is needed for the boundary commit */
 		abbrev++;
-- 
2.11.0.rc1

Re: [PATCH v1 1/3] blame: add --aggregate option

[Jeff King]

On Tue, Jan 24, 2017 at 11:27:32PM -0600, Edmundo Carmona Antoranz wrote:

> To avoid taking up so much space on normal output printing duplicate
> information on consecutive lines that "belong" to the same revision,
> this option allows to print a single line with the information about
> the revision before the lines of content themselves.

I admit I have not followed the preceding discussion closely, so ignore
me if my suggestion is way off base.

But it really seems like the various outputs you are looking for are
really all about customizing git-blame's human-readable output.

Would it be more productive to move towards a "--format" option that
shows custom items for each line? It could build on the custom formats
in the pretty-print code (though I think you would want to add some
custom ones, like filename, line number, line content, etc).

I know that only does half of what you want, which is making some output
not just per-line, but per-block. But that can come easily on top, if we
allow separate per-line and per-block formats (which would default to
the current output and the empty string, respectively).

Then you could do something like:

  git blame --format-block='%h %an <%ae>%n  %s' \
            --format-line='\t%(filename):%(linenumber) %(linecontent)'

and get something like:

    1234abcd A U Thor <author@example.com>
      initial commit
            foo.c:1 #include <foo.h>
            foo.c:2 #include <bar.h>
    5678abcd A U Thor <author@example.com>
      add third include
            foo.c:3 #include <third.h>

and so on. But people can mix and match what they want to see on each
line, and what they'd rather push to other lines.

I don't have a huge interest in the feature myself. I switched to "tig
blame" years ago and never looked back. But it just seems like your
options touch no this kind of flexibility, but will limit somebody as
soon as they want to switch around which information goes where.

-Peff