diff --git a/.changeset/git-line-ending-normalization.md b/.changeset/git-line-ending-normalization.md new file mode 100644 index 000000000..1e5ab2b9f --- /dev/null +++ b/.changeset/git-line-ending-normalization.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Respect Git line-ending normalization when reviewing working-tree changes. diff --git a/src/extensions/default/vcs/git/commands.test.ts b/src/extensions/default/vcs/git/commands.test.ts index ecadf4c0b..21c986670 100644 --- a/src/extensions/default/vcs/git/commands.test.ts +++ b/src/extensions/default/vcs/git/commands.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { buildGitDiffArgs, + buildGitDiffNumstatArgs, buildGitIgnoredDirectoryArgs, buildGitStashShowArgs, buildGitStatusArgs, @@ -14,6 +15,7 @@ import { resolveGitMetadata, runGitText, shouldSkipLargeTrackedDiff, + type GitDiffEndpoints, } from "./commands"; import type { ExtensionVcsDiffInput as VcsDiffCommandInput } from "hunkdiff/extension"; @@ -97,6 +99,51 @@ describe("git command helpers", () => { expect(buildGitDiffArgs(makeGitInput())).toContain("core.quotePath=true"); }); + test("ignores CR at EOL only when the resolved new endpoint is the worktree", () => { + const worktreeEndpoints = { + old: { kind: "index" }, + new: { kind: "worktree" }, + } satisfies GitDiffEndpoints; + const singleRefWorktreeEndpoints = { + old: { kind: "git-ref", ref: "old-ref" }, + new: { kind: "worktree" }, + } satisfies GitDiffEndpoints; + const storedComparisons = [ + { + input: makeGitInput({ staged: true }), + endpoints: { + old: { kind: "git-ref", ref: "old-ref" }, + new: { kind: "index" }, + }, + }, + { + input: makeGitInput({ range: "old-ref..new-ref" }), + endpoints: { + old: { kind: "git-ref", ref: "old-ref" }, + new: { kind: "git-ref", ref: "new-ref" }, + }, + }, + ] satisfies { input: VcsDiffCommandInput; endpoints: GitDiffEndpoints }[]; + + expect(buildGitDiffArgs(makeGitInput(), [], null, worktreeEndpoints)).toContain( + "--ignore-cr-at-eol", + ); + expect( + buildGitDiffArgs(makeGitInput({ range: "old-ref" }), [], null, singleRefWorktreeEndpoints), + ).toContain("--ignore-cr-at-eol"); + expect(buildGitDiffNumstatArgs(makeGitInput(), worktreeEndpoints)).toContain( + "--ignore-cr-at-eol", + ); + expect( + buildGitDiffNumstatArgs(makeGitInput({ range: "old-ref" }), singleRefWorktreeEndpoints), + ).toContain("--ignore-cr-at-eol"); + + for (const { input, endpoints } of storedComparisons) { + expect(buildGitDiffArgs(input, [], null, endpoints)).not.toContain("--ignore-cr-at-eol"); + expect(buildGitDiffNumstatArgs(input, endpoints)).not.toContain("--ignore-cr-at-eol"); + } + }); + test("disables external diff tools for stash patches", () => { const args = buildGitStashShowArgs({ kind: "stash-show", diff --git a/src/extensions/default/vcs/git/commands.ts b/src/extensions/default/vcs/git/commands.ts index 3526c7eee..023a22f21 100644 --- a/src/extensions/default/vcs/git/commands.ts +++ b/src/extensions/default/vcs/git/commands.ts @@ -122,9 +122,12 @@ export function buildGitDiffArgs( input: ExtensionVcsDiffInput, excludedPathspecs: string[] = [], colorMoved: GitColorMovedOptions | null = null, + endpoints: GitDiffEndpoints | null = null, ) { const args = ["diff", "--no-ext-diff", "--find-renames", ...gitPatchColorArgs(colorMoved)]; + appendGitLineEndingComparisonArgs(args, endpoints); + if (input.staged) { args.push("--staged"); } @@ -147,9 +150,14 @@ export function buildGitDiffArgs( } /** Build the cheap tracked-file stats query used to skip huge file diffs before patch output. */ -export function buildGitDiffNumstatArgs(input: ExtensionVcsDiffInput) { +export function buildGitDiffNumstatArgs( + input: ExtensionVcsDiffInput, + endpoints: GitDiffEndpoints | null = null, +) { const args = ["diff", "--no-ext-diff", "--find-renames", "--no-color", "--numstat", "-z"]; + appendGitLineEndingComparisonArgs(args, endpoints); + if (input.staged) { args.push("--staged"); } @@ -797,6 +805,13 @@ export interface GitDiffEndpoints { new: GitDiffEndpoint; } +/** Apply Git's CR-at-EOL tolerance only when the comparison reads the live worktree. */ +function appendGitLineEndingComparisonArgs(args: string[], endpoints: GitDiffEndpoints | null) { + if (endpoints?.new.kind === "worktree") { + args.push("--ignore-cr-at-eol"); + } +} + /** Parse "A...B" into its two refs, defaulting empty sides to HEAD as Git does. */ function parseSymmetricDiffRange(range: string): { left: string; right: string } | null { // Runs of four or more dots are not a valid range; bail rather than diff --git a/src/extensions/default/vcs/git/index.test.ts b/src/extensions/default/vcs/git/index.test.ts index 0c2af3f45..2bd8e48a7 100644 --- a/src/extensions/default/vcs/git/index.test.ts +++ b/src/extensions/default/vcs/git/index.test.ts @@ -43,6 +43,11 @@ function normalizeComparablePath(path: string) { return resolvedPath.replace(/\\/g, "/"); } +/** Remove Git's optional moved-line colors before asserting patch text semantics. */ +function stripAnsi(text: string) { + return text.replace(/\x1b\[[0-?]*[ -/]*[@-~]/g, ""); +} + function git(cwd: string, ...cmd: string[]) { const proc = Bun.spawnSync(["git", ...cmd], { cwd, @@ -131,6 +136,51 @@ describe("GitVcsAdapter", () => { expect(result.extraFiles ?? []).toHaveLength(0); }); + test("loads one-line edits from a CRLF checkout without replacing the whole file", async () => { + const repo = createTempRepo("hunk-git-adapter-crlf-"); + git(repo, "config", "core.autocrlf", "false"); + writeFileSync(join(repo, ".gitattributes"), "*.ts text eol=crlf\n"); + const originalLines = Array.from( + { length: 10_001 }, + (_, index) => `export const value${index} = ${index};`, + ); + writeFileSync(join(repo, "example.ts"), `${originalLines.join("\n")}\n`); + git(repo, "add", ".gitattributes", "example.ts"); + git(repo, "commit", "-m", "initial"); + + // Make Git materialize the committed attributes, then preserve CRLF while editing one line. + writeFileSync(join(repo, "example.ts"), "dirty\n"); + git(repo, "checkout", "--", "example.ts"); + + const input = { + kind: "vcs", + staged: false, + options: {}, + } satisfies ExtensionVcsDiffInput; + const operation = GitVcsAdapter.operations["working-tree-diff"]!; + expect(operation.watchSignature!(input, { cwd: repo })).toBe(""); + + const changedLines = [...originalLines]; + changedLines[5_000] = "export const value5000 = 50_000;"; + writeFileSync(join(repo, "example.ts"), `${changedLines.join("\r\n")}\r\n`); + + const result = await operation.load(input, { cwd: repo }); + const plainPatch = stripAnsi(result.patchText); + const patchLines = plainPatch.split("\n"); + const stats = { + additions: patchLines.filter((line) => line.startsWith("+") && !line.startsWith("+++")) + .length, + deletions: patchLines.filter((line) => line.startsWith("-") && !line.startsWith("---")) + .length, + }; + + expect(stats).toEqual({ additions: 1, deletions: 1 }); + expect(result.extraFiles ?? []).toHaveLength(0); + expect(plainPatch).toContain("-export const value5000 = 5000;"); + expect(plainPatch).toContain("+export const value5000 = 50_000;"); + expect(operation.watchSignature!(input, { cwd: repo })).toBe(plainPatch); + }); + test("loads revision and stash patches through adapter operations", async () => { const repo = createTempRepo("hunk-git-adapter-show-"); writeFileSync(join(repo, "file.txt"), "one\n"); diff --git a/src/extensions/default/vcs/git/index.ts b/src/extensions/default/vcs/git/index.ts index dca2453b2..776c30ab8 100644 --- a/src/extensions/default/vcs/git/index.ts +++ b/src/extensions/default/vcs/git/index.ts @@ -178,10 +178,9 @@ function createGitRevisionSourceCapability( function createGitDiffSourceCapability( input: ExtensionVcsDiffInput, repoRoot: string, - cwd: string, + endpoints: GitDiffEndpoints | null, gitExecutable: string, ): GitSourceCapability | undefined { - const endpoints = resolveGitDiffEndpoints(input, { cwd, repoRoot, gitExecutable }); return endpoints ? createGitSourceCapability(input, repoRoot, endpoints, gitExecutable) : undefined; @@ -282,6 +281,7 @@ export function createGitVcsAdapter({ "working-tree-diff": { async load(input, { cwd }) { const repoRoot = resolveGitRepoRoot(input, { cwd, gitExecutable }); + const endpoints = resolveGitDiffEndpoints(input, { cwd, repoRoot, gitExecutable }); const repoName = basename(repoRoot); const title = input.staged ? `${repoName} staged changes` @@ -291,13 +291,18 @@ export function createGitVcsAdapter({ // Ask for stats before the patch so files too large to render can be // excluded from the diff instead of generating output nobody reads. const largeTrackedFiles = parseGitNumstat( - runGitText({ input, args: buildGitDiffNumstatArgs(input), cwd, gitExecutable }), + runGitText({ + input, + args: buildGitDiffNumstatArgs(input, endpoints), + cwd, + gitExecutable, + }), ).filter((file) => shouldSkipLargeTrackedDiff(file, repoRoot)); const colorMoved = resolveGitColorMovedOptions(input, { cwd, gitExecutable }); const sourceCapability = createGitDiffSourceCapability( input, repoRoot, - cwd, + endpoints, gitExecutable, ); @@ -311,6 +316,7 @@ export function createGitVcsAdapter({ input, largeTrackedFiles.map((file) => file.path), colorMoved, + endpoints, ), cwd, gitExecutable, @@ -336,14 +342,15 @@ export function createGitVcsAdapter({ return buildGitWatchPlan(input, cwd, gitExecutable); }, watchSignature(input, { cwd }) { - const trackedPatch = runGitText({ - input, - args: buildGitDiffArgs(input), + const repoRoot = resolveGitRepoRoot(input, { cwd, gitExecutable, preventOptionalLocks: true, }); - const repoRoot = resolveGitRepoRoot(input, { + const endpoints = resolveGitDiffEndpoints(input, { cwd, repoRoot, gitExecutable }); + const trackedPatch = runGitText({ + input, + args: buildGitDiffArgs(input, [], null, endpoints), cwd, gitExecutable, preventOptionalLocks: true,