Support mentioning filenames with spaces #3044
Conversation
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 <mrubens@users.noreply.github.com>
|
dosubot
bot
added
size:XL
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * @returns A path with spaces escaped | ||
| */ | ||
| export function escapeSpaces(path: string): string { | ||
| return path.replace(/ /g, "\\ ") |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 days ago
To fix the issue, we need to ensure that backslashes in the input are escaped before escaping spaces. This can be achieved by first replacing all backslashes (\) with double backslashes (\\) and then replacing spaces with \ . This ensures that any existing backslashes are properly escaped, avoiding ambiguity or incorrect output.
The fix involves modifying the escapeSpaces function to handle backslashes in addition to spaces. Specifically:
- Replace all backslashes (
\) with double backslashes (\\) using a regular expression with the global flag. - Replace all spaces with
\as before.
No additional imports or dependencies are required for this fix.
| @@ -11,3 +11,4 @@ | ||
| export function escapeSpaces(path: string): string { | ||
| return path.replace(/ /g, "\\ ") | ||
| // Escape backslashes first, then spaces | ||
| return path.replace(/\\/g, "\\\\").replace(/ /g, "\\ "); | ||
| } |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| // Check if there's any unescaped whitespace after the '@' | ||
| // We need to check for whitespace that isn't preceded by a backslash | ||
| // Using a negative lookbehind to ensure the space isn't escaped | ||
| const hasUnescapedSpace = /(?<!\\)\s/.test(textAfterAt) |
There was a problem hiding this comment.
The negative lookbehind regex /(?<!\)\s/ in shouldShowContextMenu might not be supported in older JS engines.
| const hasUnescapedSpace = /(?<!\\)\s/.test(textAfterAt) | |
| const hasUnescapedSpace = /(^|[^\\])\s/.test(textAfterAt) |
fwiw i started working on this in #2881, and yes the scope of it is quite big, haven't got the time to continue but will do it over the weekends |
|
@cte last time i change regex to string and using replaceAll codeql make that pass ''' Something like this |
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 daunting.Adapted from 8955d45 / #2363.
Context
Addresses #2361
Screenshots
before
spaces-in-mentions-not-working.mov
after
spaces-in-mentions-escaped.mov
Important
Adds support for file path mentions with spaces by allowing spaces to be escaped with backslashes, updating regex patterns, and enhancing path handling functions.
\\inparseMentions()andopenMention()inindex.ts.mentionRegexincontext-mentions.tsto match paths with escaped spaces.insertMention()incontext-mentions.tsnow escapes spaces in file paths.index.test.ts,context-mentions.test.ts, andpath-mentions.test.ts.escapeSpaces()topath-mentions.tsto escape spaces in paths.convertToMentionPath()to handle paths with spaces.This description was created by
for 07bb1f1. You can customize this summary. It will automatically update as commits are pushed.