Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .changeset/ninety-friends-count.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
---

Refactor VCS adapter dispatch to use operation maps without a release note.
6 changes: 3 additions & 3 deletions src/core/config.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import fs from "node:fs";
import { join } from "node:path";
import { resolveGlobalConfigPath } from "./paths";
import { detectVcs, findVcsRepoRootCandidate, isVcsId } from "./vcs";
import { detectVcs, findVcsRepoRootCandidate, getDefaultVcsAdapter, isVcsId } from "./vcs";
import type {
CliInput,
CommonOptions,
Expand Down Expand Up @@ -285,7 +285,7 @@ function resolveConfigLayer(source: Record<string, unknown>, input: CliInput): C

/** Choose the VCS backend that best matches the discovered checkout. */
function detectRepoVcsMode(cwd: string): VcsMode {
return detectVcs(cwd)?.id ?? "git";
return detectVcs(cwd)?.id ?? getDefaultVcsAdapter().id;
}

/** Parse one TOML config file into a plain object. */
Expand Down Expand Up @@ -349,7 +349,7 @@ export function resolveConfiguredCliInput(
pager: input.options.pager ?? false,
watch: input.options.watch ?? resolvedOptions.watch ?? false,
excludeUntracked: resolvedOptions.excludeUntracked ?? false,
vcs: resolvedOptions.vcs ?? "git",
vcs: resolvedOptions.vcs ?? getDefaultVcsAdapter().id,
mode: resolvedOptions.mode ?? DEFAULT_VIEW_PREFERENCES.mode,
lineNumbers: resolvedOptions.lineNumbers ?? DEFAULT_VIEW_PREFERENCES.showLineNumbers,
wrapLines: resolvedOptions.wrapLines ?? DEFAULT_VIEW_PREFERENCES.wrapLines,
Expand Down
15 changes: 8 additions & 7 deletions src/core/fileSource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { createFileSourceFetcher, SourceTextTooLargeError } from "./fileSource";
import { createGitFileSourceFetcher } from "./vcs/gitSource";

const tempDirs: string[] = [];

Expand Down Expand Up @@ -127,7 +128,7 @@ describe("createFileSourceFetcher", () => {
git(repoRoot, "add", ".");
git(repoRoot, "commit", "-m", "second");

const fetcher = createFileSourceFetcher({
const fetcher = createGitFileSourceFetcher({
old: { kind: "git-blob", repoRoot, ref: "HEAD~1", path: filePath },
new: { kind: "git-blob", repoRoot, ref: "HEAD", path: filePath },
});
Expand All @@ -147,7 +148,7 @@ describe("createFileSourceFetcher", () => {
git(repoRoot, "add", filePath);
writeFileSync(join(repoRoot, filePath), "working tree\n");

const fetcher = createFileSourceFetcher({
const fetcher = createGitFileSourceFetcher({
old: { kind: "git-index", repoRoot, path: filePath },
new: { kind: "fs", absolutePath: join(repoRoot, filePath) },
});
Expand All @@ -166,7 +167,7 @@ describe("createFileSourceFetcher", () => {
writeFileSync(join(repoRoot, filePath), "staged source\n");
git(repoRoot, "add", filePath);

const fetcher = createFileSourceFetcher(
const fetcher = createGitFileSourceFetcher(
{
old: { kind: "git-blob", repoRoot, ref: "HEAD", path: filePath },
new: { kind: "git-index", repoRoot, path: filePath },
Expand Down Expand Up @@ -197,7 +198,7 @@ describe("createFileSourceFetcher", () => {
)) as typeof Bun.spawn;

try {
const fetcher = createFileSourceFetcher({
const fetcher = createGitFileSourceFetcher({
old: { kind: "git-blob", repoRoot: process.cwd(), ref: "HEAD", path: "note.txt" },
new: { kind: "none" },
});
Expand Down Expand Up @@ -235,7 +236,7 @@ describe("createFileSourceFetcher", () => {
}) as typeof Bun.spawn;

try {
const fetcher = createFileSourceFetcher(
const fetcher = createGitFileSourceFetcher(
{
old: { kind: "git-blob", repoRoot: process.cwd(), ref: "HEAD", path: "note.txt" },
new: { kind: "git-index", repoRoot: process.cwd(), path: "note.txt" },
Expand All @@ -261,7 +262,7 @@ describe("createFileSourceFetcher", () => {
git(repoRoot, "add", ".");
git(repoRoot, "commit", "-m", "first");

const fetcher = createFileSourceFetcher({
const fetcher = createGitFileSourceFetcher({
old: { kind: "git-blob", repoRoot, ref: "HEAD", path: "missing-from-history.txt" },
new: { kind: "none" },
});
Expand All @@ -274,7 +275,7 @@ describe("createFileSourceFetcher", () => {

test("logs unexpected git source failures with object context", async () => {
const repoRoot = createTempDir("hunk-source-git-not-repo-");
const fetcher = createFileSourceFetcher({
const fetcher = createGitFileSourceFetcher({
old: { kind: "git-blob", repoRoot, ref: "HEAD", path: "note.txt" },
new: { kind: "none" },
});
Expand Down
136 changes: 14 additions & 122 deletions src/core/fileSource.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,22 @@
/**
* Low-level readers for full file contents used by input and VCS adapters.
* Generic full-file source fetcher primitives used by input loaders and VCS adapters.
*
* Each `DiffFile` may carry a `FileSourceFetcher` that knows how to read the
* file's "old" and "new" sides without re-running the original diff. VCS
* adapters own VCS-specific source-spec construction; this module only executes
* the concrete reads and returns `null` when source content is unreachable.
* file's "old" and "new" sides without re-running the original diff. Provider-
* specific object reads live beside their VCS adapters; this module only owns
* provider-neutral fetcher contracts and filesystem reads.
*/

export type FileSourceSpec =
| { kind: "none" }
| { kind: "fs"; absolutePath: string }
| { kind: "git-blob"; repoRoot: string; ref: string; path: string }
| { kind: "git-index"; repoRoot: string; path: string };
export type FileSourceSpec = { kind: "none" } | { kind: "fs"; absolutePath: string };

export type FileSourceSide = "old" | "new";

export interface FileSourceFetcher {
/**
* Returns the file's full source text on the requested side, or `null` when
* the side is not reachable (deleted side, missing path, git error). Built-in
* fetchers resolve `null` instead of rejecting, but UI callers still handle
* custom fetcher rejection defensively.
* the side is not reachable (deleted side, missing path, provider error).
* Built-in fetchers resolve `null` instead of rejecting, but UI callers still
* handle custom fetcher rejection defensively.
*/
getFullText(side: FileSourceSide): Promise<string | null>;
}
Expand All @@ -36,7 +32,6 @@ export class SourceTextTooLargeError extends Error {
}

export interface FileSourceFetcherOptions {
gitExecutable?: string;
maxSourceBytes?: number;
}

Expand All @@ -54,7 +49,7 @@ function firstDiagnosticLine(text: string) {
}

/** Keep source-load diagnostics terse enough to be useful in logs. */
function logSourceDiagnostic(message: string, detail?: unknown) {
export function logSourceDiagnostic(message: string, detail?: unknown) {
if (detail instanceof Error) {
console.error(`hunk: ${message}: ${detail.message}`, detail);
return;
Expand All @@ -64,18 +59,6 @@ function logSourceDiagnostic(message: string, detail?: unknown) {
console.error(detailText ? `hunk: ${message}: ${detailText}` : `hunk: ${message}`);
}

/** Return whether a Git failure is an expected missing source side/path. */
function isExpectedMissingGitSource(stderr: string) {
const normalized = stderr.toLowerCase();
return [
"exists on disk, but not in",
"does not exist in",
"invalid object name",
"needed a single revision",
"unknown revision or path not in the working tree",
].some((fragment) => normalized.includes(fragment));
}

async function readFsSpec(
spec: Extract<FileSourceSpec, { kind: "fs" }>,
maxSourceBytes: number,
Expand All @@ -101,28 +84,7 @@ async function readFsSpec(
}
}

function readGitBlobSpec(
spec: Extract<FileSourceSpec, { kind: "git-blob" }>,
gitExecutable = "git",
maxSourceBytes: number,
): Promise<string | null> {
return readGitObjectSpec(
spec.repoRoot,
`${spec.ref}:${spec.path}`,
gitExecutable,
maxSourceBytes,
);
}

function readGitIndexSpec(
spec: Extract<FileSourceSpec, { kind: "git-index" }>,
gitExecutable = "git",
maxSourceBytes: number,
): Promise<string | null> {
return readGitObjectSpec(spec.repoRoot, `:${spec.path}`, gitExecutable, maxSourceBytes);
}

async function readStreamTextWithLimit(
export async function readStreamTextWithLimit(
stream: ReadableStream<Uint8Array> | null,
maxBytes: number,
onTooLarge?: () => void,
Expand Down Expand Up @@ -163,91 +125,21 @@ async function readStreamTextWithLimit(
return new TextDecoder().decode(combined);
}

/** Read a blob-like Git object spec such as `HEAD:path` or `:path`. */
async function readGitObjectSpec(
repoRoot: string,
objectName: string,
gitExecutable = "git",
maxSourceBytes: number,
): Promise<string | null> {
let proc: Bun.ReadableSubprocess;

try {
proc = Bun.spawn([gitExecutable, "show", objectName], {
cwd: repoRoot,
stdin: "ignore",
stdout: "pipe",
stderr: "pipe",
});
} catch (error) {
logSourceDiagnostic(`failed to run Git while reading source ${objectName}`, error);
return null;
}

let output: [number, string, string];
try {
output = await Promise.all([
proc.exited,
readStreamTextWithLimit(proc.stdout, maxSourceBytes, () => proc.kill()),
readStreamTextWithLimit(
proc.stderr,
64 * 1024,
undefined,
(maxBytes) => new Error(`Git source diagnostics exceeded ${maxBytes} bytes.`),
),
]);
} catch (error) {
if (error instanceof SourceTextTooLargeError) {
proc.kill();
await proc.exited.catch(() => undefined);
throw error;
}

logSourceDiagnostic(`failed to collect Git source ${objectName}`, error);
return null;
}

const [exitCode, stdout, stderr] = output;

if (exitCode !== 0) {
if (!isExpectedMissingGitSource(stderr)) {
logSourceDiagnostic(`failed to read Git source ${objectName} in ${repoRoot}`, stderr);
}
return null;
}

return stdout;
}

async function readSpec(
spec: FileSourceSpec,
{
gitExecutable = "git",
maxSourceBytes = DEFAULT_SOURCE_TEXT_MAX_BYTES,
}: FileSourceFetcherOptions = {},
{ maxSourceBytes = DEFAULT_SOURCE_TEXT_MAX_BYTES }: FileSourceFetcherOptions = {},
): Promise<string | null> {
if (spec.kind === "none") {
return null;
}

if (spec.kind === "fs") {
return readFsSpec(spec, maxSourceBytes);
}

if (spec.kind === "git-index") {
return readGitIndexSpec(spec, gitExecutable, maxSourceBytes);
}

return readGitBlobSpec(spec, gitExecutable, maxSourceBytes);
return readFsSpec(spec, maxSourceBytes);
}

/** Build a per-file source fetcher that caches each side's resolved text. */
export function createFileSourceFetcher(
specs: ResolvedSpecs,
{
gitExecutable = "git",
maxSourceBytes = DEFAULT_SOURCE_TEXT_MAX_BYTES,
}: Readonly<FileSourceFetcherOptions> = {},
{ maxSourceBytes = DEFAULT_SOURCE_TEXT_MAX_BYTES }: Readonly<FileSourceFetcherOptions> = {},
): FileSourceFetcher {
const cache = new Map<FileSourceSide, string | null>();

Expand All @@ -257,7 +149,7 @@ export function createFileSourceFetcher(
return cache.get(side) ?? null;
}

const text = await readSpec(specs[side], { gitExecutable, maxSourceBytes });
const text = await readSpec(specs[side], { maxSourceBytes });
cache.set(side, text);
return text;
},
Expand Down
4 changes: 2 additions & 2 deletions src/core/git.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
resolveGitDiffEndpoints,
runGitText,
} from "./git";
import type { VcsCommandInput } from "./types";
import type { VcsDiffCommandInput } from "./types";

const tempDirs: string[] = [];

Expand Down Expand Up @@ -39,7 +39,7 @@ function createTempRepo(prefix: string) {
return dir;
}

function makeGitInput(overrides: Partial<VcsCommandInput> = {}): VcsCommandInput {
function makeGitInput(overrides: Partial<VcsDiffCommandInput> = {}): VcsDiffCommandInput {
return {
kind: "vcs",
staged: false,
Expand Down
Loading