From 968e09f0b50d17f7c591baa977666b991a1e59b7 Mon Sep 17 00:00:00 2001 From: Taylor Mullen Date: Thu, 15 May 2025 23:51:53 -0700 Subject: fix: Ensure filename is available for diff rendering in write-file This commit resolves a bug where the `write-file` operation could fail to render content due to a missing filename. The fix involves: - Ensuring `fileName` is consistently passed to `DiffRenderer.tsx` through `ToolConfirmationMessage.tsx`, `ToolMessage.tsx`, and `useGeminiStream.ts`. - Modifying `edit.ts` and `write-file.ts` to include `fileName` in the `FileDiff` object. - Expanding the `FileDiff` interface in `tools.ts` to include `fileName`. Additionally, this commit enhances the diff rendering by: - Adding syntax highlighting based on file extension in `DiffRenderer.tsx`. - Adding more language mappings to `getLanguageFromExtension` in `DiffRenderer.tsx`. - Added lots of tests for all the above. Fixes https://b.corp.google.com/issues/418125982 --- .../ui/components/messages/DiffRenderer.test.tsx | 117 +++++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 packages/cli/src/ui/components/messages/DiffRenderer.test.tsx (limited to 'packages/cli/src/ui/components/messages/DiffRenderer.test.tsx') diff --git a/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx b/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx new file mode 100644 index 00000000..335ee20a --- /dev/null +++ b/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx @@ -0,0 +1,117 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { render } from 'ink-testing-library'; +import { DiffRenderer } from './DiffRenderer.js'; +import * as CodeColorizer from '../../utils/CodeColorizer.js'; +import { vi } from 'vitest'; + +describe('', () => { + const mockColorizeCode = vi.spyOn(CodeColorizer, 'colorizeCode'); + + beforeEach(() => { + mockColorizeCode.mockClear(); + }); + + it('should call colorizeCode with correct language for new file with known extension', () => { + const newFileDiffContent = ` +diff --git a/test.py b/test.py +new file mode 100644 +index 0000000..e69de29 +--- /dev/null ++++ b/test.py +@@ -0,0 +1 @@ ++print("hello world") +`; + render( + , + ); + expect(mockColorizeCode).toHaveBeenCalledWith( + 'print("hello world")', + 'python', + ); + }); + + it('should call colorizeCode with null language for new file with unknown extension', () => { + const newFileDiffContent = ` +diff --git a/test.unknown b/test.unknown +new file mode 100644 +index 0000000..e69de29 +--- /dev/null ++++ b/test.unknown +@@ -0,0 +1 @@ ++some content +`; + render( + , + ); + expect(mockColorizeCode).toHaveBeenCalledWith('some content', null); + }); + + it('should call colorizeCode with null language for new file if no filename is provided', () => { + const newFileDiffContent = ` +diff --git a/test.txt b/test.txt +new file mode 100644 +index 0000000..e69de29 +--- /dev/null ++++ b/test.txt +@@ -0,0 +1 @@ ++some text content +`; + render(); + expect(mockColorizeCode).toHaveBeenCalledWith('some text content', null); + }); + + it('should render diff content for existing file (not calling colorizeCode directly for the whole block)', () => { + const existingFileDiffContent = ` +diff --git a/test.txt b/test.txt +index 0000001..0000002 100644 +--- a/test.txt ++++ b/test.txt +@@ -1 +1 @@ +-old line ++new line +`; + const { lastFrame } = render( + , + ); + // colorizeCode is used internally by the line-by-line rendering, not for the whole block + expect(mockColorizeCode).not.toHaveBeenCalledWith( + expect.stringContaining('old line'), + expect.anything(), + ); + expect(mockColorizeCode).not.toHaveBeenCalledWith( + expect.stringContaining('new line'), + expect.anything(), + ); + const output = lastFrame(); + const lines = output!.split('\n'); + expect(lines[0]).toBe('1 - old line'); + expect(lines[1]).toBe('1 + new line'); + }); + + it('should handle diff with only header and no changes', () => { + const noChangeDiff = `diff --git a/file.txt b/file.txt +index 1234567..1234567 100644 +--- a/file.txt ++++ b/file.txt +`; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toContain('No changes detected'); + expect(mockColorizeCode).not.toHaveBeenCalled(); + }); + + it('should handle empty diff content', () => { + const { lastFrame } = render(); + expect(lastFrame()).toContain('No diff content'); + expect(mockColorizeCode).not.toHaveBeenCalled(); + }); +}); -- cgit v1.2.3