Restyle the sources and fix the audit's findings

1 file changed, 313 insertions, 314 deletions

diff --git a/source/ui-diff.c b/source/ui-diff.c
index a0790f7..09e2790 100644
--- a/source/ui-diff.c
+++ b/source/ui-diff.c
@@ -1,19 +1,34 @@
-/* ui-diff.c: show diff between two blobs
- *
- * Copyright (C) 2006-2014 cgit Development Team <cgit@lists.zx2c4.com>
- *
- * Licensed under GNU General Public License v2
- *   (see LICENSE.txt for full license text)
+/*
+ * The diff page, which shows what one revision changed against another as a
+ * summary table of the files it touched followed by the body of each change,
+ * either unified or side by side. Both halves come out of a single walk over
+ * the tree, because the summary has to be printed first while the bodies are
+ * only cheap to produce while each file is already open. A commit or a file
+ * whose diff would be too large stops at a link to a narrower view, and the
+ * raw form of the same request is handed to git and served as a plain patch.
  */
 
 #define USE_THE_REPOSITORY_VARIABLE
 
 #include "cgit.h"
-#include "ui-diff.h"
 #include "html.h"
+#include "shared.h"
+#include "ui-diff.h"
 #include "ui-shared.h"
 #include "ui-ssdiff.h"
 
+// A file's body is collected while the walk still has that file open, because
+// the stat table above it has to be printed first and rendering the bodies
+// afterwards meant a second walk with its own rename detection and its own
+// xdiff of every file. This bounds what one request may hold that way, so it
+// is not something to configure, and once it is passed the collected bodies
+// are dropped and that second walk happens after all.
+#define BODY_BUDGET (8 * 1024 * 1024)
+
+// What a context of zero means once the diff runs, mirroring the fallback in
+// cgit_diff_files, so the control offers the value the diff will really use.
+#define DEFAULT_CONTEXT_LINES 3
+
 struct fileinfo {
 	char status;
 	struct object_id old_oid[1];
@@ -30,60 +45,32 @@ struct fileinfo {
 	struct strbuf body;
 };
 
-/*
- * The diffstat has to be printed before the file bodies, but both are produced
- * by the same walk over the tree. So each body is rendered into the item it
- * belongs to while that file is already open, and replayed once the stat table
- * above it has been written. Rendering separately meant a second walk with its
- * own rename detection and its own xdiff of every file.
- *
- * A body stops being collected past max-diff-lines, since a file over that is
- * replaced by a link, and collection stops altogether past the budget below,
- * after which the bodies are produced the old way. The budget is a bound on
- * one request, not something to configure.
- *
- * Only a view that caps its bodies collects them. Without max-diff-lines to
- * stop it, a single body grows with the file it came from, which for a
- * side-by-side diff is several times the blob itself, and the budget below
- * cannot help because it is only reached once a body is already complete.
- */
-#define CGIT_DIFF_BODY_BUDGET (8 * 1024 * 1024)
-
-/* The revisions being compared, read by ui-ssdiff when it builds line links. */
 struct object_id old_rev_oid[1];
 struct object_id new_rev_oid[1];
 
-/* One entry per file in the diff, filled by the walk and replayed afterwards. */
 static struct fileinfo *items;
 static int files, slots;
 
-/* Totals for the diffstat, accumulated across that same walk. */
 static int total_adds, total_rems, max_changes;
 static int lines_added, lines_removed;
 
-/* How much body text has been collected, and whether collecting is still
- * worthwhile. Once the budget is passed the bodies are produced the old way. */
 static size_t body_bytes;
 static int bodies_usable;
 
-/* Which renderer the current file's lines go to, and whether the file has
- * already passed max-diff-lines and so stopped being rendered. */
 static linediff_fn render_line_fn;
 static int render_suppressed;
 
-/* The file being rendered right now, and the path the view is held to. */
 static struct diff_filepair *current_filepair;
 static const char *current_prefix;
 
 static int use_ssdiff;
 
-/* Caps apply only to whole-commit views. A single-file diff page must
- * always render fully, since it is where the capped views link to. */
+// Caps apply only to a whole commit view, since a single file diff is where
+// the capped views send the reader.
 static int cap_diffs;
+
 static int item_idx;
 
-/* The bodies have either been replayed or been given up on, so in both cases
- * what they hold is finished with. */
 static void release_bodies(void)
 {
 	int i;
@@ -92,18 +79,9 @@ static void release_bodies(void)
 		strbuf_release(&items[i].body);
 }
 
-struct diff_filespec *cgit_get_current_old_file(void)
-{
-	return current_filepair->one;
-}
-
-struct diff_filespec *cgit_get_current_new_file(void)
-{
-	return current_filepair->two;
-}
-
 static void print_fileinfo(struct fileinfo *info)
 {
+	int scale = max_changes ? max_changes : 1;
 	const char *class;
 
 	switch (info->status) {
@@ -167,18 +145,22 @@ static void print_fileinfo(struct fileinfo *info)
 	}
 	htmlf("%d", info->added + info->removed);
 	html("</td><td class='graph'>");
-	htmlf("<table summary='file diffstat' width='%d%%'><tr>", (max_changes > 100 ? 100 : max_changes));
+	htmlf("<table summary='file diffstat' width='%d%%'><tr>",
+	      max_changes > 100 ? 100 : max_changes);
 	htmlf("<td class='add' style='width: %.1f%%;'/>",
-	      info->added * 100.0 / (max_changes ? max_changes : 1));
+	      info->added * 100.0 / scale);
 	htmlf("<td class='rem' style='width: %.1f%%;'/>",
-	      info->removed * 100.0 / (max_changes ? max_changes : 1));
+	      info->removed * 100.0 / scale);
 	htmlf("<td class='none' style='width: %.1f%%;'/>",
-	      (max_changes - info->removed - info->added) * 100.0 / (max_changes ? max_changes : 1));
+	      (max_changes - info->removed - info->added) * 100.0 / scale);
 	html("</tr></table></td></tr>\n");
 }
 
-/* Counts every line, and renders it too until the file passes max-diff-lines,
- * at which point the body is going to be replaced by a link anyway. */
+/*
+ * Counting is only half of what this does. It also renders each line, until
+ * max-diff-lines is passed and the rest of the file is dropped, which is why
+ * the two cannot be separated into a counting pass and a rendering one.
+ */
 static void count_diff_lines(char *line, int len)
 {
 	if (line && (len > 0)) {
@@ -199,32 +181,169 @@ static void count_diff_lines(char *line, int len)
 
 static int show_filepair(struct diff_filepair *pair)
 {
-	/* Always show if we have no limiting prefix. */
 	if (!current_prefix)
 		return 1;
+	return starts_with(pair->one->path, current_prefix) ||
+	       starts_with(pair->two->path, current_prefix);
+}
 
-	/* Show if either path in the pair begins with the prefix. */
-	if (starts_with(pair->one->path, current_prefix) ||
-	    starts_with(pair->two->path, current_prefix))
-		return 1;
+/*
+ * The line xdiff produces ends in its newline, which is swapped for a
+ * terminator while the text is escaped and then put back, because the line
+ * points into a buffer xdiff still owns.
+ */
+static void print_line(char *line, int len)
+{
+	const char *class = "ctx";
+	char c = line[len - 1];
+
+	if (line[0] == '+')
+		class = "add";
+	else if (line[0] == '-')
+		class = "del";
+	else if (line[0] == '@')
+		class = "hunk";
 
-	/* Otherwise we don't want to show this filepair. */
-	return 0;
+	htmlf("<div class='%s'>", class);
+	line[len - 1] = '\0';
+	html_txt(line);
+	html("</div>");
+	line[len - 1] = c;
 }
 
-static void print_line(char *line, int len);
-static void header(const struct object_id *oid1, char *path1, int mode1,
-		   const struct object_id *oid2, char *path2, int mode2);
+/*
+ * The buffer repo_find_unique_abbrev returns is reused, so a caller printing
+ * two abbreviated names together keeps a copy of each.
+ */
+static char *abbrev_oid(const struct object_id *oid)
+{
+	return xstrdup(repo_find_unique_abbrev(the_repository, oid,
+					       DEFAULT_ABBREV));
+}
 
-/* Render one file's body into its own buffer, the way filepair_cb would have
- * written it straight out on a second walk. The line count is not known until
- * the diff has run, so the header is collected first and what follows it is
- * decided afterwards. */
-static void collect_filepair_body(struct diff_filepair *pair, int idx,
-				  int *binary, unsigned long *old_size,
-				  unsigned long *new_size)
+static void print_file_header(const struct object_id *old_oid, char *old_path,
+			      int old_mode, const struct object_id *new_oid,
+			      char *new_path, int new_mode)
 {
-	struct strbuf *body = &items[idx].body;
+	char *old_abbrev, *new_abbrev;
+	int subproject;
+
+	subproject = (S_ISGITLINK(old_mode) || S_ISGITLINK(new_mode));
+	html("<div class='head'>");
+	html("diff --git a/");
+	html_txt(old_path);
+	html(" b/");
+	html_txt(new_path);
+
+	if (old_mode == 0)
+		htmlf("<br/>new file mode %.6o", new_mode);
+
+	if (new_mode == 0)
+		htmlf("<br/>deleted file mode %.6o", old_mode);
+
+	if (!subproject) {
+		old_abbrev = abbrev_oid(old_oid);
+		new_abbrev = abbrev_oid(new_oid);
+		htmlf("<br/>index %s..%s", old_abbrev, new_abbrev);
+		free(old_abbrev);
+		free(new_abbrev);
+		if (old_mode != 0 && new_mode != 0) {
+			htmlf(" %.6o", old_mode);
+			if (new_mode != old_mode)
+				htmlf("..%.6o", new_mode);
+		}
+		if (is_null_oid(old_oid)) {
+			old_path = "dev/null";
+			html("<br/>--- /");
+		} else
+			html("<br/>--- a/");
+		if (old_mode != 0)
+			cgit_tree_link(old_path, NULL, NULL, ctx.qry.head,
+				       oid_to_hex(old_rev_oid), old_path);
+		else
+			html_txt(old_path);
+		if (is_null_oid(new_oid)) {
+			new_path = "dev/null";
+			html("<br/>+++ /");
+		} else
+			html("<br/>+++ b/");
+		if (new_mode != 0)
+			cgit_tree_link(new_path, NULL, NULL, ctx.qry.head,
+				       oid_to_hex(new_rev_oid), new_path);
+		else
+			html_txt(new_path);
+	}
+	html("</div>");
+}
+
+/*
+ * The length passed counts the terminator, because a renderer strips the last
+ * byte of every line it is given.
+ */
+static void print_subproject_lines(struct diff_filepair *pair,
+				   linediff_fn line_fn)
+{
+	if (S_ISGITLINK(pair->one->mode)) {
+		char *text = cgit_fmt("-Subproject %s",
+				      oid_to_hex(&pair->one->oid));
+		line_fn(text, strlen(text) + 1);
+	}
+	if (S_ISGITLINK(pair->two->mode)) {
+		char *text = cgit_fmt("+Subproject %s",
+				      oid_to_hex(&pair->two->oid));
+		line_fn(text, strlen(text) + 1);
+	}
+}
+
+static void print_binary_differs(void)
+{
+	if (use_ssdiff)
+		html("<tr><td colspan='4'>Binary files differ</td></tr>");
+	else
+		html("Binary files differ");
+}
+
+static void print_truncated(const char *path)
+{
+	if (use_ssdiff)
+		html("<tr><td colspan='4' class='truncated'>");
+	else
+		html("<div class='truncated'>");
+	html("This diff is too large to be rendered inline. ");
+	cgit_diff_link("View it on its own page", NULL, NULL, ctx.qry.head,
+		       ctx.qry.oid, ctx.qry.oid2, path);
+	html(".");
+	if (use_ssdiff) {
+		html("</td></tr>");
+		cgit_ssdiff_footer();
+	} else
+		html("</div>");
+}
+
+static struct fileinfo *reserve_item(void)
+{
+	files++;
+	if (files >= slots) {
+		if (slots == 0)
+			slots = 4;
+		else
+			slots = slots * 2;
+		items = xrealloc(items, slots * sizeof(struct fileinfo));
+	}
+	memset(&items[files - 1], 0, sizeof(items[files - 1]));
+	strbuf_init(&items[files - 1].body, 0);
+	return &items[files - 1];
+}
+
+/*
+ * Renders one file's body into its own buffer, the way filepair_cb would have
+ * written it straight out on a second walk.
+ */
+static void collect_body(struct diff_filepair *pair, struct fileinfo *item,
+			 int *binary, unsigned long *old_size,
+			 unsigned long *new_size)
+{
+	struct strbuf *body = &item->body;
 	linediff_fn line_fn = use_ssdiff ? cgit_ssdiff_line_cb : print_line;
 	size_t header_len;
 
@@ -233,77 +352,55 @@ static void collect_filepair_body(struct diff_filepair *pair, int idx,
 	html_capture_begin(body);
 	if (use_ssdiff)
 		cgit_ssdiff_header_begin();
-	header(&pair->one->oid, pair->one->path, pair->one->mode,
-	       &pair->two->oid, pair->two->path, pair->two->mode);
+	print_file_header(&pair->one->oid, pair->one->path, pair->one->mode,
+			  &pair->two->oid, pair->two->path, pair->two->mode);
 	if (use_ssdiff)
 		cgit_ssdiff_header_end();
-	// Everything from here on is dropped if the file turns out to be over
-	// the line budget, so remember where the header ended.
 	header_len = body->len;
 
+	// Cleared for the whole file rather than in the branch below, because
+	// the submodule branch never runs count_diff_lines with a render
+	// function. A submodule following a file that tripped the cap would
+	// otherwise inherit the flag and report itself as too large to render.
+	render_suppressed = 0;
+
 	if (S_ISGITLINK(pair->one->mode) || S_ISGITLINK(pair->two->mode)) {
-		/* The body shows the two Subproject lines, but the stat has
-		 * always counted what a diff of the pair produces, so run it
-		 * for the count alone. */
+		// The stat has always counted what a diff of the pair produces
+		// rather than the two lines the body shows, so run that diff
+		// for the count alone.
 		render_line_fn = NULL;
 		cgit_diff_files(&pair->one->oid, &pair->two->oid, old_size,
 				new_size, binary, 0, ctx.qry.ignorews,
 				count_diff_lines);
-		if (S_ISGITLINK(pair->one->mode)) {
-			char *l = cgit_fmt("-Subproject %s", oid_to_hex(&pair->one->oid));
-			line_fn(l, strlen(l) + 1);
-		}
-		if (S_ISGITLINK(pair->two->mode)) {
-			char *l = cgit_fmt("+Subproject %s", oid_to_hex(&pair->two->oid));
-			line_fn(l, strlen(l) + 1);
-		}
+		print_subproject_lines(pair, line_fn);
 	} else {
 		render_line_fn = line_fn;
-		render_suppressed = 0;
 		if (cgit_diff_files(&pair->one->oid, &pair->two->oid, old_size,
 				    new_size, binary, ctx.qry.context,
 				    ctx.qry.ignorews, count_diff_lines))
 			cgit_print_error("Error running diff");
 		render_line_fn = NULL;
-		if (*binary) {
-			if (use_ssdiff)
-				html("<tr><td colspan='4'>Binary files differ</td></tr>");
-			else
-				html("Binary files differ");
-		}
+		if (*binary)
+			print_binary_differs();
 	}
 	if (use_ssdiff)
 		cgit_ssdiff_footer();
 
 	if (render_suppressed) {
-		/* Over the line budget, so the body is a link instead. Setting
-		 * the length back would leave the buffer holding everything it
-		 * grew to while rendering, which the budget below cannot see
-		 * because it only counts what is kept, so rebuild it at the
-		 * size actually kept. */
+		// Setting the length back would leave the buffer holding
+		// everything it grew to while rendering, which the budget below
+		// cannot see because it only counts what is kept, so rebuild it
+		// at the size actually kept.
 		char *header_text = xmemdupz(body->buf, header_len);
 
 		strbuf_release(body);
 		strbuf_attach(body, header_text, header_len, header_len + 1);
-		if (use_ssdiff)
-			html("<tr><td colspan='4' class='truncated'>");
-		else
-			html("<div class='truncated'>");
-		html("This diff is too large to be rendered inline. ");
-		cgit_diff_link("View it on its own page", NULL, NULL,
-			       ctx.qry.head, ctx.qry.oid, ctx.qry.oid2,
-			       pair->two->path);
-		html(".");
-		if (use_ssdiff) {
-			html("</td></tr>");
-			cgit_ssdiff_footer();
-		} else
-			html("</div>");
+		print_truncated(pair->two->path);
 	}
 	html_capture_end();
 
 	body_bytes += body->len;
-	if (body_bytes > CGIT_DIFF_BODY_BUDGET) {
+	if (body_bytes > BODY_BUDGET) {
 		bodies_usable = 0;
 		release_bodies();
 	}
@@ -311,6 +408,7 @@ static void collect_filepair_body(struct diff_filepair *pair, int idx,
 
 static void inspect_filepair(struct diff_filepair *pair)
 {
+	struct fileinfo *item;
 	int binary = 0;
 	unsigned long old_size = 0;
 	unsigned long new_size = 0;
@@ -318,48 +416,37 @@ static void inspect_filepair(struct diff_filepair *pair)
 	if (!show_filepair(pair))
 		return;
 
-	files++;
+	item = reserve_item();
 	lines_added = 0;
 	lines_removed = 0;
-	if (files >= slots) {
-		if (slots == 0)
-			slots = 4;
-		else
-			slots = slots * 2;
-		items = xrealloc(items, slots * sizeof(struct fileinfo));
-	}
-	memset(&items[files-1], 0, sizeof(items[files-1]));
-	strbuf_init(&items[files-1].body, 0);
 
 	if (bodies_usable)
-		collect_filepair_body(pair, files - 1, &binary,
-				      &old_size, &new_size);
+		collect_body(pair, item, &binary, &old_size, &new_size);
 	else
 		cgit_diff_files(&pair->one->oid, &pair->two->oid, &old_size,
 				&new_size, &binary, 0, ctx.qry.ignorews,
 				count_diff_lines);
 
-	items[files-1].status = pair->status;
-	oidcpy(items[files-1].old_oid, &pair->one->oid);
-	oidcpy(items[files-1].new_oid, &pair->two->oid);
-	items[files-1].old_mode = pair->one->mode;
-	items[files-1].new_mode = pair->two->mode;
-	items[files-1].old_path = xstrdup(pair->one->path);
-	items[files-1].new_path = xstrdup(pair->two->path);
-	items[files-1].added = lines_added;
-	items[files-1].removed = lines_removed;
-	items[files-1].old_size = old_size;
-	items[files-1].new_size = new_size;
-	items[files-1].binary = binary;
+	item->status = pair->status;
+	oidcpy(item->old_oid, &pair->one->oid);
+	oidcpy(item->new_oid, &pair->two->oid);
+	item->old_mode = pair->one->mode;
+	item->new_mode = pair->two->mode;
+	item->old_path = xstrdup(pair->one->path);
+	item->new_path = xstrdup(pair->two->path);
+	item->added = lines_added;
+	item->removed = lines_removed;
+	item->old_size = old_size;
+	item->new_size = new_size;
+	item->binary = binary;
 	if (lines_added + lines_removed > max_changes)
 		max_changes = lines_added + lines_removed;
 	total_adds += lines_added;
 	total_rems += lines_removed;
 }
 
-static void cgit_print_diffstat(const struct object_id *old_oid,
-				const struct object_id *new_oid,
-				const char *prefix)
+static void print_diffstat(const struct object_id *old_oid,
+			   const struct object_id *new_oid, const char *prefix)
 {
 	int i;
 
@@ -376,7 +463,7 @@ static void cgit_print_diffstat(const struct object_id *old_oid,
 	max_changes = 0;
 	cgit_diff_tree(old_oid, new_oid, inspect_filepair, prefix,
 		       ctx.qry.ignorews);
-	for (i = 0; i<files; i++)
+	for (i = 0; i < files; i++)
 		print_fileinfo(&items[i]);
 	html("</table>");
 	html("<div class='diffstat-summary'>");
@@ -385,80 +472,12 @@ static void cgit_print_diffstat(const struct object_id *old_oid,
 	html("</div>");
 }
 
-/*
- * print a single line returned from xdiff
- */
-static void print_line(char *line, int len)
+static int over_line_cap(int idx)
 {
-	const char *class = "ctx";
-	char c = line[len-1];
-
-	if (line[0] == '+')
-		class = "add";
-	else if (line[0] == '-')
-		class = "del";
-	else if (line[0] == '@')
-		class = "hunk";
-
-	htmlf("<div class='%s'>", class);
-	line[len-1] = '\0';
-	html_txt(line);
-	html("</div>");
-	line[len-1] = c;
-}
-
-static void header(const struct object_id *oid1, char *path1, int mode1,
-		   const struct object_id *oid2, char *path2, int mode2)
-{
-	char *abbrev1, *abbrev2;
-	int subproject;
-
-	subproject = (S_ISGITLINK(mode1) || S_ISGITLINK(mode2));
-	html("<div class='head'>");
-	html("diff --git a/");
-	html_txt(path1);
-	html(" b/");
-	html_txt(path2);
-
-	if (mode1 == 0)
-		htmlf("<br/>new file mode %.6o", mode2);
-
-	if (mode2 == 0)
-		htmlf("<br/>deleted file mode %.6o", mode1);
-
-	if (!subproject) {
-		abbrev1 = xstrdup(repo_find_unique_abbrev(the_repository, oid1, DEFAULT_ABBREV));
-		abbrev2 = xstrdup(repo_find_unique_abbrev(the_repository, oid2, DEFAULT_ABBREV));
-		htmlf("<br/>index %s..%s", abbrev1, abbrev2);
-		free(abbrev1);
-		free(abbrev2);
-		if (mode1 != 0 && mode2 != 0) {
-			htmlf(" %.6o", mode1);
-			if (mode2 != mode1)
-				htmlf("..%.6o", mode2);
-		}
-		if (is_null_oid(oid1)) {
-			path1 = "dev/null";
-			html("<br/>--- /");
-		} else
-			html("<br/>--- a/");
-		if (mode1 != 0)
-			cgit_tree_link(path1, NULL, NULL, ctx.qry.head,
-				       oid_to_hex(old_rev_oid), path1);
-		else
-			html_txt(path1);
-		if (is_null_oid(oid2)) {
-			path2 = "dev/null";
-			html("<br/>+++ /");
-		} else
-			html("<br/>+++ b/");
-		if (mode2 != 0)
-			cgit_tree_link(path2, NULL, NULL, ctx.qry.head,
-				       oid_to_hex(new_rev_oid), path2);
-		else
-			html_txt(path2);
-	}
-	html("</div>");
+	return cap_diffs && ctx.cfg.max_diff_lines > 0 &&
+	       idx < files && !items[idx].binary &&
+	       items[idx].added + items[idx].removed >
+			(unsigned int)ctx.cfg.max_diff_lines;
 }
 
 static void filepair_cb(struct diff_filepair *pair)
@@ -467,7 +486,7 @@ static void filepair_cb(struct diff_filepair *pair)
 	unsigned long new_size = 0;
 	int binary = 0;
 	int idx;
-	linediff_fn print_line_fn = print_line;
+	linediff_fn line_fn = print_line;
 
 	if (!show_filepair(pair))
 		return;
@@ -476,65 +495,66 @@ static void filepair_cb(struct diff_filepair *pair)
 	current_filepair = pair;
 	if (use_ssdiff) {
 		cgit_ssdiff_header_begin();
-		print_line_fn = cgit_ssdiff_line_cb;
+		line_fn = cgit_ssdiff_line_cb;
 	}
-	header(&pair->one->oid, pair->one->path, pair->one->mode,
-	       &pair->two->oid, pair->two->path, pair->two->mode);
+	print_file_header(&pair->one->oid, pair->one->path, pair->one->mode,
+			  &pair->two->oid, pair->two->path, pair->two->mode);
 	if (use_ssdiff)
 		cgit_ssdiff_header_end();
 
-	/* The diffstat pass counted this file's lines. Over the budget,
-	 * print the header and hand off to the single-file diff page
-	 * instead of rendering the whole thing inline. */
-	if (cap_diffs && ctx.cfg.max_diff_lines > 0 && idx < files &&
-	    !items[idx].binary && items[idx].added + items[idx].removed >
-				  (unsigned int)ctx.cfg.max_diff_lines) {
-		if (use_ssdiff)
-			html("<tr><td colspan='4' class='truncated'>");
-		else
-			html("<div class='truncated'>");
-		html("This diff is too large to be rendered inline. ");
-		cgit_diff_link("View it on its own page", NULL, NULL,
-			       ctx.qry.head, ctx.qry.oid, ctx.qry.oid2,
-			       items[idx].new_path);
-		html(".");
-		if (use_ssdiff) {
-			html("</td></tr>");
-			cgit_ssdiff_footer();
-		} else
-			html("</div>");
+	if (over_line_cap(idx)) {
+		print_truncated(items[idx].new_path);
 		return;
 	}
 	if (S_ISGITLINK(pair->one->mode) || S_ISGITLINK(pair->two->mode)) {
-		if (S_ISGITLINK(pair->one->mode)) {
-			char *l = cgit_fmt("-Subproject %s", oid_to_hex(&pair->one->oid));
-			print_line_fn(l, strlen(l) + 1);
-		}
-		if (S_ISGITLINK(pair->two->mode)) {
-			char *l = cgit_fmt("+Subproject %s", oid_to_hex(&pair->two->oid));
-			print_line_fn(l, strlen(l) + 1);
-		}
+		print_subproject_lines(pair, line_fn);
 		if (use_ssdiff)
 			cgit_ssdiff_footer();
 		return;
 	}
 	if (cgit_diff_files(&pair->one->oid, &pair->two->oid, &old_size,
 			    &new_size, &binary, ctx.qry.context,
-			    ctx.qry.ignorews, print_line_fn))
+			    ctx.qry.ignorews, line_fn))
 		cgit_print_error("Error running diff");
-	if (binary) {
-		if (use_ssdiff)
-			html("<tr><td colspan='4'>Binary files differ</td></tr>");
-		else
-			html("Binary files differ");
-	}
+	if (binary)
+		print_binary_differs();
 	if (use_ssdiff)
 		cgit_ssdiff_footer();
 }
 
+static void print_raw_patch(const struct object_id *old_tree_oid,
+			    const struct object_id *new_tree_oid)
+{
+	struct diff_options diffopt;
+
+	repo_diff_setup(the_repository, &diffopt);
+	diffopt.output_format = DIFF_FORMAT_PATCH;
+	diffopt.flags.recursive = 1;
+	diff_setup_done(&diffopt);
+
+	ctx.page.mimetype = "text/plain";
+	cgit_print_http_headers();
+	if (old_tree_oid)
+		diff_tree_oid(old_tree_oid, new_tree_oid, "", &diffopt);
+	else
+		diff_root_tree_oid(new_tree_oid, "", &diffopt);
+	diffcore_std(&diffopt);
+	diff_flush(&diffopt);
+}
+
+struct diff_filespec *cgit_get_current_old_file(void)
+{
+	return current_filepair->one;
+}
+
+struct diff_filespec *cgit_get_current_new_file(void)
+{
+	return current_filepair->two;
+}
+
 void cgit_print_diff_ctrls(void)
 {
-	int i, curr;
+	int i, selected;
 
 	html("<div class='cgit-panel'>");
 	html("<b>diff options</b>");
@@ -546,13 +566,13 @@ void cgit_print_diff_ctrls(void)
 	html("<td class='label'>context:</td>");
 	html("<td class='ctrl'>");
 	html("<select name='context' data-autosubmit='1'>");
-	curr = ctx.qry.context;
-	if (!curr)
-		curr = 3;
+	selected = ctx.qry.context;
+	if (!selected)
+		selected = DEFAULT_CONTEXT_LINES;
 	for (i = 1; i <= 10; i++)
-		html_intoption(i, cgit_fmt("%d", i), curr);
-	for (i = 15; i <= 40; i += 5)
-		html_intoption(i, cgit_fmt("%d", i), curr);
+		html_intoption(i, cgit_fmt("%d", i), selected);
+	for (i = 15; i <= MAX_DIFF_CONTEXT_LINES; i += 5)
+		html_intoption(i, cgit_fmt("%d", i), selected);
 	html("</select>");
 	html("</td>");
 	html("</tr><tr>");
@@ -567,10 +587,10 @@ void cgit_print_diff_ctrls(void)
 	html("<td class='label'>mode:</td>");
 	html("<td class='ctrl'>");
 	html("<select name='dt' data-autosubmit='1'>");
-	curr = ctx.qry.has_difftype ? ctx.qry.difftype : ctx.cfg.difftype;
-	html_intoption(0, "unified", curr);
-	html_intoption(1, "ssdiff", curr);
-	html_intoption(2, "stat only", curr);
+	selected = ctx.qry.has_difftype ? ctx.qry.difftype : ctx.cfg.difftype;
+	html_intoption(0, "unified", selected);
+	html_intoption(1, "ssdiff", selected);
+	html_intoption(2, "stat only", selected);
 	html("</select></td></tr>");
 	html("<tr><td/><td class='ctrl'>");
 	html("<noscript><input type='submit' value='reload'/></noscript>");
@@ -582,19 +602,17 @@ void cgit_print_diff_ctrls(void)
 void cgit_print_diff(const char *new_rev, const char *old_rev,
 		     const char *prefix, int show_ctrls, int raw)
 {
-	struct commit *commit, *commit2;
+	struct commit *new_commit, *old_commit;
 	const struct object_id *old_tree_oid, *new_tree_oid;
 	diff_type difftype;
 
-	// A path-limited diff is naturally bounded, so the size caps only
-	// apply to a whole-commit diff. Decide that from the caller's prefix
-	// before the follow logic rewrites it to "", otherwise follow=1
-	// silently disables the caps.
+	// Decided from the caller's prefix before the follow logic below
+	// rewrites it to "", otherwise follow=1 silently disables the caps.
 	cap_diffs = !prefix;
 
-	// If "follow" is set then the diff machinery needs to examine the
-	// entire commit to detect renames so we must limit the paths in our
-	// own callbacks and not pass the prefix to the diff machinery.
+	// Detecting renames needs the diff machinery to examine the whole
+	// commit, so with follow set the prefix is applied in show_filepair
+	// instead of being passed down.
 	if (ctx.qry.follow && ctx.repo->enable_follow_links) {
 		current_prefix = prefix;
 		prefix = "";
@@ -609,13 +627,13 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
 			"Bad object name: %s", new_rev);
 		return;
 	}
-	commit = lookup_commit_reference(the_repository, new_rev_oid);
-	if (!commit || repo_parse_commit(the_repository, commit)) {
+	new_commit = lookup_commit_reference(the_repository, new_rev_oid);
+	if (!new_commit || repo_parse_commit(the_repository, new_commit)) {
 		cgit_print_error_page(404, "Not found",
 			"Bad commit: %s", oid_to_hex(new_rev_oid));
 		return;
 	}
-	new_tree_oid = get_commit_tree_oid(commit);
+	new_tree_oid = get_commit_tree_oid(new_commit);
 
 	if (old_rev) {
 		if (repo_get_oid(the_repository, old_rev, old_rev_oid)) {
@@ -623,68 +641,52 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
 				"Bad object name: %s", old_rev);
 			return;
 		}
-	} else if (commit->parents && commit->parents->item) {
-		oidcpy(old_rev_oid, &commit->parents->item->object.oid);
+	} else if (new_commit->parents && new_commit->parents->item) {
+		oidcpy(old_rev_oid, &new_commit->parents->item->object.oid);
 	} else {
 		oidclr(old_rev_oid, the_repository->hash_algo);
 	}
 
 	if (!is_null_oid(old_rev_oid)) {
-		commit2 = lookup_commit_reference(the_repository, old_rev_oid);
-		if (!commit2 || repo_parse_commit(the_repository, commit2)) {
+		old_commit = lookup_commit_reference(the_repository,
+						     old_rev_oid);
+		if (!old_commit ||
+		    repo_parse_commit(the_repository, old_commit)) {
 			cgit_print_error_page(404, "Not found",
 				"Bad commit: %s", oid_to_hex(old_rev_oid));
 			return;
 		}
-		old_tree_oid = get_commit_tree_oid(commit2);
+		old_tree_oid = get_commit_tree_oid(old_commit);
 	} else {
 		old_tree_oid = NULL;
 	}
 
 	if (raw) {
-		struct diff_options diffopt;
-
-		repo_diff_setup(the_repository, &diffopt);
-		diffopt.output_format = DIFF_FORMAT_PATCH;
-		diffopt.flags.recursive = 1;
-		diff_setup_done(&diffopt);
-
-		ctx.page.mimetype = "text/plain";
-		cgit_print_http_headers();
-		if (old_tree_oid) {
-			diff_tree_oid(old_tree_oid, new_tree_oid, "",
-				       &diffopt);
-		} else {
-			diff_root_tree_oid(new_tree_oid, "", &diffopt);
-		}
-		diffcore_std(&diffopt);
-		diff_flush(&diffopt);
-
+		print_raw_patch(old_tree_oid, new_tree_oid);
 		return;
 	}
 
 	difftype = ctx.qry.has_difftype ? ctx.qry.difftype : ctx.cfg.difftype;
 	use_ssdiff = difftype == DIFF_SSDIFF;
-	/* A stat-only view never shows a body, and only a capped view bounds
-	 * the size of the ones it does show. */
-	bodies_usable = difftype != DIFF_STATONLY && cap_diffs;
+	// Without max-diff-lines to stop it a single body grows with the file
+	// it came from, which for a side by side diff is several times the blob
+	// itself, and the budget cannot help there because it is only reached
+	// once a body is complete.
+	bodies_usable = difftype != DIFF_STATONLY && cap_diffs &&
+			ctx.cfg.max_diff_lines > 0;
 
 	if (show_ctrls) {
 		cgit_print_layout_start();
 		cgit_print_diff_ctrls();
 	}
 
-	/*
-	 * Clicking on a link to a file in the diff stat should show a diff
-	 * of the file, showing the diff stat limited to a single file is
-	 * pretty useless.  All links from this point on will be to
-	 * individual files, so we simply reset the difftype in the query
-	 * here to avoid propagating DIFF_STATONLY to the individual files.
-	 */
+	// Every link from here on leads to a single file, and a stat limited
+	// to one file is useless, so the difftype those links carry is reset
+	// rather than propagating DIFF_STATONLY to them.
 	if (difftype == DIFF_STATONLY)
 		ctx.qry.difftype = ctx.cfg.difftype;
 
-	cgit_print_diffstat(old_rev_oid, new_rev_oid, prefix);
+	print_diffstat(old_rev_oid, new_rev_oid, prefix);
 
 	if (difftype == DIFF_STATONLY) {
 		if (show_ctrls)
@@ -692,8 +694,6 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
 		return;
 	}
 
-	/* A commit touching more files than max-diff-files stops at the
-	 * stat above, where every file links to its own diff page. */
 	if (cap_diffs && ctx.cfg.max_diff_files > 0 &&
 	    files > ctx.cfg.max_diff_files) {
 		html("<div class='truncated'>");
@@ -715,12 +715,11 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
 	}
 	if (bodies_usable) {
 		int i;
+
 		for (i = 0; i < files; i++)
 			html_raw(items[i].body.buf, items[i].body.len);
 		release_bodies();
 	} else {
-		/* The bodies outgrew the budget, so they are produced the way
-		 * they were before, by walking the tree again. */
 		item_idx = 0;
 		cgit_diff_tree(old_rev_oid, new_rev_oid, filepair_cb, prefix,
 			       ctx.qry.ignorews);