Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions src/core/mentions/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,91 @@ Detailed commit message with multiple lines
expect(result).toContain(`<git_commit hash="${commitHash}">`)
expect(result).toContain(`Error fetching commit info: ${errorMessage}`)
})

it("should parse file paths with escaped spaces", async () => {
// Mock the file content fetching
const fileContent = "This is the content of the file with spaces in its name"

// Mock the getFileOrFolderContent function (which is called internally by parseMentions)
// This is done by mocking the fs.readFile that would be called inside getFileOrFolderContent
const fs = require("fs/promises")
jest.spyOn(fs, "readFile").mockResolvedValue(fileContent)
jest.spyOn(fs, "stat").mockResolvedValue({ isFile: () => true, isDirectory: () => false } as any)

// Test with a file path containing escaped spaces
const filePath = "/path/with\ spaces/my\ file.txt"

// First, verify that the regex pattern correctly matches the entire path
// Import the regex pattern directly to test it
const { mentionRegexGlobal } = require("../../../shared/context-mentions")
const mentionMatch = `@${filePath}`.match(mentionRegexGlobal)
expect(mentionMatch).not.toBeNull()
expect(mentionMatch![0]).toBe(`@${filePath}`)

// Now test the full parseMentions function
const result = await parseMentions(`Check out this file @${filePath}`, mockCwd, mockUrlContentFetcher)

// Verify the file path with escaped spaces was correctly parsed
// The spaces should be unescaped when displayed
expect(result).toContain(`'path/with spaces/my file.txt' (see below for file content)`)
expect(result).toContain(`<file_content path="path/with spaces/my file.txt">`)
})

it("should parse folder paths with escaped spaces", async () => {
// Mock the folder content fetching

// Mock the getFileOrFolderContent function (which is called internally by parseMentions)
// This is done by mocking the fs.readdir and fs.stat that would be called inside getFileOrFolderContent
const fs = require("fs/promises")
jest.spyOn(fs, "readdir").mockResolvedValue([
{ name: "file1.txt", isFile: () => true, isDirectory: () => false },
{ name: "file2.txt", isFile: () => true, isDirectory: () => false },
{ name: "subfolder", isFile: () => false, isDirectory: () => true }
])
jest.spyOn(fs, "stat").mockResolvedValue({ isFile: () => false, isDirectory: () => true } as any)

// Test with a folder path containing escaped spaces
const folderPath = "/folder\ with\ spaces/"

// First, verify that the regex pattern correctly matches the entire path
const { mentionRegexGlobal } = require("../../../shared/context-mentions")
const mentionMatch = `@${folderPath}`.match(mentionRegexGlobal)
expect(mentionMatch).not.toBeNull()
expect(mentionMatch![0]).toBe(`@${folderPath}`)

const result = await parseMentions(`Check out this folder @${folderPath}`, mockCwd, mockUrlContentFetcher)

// Verify the folder path with escaped spaces was correctly parsed
// The spaces should be unescaped when displayed
expect(result).toContain(`'folder with spaces/' (see below for folder content)`)
expect(result).toContain(`<folder_content path="folder with spaces/">`)
})

it("should parse nested paths with multiple escaped spaces", async () => {
// Mock the file content fetching
const fileContent = "This is the content of the file with multiple spaces in its path"

// Mock the getFileOrFolderContent function
const fs = require("fs/promises")
jest.spyOn(fs, "readFile").mockResolvedValue(fileContent)
jest.spyOn(fs, "stat").mockResolvedValue({ isFile: () => true, isDirectory: () => false } as any)

// Test with a deeply nested path containing multiple escaped spaces
const filePath = "/root\ dir/my\ documents/project\ files/important\ notes/final\ draft\ v2.txt"

// Verify the regex pattern correctly matches the entire path
const { mentionRegexGlobal } = require("../../../shared/context-mentions")
const mentionMatch = `@${filePath}`.match(mentionRegexGlobal)
expect(mentionMatch).not.toBeNull()
expect(mentionMatch![0]).toBe(`@${filePath}`)

// Test the full parseMentions function
const result = await parseMentions(`Check out this file @${filePath}`, mockCwd, mockUrlContentFetcher)

// Verify the complex path was correctly parsed with all spaces unescaped
expect(result).toContain(`'root dir/my documents/project files/important notes/final draft v2.txt' (see below for file content)`)
expect(result).toContain(`<file_content path="root dir/my documents/project files/important notes/final draft v2.txt">`)
})
})

describe("openMention", () => {
Expand Down
23 changes: 13 additions & 10 deletions src/core/mentions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,24 @@ export async function parseMentions(
): Promise<string> {
const mentions: Set<string> = new Set()
let parsedText = text.replace(mentionRegexGlobal, (match, mention) => {
mentions.add(mention)
if (mention.startsWith("http")) {
return `'${mention}' (see below for site content)`
} else if (mention.startsWith("/")) {
const mentionPath = mention.slice(1)
// Unescape spaces in the mention (convert "\s" to " ")
const unescapedMention = mention.replace(/\\\s/g, " ")
mentions.add(unescapedMention)

if (unescapedMention.startsWith("http")) {
return `'${unescapedMention}' (see below for site content)`
} else if (unescapedMention.startsWith("/")) {
const mentionPath = unescapedMention.slice(1)
return mentionPath.endsWith("/")
? `'${mentionPath}' (see below for folder content)`
: `'${mentionPath}' (see below for file content)`
} else if (mention === "problems") {
} else if (unescapedMention === "problems") {
return `Workspace Problems (see below for diagnostics)`
} else if (mention === "git-changes") {
} else if (unescapedMention === "git-changes") {
return `Working directory changes (see below for details)`
} else if (/^[a-f0-9]{7,40}$/.test(mention)) {
return `Git commit '${mention}' (see below for commit info)`
} else if (mention === "terminal") {
} else if (/^[a-f0-9]{7,40}$/.test(unescapedMention)) {
return `Git commit '${unescapedMention}' (see below for commit info)`
} else if (unescapedMention === "terminal") {
return `Terminal Output (see below for output)`
}
return match
Expand Down
38 changes: 21 additions & 17 deletions src/shared/context-mentions.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,31 @@
/*
Mention regex:
- **Purpose**:
- To identify and highlight specific mentions in text that start with '@'.
- **Purpose**:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kunwarVivek can you pleaes remove the unnecessary changes to spaces and comments to focus the PR? Minor tweak. When using AI to write fixes its usually good practice to remove any uneeded changes to lines like this.

- To identify and highlight specific mentions in text that start with '@'.
- These mentions can be file paths, URLs, or the exact word 'problems'.
- Ensures that trailing punctuation marks (like commas, periods, etc.) are not included in the match, allowing punctuation to follow the mention without being part of it.

- **Regex Breakdown**:
- `/@`:
- `/@`:
- **@**: The mention must start with the '@' symbol.
- `((?:\/|\w+:\/\/)[^\s]+?|problems\b|git-changes\b)`:

- `((?:\/|\w+:\/\/)[^\r\n]*?(?=\s*$|\s+@|[.,;:!?](?=[\s\r\n]|$))|problems\b|git-changes\b)`:
- **Capturing Group (`(...)`)**: Captures the part of the string that matches one of the specified patterns.
- `(?:\/|\w+:\/\/)`:
- `(?:\/|\w+:\/\/)`:
- **Non-Capturing Group (`(?:...)`)**: Groups the alternatives without capturing them for back-referencing.
- `\/`:
- `\/`:
- **Slash (`/`)**: Indicates that the mention is a file or folder path starting with a '/'.
- `|`: Logical OR.
- `\w+:\/\/`:
- `\w+:\/\/`:
- **Protocol (`\w+://`)**: Matches URLs that start with a word character sequence followed by '://', such as 'http://', 'https://', 'ftp://', etc.
- `[^\s]+?`:
- **Non-Whitespace Characters (`[^\s]+`)**: Matches one or more characters that are not whitespace.
- **Non-Greedy (`+?`)**: Ensures the smallest possible match, preventing the inclusion of trailing punctuation.
- `[^\s\r\n]*?(?:\\[\s][^\s\r\n]*?)*?(?=\s*$|\s+@|[.,;:!?](?=[\s\r\n]|$))`:
- **Character Pattern**: Matches any characters except whitespace and line breaks.
- **Escaped Spaces**: The `(?:\\[\s][^\s\r\n]*?)*?` part allows for escaped spaces (like `\s`) in the path.
- **Followed by a lookahead**: Ensures the match ends at the end of the line, before another @ symbol, or before punctuation followed by whitespace or end of line.
- **This handles paths with escaped spaces (e.g., `my\ folder/my\ file.txt`)**.
- **Non-Greedy (`*?`)**: Ensures the smallest possible match.
- `|`: Logical OR.
- `problems\b`:
- `problems\b`:
- **Exact Word ('problems')**: Matches the exact word 'problems'.
- **Word Boundary (`\b`)**: Ensures that 'problems' is matched as a whole word and not as part of another word (e.g., 'problematic').
- `|`: Logical OR.
Expand All @@ -31,14 +34,15 @@ Mention regex:
- **Word Boundary (`\b`)**: Ensures that 'terminal' is matched as a whole word and not as part of another word (e.g., 'terminals').
- `(?=[.,;:!?]?(?=[\s\r\n]|$))`:
- **Positive Lookahead (`(?=...)`)**: Ensures that the match is followed by specific patterns without including them in the match.
- `[.,;:!?]?`:
- `[.,;:!?]?`:
- **Optional Punctuation (`[.,;:!?]?`)**: Matches zero or one of the specified punctuation marks.
- `(?=[\s\r\n]|$)`:
- `(?=[\s\r\n]|$)`:
- **Nested Positive Lookahead (`(?=[\s\r\n]|$)`)**: Ensures that the punctuation (if present) is followed by a whitespace character, a line break, or the end of the string.

- **Summary**:
- The regex effectively matches:
- Mentions that are file or folder paths starting with '/' and containing any non-whitespace characters (including periods within the path).
- Mentions that are file or folder paths starting with '/' and can contain escaped spaces within the path (e.g., 'my\ folder/my\ file.txt').
The regex properly handles paths with escaped spaces and ensures the entire path is captured.
- URLs that start with a protocol (like 'http://') followed by any non-whitespace characters (including query parameters).
- The exact word 'problems'.
- The exact word 'git-changes'.
Expand All @@ -50,7 +54,7 @@ Mention regex:

*/
export const mentionRegex =
/@((?:\/|\w+:\/\/)[^\s]+?|[a-f0-9]{7,40}\b|problems\b|git-changes\b|terminal\b)(?=[.,;:!?]?(?=[\s\r\n]|$))/
/@((?:\/|\w+:\/\/)[^\s\r\n]*?(?:\\[\s][^\s\r\n]*?)*?(?=\s*$|\s+@|[.,;:!?](?=[\s\r\n]|$))|[a-f0-9]{7,40}\b|problems\b|git-changes\b|terminal\b)(?=[.,;:!?]?(?=[\s\r\n]|$))/
export const mentionRegexGlobal = new RegExp(mentionRegex.source, "g")

export interface MentionSuggestion {
Expand Down
15 changes: 15 additions & 0 deletions webview-ui/src/utils/__tests__/context-mentions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,4 +372,19 @@ describe("shouldShowContextMenu", () => {
// Position cursor at the end to test the full word
expect(shouldShowContextMenu("@problems", 9)).toBe(true)
})

it("should return true for file paths with escaped spaces", () => {
// Test with a file path containing escaped spaces
expect(shouldShowContextMenu("@/path/to/my\ file.txt", 20)).toBe(true)
})

it("should return true for folder paths with escaped spaces", () => {
// Test with a folder path containing escaped spaces
expect(shouldShowContextMenu("@/path/to/my\ folder/", 20)).toBe(true)
})

it("should return true for nested paths with multiple escaped spaces", () => {
// Test with a deeply nested path containing multiple escaped spaces
expect(shouldShowContextMenu("@/root\ dir/my\ documents/project\ files/file.txt", 50)).toBe(true)
})
})
5 changes: 3 additions & 2 deletions webview-ui/src/utils/context-mentions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,9 @@ export function shouldShowContextMenu(text: string, position: number): boolean {

const textAfterAt = beforeCursor.slice(atIndex + 1)

// Check if there's any whitespace after the '@'
if (/\s/.test(textAfterAt)) return false
// Check if there's any whitespace immediately after the '@'
// This only checks the first character after @ to allow for paths with spaces
if (textAfterAt.startsWith(" ")) return false

// Don't show the menu if it's clearly a URL
if (textAfterAt.toLowerCase().startsWith("http")) {
Expand Down
Loading