Support mentioning filenames with spaces

Author: mrubens

src/core/mentions/__tests__/index.test.ts

@@ -1,187 +1,198 @@
-// Create mock vscode module before importing anything
-const createMockUri = (scheme: string, path: string) => ({
-	scheme,
-	authority: "",
-	path,
-	query: "",
-	fragment: "",
-	fsPath: path,
-	with: jest.fn(),
-	toString: () => path,
-	toJSON: () => ({
-		scheme,
-		authority: "",
-		path,
-		query: "",
-		fragment: "",
-	}),
-})
+import * as vscode from "vscode"
+import * as path from "path"
+import fs from "fs/promises"
+import { openFile } from "../../../integrations/misc/open-file"
+import { getWorkspacePath } from "../../../utils/path"
+// Test through exported functions parseMentions and openMention
+import { parseMentions, openMention } from "../index"
+import { UrlContentFetcher } from "../../../services/browser/UrlContentFetcher"
 
-const mockExecuteCommand = jest.fn()
-const mockOpenExternal = jest.fn()
-const mockShowErrorMessage = jest.fn()
-
-const mockVscode = {
-	workspace: {
-		workspaceFolders: [
-			{
-				uri: { fsPath: "/test/workspace" },
-			},
-		] as { uri: { fsPath: string } }[] | undefined,
-		getWorkspaceFolder: jest.fn().mockReturnValue("/test/workspace"),
-		fs: {
-			stat: jest.fn(),
-			writeFile: jest.fn(),
+// Mocks
+jest.mock("fs/promises", () => ({
+	stat: jest.fn(),
+	readdir: jest.fn(),
+}))
+jest.mock("../../../integrations/misc/open-file", () => ({
+	openFile: jest.fn(),
+}))
+jest.mock("../../../utils/path", () => ({
+	getWorkspacePath: jest.fn(),
+}))
+jest.mock(
+	"vscode",
+	() => ({
+		commands: {
+			executeCommand: jest.fn(),
 		},
-		openTextDocument: jest.fn().mockResolvedValue({}),
-	},
-	window: {
-		showErrorMessage: mockShowErrorMessage,
-		showInformationMessage: jest.fn(),
-		showWarningMessage: jest.fn(),
-		createTextEditorDecorationType: jest.fn(),
-		createOutputChannel: jest.fn(),
-		createWebviewPanel: jest.fn(),
-		showTextDocument: jest.fn().mockResolvedValue({}),
-		activeTextEditor: undefined as
-			| undefined
-			| {
-					document: {
-						uri: { fsPath: string }
-					}
-			  },
-	},
-	commands: {
-		executeCommand: mockExecuteCommand,
-	},
-	env: {
-		openExternal: mockOpenExternal,
-	},
-	Uri: {
-		parse: jest.fn((url: string) => createMockUri("https", url)),
-		file: jest.fn((path: string) => createMockUri("file", path)),
-	},
-	Position: jest.fn(),
-	Range: jest.fn(),
-	TextEdit: jest.fn(),
-	WorkspaceEdit: jest.fn(),
-	DiagnosticSeverity: {
-		Error: 0,
-		Warning: 1,
-		Information: 2,
-		Hint: 3,
-	},
-}
-
-// Mock modules
-jest.mock("vscode", () => mockVscode)
+		Uri: {
+			file: jest.fn((p) => ({ fsPath: p })), // Simple mock for Uri.file
+		},
+		window: {
+			showErrorMessage: jest.fn(),
+		},
+		env: {
+			openExternal: jest.fn(),
+		},
+	}),
+	{ virtual: true },
+)
+jest.mock("../../../integrations/misc/extract-text", () => ({
+	extractTextFromFile: jest.fn(),
+}))
+jest.mock("isbinaryfile", () => ({
+	isBinaryFile: jest.fn().mockResolvedValue(false),
+}))
 jest.mock("../../../services/browser/UrlContentFetcher")
-jest.mock("../../../utils/git")
-jest.mock("../../../utils/path")
 
-// Now import the modules that use the mocks
-import { parseMentions, openMention } from "../index"
-import { UrlContentFetcher } from "../../../services/browser/UrlContentFetcher"
-import * as git from "../../../utils/git"
-
-import { getWorkspacePath } from "../../../utils/path"
-;(getWorkspacePath as jest.Mock).mockReturnValue("/test/workspace")
+// Helper to reset mocks between tests
+const resetMocks = () => {
+	;(fs.stat as jest.Mock).mockClear()
+	;(fs.readdir as jest.Mock).mockClear()
+	;(openFile as jest.Mock).mockClear()
+	;(getWorkspacePath as jest.Mock).mockClear()
+	;(vscode.commands.executeCommand as jest.Mock).mockClear()
+	;(vscode.Uri.file as jest.Mock).mockClear()
+	;(require("../../../integrations/misc/extract-text").extractTextFromFile as jest.Mock).mockClear()
+	;(require("isbinaryfile").isBinaryFile as jest.Mock).mockClear()
+}
 
-describe("mentions", () => {
-	const mockCwd = "/test/workspace"
-	let mockUrlContentFetcher: UrlContentFetcher
+describe("Core Mentions Logic", () => {
+	const MOCK_CWD = "/mock/workspace"
 
 	beforeEach(() => {
-		jest.clearAllMocks()
-
-		// Create a mock instance with just the methods we need
-		mockUrlContentFetcher = {
-			launchBrowser: jest.fn().mockResolvedValue(undefined),
-			closeBrowser: jest.fn().mockResolvedValue(undefined),
-			urlToMarkdown: jest.fn().mockResolvedValue(""),
-		} as unknown as UrlContentFetcher
-
-		// Reset all vscode mocks
-		mockVscode.workspace.fs.stat.mockReset()
-		mockVscode.workspace.fs.writeFile.mockReset()
-		mockVscode.workspace.openTextDocument.mockReset().mockResolvedValue({})
-		mockVscode.window.showTextDocument.mockReset().mockResolvedValue({})
-		mockVscode.window.showErrorMessage.mockReset()
-		mockExecuteCommand.mockReset()
-		mockOpenExternal.mockReset()
+		resetMocks()
+		// Default mock implementations
+		;(getWorkspacePath as jest.Mock).mockReturnValue(MOCK_CWD)
 	})
 
 	describe("parseMentions", () => {
-		it("should parse git commit mentions", async () => {
-			const commitHash = "abc1234"
-			const commitInfo = `abc1234 Fix bug in parser
+		let mockUrlFetcher: UrlContentFetcher
 
-Author: John Doe
-Date: Mon Jan 5 23:50:06 2025 -0500
+		beforeEach(() => {
+			mockUrlFetcher = new (UrlContentFetcher as jest.Mock<UrlContentFetcher>)()
+			;(fs.stat as jest.Mock).mockResolvedValue({ isFile: () => true, isDirectory: () => false })
+			;(require("../../../integrations/misc/extract-text").extractTextFromFile as jest.Mock).mockResolvedValue(
+				"Mock file content",
+			)
+		})
 
-Detailed commit message with multiple lines
-- Fixed parsing issue
-- Added tests`
+		it("should correctly parse mentions with escaped spaces and fetch content", async () => {
+			const text = "Please check the file @/path/to/file\\ with\\ spaces.txt"
+			const mentionPath = "/path/to/file\\ with\\ spaces.txt"
+			const expectedUnescaped = "path/to/file with spaces.txt" // Note: leading '/' removed by slice(1) in parseMentions
+			const expectedAbsPath = path.resolve(MOCK_CWD, expectedUnescaped)
 
-			jest.mocked(git.getCommitInfo).mockResolvedValue(commitInfo)
+			const result = await parseMentions(text, MOCK_CWD, mockUrlFetcher)
 
-			const result = await parseMentions(`Check out this commit @${commitHash}`, mockCwd, mockUrlContentFetcher)
+			// Check if fs.stat was called with the unescaped path
+			expect(fs.stat).toHaveBeenCalledWith(expectedAbsPath)
+			// Check if extractTextFromFile was called with the unescaped path
+			expect(require("../../../integrations/misc/extract-text").extractTextFromFile).toHaveBeenCalledWith(
+				expectedAbsPath,
+			)
 
-			expect(result).toContain(`'${commitHash}' (see below for commit info)`)
-			expect(result).toContain(`<git_commit hash="${commitHash}">`)
-			expect(result).toContain(commitInfo)
+			// Check the output format
+			expect(result).toContain(`'path/to/file\\ with\\ spaces.txt' (see below for file content)`)
+			expect(result).toContain(
+				`<file_content path="path/to/file\\ with\\ spaces.txt">\nMock file content\n</file_content>`,
+			)
 		})
 
-		it("should handle errors fetching git info", async () => {
-			const commitHash = "abc1234"
-			const errorMessage = "Failed to get commit info"
+		it("should handle folder mentions with escaped spaces", async () => {
+			const text = "Look in @/my\\ documents/folder\\ name/"
+			const mentionPath = "/my\\ documents/folder\\ name/"
+			const expectedUnescaped = "my documents/folder name/"
+			const expectedAbsPath = path.resolve(MOCK_CWD, expectedUnescaped)
+			;(fs.stat as jest.Mock).mockResolvedValue({ isFile: () => false, isDirectory: () => true })
+			;(fs.readdir as jest.Mock).mockResolvedValue([]) // Empty directory
+
+			const result = await parseMentions(text, MOCK_CWD, mockUrlFetcher)
 
-			jest.mocked(git.getCommitInfo).mockRejectedValue(new Error(errorMessage))
+			expect(fs.stat).toHaveBeenCalledWith(expectedAbsPath)
+			expect(fs.readdir).toHaveBeenCalledWith(expectedAbsPath, { withFileTypes: true })
+			expect(result).toContain(`'my\\ documents/folder\\ name/' (see below for folder content)`)
+			expect(result).toContain(`<folder_content path="my\\ documents/folder\\ name/">`) // Content check might be more complex
+		})
 
-			const result = await parseMentions(`Check out this commit @${commitHash}`, mockCwd, mockUrlContentFetcher)
+		it("should handle errors when accessing paths with escaped spaces", async () => {
+			const text = "Check @/nonexistent\\ file.txt"
+			const mentionPath = "/nonexistent\\ file.txt"
+			const expectedUnescaped = "nonexistent file.txt"
+			const expectedAbsPath = path.resolve(MOCK_CWD, expectedUnescaped)
+			const mockError = new Error("ENOENT: no such file or directory")
+			;(fs.stat as jest.Mock).mockRejectedValue(mockError)
 
-			expect(result).toContain(`'${commitHash}' (see below for commit info)`)
-			expect(result).toContain(`<git_commit hash="${commitHash}">`)
-			expect(result).toContain(`Error fetching commit info: ${errorMessage}`)
+			const result = await parseMentions(text, MOCK_CWD, mockUrlFetcher)
+
+			expect(fs.stat).toHaveBeenCalledWith(expectedAbsPath)
+			expect(result).toContain(
+				`<file_content path="nonexistent\\ file.txt">\nError fetching content: Failed to access path "nonexistent\\ file.txt": ${mockError.message}\n</file_content>`,
+			)
 		})
+
+		// Add more tests for parseMentions if needed (URLs, other mentions combined with escaped paths etc.)
 	})
 
 	describe("openMention", () => {
-		it("should handle file paths and problems", async () => {
-			// Mock stat to simulate file not existing
-			mockVscode.workspace.fs.stat.mockRejectedValueOnce(new Error("File does not exist"))
+		beforeEach(() => {
+			;(getWorkspacePath as jest.Mock).mockReturnValue(MOCK_CWD)
+		})
+
+		it("should unescape file path before opening", async () => {
+			const mention = "/file\\ with\\ spaces.txt"
+			const expectedUnescaped = "file with spaces.txt"
+			const expectedAbsPath = path.resolve(MOCK_CWD, expectedUnescaped)
 
-			// Call openMention and wait for it to complete
-			await openMention("/path/to/file")
+			await openMention(mention)
+
+			expect(openFile).toHaveBeenCalledWith(expectedAbsPath)
+			expect(vscode.commands.executeCommand).not.toHaveBeenCalled()
+		})
 
-			// Verify error handling
-			expect(mockExecuteCommand).not.toHaveBeenCalled()
-			expect(mockOpenExternal).not.toHaveBeenCalled()
-			expect(mockVscode.window.showErrorMessage).toHaveBeenCalledWith("Could not open file: File does not exist")
+		it("should unescape folder path before revealing", async () => {
+			const mention = "/folder\\ with\\ spaces/"
+			const expectedUnescaped = "folder with spaces/"
+			const expectedAbsPath = path.resolve(MOCK_CWD, expectedUnescaped)
+			const expectedUri = { fsPath: expectedAbsPath } // From mock
+			;(vscode.Uri.file as jest.Mock).mockReturnValue(expectedUri)
 
-			// Reset mocks for next test
-			jest.clearAllMocks()
+			await openMention(mention)
 
-			// Test problems command
-			await openMention("problems")
-			expect(mockExecuteCommand).toHaveBeenCalledWith("workbench.actions.view.problems")
+			expect(vscode.commands.executeCommand).toHaveBeenCalledWith("revealInExplorer", expectedUri)
+			expect(vscode.Uri.file).toHaveBeenCalledWith(expectedAbsPath)
+			expect(openFile).not.toHaveBeenCalled()
 		})
 
-		it("should handle URLs", async () => {
-			const url = "https://example.com"
-			await openMention(url)
-			const mockUri = mockVscode.Uri.parse(url)
-			expect(mockVscode.env.openExternal).toHaveBeenCalled()
-			const calledArg = mockVscode.env.openExternal.mock.calls[0][0]
-			expect(calledArg).toEqual(
-				expect.objectContaining({
-					scheme: mockUri.scheme,
-					authority: mockUri.authority,
-					path: mockUri.path,
-					query: mockUri.query,
-					fragment: mockUri.fragment,
-				}),
-			)
+		it("should handle mentions without paths correctly", async () => {
+			await openMention("@problems")
+			expect(vscode.commands.executeCommand).toHaveBeenCalledWith("workbench.actions.view.problems")
+
+			await openMention("@terminal")
+			expect(vscode.commands.executeCommand).toHaveBeenCalledWith("workbench.action.terminal.focus")
+
+			await openMention("@http://example.com")
+			expect(vscode.env.openExternal).toHaveBeenCalled() // Check if called, specific URI mock might be needed for detailed check
+
+			await openMention("@git-changes") // Assuming no specific action for this yet
+			// Add expectations if an action is defined for git-changes
+
+			await openMention("@a1b2c3d") // Assuming no specific action for commit hashes yet
+			// Add expectations if an action is defined for commit hashes
+		})
+
+		it("should do nothing if mention is undefined or empty", async () => {
+			await openMention(undefined)
+			await openMention("")
+			expect(openFile).not.toHaveBeenCalled()
+			expect(vscode.commands.executeCommand).not.toHaveBeenCalled()
+			expect(vscode.env.openExternal).not.toHaveBeenCalled()
+		})
+
+		it("should do nothing if cwd is not available", async () => {
+			;(getWorkspacePath as jest.Mock).mockReturnValue(undefined)
+			await openMention("/some\\ path.txt")
+			expect(openFile).not.toHaveBeenCalled()
+			expect(vscode.commands.executeCommand).not.toHaveBeenCalled()
 		})
 	})
 })

src/core/mentions/index.ts

@@ -22,7 +22,8 @@ export async function openMention(mention?: string): Promise<void> {
 	}
 
 	if (mention.startsWith("/")) {
-		const relPath = mention.slice(1)
+		// Slice off the leading slash and unescape any spaces in the path
+		const relPath = unescapeSpaces(mention.slice(1))
 		const absPath = path.resolve(cwd, relPath)
 		if (mention.endsWith("/")) {
 			vscode.commands.executeCommand("revealInExplorer", vscode.Uri.file(absPath))
@@ -145,8 +146,15 @@ export async function parseMentions(text: string, cwd: string, urlContentFetcher
 	return parsedText
 }
 
+// Helper function to unescape paths with backslash-escaped spaces
+function unescapeSpaces(path: string): string {
+	return path.replace(/\\ /g, " ")
+}
+
 async function getFileOrFolderContent(mentionPath: string, cwd: string): Promise<string> {
-	const absPath = path.resolve(cwd, mentionPath)
+	// Unescape spaces in the path before resolving it
+	const unescapedPath = unescapeSpaces(mentionPath)
+	const absPath = path.resolve(cwd, unescapedPath)
 
 	try {
 		const stats = await fs.stat(absPath)