diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index b97886d32d..a138e42b47 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -15,6 +15,7 @@ import { Task } from "../../core/task/Task" import { DecorationController } from "./DecorationController" export const DIFF_VIEW_URI_SCHEME = "cline-diff" +export const DIFF_VIEW_LABEL_CHANGES = "Original ↔ Roo's Changes" // TODO: https://github.com/cline/cline/pull/3354 export class DiffViewProvider { @@ -384,12 +385,25 @@ export class DiffViewProvider { private async closeAllDiffViews(): Promise { const closeOps = vscode.window.tabGroups.all .flatMap((group) => group.tabs) - .filter( - (tab) => + .filter((tab) => { + // Check for standard diff views with our URI scheme + if ( tab.input instanceof vscode.TabInputTextDiff && tab.input.original.scheme === DIFF_VIEW_URI_SCHEME && - !tab.isDirty, - ) + !tab.isDirty + ) { + return true + } + + // Also check by tab label for our specific diff views + // This catches cases where the diff view might be created differently + // when files are pre-opened as text documents + if (tab.label.includes(DIFF_VIEW_LABEL_CHANGES) && !tab.isDirty) { + return true + } + + return false + }) .map((tab) => vscode.window.tabGroups.close(tab).then( () => undefined, @@ -487,17 +501,22 @@ export class DiffViewProvider { }), ) - // Execute the diff command - vscode.commands - .executeCommand( - "vscode.diff", - vscode.Uri.parse(`${DIFF_VIEW_URI_SCHEME}:${fileName}`).with({ - query: Buffer.from(this.originalContent ?? "").toString("base64"), - }), - uri, - `${fileName}: ${fileExists ? "Original ↔ Roo's Changes" : "New File"} (Editable)`, - { preserveFocus: true }, - ) + // Pre-open the file as a text document to ensure it doesn't open in preview mode + // This fixes issues with files that have custom editor associations (like markdown preview) + vscode.window + .showTextDocument(uri, { preview: false, viewColumn: vscode.ViewColumn.Active }) + .then(() => { + // Execute the diff command after ensuring the file is open as text + return vscode.commands.executeCommand( + "vscode.diff", + vscode.Uri.parse(`${DIFF_VIEW_URI_SCHEME}:${fileName}`).with({ + query: Buffer.from(this.originalContent ?? "").toString("base64"), + }), + uri, + `${fileName}: ${fileExists ? `${DIFF_VIEW_LABEL_CHANGES}` : "New File"} (Editable)`, + { preserveFocus: true }, + ) + }) .then( () => { // Command executed successfully, now wait for the editor to appear diff --git a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts index aa6e492bcd..44b3aaba2a 100644 --- a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts +++ b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts @@ -1,24 +1,78 @@ -import { DiffViewProvider } from "../DiffViewProvider" +import { DiffViewProvider, DIFF_VIEW_URI_SCHEME, DIFF_VIEW_LABEL_CHANGES } from "../DiffViewProvider" import * as vscode from "vscode" +import * as path from "path" + +// Mock fs/promises +vi.mock("fs/promises", () => ({ + readFile: vi.fn().mockResolvedValue("file content"), + writeFile: vi.fn().mockResolvedValue(undefined), +})) + +// Mock utils +vi.mock("../../../utils/fs", () => ({ + createDirectoriesForFile: vi.fn().mockResolvedValue([]), +})) + +// Mock path +vi.mock("path", () => ({ + resolve: vi.fn((cwd, relPath) => `${cwd}/${relPath}`), + basename: vi.fn((path) => path.split("/").pop()), +})) // Mock vscode vi.mock("vscode", () => ({ workspace: { applyEdit: vi.fn(), + onDidOpenTextDocument: vi.fn(() => ({ dispose: vi.fn() })), + textDocuments: [], + fs: { + stat: vi.fn(), + }, }, window: { createTextEditorDecorationType: vi.fn(), + showTextDocument: vi.fn(), + onDidChangeVisibleTextEditors: vi.fn(() => ({ dispose: vi.fn() })), + tabGroups: { + all: [], + close: vi.fn(), + }, + visibleTextEditors: [], + }, + commands: { + executeCommand: vi.fn(), + }, + languages: { + getDiagnostics: vi.fn(() => []), }, WorkspaceEdit: vi.fn().mockImplementation(() => ({ replace: vi.fn(), delete: vi.fn(), })), + ViewColumn: { + Active: 1, + Beside: 2, + One: 1, + Two: 2, + Three: 3, + Four: 4, + Five: 5, + Six: 6, + Seven: 7, + Eight: 8, + Nine: 9, + }, Range: vi.fn(), Position: vi.fn(), Selection: vi.fn(), TextEditorRevealType: { InCenter: 2, }, + TabInputTextDiff: class TabInputTextDiff {}, + Uri: { + file: vi.fn((path) => ({ fsPath: path })), + parse: vi.fn((uri) => ({ with: vi.fn(() => ({})) })), + }, })) // Mock DecorationController @@ -26,6 +80,7 @@ vi.mock("../DecorationController", () => ({ DecorationController: vi.fn().mockImplementation(() => ({ setActiveLine: vi.fn(), updateOverlayAfterLine: vi.fn(), + addLines: vi.fn(), clear: vi.fn(), })), })) @@ -60,7 +115,11 @@ describe("DiffViewProvider", () => { revealRange: vi.fn(), } ;(diffViewProvider as any).activeLineController = { setActiveLine: vi.fn(), clear: vi.fn() } - ;(diffViewProvider as any).fadedOverlayController = { updateOverlayAfterLine: vi.fn(), clear: vi.fn() } + ;(diffViewProvider as any).fadedOverlayController = { + updateOverlayAfterLine: vi.fn(), + addLines: vi.fn(), + clear: vi.fn(), + } }) describe("update method", () => { @@ -93,4 +152,179 @@ describe("DiffViewProvider", () => { expect(mockWorkspaceEdit.replace).toHaveBeenCalledWith(expect.anything(), expect.anything(), "New content") }) }) + + describe("open method", () => { + it("should pre-open file as text document before executing diff command", async () => { + // Setup + const mockEditor = { + document: { + uri: { fsPath: `${mockCwd}/test.md` }, + getText: vi.fn().mockReturnValue(""), + lineCount: 0, + }, + selection: { + active: { line: 0, character: 0 }, + anchor: { line: 0, character: 0 }, + }, + edit: vi.fn().mockResolvedValue(true), + revealRange: vi.fn(), + } + + // Track the order of calls + const callOrder: string[] = [] + + // Mock showTextDocument to track when it's called + vi.mocked(vscode.window.showTextDocument).mockImplementation(async (uri, options) => { + callOrder.push("showTextDocument") + expect(options).toEqual({ preview: false, viewColumn: vscode.ViewColumn.Active }) + return mockEditor as any + }) + + // Mock executeCommand to track when it's called + vi.mocked(vscode.commands.executeCommand).mockImplementation(async (command) => { + callOrder.push("executeCommand") + expect(command).toBe("vscode.diff") + return undefined + }) + + // Mock workspace.onDidOpenTextDocument to trigger immediately + vi.mocked(vscode.workspace.onDidOpenTextDocument).mockImplementation((callback) => { + // Trigger the callback immediately with the document + setTimeout(() => { + callback({ uri: { fsPath: `${mockCwd}/test.md` } } as any) + }, 0) + return { dispose: vi.fn() } + }) + + // Mock window.visibleTextEditors to return our editor + vi.mocked(vscode.window).visibleTextEditors = [mockEditor as any] + + // Set up for file + ;(diffViewProvider as any).editType = "modify" + + // Execute open + await diffViewProvider.open("test.md") + + // Verify that showTextDocument was called before executeCommand + expect(callOrder).toEqual(["showTextDocument", "executeCommand"]) + + // Verify that showTextDocument was called with preview: false + expect(vscode.window.showTextDocument).toHaveBeenCalledWith( + expect.objectContaining({ fsPath: `${mockCwd}/test.md` }), + { preview: false, viewColumn: vscode.ViewColumn.Active }, + ) + + // Verify that the diff command was executed + expect(vscode.commands.executeCommand).toHaveBeenCalledWith( + "vscode.diff", + expect.any(Object), + expect.any(Object), + `test.md: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`, + { preserveFocus: true }, + ) + }) + + it("should handle showTextDocument failure", async () => { + // Mock showTextDocument to fail + vi.mocked(vscode.window.showTextDocument).mockRejectedValue(new Error("Cannot open file")) + + // Mock workspace.onDidOpenTextDocument + vi.mocked(vscode.workspace.onDidOpenTextDocument).mockReturnValue({ dispose: vi.fn() }) + + // Mock window.onDidChangeVisibleTextEditors + vi.mocked(vscode.window.onDidChangeVisibleTextEditors).mockReturnValue({ dispose: vi.fn() }) + + // Set up for file + ;(diffViewProvider as any).editType = "modify" + + // Try to open and expect rejection + await expect(diffViewProvider.open("test.md")).rejects.toThrow( + "Failed to execute diff command for /mock/cwd/test.md: Cannot open file", + ) + }) + }) + + describe("closeAllDiffViews method", () => { + it("should close diff views including those identified by label", async () => { + // Mock tab groups with various types of tabs + const mockTabs = [ + // Normal diff view + { + input: { + constructor: { name: "TabInputTextDiff" }, + original: { scheme: DIFF_VIEW_URI_SCHEME }, + modified: { fsPath: "/test/file1.ts" }, + }, + label: `file1.ts: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`, + isDirty: false, + }, + // Diff view identified by label (for pre-opened files) + { + input: { + constructor: { name: "TabInputTextDiff" }, + original: { scheme: "file" }, // Different scheme due to pre-opening + modified: { fsPath: "/test/file2.md" }, + }, + label: `file2.md: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`, + isDirty: false, + }, + // Regular file tab (should not be closed) + { + input: { + constructor: { name: "TabInputText" }, + uri: { fsPath: "/test/file3.js" }, + }, + label: "file3.js", + isDirty: false, + }, + // Dirty diff view (should not be closed) + { + input: { + constructor: { name: "TabInputTextDiff" }, + original: { scheme: DIFF_VIEW_URI_SCHEME }, + modified: { fsPath: "/test/file4.ts" }, + }, + label: `file4.ts: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`, + isDirty: true, + }, + ] + + // Make tabs appear as TabInputTextDiff instances + mockTabs.forEach((tab) => { + if (tab.input.constructor.name === "TabInputTextDiff") { + Object.setPrototypeOf(tab.input, vscode.TabInputTextDiff.prototype) + } + }) + + // Mock the tabGroups getter + Object.defineProperty(vscode.window.tabGroups, "all", { + get: () => [ + { + tabs: mockTabs as any, + }, + ], + configurable: true, + }) + + const closedTabs: any[] = [] + vi.mocked(vscode.window.tabGroups.close).mockImplementation((tab) => { + closedTabs.push(tab) + return Promise.resolve(true) + }) + + // Execute closeAllDiffViews + await (diffViewProvider as any).closeAllDiffViews() + + // Verify that only the appropriate tabs were closed + expect(closedTabs).toHaveLength(2) + expect(closedTabs[0].label).toBe(`file1.ts: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`) + expect(closedTabs[1].label).toBe(`file2.md: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`) + + // Verify that the regular file and dirty diff were not closed + expect(closedTabs.find((t) => t.label === "file3.js")).toBeUndefined() + expect( + closedTabs.find((t) => t.label === `file4.ts: ${DIFF_VIEW_LABEL_CHANGES} (Editable)` && t.isDirty), + ).toBeUndefined() + }) + }) })