From 22eef7a791b53f00ad3072fa00c65cca7da47a5b Mon Sep 17 00:00:00 2001 From: Justin Ling Date: Thu, 30 Jul 2026 22:56:03 +0800 Subject: [PATCH] Render uncommon diff entries correctly --- app/page.tsx | 11 ++ scripts/build-diff-data.mjs | 60 +++++++++++ tests/remote-targets.test.mjs | 185 +++++++++++++++++++++++++++++++++- 3 files changed, 253 insertions(+), 3 deletions(-) diff --git a/app/page.tsx b/app/page.tsx index 64084f1..dd0ebc4 100644 --- a/app/page.tsx +++ b/app/page.tsx @@ -30,6 +30,7 @@ type DiffFile = { patch: string; snippet: string; sourceUrl?: string; + comparisonUrl?: string; summary: FileSummary; noteReady?: boolean; }; @@ -577,6 +578,16 @@ export default function Home() { Open file ↗ ) : null} + {currentFile.comparisonUrl ? ( + + Open comparison ↗ + + ) : null} diff --git a/scripts/build-diff-data.mjs b/scripts/build-diff-data.mjs index a2f0bc6..df671c3 100644 --- a/scripts/build-diff-data.mjs +++ b/scripts/build-diff-data.mjs @@ -387,6 +387,34 @@ function remoteDefaultBranch(remoteUrl) { return remoteDefaultBranchInfo(remoteUrl).name; } +function remoteContainsCommits(remoteUrl, commits) { + let raw; + try { + raw = runRepo(['ls-remote', remoteUrl]); + } catch { + return false; + } + const tips = [ + ...new Set( + raw + .split('\n') + .map((line) => line.trim().split(/\s+/, 1)[0]) + .filter(Boolean), + ), + ]; + return commits.every((commit) => + tips.some((tip) => { + if (tip === commit) return true; + const result = spawnSync( + 'git', + ['-C', repo, 'merge-base', '--is-ancestor', commit, tip], + { stdio: 'ignore' }, + ); + return result.status === 0; + }), + ); +} + function localDefaultBranch(remote) { if (baseOption) return { name: baseOption }; @@ -535,6 +563,7 @@ function resolveBranchTarget() { remote, sourceRepositoryUrl: repository?.webUrl, baseRepositoryUrl: repository?.webUrl, + comparisonCommitsOnRemote: true, target: { kind: 'branch', remote: remote.name, @@ -600,6 +629,7 @@ function resolvePullRequestTarget() { pr.url.replace(/\/pull\/\d+(?:\/.*)?$/, ''), baseRepositoryUrl: repository?.webUrl || pr.url.replace(/\/pull\/\d+(?:\/.*)?$/, ''), + comparisonCommitsOnRemote: true, target: { kind: 'pull-request', remote: remote.name, @@ -646,6 +676,9 @@ function resolveCheckoutTarget() { const repository = githubRepository(remoteUrl); const headLabel = branch || currentHead; const hasCommittedChanges = mergeBaseOid !== currentHead; + const hasUncommittedChanges = Boolean( + tryRepo(['status', '--porcelain=v1', '-z']), + ); const isDefaultBranchCheckout = branch === defaultBranch.name; return { @@ -659,6 +692,12 @@ function resolveCheckoutTarget() { remote, sourceRepositoryUrl: repository?.webUrl, baseRepositoryUrl: repository?.webUrl, + comparisonCommitsOnRemote: + !hasUncommittedChanges && + Boolean( + remote?.url && + remoteContainsCommits(remote.url, [mergeBaseOid, currentHead]), + ), target: { kind: 'checkout', ...(remote ? { remote: remote.name } : {}), @@ -712,6 +751,12 @@ function resolveLocalTarget() { baseRepositoryUrl: worktree ? undefined : githubRepository(remoteUrl)?.webUrl, + comparisonCommitsOnRemote: + !worktree && + Boolean( + remoteUrl && + remoteContainsCommits(remoteUrl, [resolvedBase, resolvedHead]), + ), target: worktree ? { kind: 'worktree', base: { ref: 'HEAD', oid: currentHead || null } } : { @@ -812,6 +857,13 @@ function githubFileUrl(repositoryUrl, ref, path) { return `${repositoryUrl}/blob/${encodeURIComponent(ref)}/${filePath}`; } +function githubComparisonUrl(repositoryUrl, base, head) { + if (!repositoryUrl || !base || !head || head === 'WORKTREE') { + return undefined; + } + return `${repositoryUrl}/compare/${encodeURIComponent(base)}...${encodeURIComponent(head)}`; +} + function build() { const localWorkspace = tryRepo(['rev-parse', '--is-inside-work-tree']) === 'true'; @@ -887,6 +939,13 @@ function build() { file.status === 'deleted' ? target.base : target.head, file.path, ); + const comparisonUrl = githubComparisonUrl( + target.comparisonCommitsOnRemote + ? target.sourceRepositoryUrl + : undefined, + target.base, + target.head, + ); return { path: file.path, ...(file.oldPath ? { oldPath: file.oldPath } : {}), @@ -899,6 +958,7 @@ function build() { patch: textPatch, snippet: binary ? '' : compactSnippet(textPatch), ...(sourceUrl ? { sourceUrl } : {}), + ...(comparisonUrl ? { comparisonUrl } : {}), }; }); diff --git a/tests/remote-targets.test.mjs b/tests/remote-targets.test.mjs index ee3b164..1f82b11 100644 --- a/tests/remote-targets.test.mjs +++ b/tests/remote-targets.test.mjs @@ -23,9 +23,38 @@ function run(repo, args, options = {}) { ); } +async function proxyRemote(fixture, remoteUrl) { + const bin = join(fixture.root, "git-proxy"); + const proxy = join(bin, "git"); + await mkdir(bin); + await writeFile( + proxy, + `#!/usr/bin/env node +const { spawnSync } = require("node:child_process"); +const args = process.argv.slice(2).map((arg) => + arg === ${JSON.stringify(remoteUrl)} + ? ${JSON.stringify(fixture.remote)} + : arg +); +const result = spawnSync("git", args, { + env: { ...process.env, PATH: process.env.DIFFSPLAIN_REAL_PATH }, + stdio: "inherit", +}); +process.exit(result.status ?? 1); +`, + ); + await chmod(proxy, 0o755); + return { + ...process.env, + DIFFSPLAIN_REAL_PATH: process.env.PATH, + PATH: `${bin}:${process.env.PATH}`, + }; +} + async function makeRemoteRepo() { const root = await mkdtemp(join(tmpdir(), "diffsplain-remote-")); - const remote = join(root, "origin.git"); + await mkdir(join(root, "example")); + const remote = join(root, "example", "diffsplain.git"); const repo = join(root, "checkout"); execFileSync("git", ["init", "--bare", "-q", remote]); execFileSync("git", ["clone", "-q", remote, repo]); @@ -141,8 +170,17 @@ test("names worktree-only checkout changes without comparing a branch to itself" try { await writeFile(join(fixture.repo, "working.txt"), "working tree work\n"); + const githubRemote = "https://github.com/example/diffsplain.git"; + const env = await proxyRemote(fixture, githubRemote); + git( + fixture.repo, + "remote", + "set-url", + "origin", + githubRemote, + ); - run(fixture.repo, ["--checkout", "--output", output]); + run(fixture.repo, ["--checkout", "--output", output], { env }); const payload = JSON.parse(await readFile(output, "utf8")); assert.equal(payload.repo.base, payload.repo.head); @@ -152,15 +190,34 @@ test("names worktree-only checkout changes without comparing a branch to itself" payload.change.summary, "Shows staged, unstaged, and untracked changes in the current checkout.", ); + assert.ok( + payload.files.every((file) => file.comparisonUrl === undefined), + "uncommitted work must not link to a commit-only comparison", + ); git(fixture.repo, "add", "working.txt"); git(fixture.repo, "commit", "-qm", "local main work"); - run(fixture.repo, ["--checkout", "--output", output]); + run(fixture.repo, ["--checkout", "--output", output], { env }); const committed = JSON.parse(await readFile(output, "utf8")); assert.notEqual(committed.repo.base, committed.repo.head); assert.equal(committed.change.title, "Local changes on main"); + assert.ok( + committed.files.every((file) => file.comparisonUrl === undefined), + "local-only commits must not link to a remote comparison", + ); + + git(fixture.repo, "remote", "set-url", "origin", fixture.remote); + git(fixture.repo, "push", "-q", "origin", "HEAD:refs/heads/local-main"); + git(fixture.repo, "remote", "set-url", "origin", githubRemote); + run(fixture.repo, ["--checkout", "--output", output], { env }); + const pushed = JSON.parse(await readFile(output, "utf8")); + + assert.match( + pushed.files[0].comparisonUrl, + /^https:\/\/github\.com\/example\/diffsplain\/compare\//, + ); } finally { await rm(fixture.root, { recursive: true, force: true }); } @@ -199,6 +256,128 @@ test("builds a remote repo target without a local checkout", async () => { } }); +test("renders uncommon range entries with the right content and GitHub links", async () => { + const fixture = await makeRemoteRepo(); + const output = join(fixture.root, "uncommon-range.json"); + + try { + await writeFile(join(fixture.repo, "deleted.txt"), "remove me\n"); + await writeFile(join(fixture.repo, "moved-from.txt"), "move me\n"); + await writeFile(join(fixture.repo, "changed.bin"), Buffer.from([0, 1])); + await writeFile( + join(fixture.repo, "long.txt"), + Array.from({ length: 240 }, (_, index) => `before ${index}\n`).join(""), + ); + git(fixture.repo, "add", "."); + git(fixture.repo, "commit", "-qm", "uncommon base"); + const base = git(fixture.repo, "rev-parse", "HEAD"); + + await writeFile(join(fixture.repo, "changed.bin"), Buffer.from([0, 2])); + await writeFile(join(fixture.repo, "added.bin"), Buffer.from([0, 4])); + await rm(join(fixture.repo, "deleted.txt")); + git(fixture.repo, "mv", "moved-from.txt", "moved-to.txt"); + await writeFile( + join(fixture.repo, "long.txt"), + Array.from({ length: 240 }, (_, index) => `after ${index}\n`).join(""), + ); + git(fixture.repo, "add", "."); + git(fixture.repo, "commit", "-qm", "uncommon changes"); + const head = git(fixture.repo, "rev-parse", "HEAD"); + const githubRemote = "https://github.com/example/diffsplain.git"; + const env = await proxyRemote(fixture, githubRemote); + git(fixture.repo, "remote", "set-url", "origin", githubRemote); + const beforeLocalOnly = checkoutState(fixture.repo); + + run( + fixture.repo, + ["--base", base, "--head", head, "--output", output], + { env }, + ); + const localOnly = JSON.parse(await readFile(output, "utf8")); + + assert.ok( + localOnly.files.every((file) => file.comparisonUrl === undefined), + "local-only ranges must not link to a remote comparison", + ); + assert.deepEqual(checkoutState(fixture.repo), beforeLocalOnly); + + git(fixture.repo, "remote", "set-url", "origin", fixture.remote); + git(fixture.repo, "push", "-q", "origin", "HEAD:refs/heads/uncommon"); + git(fixture.repo, "remote", "set-url", "origin", githubRemote); + const before = checkoutState(fixture.repo); + + run( + fixture.repo, + ["--base", base, "--head", head, "--output", output], + { env }, + ); + const payload = JSON.parse(await readFile(output, "utf8")); + const files = Object.fromEntries(payload.files.map((file) => [file.path, file])); + const source = (ref, path) => + `https://github.com/example/diffsplain/blob/${ref}/${path}`; + const comparison = `https://github.com/example/diffsplain/compare/${base}...${head}`; + + assert.deepEqual( + payload.files.map((file) => file.path), + ["added.bin", "changed.bin", "deleted.txt", "long.txt", "moved-to.txt"], + ); + assert.equal(files["added.bin"].status, "binary"); + assert.equal(files["added.bin"].isBinary, true); + assert.equal(files["added.bin"].patch, ""); + assert.equal(files["added.bin"].sourceUrl, source(head, "added.bin")); + assert.equal(files["added.bin"].comparisonUrl, comparison); + assert.equal(files["changed.bin"].status, "binary"); + assert.equal(files["changed.bin"].isBinary, true); + assert.equal(files["changed.bin"].patch, ""); + assert.equal(files["changed.bin"].sourceUrl, source(head, "changed.bin")); + assert.equal(files["changed.bin"].comparisonUrl, comparison); + assert.equal(files["deleted.txt"].status, "deleted"); + assert.equal(files["deleted.txt"].isBinary, false); + assert.match(files["deleted.txt"].patch, /-remove me/); + assert.equal(files["deleted.txt"].sourceUrl, source(base, "deleted.txt")); + assert.equal(files["deleted.txt"].comparisonUrl, comparison); + assert.equal(files["moved-to.txt"].status, "renamed"); + assert.equal(files["moved-to.txt"].oldPath, "moved-from.txt"); + assert.match(files["moved-to.txt"].patch, /similarity index 100%/); + assert.equal(files["moved-to.txt"].sourceUrl, source(head, "moved-to.txt")); + assert.equal(files["moved-to.txt"].comparisonUrl, comparison); + assert.equal(files["long.txt"].status, "modified"); + assert.equal(files["long.txt"].isBinary, false); + assert.equal(files["long.txt"].isTruncated, true); + assert.ok(files["long.txt"].snippet.split("\n").length <= 180); + assert.match(files["long.txt"].snippet, /^@@ /m); + assert.equal(files["long.txt"].sourceUrl, source(head, "long.txt")); + assert.equal(files["long.txt"].comparisonUrl, comparison); + assert.deepEqual(checkoutState(fixture.repo), before); + } finally { + await rm(fixture.root, { recursive: true, force: true }); + } +}); + +test("keeps links out of worktree entries and leaves the checkout untouched", async () => { + const fixture = await makeRemoteRepo(); + const output = join(fixture.root, "uncommon-worktree.json"); + + try { + await writeFile(join(fixture.repo, "worktree.bin"), Buffer.from([0, 1])); + const before = checkoutState(fixture.repo); + + run(fixture.repo, ["--worktree", "--output", output]); + const payload = JSON.parse(await readFile(output, "utf8")); + const [file] = payload.files; + + assert.equal(file.path, "worktree.bin"); + assert.equal(file.status, "binary"); + assert.equal(file.isBinary, true); + assert.equal(file.patch, ""); + assert.equal(file.sourceUrl, undefined); + assert.equal(file.comparisonUrl, undefined); + assert.deepEqual(checkoutState(fixture.repo), before); + } finally { + await rm(fixture.root, { recursive: true, force: true }); + } +}); + test("builds a pull request range through gh without changing the checkout", async () => { const fixture = await makeRemoteRepo(); const bin = join(fixture.root, "bin");