From 80767bc9732bf6716697198e53ff2cb8d4ae96be Mon Sep 17 00:00:00 2001 From: Bryce Kwon Date: Wed, 12 Aug 2026 18:23:17 -1000 Subject: Restyle the sources and fix the audit's findings --- source/ui-blame.c | 324 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 193 insertions(+), 131 deletions(-) (limited to 'source/ui-blame.c') 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 - * - * 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(""); - 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(""); - 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(""); + for (ent = sb->ent; ent; ent = ent->next) { + html("
");
+		emit_entry_hash(ent);
+		html("
"); + } + html("\n"); +} + +static void emit_entry_linenumbers(struct blame_entry *ent) { const char *numberfmt = "%1$d\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(""); + for (ent = sb->ent; ent; ent = ent->next) { + html("
");
+		emit_entry_linenumbers(ent);
+		html("
"); + } + html("\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("
"); + while (ent) { + struct blame_entry *next = ent->next; + + html("
");
+		emit_entry_background(sb, ent);
+		html("
"); + free(ent); + ent = next; + } + html("
"); +} + +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("\n\n"); - /* Commit hashes */ - html("\n"); + emit_hashes(&sb); - /* Line numbers */ - if (ctx.cfg.enable_tree_linenumbers) { - html("\n"); - } + if (ctx.cfg.enable_tree_linenumbers) + emit_linenumbers(&sb); html("\n
"); - for (ent = sb.ent; ent; ent = ent->next) { - html("
");
-		emit_blame_entry_hash(ent);
-		html("
"); - } - html("
"); - for (ent = sb.ent; ent; ent = ent->next) { - html("
");
-			emit_blame_entry_linenumber(ent);
-			html("
"); - } - html("
"); - - /* Colored bars behind lines */ - html("
"); - for (ent = sb.ent; ent; ) { - struct blame_entry *e = ent->next; - html("
");
-		emit_blame_entry_line_background(&sb, ent);
-		html("
"); - free(ent); - ent = e; - } - html("
"); + emit_line_backgrounds(&sb); free((void *)sb.final_buf); - /* Lines */ html("
");
 	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("
\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); } -- cgit v2.8.0