fix: guard active @ replacement and extract needsSpaceBeforeMention helper

- Extract needsSpaceBeforeMention(text) helper for consistent space detection
- Use helper across insertMention, ChatTextArea drag/drop, and text insertion
- Add findActiveAtIndex to guard @ replacement (only replace @ with no whitespace between it and cursor)
- Handle \n and \r as whitespace in space-before-mention check
- Add comprehensive tests for active @ detection and helper function

Addresses PR review feedback:
1. Guard replacement to active @ segment only
2. Align space-before-mention behavior across insertion paths
This commit is contained in:
Roo Code 2026-01-06 17:32:25 +00:00
parent 7ca604600f
commit 5b450e0908
3 changed files with 118 additions and 17 deletions

View file

@ -15,6 +15,7 @@ import {
ContextMenuOptionType,
getContextMenuOptions,
insertMention,
needsSpaceBeforeMention,
removeMention,
shouldShowContextMenu,
SearchResult,
@ -153,8 +154,7 @@ export const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
// Check if we need to add a space before the command
const textBefore = currentValue.slice(0, cursorPos)
const needsSpaceBefore = textBefore.length > 0 && !textBefore.endsWith(" ")
const prefix = needsSpaceBefore ? " " : ""
const prefix = needsSpaceBeforeMention(textBefore) ? " " : ""
// Insert the text at cursor position
const newValue =
@ -792,9 +792,7 @@ export const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
// Check if we need to add a space before the first mention
const textBefore = inputValue.slice(0, cursorPosition)
const needsSpaceBefore =
textBefore.length > 0 && !textBefore.endsWith(" ") && !textBefore.endsWith("\n")
if (needsSpaceBefore) {
if (needsSpaceBeforeMention(textBefore)) {
newValue += " "
totalLength += 1
}

View file

@ -1,5 +1,6 @@
import {
insertMention,
needsSpaceBeforeMention,
removeMention,
getContextMenuOptions,
shouldShowContextMenu,
@ -8,6 +9,28 @@ import {
SearchResult,
} from "@src/utils/context-mentions"
describe("needsSpaceBeforeMention", () => {
it("should return false for empty text", () => {
expect(needsSpaceBeforeMention("")).toBe(false)
})
it("should return true for text ending with non-whitespace", () => {
expect(needsSpaceBeforeMention("Hello")).toBe(true)
expect(needsSpaceBeforeMention("Check")).toBe(true)
expect(needsSpaceBeforeMention("test123")).toBe(true)
})
it("should return false for text ending with space", () => {
expect(needsSpaceBeforeMention("Hello ")).toBe(false)
expect(needsSpaceBeforeMention("Check ")).toBe(false)
})
it("should return false for text ending with newline", () => {
expect(needsSpaceBeforeMention("Hello\n")).toBe(false)
expect(needsSpaceBeforeMention("Check\r")).toBe(false)
})
})
describe("insertMention", () => {
it("should insert mention at cursor position when no @ symbol exists", () => {
const result = insertMention("Hello world", 5, "test")
@ -116,6 +139,47 @@ describe("insertMention", () => {
expect(result.newValue.includes("\\ ")).toBe(false)
})
// --- Tests for active @ detection (guarding earlier mentions) ---
describe("active @ detection", () => {
it("should not replace earlier @ when cursor is after whitespace", () => {
// Text: "Hello @file.ts now I want another"
// Cursor at position 21 (at 'w' in "want"), so beforeCursor = "Hello @file.ts now I " (ends with space)
// The @ at position 6 should NOT be replaced because there's whitespace between it and cursor
const result = insertMention("Hello @file.ts now I want another", 21, "test")
// Should insert new mention at cursor position, not replace the earlier @file.ts
// Since beforeCursor ends with space, no extra space is added
expect(result.newValue).toBe("Hello @file.ts now I @test want another")
expect(result.mentionIndex).toBe(21)
})
it("should replace @ when cursor is directly after it with no whitespace", () => {
// Text: "Hello @fi"
// Cursor at position 9 (right after "fi")
const result = insertMention("Hello @fi", 9, "file.ts")
// Should replace from @ since cursor is directly after partial mention
expect(result.newValue).toBe("Hello @file.ts ")
expect(result.mentionIndex).toBe(6)
})
it("should handle multiple @ symbols - only active one is affected", () => {
// Text: "Check @file1.ts and @fi"
// Cursor at position 23 (after "@fi")
// Should replace from the second @ (at position 20), not the first one
const result = insertMention("Check @file1.ts and @fi", 23, "file2.ts")
expect(result.newValue).toBe("Check @file1.ts and @file2.ts ")
expect(result.mentionIndex).toBe(20)
})
it("should insert new mention when cursor is far from any @", () => {
// Text: "Check @file.ts and then some text here"
// Cursor at position 38 (at the end after "here")
const result = insertMention("Check @file.ts and then some text here", 38, "test")
// No active @, should insert new mention at cursor
expect(result.newValue).toBe("Check @file.ts and then some text here @test ")
expect(result.mentionIndex).toBe(39)
})
})
// --- Tests for isSlashCommand parameter ---
describe("isSlashCommand parameter", () => {
it("should replace entire text when isSlashCommand is true", () => {

View file

@ -25,6 +25,45 @@ function getBasename(filepath: string): string {
return filepath.split("/").pop() || filepath
}
/**
* Helper function to determine if a space is needed before inserting a mention.
* Returns true if the text has content and doesn't end with whitespace.
* @param text - The text to check
* @returns true if a space should be inserted before the mention
*/
export function needsSpaceBeforeMention(text: string): boolean {
if (text.length === 0) {
return false
}
const lastChar = text[text.length - 1]
return lastChar !== " " && lastChar !== "\n" && lastChar !== "\r"
}
/**
* Finds the active @ symbol position for mention insertion.
* An @ is "active" if there's no unescaped whitespace between it and the cursor.
* @param text - Text before cursor
* @returns The index of the active @, or -1 if no active @ exists
*/
function findActiveAtIndex(text: string): number {
const lastAtIndex = text.lastIndexOf("@")
if (lastAtIndex === -1) {
return -1
}
// Check if there's any unescaped whitespace between @ and cursor
const textAfterAt = text.slice(lastAtIndex + 1)
// Look for unescaped whitespace (space not preceded by backslash)
const hasUnescapedWhitespace = /(?<!\\)\s/.test(textAfterAt)
if (hasUnescapedWhitespace) {
// The @ is not active - there's whitespace between @ and cursor
return -1
}
return lastAtIndex
}
export function insertMention(
text: string,
position: number,
@ -42,8 +81,8 @@ export function insertMention(
const beforeCursor = text.slice(0, position)
const afterCursor = text.slice(position)
// Find the position of the last '@' symbol before the cursor
const lastAtIndex = beforeCursor.lastIndexOf("@")
// Find the active '@' symbol - only considers @ with no whitespace between it and cursor
const activeAtIndex = findActiveAtIndex(beforeCursor)
// Process the value - escape spaces if it's a file path
let processedValue = value
@ -57,13 +96,13 @@ export function insertMention(
let newValue: string
let mentionIndex: number
if (lastAtIndex !== -1) {
// If there's an '@' symbol, check if we need to add a space before it
const beforeAt = text.slice(0, lastAtIndex)
const needsSpaceBefore = beforeAt.length > 0 && !beforeAt.endsWith(" ") && !beforeAt.endsWith("\n")
if (activeAtIndex !== -1) {
// If there's an active '@' symbol, check if we need to add a space before it
const beforeAt = text.slice(0, activeAtIndex)
const spaceBefore = needsSpaceBeforeMention(beforeAt)
// If we need a space before @, add it
const beforeMention = needsSpaceBefore ? beforeAt + " " : beforeAt
const beforeMention = spaceBefore ? beforeAt + " " : beforeAt
// Only replace if afterCursor is all alphanumerical
// This is required to handle languages that don't use space as a word separator (chinese, japanese, korean, etc)
@ -71,15 +110,15 @@ export function insertMention(
? afterCursor.replace(/^[^\s]*/, "")
: afterCursor
newValue = beforeMention + "@" + processedValue + " " + afterCursorContent
mentionIndex = needsSpaceBefore ? lastAtIndex + 1 : lastAtIndex
mentionIndex = spaceBefore ? activeAtIndex + 1 : activeAtIndex
} else {
// If there's no '@' symbol, insert the mention at the cursor position
// If there's no active '@' symbol, insert the mention at the cursor position
// Check if we need to add a space before the mention
const needsSpaceBefore = beforeCursor.length > 0 && !beforeCursor.endsWith(" ") && !beforeCursor.endsWith("\n")
const prefix = needsSpaceBefore ? " " : ""
const spaceBefore = needsSpaceBeforeMention(beforeCursor)
const prefix = spaceBefore ? " " : ""
newValue = beforeCursor + prefix + "@" + processedValue + " " + afterCursor
mentionIndex = position + (needsSpaceBefore ? 1 : 0)
mentionIndex = position + (spaceBefore ? 1 : 0)
}
return { newValue, mentionIndex }