1 file changed, 193 insertions, 131 deletions
diff --git a/source/ui-blame.c b/source/ui-blame.c
index 132121b..80512a0 100644
--- a/source/ui-blame.c
+++ b/source/ui-blame.c
@@ -1,25 +1,42 @@
-/* ui-blame.c: functions for blame output
- *
- * Copyright (C) 2006-2017 cgit Development Team <cgit@lists.zx2c4.com>
- *
- * Licensed under GNU General Public License v2
- *   (see LICENSE.txt for full license text)
+/*
+ * The blame page, which shows a file next to the commit that last touched
+ * each of its lines. git returns blame as runs of neighbouring lines that
+ * share a commit, and the page turns every run into one block in each column
+ * so the hashes, the line numbers and the striped background all line up with
+ * the source. Only a regular file can be blamed, so a path naming a folder is
+ * turned away.
  */
 
 #define USE_THE_REPOSITORY_VARIABLE
 
 #include "cgit.h"
-#include "ui-blame.h"
+#include "filter.h"
 #include "html.h"
+#include "parsing.h"
+#include "shared.h"
+#include "ui-blame.h"
 #include "ui-shared.h"
-#include "strvec.h"
-#include "blame.h"
 
+// A tab in the rendered source runs on to the next multiple of this. The
+// stylesheet leaves tab-size alone, so the measurement has to match what the
+// browser does on its own rather than anything cgit picks.
+#define TAB_WIDTH 8
 
-/* Blame coalesces neighbouring lines from one commit, but a commit that
- * touched several separate parts of the file still comes back once per part.
- * Each of those repeats a full commit parse, so the rendered detail is kept
- * and looked up by object id. */
+enum blame_target {
+	TARGET_MISSING,
+	TARGET_FILE,
+	TARGET_FOLDER,
+};
+
+struct walk_tree_context {
+	char *rev;
+	int match_baselen;
+	enum blame_target found;
+};
+
+// A commit that touched several separate parts of the file comes back once
+// per part, and each repeat would parse the commit again, so the rendered
+// detail is cached here and looked up by object id.
 static struct string_list suspect_details = STRING_LIST_INIT_DUP;
 
 static void free_suspect_details(void)
@@ -31,8 +48,40 @@ static void free_suspect_details(void)
 	string_list_clear(&suspect_details, 0);
 }
 
-/* Write out a run of one repeated character. A single blame entry can cover
- * the whole file, so the run is handed on in batches rather than built whole. */
+/*
+ * The scoreboard keeps a pointer to revs, so the caller owns both and has to
+ * keep them alive together.
+ */
+static void run_blame(struct blame_scoreboard *sb, struct rev_info *revs,
+		      const char *path, const char *rev)
+{
+	struct strvec argv = STRVEC_INIT;
+	struct blame_origin *origin;
+
+	strvec_push(&argv, "blame");
+	strvec_push(&argv, rev);
+	repo_init_revisions(the_repository, revs, NULL);
+	revs->diffopt.flags.allow_textconv = 1;
+	setup_revisions(argv.nr, argv.v, revs, NULL);
+	init_scoreboard(sb);
+	sb->revs = revs;
+	sb->repo = the_repository;
+	sb->path = path;
+	setup_scoreboard(sb, &origin);
+	origin->suspects = blame_entry_prepend(NULL, 0, sb->num_lines, origin);
+	prio_queue_put(&sb->commits, origin->commit);
+	blame_origin_decref(origin);
+	sb->ent = NULL;
+	sb->path = path;
+	assign_blame(sb, 0);
+	blame_sort_final(sb);
+	blame_coalesce(sb);
+}
+
+/*
+ * A single blame entry can cover the whole file, so the run is handed on in
+ * batches rather than built whole.
+ */
 static void emit_chars(char ch, unsigned long count)
 {
 	struct strbuf run = STRBUF_INIT;
@@ -48,7 +97,10 @@ static void emit_chars(char ch, unsigned long count)
 	strbuf_release(&run);
 }
 
-static char *emit_suspect_detail(struct blame_origin *suspect)
+/*
+ * The returned string belongs to suspect_details, not the caller.
+ */
+static char *suspect_detail(struct blame_origin *suspect)
 {
 	struct commitinfo *info;
 	struct strbuf detail = STRBUF_INIT;
@@ -87,18 +139,21 @@ static char *emit_suspect_detail(struct blame_origin *suspect)
 	return cached->util;
 }
 
-static void emit_blame_entry_hash(struct blame_entry *ent)
+static void emit_entry_hash(struct blame_entry *ent)
 {
 	struct blame_origin *suspect = ent->suspect;
 	struct object_id *oid = &suspect->commit->object.oid;
+	const char *detail = suspect_detail(suspect);
 
-	const char *detail = emit_suspect_detail(suspect);
 	html("<span class='oid'>");
-	cgit_commit_link(repo_find_unique_abbrev(the_repository, oid, DEFAULT_ABBREV), detail,
-			 NULL, ctx.qry.head, oid_to_hex(oid), suspect->path);
+	cgit_commit_link(repo_find_unique_abbrev(the_repository, oid,
+						 DEFAULT_ABBREV),
+			 detail, NULL, ctx.qry.head, oid_to_hex(oid),
+			 suspect->path);
 	html("</span>");
 
-	if (!repo_parse_commit(the_repository, suspect->commit) && suspect->commit->parents) {
+	if (!repo_parse_commit(the_repository, suspect->commit) &&
+	    suspect->commit->parents) {
 		struct commit *parent = suspect->commit->parents->item;
 
 		html(" ");
@@ -107,17 +162,30 @@ static void emit_blame_entry_hash(struct blame_entry *ent)
 				suspect->path);
 	}
 
-	// Batched rather than one write per line of the entry.
+	// The stripes only line up across the columns if each column gives an
+	// entry the same height, so pad out to the lines the entry covers.
 	emit_chars('\n', ent->num_lines);
 }
 
-static void emit_blame_entry_linenumber(struct blame_entry *ent)
+static void emit_hashes(struct blame_scoreboard *sb)
+{
+	struct blame_entry *ent;
+
+	html("<td class='hashes'>");
+	for (ent = sb->ent; ent; ent = ent->next) {
+		html("<div class='alt'><pre>");
+		emit_entry_hash(ent);
+		html("</pre></div>");
+	}
+	html("</td>\n");
+}
+
+static void emit_entry_linenumbers(struct blame_entry *ent)
 {
 	const char *numberfmt = "<a id='n%1$d' href='#n%1$d'>%1$d</a>\n";
 	struct strbuf numbers = STRBUF_INIT;
 	int lineno = ent->lno;
 
-	// Batched rather than a formatted write per line of the entry.
 	while (lineno < ent->lno + ent->num_lines) {
 		strbuf_addf(&numbers, numberfmt, ++lineno);
 		if (numbers.len >= HTML_BATCH) {
@@ -129,50 +197,84 @@ static void emit_blame_entry_linenumber(struct blame_entry *ent)
 	strbuf_release(&numbers);
 }
 
-static void emit_blame_entry_line_background(struct blame_scoreboard *sb,
-					     struct blame_entry *ent)
+static void emit_linenumbers(struct blame_scoreboard *sb)
 {
+	struct blame_entry *ent;
+
+	html("<td class='linenumbers'>");
+	for (ent = sb->ent; ent; ent = ent->next) {
+		html("<div class='alt'><pre>");
+		emit_entry_linenumbers(ent);
+		html("</pre></div>");
+	}
+	html("</td>\n");
+}
+
+static size_t line_width(struct blame_scoreboard *sb, int line)
+{
+	const char *pos = blame_nth_line(sb, line);
+	const char *end = blame_nth_line(sb, line + 1);
+	size_t width = 0;
+
+	while (pos < end) {
+		width++;
+		if (*pos++ == '\t')
+			width = (width + TAB_WIDTH - 1) & ~(TAB_WIDTH - 1);
+	}
+	return width;
+}
+
+/*
+ * The stylesheet takes the source pre out of flow and positions it over these
+ * blocks, so nothing else gives the cell a size and each block has to be
+ * padded to the height and the width of the lines it stands behind.
+ */
+static void emit_entry_background(struct blame_scoreboard *sb,
+				  struct blame_entry *ent)
+{
+	size_t widest = 2;
 	int line;
-	size_t len, maxlen = 2;
-	const char* pos, *endpos;
 
 	for (line = ent->lno; line < ent->lno + ent->num_lines; line++) {
-		pos = blame_nth_line(sb, line);
-		endpos = blame_nth_line(sb, line + 1);
-		len = 0;
-		while (pos < endpos) {
-			len++;
-			if (*pos++ == '\t')
-				len = (len + 7) & ~7;
-		}
-		if (len > maxlen)
-			maxlen = len;
+		size_t width = line_width(sb, line);
+
+		if (width > widest)
+			widest = width;
 	}
 
-	// The widest line decides the padding, so the entry has to be measured
-	// before any of it is written. The newlines do not depend on that
-	// measurement, so both runs go out batched once it is known.
 	emit_chars('\n', ent->num_lines);
-	emit_chars(' ', maxlen - 1);
+	emit_chars(' ', widest - 1);
 }
 
-struct walk_tree_context {
-	char *curr_rev;
-	int match_baselen;
-	int state;
-};
+/*
+ * Frees each entry on the way past, so this has to be the last pass over the
+ * scoreboard's entries.
+ */
+static void emit_line_backgrounds(struct blame_scoreboard *sb)
+{
+	struct blame_entry *ent = sb->ent;
 
-static void print_object(const struct object_id *oid, const char *path,
-			 const char *basename, const char *rev)
+	html("<div>");
+	while (ent) {
+		struct blame_entry *next = ent->next;
+
+		html("<div class='alt'><pre>");
+		emit_entry_background(sb, ent);
+		html("</pre></div>");
+		free(ent);
+		ent = next;
+	}
+	html("</div>");
+}
+
+static void print_blame_page(const struct object_id *oid, const char *path,
+			     const char *filename, const char *rev)
 {
 	enum object_type type;
 	char *buf;
 	unsigned long size;
-	struct strvec rev_argv = STRVEC_INIT;
 	struct rev_info revs;
 	struct blame_scoreboard sb;
-	struct blame_origin *o;
-	struct blame_entry *ent = NULL;
 
 	type = odb_read_object_info(the_repository->objects, oid, &size);
 	if (type == OBJ_BAD) {
@@ -194,24 +296,7 @@ static void print_object(const struct object_id *oid, const char *path,
 		return;
 	}
 
-	strvec_push(&rev_argv, "blame");
-	strvec_push(&rev_argv, rev);
-	repo_init_revisions(the_repository, &revs, NULL);
-	revs.diffopt.flags.allow_textconv = 1;
-	setup_revisions(rev_argv.nr, rev_argv.v, &revs, NULL);
-	init_scoreboard(&sb);
-	sb.revs = &revs;
-	sb.repo = the_repository;
-	sb.path = path;
-	setup_scoreboard(&sb, &o);
-	o->suspects = blame_entry_prepend(NULL, 0, sb.num_lines, o);
-	prio_queue_put(&sb.commits, o->commit);
-	blame_origin_decref(o);
-	sb.ent = NULL;
-	sb.path = path;
-	assign_blame(&sb, 0);
-	blame_sort_final(&sb);
-	blame_coalesce(&sb);
+	run_blame(&sb, &revs, path, rev);
 
 	cgit_set_title_from_path(path);
 
@@ -228,46 +313,19 @@ static void print_object(const struct object_id *oid, const char *path,
 	}
 	html("<table class='blame blob'>\n<tr>\n");
 
-	/* Commit hashes */
-	html("<td class='hashes'>");
-	for (ent = sb.ent; ent; ent = ent->next) {
-		html("<div class='alt'><pre>");
-		emit_blame_entry_hash(ent);
-		html("</pre></div>");
-	}
-	html("</td>\n");
+	emit_hashes(&sb);
 
-	/* Line numbers */
-	if (ctx.cfg.enable_tree_linenumbers) {
-		html("<td class='linenumbers'>");
-		for (ent = sb.ent; ent; ent = ent->next) {
-			html("<div class='alt'><pre>");
-			emit_blame_entry_linenumber(ent);
-			html("</pre></div>");
-		}
-		html("</td>\n");
-	}
+	if (ctx.cfg.enable_tree_linenumbers)
+		emit_linenumbers(&sb);
 
 	html("<td class='lines'><div>");
-
-	/* Colored bars behind lines */
-	html("<div>");
-	for (ent = sb.ent; ent; ) {
-		struct blame_entry *e = ent->next;
-		html("<div class='alt'><pre>");
-		emit_blame_entry_line_background(&sb, ent);
-		html("</pre></div>");
-		free(ent);
-		ent = e;
-	}
-	html("</div>");
+	emit_line_backgrounds(&sb);
 
 	free((void *)sb.final_buf);
 
-	/* Lines */
 	html("<pre><code>");
 	if (ctx.repo->source_filter) {
-		char *filter_arg = xstrdup(basename);
+		char *filter_arg = xstrdup(filename);
 		cgit_open_filter(ctx.repo->source_filter, filter_arg);
 		html_raw(buf, size);
 		cgit_close_filter(ctx.repo->source_filter);
@@ -282,35 +340,34 @@ static void print_object(const struct object_id *oid, const char *path,
 	html("</tr>\n</table>\n");
 
 cleanup:
-	/* The binary and oversized branches jump here with the layout still
-	 * open, so close it on every path rather than only the normal one. */
 	cgit_print_layout_end();
 	free_suspect_details();
 	free(buf);
 }
 
 static int walk_tree(const struct object_id *oid, struct strbuf *base,
-		     const char *pathname, unsigned mode, void *cbdata)
+		     const char *pathname, unsigned mode, void *data)
 {
-	struct walk_tree_context *walk_tree_ctx = cbdata;
+	struct walk_tree_context *walk = data;
 
 	// match_baselen is -1 when no path was given, which no length equals.
-	if (walk_tree_ctx->match_baselen >= 0 &&
-	    base->len == (size_t)walk_tree_ctx->match_baselen) {
+	if (walk->match_baselen >= 0 &&
+	    base->len == (size_t)walk->match_baselen) {
 		if (S_ISREG(mode)) {
-			struct strbuf buffer = STRBUF_INIT;
-			strbuf_addbuf(&buffer, base);
-			strbuf_addstr(&buffer, pathname);
-			print_object(oid, buffer.buf, pathname,
-				     walk_tree_ctx->curr_rev);
-			strbuf_release(&buffer);
-			walk_tree_ctx->state = 1;
+			struct strbuf fullpath = STRBUF_INIT;
+
+			strbuf_addbuf(&fullpath, base);
+			strbuf_addstr(&fullpath, pathname);
+			print_blame_page(oid, fullpath.buf, pathname,
+					 walk->rev);
+			strbuf_release(&fullpath);
+			walk->found = TARGET_FILE;
 		} else if (S_ISDIR(mode)) {
-			walk_tree_ctx->state = 2;
+			walk->found = TARGET_FOLDER;
 		}
 	} else if (base->len < INT_MAX
-			&& (int)base->len > walk_tree_ctx->match_baselen) {
-		walk_tree_ctx->state = 2;
+			&& (int)base->len > walk->match_baselen) {
+		walk->found = TARGET_FOLDER;
 	} else if (S_ISDIR(mode)) {
 		return READ_TREE_RECURSIVE;
 	}
@@ -319,9 +376,10 @@ static int walk_tree(const struct object_id *oid, struct strbuf *base,
 
 static int basedir_len(const char *path)
 {
-	const char *p = strrchr(path, '/');
-	if (p)
-		return p - path + 1;
+	const char *slash = strrchr(path, '/');
+
+	if (slash)
+		return slash - path + 1;
 	return 0;
 }
 
@@ -330,16 +388,20 @@ void cgit_print_blame(void)
 	const char *rev = ctx.qry.oid;
 	struct object_id oid;
 	struct commit *commit;
+	int path_len = ctx.qry.path ? strlen(ctx.qry.path) : 0;
+	// nowildcard_len matches len so git treats the path as literal rather
+	// than as a glob, which is what a request naming one file means.
 	struct pathspec_item path_items = {
 		.match = ctx.qry.path,
-		.len = ctx.qry.path ? strlen(ctx.qry.path) : 0
+		.len = path_len,
+		.nowildcard_len = path_len
 	};
 	struct pathspec paths = {
 		.nr = 1,
 		.items = &path_items
 	};
-	struct walk_tree_context walk_tree_ctx = {
-		.state = 0
+	struct walk_tree_context walk = {
+		.found = TARGET_MISSING
 	};
 
 	if (!rev)
@@ -357,17 +419,17 @@ void cgit_print_blame(void)
 		return;
 	}
 
-	walk_tree_ctx.curr_rev = xstrdup(rev);
-	walk_tree_ctx.match_baselen = (path_items.match) ?
-				       basedir_len(path_items.match) : -1;
+	walk.rev = xstrdup(rev);
+	walk.match_baselen = path_items.match ?
+			     basedir_len(path_items.match) : -1;
 
 	read_tree(the_repository, repo_get_commit_tree(the_repository, commit),
-		  &paths, walk_tree, &walk_tree_ctx);
-	if (!walk_tree_ctx.state)
+		  &paths, walk_tree, &walk);
+	if (walk.found == TARGET_MISSING)
 		cgit_print_error_page(404, "Not found", "Not found");
-	else if (walk_tree_ctx.state == 2)
+	else if (walk.found == TARGET_FOLDER)
 		cgit_print_error_page(404, "No blame for folders",
 			"Blame is not available for folders.");
 
-	free(walk_tree_ctx.curr_rev);
+	free(walk.rev);
 }