From 22f8de0f4f9b60ba3c6d3cc164cdf07ff988d2a4 Mon Sep 17 00:00:00 2001 From: Bryce Kwon Date: Sat, 31 Oct 2026 22:07:47 -1000 Subject: Show a renamed file's diff as the rename A diff limited to the new name cannot see the old one, so the diffstat's link for a renamed file, the commit page reached from that file's log, and the file's rawdiff and patch all showed every line as added. A path the old tree lacks is now diffed across the whole commit with rename detection aimed at it alone, the way git blame looks for a rename, so it is compared with the commit's deleted files and nothing else. Doing it in cgit_diff_tree keeps the log's counts for that commit in step with the page they link to. The rawdiff and patch pages leave the diff to git's own output, so for them that search finds the old name, and both names go into the limit with rename detection on. --- source/shared.c | 79 +++++++++++++++++++++++++++++++++++++++++++++----- source/shared.h | 24 +++++++++++++-- source/ui-diff.c | 11 +++++-- source/ui-patch.c | 13 +++++++++ tests/t0107-diff.sh | 35 ++++++++++++++++++++++ tests/t0108-rawdiff.sh | 21 ++++++++++++++ tests/t0109-patch.sh | 19 ++++++++++++ 7 files changed, 191 insertions(+), 11 deletions(-) diff --git a/source/shared.c b/source/shared.c index c189b11..fcdcd24 100644 --- a/source/shared.c +++ b/source/shared.c @@ -87,15 +87,19 @@ struct cgit_repo *cgit_get_repoinfo(const char *url) return NULL; } -void cgit_diff_tree_cb(struct diff_queue_struct *q, struct diff_options *options UNUSED, void *data) +void cgit_diff_tree_cb(struct diff_queue_struct *q, struct diff_options *options, void *data) { filepair_fn fn = *(filepair_fn *)data; + const char *wanted = options->single_follow; int i; for (i = 0; i < q->nr; i++) { // An unmerged path has no single pair of blobs to show. if (q->queue[i]->status == 'U') continue; + // A diff aimed at one path ran over the whole commit. + if (wanted && strcmp(q->queue[i]->two->path, wanted)) + continue; fn(q->queue[i]); } } @@ -215,11 +219,48 @@ int cgit_diff_files(const struct object_id *old_oid, const struct object_id *new return 0; } +static void set_literal_item(struct pathspec_item *item, const char *path) +{ + int len = path ? strlen(path) : 0; + + // nowildcard_len matching len makes git treat the path as literal + // rather than as a glob. + *item = (struct pathspec_item){ .match = path, .len = len, .nowildcard_len = len }; +} + void cgit_diff_limit_path(struct diff_options *opt, const char *prefix) { cgit_literal_pathspec(&opt->pathspec, xmalloc(sizeof(struct pathspec_item)), xstrdup(prefix)); } +void cgit_diff_limit_rename(struct diff_options *opt, const char *path, const char *source) +{ + struct pathspec_item *items = xcalloc(2, sizeof(*items)); + + set_literal_item(&items[0], xstrdup(path)); + set_literal_item(&items[1], xstrdup(source)); + opt->pathspec.nr = 2; + opt->pathspec.items = items; + opt->detect_rename = DIFF_DETECT_RENAME; +} + +/* + * Whether path names something other than a directory in the new tree that + * the old tree lacks, which is where a rename lands. + */ +static int path_is_new(const struct object_id *old_oid, const struct object_id *new_oid, + const char *path) +{ + struct object_id oid; + unsigned short mode; + + if (!*path || !old_oid || is_null_oid(old_oid)) + return 0; + if (get_tree_entry(the_repository, new_oid, path, &oid, &mode) || S_ISDIR(mode)) + return 0; + return get_tree_entry(the_repository, old_oid, path, &oid, &mode) != 0; +} + void cgit_diff_tree(const struct object_id *old_oid, const struct object_id *new_oid, filepair_fn fn, const char *prefix, int ignorews) { @@ -234,7 +275,12 @@ void cgit_diff_tree(const struct object_id *old_oid, const struct object_id *new DIFF_XDL_SET(&opt, IGNORE_WHITESPACE); opt.format_callback = cgit_diff_tree_cb; opt.format_callback_data = &fn; - if (prefix) + // The source of a rename lies outside a limit naming its destination, + // so a new path is diffed across the whole commit instead, with + // rename detection aimed at that path alone. + if (prefix && path_is_new(old_oid, new_oid, prefix)) + opt.single_follow = prefix; + else if (prefix) cgit_diff_limit_path(&opt, prefix); diff_setup_done(&opt); @@ -246,6 +292,29 @@ void cgit_diff_tree(const struct object_id *old_oid, const struct object_id *new diff_flush(&opt); } +// The old name cgit_rename_source's diff finds, which a filepair_fn has no +// other way to hand back. +static char *rename_source; + +static void note_rename_source(struct diff_filepair *pair) +{ + if (pair->status == DIFF_STATUS_RENAMED && !rename_source) + rename_source = xstrdup(pair->one->path); +} + +char *cgit_rename_source(const struct object_id *old_oid, const struct object_id *new_oid, + const char *path) +{ + char *source; + + if (!path_is_new(old_oid, new_oid, path)) + return NULL; + cgit_diff_tree(old_oid, new_oid, note_rename_source, path, 0); + source = rename_source; + rename_source = NULL; + return source; +} + void cgit_diff_commit(struct commit *commit, filepair_fn fn, const char *prefix) { const struct object_id *old_oid = NULL; @@ -262,11 +331,7 @@ int cgit_blob_too_large(size_t size) void cgit_literal_pathspec(struct pathspec *ps, struct pathspec_item *item, const char *path) { - int len = path ? strlen(path) : 0; - - // nowildcard_len matching len makes git treat the path as literal - // rather than as a glob. - *item = (struct pathspec_item){ .match = path, .len = len, .nowildcard_len = len }; + set_literal_item(item, path); ps->nr = path ? 1 : 0; ps->items = item; } diff --git a/source/shared.h b/source/shared.h index f182962..b24c690 100644 --- a/source/shared.h +++ b/source/shared.h @@ -25,8 +25,9 @@ extern struct cgit_repo *cgit_get_repoinfo(const char *url); /* * A diff format callback that hands each filepair to the filepair_fn data - * points at. ISO C has no conversion between a function pointer and void *, - * so the function travels by address. + * points at, or only the pair landing on single_follow when that is set. ISO + * C has no conversion between a function pointer and void *, so the function + * travels by address. */ extern void cgit_diff_tree_cb(struct diff_queue_struct *q, struct diff_options *options, void *data); @@ -45,6 +46,25 @@ extern int cgit_diff_files(const struct object_id *old_oid, const struct object_ */ extern void cgit_diff_limit_path(struct diff_options *opt, const char *prefix); +/* + * Limit a diff to path and to source, the name it was renamed from, both + * taken literally, with rename detection on so that git pairs the two. + */ +extern void cgit_diff_limit_rename(struct diff_options *opt, const char *path, const char *source); + +/* + * The name path had in the old tree when the change between the two trees + * renamed it, or NULL when path is no renamed file's new name. The result is + * the caller's to free. + */ +extern char *cgit_rename_source(const struct object_id *old_oid, const struct object_id *new_oid, + const char *path); + +/* + * Hand fn each filepair between the two trees, limited to the paths below + * prefix when one is given. A prefix naming a file the old tree lacks still + * comes back as the rename that produced it. + */ extern void cgit_diff_tree(const struct object_id *old_oid, const struct object_id *new_oid, filepair_fn fn, const char *prefix, int ignorews); extern void cgit_diff_commit(struct commit *commit, filepair_fn fn, const char *prefix); diff --git a/source/ui-diff.c b/source/ui-diff.c index 315a8f0..0e2e75e 100644 --- a/source/ui-diff.c +++ b/source/ui-diff.c @@ -510,8 +510,15 @@ static void print_raw_patch(const struct object_id *old_tree_oid, const struct o repo_diff_setup(the_repository, &diffopt); diffopt.output_format = DIFF_FORMAT_PATCH; diffopt.flags.recursive = 1; - if (prefix) - cgit_diff_limit_path(&diffopt, prefix); + if (prefix) { + char *source = cgit_rename_source(old_tree_oid, new_tree_oid, prefix); + + if (source) + cgit_diff_limit_rename(&diffopt, prefix, source); + else + cgit_diff_limit_path(&diffopt, prefix); + free(source); + } diff_setup_done(&diffopt); ctx.page.mimetype = "text/plain"; diff --git a/source/ui-patch.c b/source/ui-patch.c index 0fa9a2e..a60e7cc 100644 --- a/source/ui-patch.c +++ b/source/ui-patch.c @@ -12,6 +12,7 @@ #include "html.h" #include "ui-patch.h" #include "ui-shared.h" +#include "shared.h" // Room for two hex object ids, the two dots between them, and the null byte. #define REV_RANGE_LEN (2 * GIT_MAX_HEXSZ + 3) @@ -24,6 +25,7 @@ void cgit_print_patch(const char *new_rev, const char *old_rev, const char *pref struct strvec rev_argv = STRVEC_INIT; char rev_range[REV_RANGE_LEN]; const char *version; + char *source = NULL; if (!new_rev) new_rev = ctx.qry.head; @@ -47,9 +49,20 @@ void cgit_print_patch(const char *new_rev, const char *old_rev, const char *pref else strvec_push(&rev_argv, "--format=format:From %H Mon Sep 17 00:00:00 2001%n" "From: %an%nDate: %aD%n%w(78,0,1)Subject: %s%n%n%w(0)%b"); + // A file the change renamed is limited by its old name as well, with + // rename detection on, so that git pairs the two into the rename. That + // name comes from the tree rather than the request, so it is matched + // literally. + if (prefix) + source = cgit_rename_source(&old_oid, &new_oid, prefix); + if (source) + strvec_push(&rev_argv, "-M"); strvec_pushl(&rev_argv, rev_range, "--", NULL); if (prefix) strvec_push(&rev_argv, prefix); + if (source) + strvec_pushf(&rev_argv, ":(literal)%s", source); + free(source); // Git inlines a blob up to its own big-file threshold, far past what // any other view inflates, so max-blob-size is applied through it. diff --git a/tests/t0107-diff.sh b/tests/t0107-diff.sh index e173d00..d03ecd2 100755 --- a/tests/t0107-diff.sh +++ b/tests/t0107-diff.sh @@ -104,4 +104,39 @@ test_expect_success 'stripping the highlight leaves every line intact' ' done old.txt && + git add old.txt && + git commit -m "add old.txt" && + git mv old.txt new.txt && + git commit -m "rename old.txt" + ) && + cgitrc_repo movedrc moved +' + +test_expect_success 'the diff of a renamed file shows the rename' ' + cgit_rc movedrc "url=moved/diff/new.txt" >tmp && + grep "diff --git a/old.txt b/new.txt" tmp && + grep " (renamed from old.txt)" tmp && + ! grep "new file mode" tmp +' + +test_expect_success 'so does the commit page limited to the new name' ' + cgit_rc movedrc "url=moved/commit/new.txt" >tmp && + grep "diff --git a/old.txt b/new.txt" tmp && + ! grep "new file mode" tmp +' + +test_expect_success 'a file added outright still shows as new' ' + added=$(git -C repos/moved rev-parse HEAD~1) && + cgit_rc movedrc "url=moved/diff/old.txt&id=$added" >tmp && + grep "new file mode 100644" tmp && + grep "
+10
" tmp +' + test_done diff --git a/tests/t0108-rawdiff.sh b/tests/t0108-rawdiff.sh index 9b4eb37..66f0cbe 100755 --- a/tests/t0108-rawdiff.sh +++ b/tests/t0108-rawdiff.sh @@ -52,4 +52,25 @@ test_expect_success 'a path limits the diff to that file' ' ! grep "file-5" actual ' +# The old name of a file renamed in the change joins the limit, so the diff +# shows the rename, the same as git diff given both names. +test_expect_success 'a renamed file is diffed as the rename' ' + mkrepo repos/moved 1 && + ( + cd repos/moved && + test_seq 1 10 >old.txt && + git add old.txt && + git commit -m "add old.txt" && + git mv old.txt new.txt && + echo eleven >>new.txt && + git commit -am "rename old.txt" + ) && + cgitrc_repo movedrc moved && + cgit_rc movedrc "url=moved/rawdiff/new.txt" >tmp && + git -C repos/moved diff HEAD^ HEAD -- old.txt new.txt >expect && + strip_headers actual && + test_cmp expect actual && + grep "^rename from old.txt$" actual +' + test_done diff --git a/tests/t0109-patch.sh b/tests/t0109-patch.sh index bc942fe..6aae556 100755 --- a/tests/t0109-patch.sh +++ b/tests/t0109-patch.sh @@ -70,4 +70,23 @@ test_expect_success 'the signature names the version when asked to' ' check_cgit_signature "$CGIT_VERSION" ' +# The old name of a file renamed in the commit joins the limit, so the patch +# carries the rename rather than a file added whole. +test_expect_success 'a renamed file is patched as the rename' ' + mkrepo repos/moved 1 && + ( + cd repos/moved && + test_seq 1 10 >old.txt && + git add old.txt && + git commit -m "add old.txt" && + git mv old.txt new.txt && + git commit -m "rename old.txt" + ) && + cgitrc_repo movedrc moved && + cgit_rc movedrc "url=moved/patch/new.txt" >tmp && + grep "^rename from old.txt$" tmp && + grep "^rename to new.txt$" tmp && + ! grep "^new file mode" tmp +' + test_done -- cgit 2.12.1