Skip to content

fix(mention): conditionally remove aftercursor content #2732

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 23, 2025
Merged

fix(mention): conditionally remove aftercursor content #2732

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 6 additions & 1 deletion webview-ui/src/utils/context-mentions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ export function insertMention(
if (lastAtIndex !== -1) {
// If there's an '@' symbol, replace everything after it with the new mention
const beforeMention = text.slice(0, lastAtIndex)
newValue = beforeMention + "@" + value + " " + afterCursor.replace(/^[^\s]*/, "")
// 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)
const afterCursorContent = /^[a-zA-Z0-9\s]*$/.test(afterCursor)
? afterCursor.replace(/^[^\s]*/, "")
: afterCursor
newValue = beforeMention + "@" + value + " " + afterCursorContent
Comment on lines +36 to +41
Copy link
Collaborator

@mrubens mrubens Apr 18, 2025

Choose a reason for hiding this comment

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

I don't know that I see why this replacing is necessary at all. What's the downside of just always doing newValue = beforeMention + "@" + value + " " + afterCursor?

Copy link
Contributor Author

@elianiva elianiva Apr 18, 2025

Choose a reason for hiding this comment

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

my best guess would be for this case (hope this makes sense)

suppose that we have this scenario for the prompt

@/src/pages/feature-a|/index.jsx implement something for this page
                     ^ cursor position

with the current code, when the user choose other folder, such as feature-b and then press enter, it will be replaced as

@/src/pages/feature-b/index.jsx| implement ...
                               ^ cursor position

if we don't do the replacement, we'll end up with

@/src/pages/feature-b/index.jsx|/index.jsx implement ...
                               ^ cursor position

i'm not sure if i like this behaviour either, i'm planning to simplify the display so that it will only show folder name for folders and filename for files, just like how cursor does it

most of the time you'll remove the full path and start over with the mention anyway, and it gets pretty long on the prompt UI if you include a lot of files

so instead of this

@/src/pages/feature-b/index.jsx implement this feature

we'll have something like this instead

[index.jsx] implement this feature
^ an actual box, not [] character

so in the case of longer contexts, instead of this:

@/src/pages/feature-b/index.jsx implement @/src/components/card.tsx to handle data coming from @/src/services/data.ts

we'll have this instead

[index.jsx] implement [card.tsx] to handle data coming from [data.ts]

and the current behaviour of the infix-cursor-word-replacement would be no longer needed

mentionIndex = lastAtIndex
} else {
// If there's no '@' symbol, insert the mention at the cursor position
Expand Down
Loading