From f8eddb387ee5d6e6f9f751888d71ffd80974cb2d Mon Sep 17 00:00:00 2001 From: Bryce Kwon Date: Sat, 8 Aug 2026 12:50:00 -1000 Subject: Render each diff body while its file is open --- source/html.c | 27 ++++++++ source/html.h | 5 ++ source/ui-diff.c | 206 ++++++++++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 222 insertions(+), 16 deletions(-) diff --git a/source/html.c b/source/html.c index dbb0a57..6e023d1 100644 --- a/source/html.c +++ b/source/html.c @@ -113,8 +113,35 @@ void html_flush(void) html_write(html_buf, len); } +/* + * While a capture is in effect, page output is collected into the caller's + * buffer instead of being written. The diff view uses this to render a file's + * body at the point it already has the file open, rather than walking the whole + * tree a second time to produce what the diffstat above it has to be printed + * before. + * + * A capture must not span anything that writes to stdout by another route, a + * filter in particular, since that output would escape the capture. + */ +static struct strbuf *html_capture; + +void html_capture_begin(struct strbuf *sb) +{ + html_flush(); + html_capture = sb; +} + +void html_capture_end(void) +{ + html_capture = NULL; +} + void html_raw(const char *data, size_t size) { + if (html_capture) { + strbuf_add(html_capture, data, size); + return; + } if (size >= HTML_WRITE_BUFSIZE) { // Nothing is gained by copying a blob through the buffer. html_flush(); diff --git a/source/html.h b/source/html.h index 353254e..0818ab5 100644 --- a/source/html.h +++ b/source/html.h @@ -17,6 +17,11 @@ extern void html(const char *txt); * other than html_raw writes to stdout. */ extern void html_flush(void); +/* Collect page output into sb rather than writing it, until html_capture_end. + * Must not span a filter or anything else that writes to stdout directly. */ +extern void html_capture_begin(struct strbuf *sb); +extern void html_capture_end(void); + __attribute__((format (printf,1,2))) extern void htmlf(const char *format,...); diff --git a/source/ui-diff.c b/source/ui-diff.c index ebf9425..a0790f7 100644 --- a/source/ui-diff.c +++ b/source/ui-diff.c @@ -14,14 +14,7 @@ #include "ui-shared.h" #include "ui-ssdiff.h" -struct object_id old_rev_oid[1]; -struct object_id new_rev_oid[1]; - -static int files, slots; -static int total_adds, total_rems, max_changes; -static int lines_added, lines_removed; - -static struct fileinfo { +struct fileinfo { char status; struct object_id old_oid[1]; struct object_id new_oid[1]; @@ -34,17 +27,71 @@ static struct fileinfo { unsigned long old_size; unsigned long new_size; unsigned int binary:1; -} *items; + 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; -static int use_ssdiff = 0; +/* 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. */ 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; + + for (i = 0; i < files; i++) + strbuf_release(&items[i].body); +} + struct diff_filespec *cgit_get_current_old_file(void) { return current_filepair->one; @@ -130,6 +177,8 @@ static void print_fileinfo(struct fileinfo *info) html("\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. */ static void count_diff_lines(char *line, int len) { if (line && (len > 0)) { @@ -138,6 +187,14 @@ static void count_diff_lines(char *line, int len) else if (line[0] == '-') lines_removed++; } + if (!render_line_fn || render_suppressed) + return; + if (cap_diffs && ctx.cfg.max_diff_lines > 0 && + lines_added + lines_removed > ctx.cfg.max_diff_lines) { + render_suppressed = 1; + return; + } + render_line_fn(line, len); } static int show_filepair(struct diff_filepair *pair) @@ -155,6 +212,103 @@ static int show_filepair(struct diff_filepair *pair) return 0; } +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); + +/* 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) +{ + struct strbuf *body = &items[idx].body; + linediff_fn line_fn = use_ssdiff ? cgit_ssdiff_line_cb : print_line; + size_t header_len; + + current_filepair = pair; + + 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); + 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; + + 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. */ + 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); + } + } 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("Binary files differ"); + else + html("Binary files differ"); + } + } + 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. */ + 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(""); + else + html("
"); + 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(""); + cgit_ssdiff_footer(); + } else + html("
"); + } + html_capture_end(); + + body_bytes += body->len; + if (body_bytes > CGIT_DIFF_BODY_BUDGET) { + bodies_usable = 0; + release_bodies(); + } +} + static void inspect_filepair(struct diff_filepair *pair) { int binary = 0; @@ -167,8 +321,6 @@ static void inspect_filepair(struct diff_filepair *pair) files++; lines_added = 0; lines_removed = 0; - cgit_diff_files(&pair->one->oid, &pair->two->oid, &old_size, &new_size, - &binary, 0, ctx.qry.ignorews, count_diff_lines); if (files >= slots) { if (slots == 0) slots = 4; @@ -176,6 +328,17 @@ static void inspect_filepair(struct diff_filepair *pair) 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); + 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); @@ -222,7 +385,6 @@ static void cgit_print_diffstat(const struct object_id *old_oid, html(""); } - /* * print a single line returned from xdiff */ @@ -503,6 +665,9 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, 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; if (show_ctrls) { cgit_print_layout_start(); @@ -548,9 +713,18 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, html(""); html(""); html("
"); } - item_idx = 0; - cgit_diff_tree(old_rev_oid, new_rev_oid, filepair_cb, prefix, - ctx.qry.ignorews); + 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); + } if (!use_ssdiff) html("
"); -- cgit v2.8.0