-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix: restrict @-mention parsing to line-start or whitespace boundaries #7876
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,25 @@ | ||
| /* | ||
| Mention regex: | ||
| - **Purpose**: | ||
| - To identify and highlight specific mentions in text that start with '@'. | ||
| - **Purpose**: | ||
| - 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. | ||
| - Restricts @ parsing to line-start or after whitespace to avoid accidental loading from pasted logs. | ||
|
|
||
| - **Regex Breakdown**: | ||
| - `/@`: | ||
| - `(?:^|\s)`: | ||
| - **Non-Capturing Group (`(?:...)`)**: Groups the alternatives without capturing them. | ||
| - **Line Start or Whitespace (`^|\s`)**: The @ must be at the start of a line or preceded by whitespace. | ||
|
|
||
| - `(?<!\\)@`: | ||
| - **Negative Lookbehind (`(?<!\\)`)**: Ensures the @ is not escaped with a backslash. | ||
| - **@**: The mention must start with the '@' symbol. | ||
|
|
||
| - `((?:\/|\w+:\/\/)[^\s]+?|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+:\/\/`: | ||
|
|
@@ -25,7 +31,7 @@ Mention regex: | |
| - **Escaped Space (`\\ `)**: Matches a backslash followed by a space (an escaped space). | ||
| - **Non-Greedy (`+?`)**: Ensures the smallest possible match, preventing the inclusion of trailing punctuation. | ||
| - `|`: 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. | ||
|
|
@@ -34,9 +40,9 @@ 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**: | ||
|
|
@@ -48,13 +54,14 @@ Mention regex: | |
| - The exact word 'git-changes'. | ||
| - The exact word 'terminal'. | ||
| - It ensures that any trailing punctuation marks (such as ',', '.', '!', etc.) are not included in the matched mention, allowing the punctuation to follow the mention naturally in the text. | ||
| - **NEW**: The @ symbol must be at the start of a line or preceded by whitespace to prevent accidental matches in pasted logs. | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding examples of valid vs invalid patterns in the comments to make it immediately clear for future developers. For instance, showing that '@/path/to/file.txt' at line start is valid, 'Check @problems here' after space is valid, but 'error@https://api.com' mid-word is invalid. |
||
|
|
||
| - **Global Regex**: | ||
| - `mentionRegexGlobal`: Creates a global version of the `mentionRegex` to find all matches within a given string. | ||
|
|
||
| */ | ||
| export const mentionRegex = | ||
| /(?<!\\)@((?:\/|\w+:\/\/)(?:[^\s\\]|\\ )+?|[a-f0-9]{7,40}\b|problems\b|git-changes\b|terminal\b)(?=[.,;:!?]?(?=[\s\r\n]|$))/ | ||
| /(?:^|(?<=\s))(?<!\\)@((?:\/|\w+:\/\/)(?:[^\s\\]|\\ )+?|[a-f0-9]{7,40}\b|problems\b|git-changes\b|terminal\b)(?=[.,;:!?]?(?=[\s\r\n]|$))/ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The regex pattern has become quite complex with the addition of (?:^|(?<=\s)). Have you considered adding a helper function that validates mention boundaries separately? This could improve readability and make the logic easier to maintain. |
||
| export const mentionRegexGlobal = new RegExp(mentionRegex.source, "g") | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it intentional that the regex doesn't use the multiline flag? Without it, ^ only matches the absolute start of the string. Mentions at the beginning of lines after newlines currently work because \n counts as whitespace in (?<=\s), but this might be worth clarifying in the documentation. |
||
|
|
||
| // Regex to match command mentions like /command-name anywhere in text | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add a test case for mentions after punctuation with a space? For example, 'Hello! @problems' should work but 'Hello!@problems' shouldn't. This would explicitly document the expected behavior for this edge case.