diff --git a/src/extension/prompt/vscode-node/gitDiffService.ts b/src/extension/prompt/vscode-node/gitDiffService.ts index 4d01592d5e..6a5811126d 100644 --- a/src/extension/prompt/vscode-node/gitDiffService.ts +++ b/src/extension/prompt/vscode-node/gitDiffService.ts @@ -114,22 +114,38 @@ export class GitDiffService implements IGitDiffService { try { const buffer = await workspace.fs.readFile(resource); const relativePath = path.relative(repository.rootUri.fsPath, resource.fsPath); + const content = buffer.toString(); // Header patch.push(`diff --git a/${relativePath} b/${relativePath}`); - - // Add original/modified file paths + // 100644 is standard file mode for new git files. Saves us from trying to check file permissions and handling + // UNIX vs Windows permission differences. Skipping calculating the SHA1 hashes as well since they are not strictly necessary + // to apply the patch. + patch.push('new file mode 100644'); patch.push('--- /dev/null', `+++ b/${relativePath}`); - // Add range header - patch.push(`@@ -0,0 +1,${buffer.length} @@`); - - // Add content - patch.push(...buffer.toString().split('\n').map(line => `+${line}`)); + // For non-empty files, add range header and content (empty files omit this) + if (content.length > 0) { + const lines = content.split('\n'); + if (content.endsWith('\n')) { + // Prevent an extra empty line at the end + lines.pop(); + } + + // Range header and content + patch.push(`@@ -0,0 +1,${lines.length} @@`); + patch.push(...lines.map(line => `+${line}`)); + + // Git standard to add this comment if the file does not end with a newline + if (!content.endsWith('\n')) { + patch.push('\\ No newline at end of file'); + } + } } catch (err) { console.error(err, `Failed to generate patch file for untracked file: ${resource.toString()}`); } - return patch.join('\n'); + // The patch itself should always end with a newline per git patch standards + return patch.join('\n') + '\n'; } } diff --git a/src/extension/prompt/vscode-node/test/gitDiffService.spec.ts b/src/extension/prompt/vscode-node/test/gitDiffService.spec.ts new file mode 100644 index 0000000000..e98dba2a02 --- /dev/null +++ b/src/extension/prompt/vscode-node/test/gitDiffService.spec.ts @@ -0,0 +1,179 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { MockInstance, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as vscode from 'vscode'; +import { IGitExtensionService } from '../../../../platform/git/common/gitExtensionService'; +import { API, Change, Repository } from '../../../../platform/git/vscode/git'; +import { ITestingServicesAccessor } from '../../../../platform/test/node/services'; +import { IInstantiationService } from '../../../../util/vs/platform/instantiation/common/instantiation'; +import { Uri } from '../../../../vscodeTypes'; +import { createExtensionUnitTestingServices } from '../../../test/node/services'; +import { GitDiffService } from '../gitDiffService'; + +describe('GitDiffService', () => { + let readFileSpy: MockInstance; + let accessor: ITestingServicesAccessor; + let gitDiffService: GitDiffService; + let mockRepository: Partial; + + beforeEach(() => { + // Create mock workspace.fs.readFile if it doesn't exist + if (!vscode.workspace?.fs?.readFile) { + const workspaceWithFs = vscode as unknown as { workspace: typeof vscode.workspace }; + workspaceWithFs.workspace = { + ...vscode.workspace, + fs: { + ...vscode.workspace?.fs, + readFile: vi.fn() + } + }; + } + + // Spy on workspace.fs.readFile + readFileSpy = vi.spyOn(vscode.workspace.fs, 'readFile').mockImplementation(() => Promise.resolve(new Uint8Array())); + + mockRepository = { + rootUri: Uri.file('/repo'), + diffWith: vi.fn(), + diffIndexWithHEAD: vi.fn(), + diffWithHEAD: vi.fn() + }; + + const services = createExtensionUnitTestingServices(); + + const mockGitExtensionService = { + getExtensionApi: vi.fn().mockReturnValue({ + getRepository: vi.fn().mockReturnValue(mockRepository), + openRepository: vi.fn(), + repositories: [mockRepository as Repository] + } as unknown as API) + } as unknown as IGitExtensionService; + services.set(IGitExtensionService, mockGitExtensionService); + + accessor = services.createTestingAccessor(); + gitDiffService = accessor.get(IInstantiationService).createInstance(GitDiffService); + }); + + afterEach(() => { + readFileSpy.mockRestore(); + }); + + describe('_getUntrackedChangePatch', () => { + it('should generate correct patch for untracked file', async () => { + const fileUri = Uri.file('/repo/newfile.txt'); + const fileContent = 'line1\nline2\n'; + + readFileSpy.mockResolvedValue(Buffer.from(fileContent)); + + const changes: Change[] = [{ + uri: fileUri, + originalUri: fileUri, + renameUri: undefined, + status: 7 /* UNTRACKED */ + }]; + + const diffs = await gitDiffService.getChangeDiffs(mockRepository as Repository, changes); + + expect(diffs).toHaveLength(1); + const patch = diffs[0].diff; + + // Verify standard git patch headers + expect(patch).toContain('diff --git a/newfile.txt b/newfile.txt'); + expect(patch).toContain('new file mode 100644'); + expect(patch).toContain('--- /dev/null'); + expect(patch).toContain('+++ b/newfile.txt'); + + // Verify range header uses line count (2 lines), not byte length + expect(patch).toContain('@@ -0,0 +1,2 @@'); + + // Verify content + expect(patch).toContain('+line1'); + expect(patch).toContain('+line2'); + + // Verify final newline + expect(patch.endsWith('\n')).toBe(true); + + // Verify no "No newline at end of file" warning since file ends with \n + expect(patch).not.toContain('\\ No newline at end of file'); + }); + + it('should handle file without trailing newline', async () => { + const fileUri = Uri.file('/repo/no-newline.txt'); + const fileContent = 'line1'; // No trailing \n + + readFileSpy.mockResolvedValue(Buffer.from(fileContent)); + + const changes: Change[] = [{ + uri: fileUri, + originalUri: fileUri, + renameUri: undefined, + status: 7 /* UNTRACKED */ + }]; + + const diffs = await gitDiffService.getChangeDiffs(mockRepository as Repository, changes); + const patch = diffs[0].diff; + + expect(patch).toContain('@@ -0,0 +1,1 @@'); + expect(patch).toContain('+line1'); + expect(patch).toContain('\\ No newline at end of file'); + expect(patch.endsWith('\n')).toBe(true); + }); + + it('should handle empty file', async () => { + const fileUri = Uri.file('/repo/empty.txt'); + const fileContent = ''; + + // Mock readFile to return an empty buffer + readFileSpy.mockResolvedValue(Buffer.from(fileContent)); + + const changes: Change[] = [{ + uri: fileUri, + originalUri: fileUri, + renameUri: undefined, + status: 7 /* UNTRACKED */ + }]; + + const diffs = await gitDiffService.getChangeDiffs(mockRepository as Repository, changes); + + // Empty file case: git omits range header and content for totally empty files + const patch = diffs[0].diff; + expect(patch).toContain('diff --git a/empty.txt b/empty.txt'); + expect(patch).toContain('new file mode 100644'); + expect(patch).toContain('--- /dev/null'); + expect(patch).toContain('+++ b/empty.txt'); + // No range header for empty files + expect(patch).not.toContain('@@'); + // No content lines + expect(patch).not.toMatch(/^\+[^+]/m); + }); + + it('should handle file with single blank line', async () => { + const fileUri = Uri.file('/repo/blank-line.txt'); + const fileContent = '\n'; // Single newline + + readFileSpy.mockResolvedValue(Buffer.from(fileContent)); + + const changes: Change[] = [{ + uri: fileUri, + originalUri: fileUri, + renameUri: undefined, + status: 7 /* UNTRACKED */ + }]; + + const diffs = await gitDiffService.getChangeDiffs(mockRepository as Repository, changes); + + // Single blank line: should have range header and one empty line addition + const patch = diffs[0].diff; + expect(patch).toContain('diff --git a/blank-line.txt b/blank-line.txt'); + expect(patch).toContain('new file mode 100644'); + expect(patch).toContain('--- /dev/null'); + expect(patch).toContain('+++ b/blank-line.txt'); + expect(patch).toContain('@@ -0,0 +1,1 @@'); + expect(patch).toContain('+'); // One empty line + expect(patch.endsWith('\n')).toBe(true); + }); + }); +});