Skip to content

Commit 272e0d5

Browse files
jrmrubens
andcommitted
Support mentioning filenames with spaces
Allows spaces to be escaped in mentions with \. Does not attempt more comprehensive escape handling. (IMO a more structured representation or moving to something like @[something complex] is probably a better long term approach, but the scope of that seems problematic. Adapted from 8955d45. Co-authored-by: Matt Rubens <[email protected]>
1 parent 664d5b1 commit 272e0d5

File tree

9 files changed

+552
-82
lines changed

9 files changed

+552
-82
lines changed

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

Lines changed: 141 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,24 @@ import * as git from "../../../utils/git"
8787
import { getWorkspacePath } from "../../../utils/path"
8888
;(getWorkspacePath as jest.Mock).mockReturnValue("/test/workspace")
8989

90+
jest.mock("fs/promises", () => ({
91+
stat: jest.fn(),
92+
readdir: jest.fn(),
93+
}))
94+
import fs from "fs/promises"
95+
import * as path from "path"
96+
97+
jest.mock("../../../integrations/misc/open-file", () => ({
98+
openFile: jest.fn(),
99+
}))
100+
import { openFile } from "../../../integrations/misc/open-file"
101+
102+
jest.mock("../../../integrations/misc/extract-text", () => ({
103+
extractTextFromFile: jest.fn(),
104+
}))
105+
106+
import * as vscode from "vscode"
107+
90108
describe("mentions", () => {
91109
const mockCwd = "/test/workspace"
92110
let mockUrlContentFetcher: UrlContentFetcher
@@ -112,6 +130,16 @@ describe("mentions", () => {
112130
})
113131

114132
describe("parseMentions", () => {
133+
let mockUrlFetcher: UrlContentFetcher
134+
135+
beforeEach(() => {
136+
mockUrlFetcher = new (UrlContentFetcher as jest.Mock<UrlContentFetcher>)()
137+
;(fs.stat as jest.Mock).mockResolvedValue({ isFile: () => true, isDirectory: () => false })
138+
;(require("../../../integrations/misc/extract-text").extractTextFromFile as jest.Mock).mockResolvedValue(
139+
"Mock file content",
140+
)
141+
})
142+
115143
it("should parse git commit mentions", async () => {
116144
const commitHash = "abc1234"
117145
const commitInfo = `abc1234 Fix bug in parser
@@ -144,35 +172,72 @@ Detailed commit message with multiple lines
144172
expect(result).toContain(`<git_commit hash="${commitHash}">`)
145173
expect(result).toContain(`Error fetching commit info: ${errorMessage}`)
146174
})
147-
})
148175

149-
describe("openMention", () => {
150-
it("should handle file paths and problems", async () => {
151-
// Mock stat to simulate file not existing
152-
mockVscode.workspace.fs.stat.mockRejectedValueOnce(new Error("File does not exist"))
176+
it("should correctly parse mentions with escaped spaces and fetch content", async () => {
177+
const text = "Please check the file @/path/to/file\\ with\\ spaces.txt"
178+
const expectedUnescaped = "path/to/file with spaces.txt" // Note: leading '/' removed by slice(1) in parseMentions
179+
const expectedAbsPath = path.resolve(mockCwd, expectedUnescaped)
153180

154-
// Call openMention and wait for it to complete
155-
await openMention("/path/to/file")
181+
const result = await parseMentions(text, mockCwd, mockUrlFetcher)
156182

157-
// Verify error handling
158-
expect(mockExecuteCommand).not.toHaveBeenCalled()
159-
expect(mockOpenExternal).not.toHaveBeenCalled()
160-
expect(mockVscode.window.showErrorMessage).toHaveBeenCalledWith("Could not open file: File does not exist")
183+
// Check if fs.stat was called with the unescaped path
184+
expect(fs.stat).toHaveBeenCalledWith(expectedAbsPath)
185+
// Check if extractTextFromFile was called with the unescaped path
186+
expect(require("../../../integrations/misc/extract-text").extractTextFromFile).toHaveBeenCalledWith(
187+
expectedAbsPath,
188+
)
161189

162-
// Reset mocks for next test
163-
jest.clearAllMocks()
190+
// Check the output format
191+
expect(result).toContain(`'path/to/file\\ with\\ spaces.txt' (see below for file content)`)
192+
expect(result).toContain(
193+
`<file_content path="path/to/file\\ with\\ spaces.txt">\nMock file content\n</file_content>`,
194+
)
195+
})
164196

165-
// Test problems command
166-
await openMention("problems")
167-
expect(mockExecuteCommand).toHaveBeenCalledWith("workbench.actions.view.problems")
197+
it("should handle folder mentions with escaped spaces", async () => {
198+
const text = "Look in @/my\\ documents/folder\\ name/"
199+
const expectedUnescaped = "my documents/folder name/"
200+
const expectedAbsPath = path.resolve(mockCwd, expectedUnescaped)
201+
;(fs.stat as jest.Mock).mockResolvedValue({ isFile: () => false, isDirectory: () => true })
202+
;(fs.readdir as jest.Mock).mockResolvedValue([]) // Empty directory
203+
204+
const result = await parseMentions(text, mockCwd, mockUrlFetcher)
205+
206+
expect(fs.stat).toHaveBeenCalledWith(expectedAbsPath)
207+
expect(fs.readdir).toHaveBeenCalledWith(expectedAbsPath, { withFileTypes: true })
208+
expect(result).toContain(`'my\\ documents/folder\\ name/' (see below for folder content)`)
209+
expect(result).toContain(`<folder_content path="my\\ documents/folder\\ name/">`) // Content check might be more complex
210+
})
211+
212+
it("should handle errors when accessing paths with escaped spaces", async () => {
213+
const text = "Check @/nonexistent\\ file.txt"
214+
const expectedUnescaped = "nonexistent file.txt"
215+
const expectedAbsPath = path.resolve(mockCwd, expectedUnescaped)
216+
const mockError = new Error("ENOENT: no such file or directory")
217+
;(fs.stat as jest.Mock).mockRejectedValue(mockError)
218+
219+
const result = await parseMentions(text, mockCwd, mockUrlFetcher)
220+
221+
expect(fs.stat).toHaveBeenCalledWith(expectedAbsPath)
222+
expect(result).toContain(
223+
`<file_content path="nonexistent\\ file.txt">\nError fetching content: Failed to access path "nonexistent\\ file.txt": ${mockError.message}\n</file_content>`,
224+
)
225+
})
226+
227+
// Add more tests for parseMentions if needed (URLs, other mentions combined with escaped paths etc.)
228+
})
229+
230+
describe("openMention", () => {
231+
beforeEach(() => {
232+
;(getWorkspacePath as jest.Mock).mockReturnValue(mockCwd)
168233
})
169234

170235
it("should handle URLs", async () => {
171236
const url = "https://example.com"
172237
await openMention(url)
173-
const mockUri = mockVscode.Uri.parse(url)
174-
expect(mockVscode.env.openExternal).toHaveBeenCalled()
175-
const calledArg = mockVscode.env.openExternal.mock.calls[0][0]
238+
const mockUri = vscode.Uri.parse(url)
239+
expect(vscode.env.openExternal).toHaveBeenCalled()
240+
const calledArg = (vscode.env.openExternal as jest.Mock).mock.calls[0][0]
176241
expect(calledArg).toEqual(
177242
expect.objectContaining({
178243
scheme: mockUri.scheme,
@@ -183,5 +248,62 @@ Detailed commit message with multiple lines
183248
}),
184249
)
185250
})
251+
252+
it("should unescape file path before opening", async () => {
253+
const mention = "/file\\ with\\ spaces.txt"
254+
const expectedUnescaped = "file with spaces.txt"
255+
const expectedAbsPath = path.resolve(mockCwd, expectedUnescaped)
256+
257+
await openMention(mention)
258+
259+
expect(openFile).toHaveBeenCalledWith(expectedAbsPath)
260+
expect(vscode.commands.executeCommand).not.toHaveBeenCalled()
261+
})
262+
263+
it("should unescape folder path before revealing", async () => {
264+
const mention = "/folder\\ with\\ spaces/"
265+
const expectedUnescaped = "folder with spaces/"
266+
const expectedAbsPath = path.resolve(mockCwd, expectedUnescaped)
267+
const expectedUri = { fsPath: expectedAbsPath } // From mock
268+
;(vscode.Uri.file as jest.Mock).mockReturnValue(expectedUri)
269+
270+
await openMention(mention)
271+
272+
expect(vscode.commands.executeCommand).toHaveBeenCalledWith("revealInExplorer", expectedUri)
273+
expect(vscode.Uri.file).toHaveBeenCalledWith(expectedAbsPath)
274+
expect(openFile).not.toHaveBeenCalled()
275+
})
276+
277+
it("should handle mentions without paths correctly", async () => {
278+
await openMention("problems")
279+
expect(vscode.commands.executeCommand).toHaveBeenCalledWith("workbench.actions.view.problems")
280+
281+
await openMention("terminal")
282+
expect(vscode.commands.executeCommand).toHaveBeenCalledWith("workbench.action.terminal.focus")
283+
284+
await openMention("http://example.com")
285+
expect(vscode.env.openExternal).toHaveBeenCalled() // Check if called, specific URI mock might be needed for detailed check
286+
287+
await openMention("git-changes") // Assuming no specific action for this yet
288+
// Add expectations if an action is defined for git-changes
289+
290+
await openMention("a1b2c3d") // Assuming no specific action for commit hashes yet
291+
// Add expectations if an action is defined for commit hashes
292+
})
293+
294+
it("should do nothing if mention is undefined or empty", async () => {
295+
await openMention(undefined)
296+
await openMention("")
297+
expect(openFile).not.toHaveBeenCalled()
298+
expect(vscode.commands.executeCommand).not.toHaveBeenCalled()
299+
expect(vscode.env.openExternal).not.toHaveBeenCalled()
300+
})
301+
302+
it("should do nothing if cwd is not available", async () => {
303+
;(getWorkspacePath as jest.Mock).mockReturnValue(undefined)
304+
await openMention("/some\\ path.txt")
305+
expect(openFile).not.toHaveBeenCalled()
306+
expect(vscode.commands.executeCommand).not.toHaveBeenCalled()
307+
})
186308
})
187309
})

src/core/mentions/index.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { isBinaryFile } from "isbinaryfile"
66

77
import { openFile } from "../../integrations/misc/open-file"
88
import { UrlContentFetcher } from "../../services/browser/UrlContentFetcher"
9-
import { mentionRegexGlobal } from "../../shared/context-mentions"
9+
import { mentionRegexGlobal, unescapeSpaces } from "../../shared/context-mentions"
1010

1111
import { extractTextFromFile } from "../../integrations/misc/extract-text"
1212
import { diagnosticsToProblemsString } from "../../integrations/diagnostics"
@@ -25,7 +25,8 @@ export async function openMention(mention?: string): Promise<void> {
2525
}
2626

2727
if (mention.startsWith("/")) {
28-
const relPath = mention.slice(1)
28+
// Slice off the leading slash and unescape any spaces in the path
29+
const relPath = unescapeSpaces(mention.slice(1))
2930
const absPath = path.resolve(cwd, relPath)
3031
if (mention.endsWith("/")) {
3132
vscode.commands.executeCommand("revealInExplorer", vscode.Uri.file(absPath))
@@ -158,7 +159,9 @@ export async function parseMentions(
158159
}
159160

160161
async function getFileOrFolderContent(mentionPath: string, cwd: string): Promise<string> {
161-
const absPath = path.resolve(cwd, mentionPath)
162+
// Unescape spaces in the path before resolving it
163+
const unescapedPath = unescapeSpaces(mentionPath)
164+
const absPath = path.resolve(cwd, unescapedPath)
162165

163166
try {
164167
const stats = await fs.stat(absPath)
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { mentionRegex, mentionRegexGlobal } from "../context-mentions"
2+
3+
describe("mentionRegex and mentionRegexGlobal", () => {
4+
// Test cases for various mention types
5+
const testCases = [
6+
// Basic file paths
7+
{ input: "@/path/to/file.txt", expected: ["@/path/to/file.txt"] },
8+
{ input: "@/file.js", expected: ["@/file.js"] },
9+
{ input: "@/folder/", expected: ["@/folder/"] },
10+
11+
// File paths with escaped spaces
12+
{ input: "@/path/to/file\\ with\\ spaces.txt", expected: ["@/path/to/file\\ with\\ spaces.txt"] },
13+
{ input: "@/users/my\\ project/report\\ final.pdf", expected: ["@/users/my\\ project/report\\ final.pdf"] },
14+
{ input: "@/folder\\ with\\ spaces/", expected: ["@/folder\\ with\\ spaces/"] },
15+
{ input: "@/a\\ b\\ c.txt", expected: ["@/a\\ b\\ c.txt"] },
16+
17+
// URLs
18+
{ input: "@http://example.com", expected: ["@http://example.com"] },
19+
{ input: "@https://example.com/path?query=1", expected: ["@https://example.com/path?query=1"] },
20+
21+
// Other mentions
22+
{ input: "@problems", expected: ["@problems"] },
23+
{ input: "@git-changes", expected: ["@git-changes"] },
24+
{ input: "@terminal", expected: ["@terminal"] },
25+
{ input: "@a1b2c3d", expected: ["@a1b2c3d"] }, // Git commit hash (short)
26+
{ input: "@a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0", expected: ["@a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0"] }, // Git commit hash (long)
27+
28+
// Mentions within text
29+
{
30+
input: "Check file @/path/to/file\\ with\\ spaces.txt for details.",
31+
expected: ["@/path/to/file\\ with\\ spaces.txt"],
32+
},
33+
{ input: "See @problems and @terminal output.", expected: ["@problems", "@terminal"] },
34+
{ input: "URL: @https://example.com.", expected: ["@https://example.com"] }, // Trailing punctuation
35+
{ input: "Commit @a1b2c3d, then check @/file.txt", expected: ["@a1b2c3d", "@/file.txt"] },
36+
37+
// Negative cases (should not match or match partially)
38+
{ input: "@/path/with unescaped space.txt", expected: ["@/path/with"] }, // Unescaped space
39+
{ input: "@ /path/leading-space.txt", expected: null }, // Space after @
40+
{ input: "[email protected]", expected: null }, // Email address
41+
{ input: "mention@", expected: null }, // Trailing @
42+
{ input: "@/path/trailing\\", expected: null }, // Trailing backslash (invalid escape)
43+
{ input: "@/path/to/file\\not-a-space", expected: null }, // Backslash not followed by space
44+
]
45+
46+
testCases.forEach(({ input, expected }) => {
47+
it(`should handle input: "${input}"`, () => {
48+
// Test mentionRegex (first match)
49+
const match = input.match(mentionRegex)
50+
const firstExpected = expected ? expected[0] : null
51+
if (firstExpected) {
52+
expect(match).not.toBeNull()
53+
// Check the full match (group 0)
54+
expect(match?.[0]).toBe(firstExpected)
55+
// Check the captured group (group 1) - remove leading '@'
56+
expect(match?.[1]).toBe(firstExpected.slice(1))
57+
} else {
58+
expect(match).toBeNull()
59+
}
60+
61+
// Test mentionRegexGlobal (all matches)
62+
const globalMatches = [...input.matchAll(mentionRegexGlobal)].map((m) => m[0])
63+
if (expected) {
64+
expect(globalMatches).toEqual(expected)
65+
} else {
66+
expect(globalMatches).toEqual([])
67+
}
68+
})
69+
})
70+
71+
it("should correctly capture the mention part (group 1)", () => {
72+
const input = "Mention @/path/to/escaped\\ file.txt and @problems"
73+
const matches = [...input.matchAll(mentionRegexGlobal)]
74+
75+
expect(matches.length).toBe(2)
76+
expect(matches[0][1]).toBe("/path/to/escaped\\ file.txt") // Group 1 should not include '@'
77+
expect(matches[1][1]).toBe("problems")
78+
})
79+
})

src/shared/context-mentions.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@ Mention regex:
1616
- `\/`:
1717
- **Slash (`/`)**: Indicates that the mention is a file or folder path starting with a '/'.
1818
- `|`: Logical OR.
19-
- `\w+:\/\/`:
20-
- **Protocol (`\w+://`)**: Matches URLs that start with a word character sequence followed by '://', such as 'http://', 'https://', 'ftp://', etc.
21-
- `[^\s]+?`:
22-
- **Non-Whitespace Characters (`[^\s]+`)**: Matches one or more characters that are not whitespace.
19+
- `\w+:\/\/`:
20+
- **Protocol (`\w+://`)**: Matches URLs that start with a word character sequence followed by '://', such as 'http://', 'https://', 'ftp://', etc.
21+
- `(?:[^\s\\]|\\ )+?`:
22+
- **Non-Capturing Group (`(?:...)`)**: Groups the alternatives without capturing them.
23+
- **Non-Whitespace and Non-Backslash (`[^\s\\]`)**: Matches any character that is not whitespace or a backslash.
24+
- **OR (`|`)**: Logical OR.
25+
- **Escaped Space (`\\ `)**: Matches a backslash followed by a space (an escaped space).
2326
- **Non-Greedy (`+?`)**: Ensures the smallest possible match, preventing the inclusion of trailing punctuation.
2427
- `|`: Logical OR.
2528
- `problems\b`:
@@ -39,6 +42,7 @@ Mention regex:
3942
- **Summary**:
4043
- The regex effectively matches:
4144
- Mentions that are file or folder paths starting with '/' and containing any non-whitespace characters (including periods within the path).
45+
- File paths can include spaces if they are escaped with a backslash (e.g., `@/path/to/file\ with\ spaces.txt`).
4246
- URLs that start with a protocol (like 'http://') followed by any non-whitespace characters (including query parameters).
4347
- The exact word 'problems'.
4448
- The exact word 'git-changes'.
@@ -50,7 +54,7 @@ Mention regex:
5054
5155
*/
5256
export const mentionRegex =
53-
/@((?:\/|\w+:\/\/)[^\s]+?|[a-f0-9]{7,40}\b|problems\b|git-changes\b|terminal\b)(?=[.,;:!?]?(?=[\s\r\n]|$))/
57+
/@((?:\/|\w+:\/\/)(?:[^\s\\]|\\ )+?|[a-f0-9]{7,40}\b|problems\b|git-changes\b|terminal\b)(?=[.,;:!?]?(?=[\s\r\n]|$))/
5458
export const mentionRegexGlobal = new RegExp(mentionRegex.source, "g")
5559

5660
export interface MentionSuggestion {
@@ -90,3 +94,8 @@ export function formatGitSuggestion(commit: {
9094
date: commit.date,
9195
}
9296
}
97+
98+
// Helper function to unescape paths with backslash-escaped spaces
99+
export function unescapeSpaces(path: string): string {
100+
return path.replace(/\\ /g, " ")
101+
}

webview-ui/src/components/chat/ChatTextArea.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import React, { forwardRef, useCallback, useEffect, useLayoutEffect, useMemo, us
22
import { useEvent } from "react-use"
33
import DynamicTextArea from "react-textarea-autosize"
44

5-
import { mentionRegex, mentionRegexGlobal } from "@roo/shared/context-mentions"
5+
import { mentionRegex, mentionRegexGlobal, unescapeSpaces } from "@roo/shared/context-mentions"
66
import { WebviewMessage } from "@roo/shared/WebviewMessage"
77
import { Mode, getAllModes } from "@roo/shared/modes"
88
import { ExtensionMessage } from "@roo/shared/ExtensionMessage"
@@ -470,7 +470,7 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
470470
// Send message to extension to search files
471471
vscode.postMessage({
472472
type: "searchFiles",
473-
query: query,
473+
query: unescapeSpaces(query),
474474
requestId: reqId,
475475
})
476476
}, 200) // 200ms debounce

0 commit comments

Comments
 (0)