-
-
Notifications
You must be signed in to change notification settings - Fork 390
fix: skip emoji conversion inside code blocks #2373
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -1905,15 +1905,16 @@ const emojis = { | |
| 'wales': '🏴', | ||
| } | ||
|
|
||
| const emojisKeysRegex = new RegExp( | ||
| Object.keys(emojis) | ||
| .map(key => `:${key}:`) | ||
| .join('|'), | ||
| 'g', | ||
| ) | ||
| export function convertToEmoji(text: string): string { | ||
| return text.replace(emojisKeysRegex, match => { | ||
| const key = match.slice(1, -1) as keyof typeof emojis | ||
| return emojis[key] || match | ||
| }) | ||
| export function convertToEmoji(html: string): string { | ||
| return html.replace( | ||
| /(<code[\s>][\s\S]*?<\/code>|<pre[\s>][\s\S]*?<\/pre>)|(:[\w+-]+:)/gi, | ||
| (match, codeBlock: string | undefined, shortcode: string | undefined) => { | ||
| if (codeBlock) return codeBlock | ||
| if (shortcode) { | ||
| const key = shortcode.slice(1, -1) as keyof typeof emojis | ||
| return emojis[key] || shortcode | ||
|
Comment on lines
+1914
to
+1915
Contributor
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. 🛠️ Refactor suggestion | 🟠 Major 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify TypeScript strictness settings and locate assertion-based lookup in this utility.
fd -i 'tsconfig*.json' --exec sh -c 'echo "=== {} ==="; cat "{}"' \;
echo
rg -n --type=ts -C2 'as keyof typeof emojis|const emojis\s*:\s*Record<string,\s*string>|emojis\[key\]' shared/utils/emoji.tsRepository: npmx-dev/npmx.dev Length of output: 1366 🏁 Script executed: # Check file size first
wc -l shared/utils/emoji.ts
# Read the area around the disputed lines and the emojis definition
head -n 100 shared/utils/emoji.ts | tail -n 50
# Also check the beginning to find the emojis object definition
head -n 50 shared/utils/emoji.tsRepository: npmx-dev/npmx.dev Length of output: 2543 🏁 Script executed: # Read the area around the disputed lines
sed -n '1900,1920p' shared/utils/emoji.ts
# Also check the end of the file to see if there's an export
tail -n 30 shared/utils/emoji.ts
# Check for how many entries are in the emojis object
grep -n "^const emojis\|^}" shared/utils/emoji.ts | head -n 5Repository: npmx-dev/npmx.dev Length of output: 1512 Remove assertion-based key narrowing in emoji lookup to improve type safety. Line 1914 uses Suggested refactor-const emojis = {
+const emojis: Record<string, string> = {
'100': '💯',
'1234': '🔢',
...
}
@@
- const key = shortcode.slice(1, -1) as keyof typeof emojis
- return emojis[key] || shortcode
+ const key = shortcode.slice(1, -1)
+ return emojis[key] ?? shortcode |
||
| } | ||
| return match | ||
| }, | ||
| ) | ||
| } | ||
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.
this previously took in
text, but nowhtml- is this definitely the case everywhere? and if not I guess it doesn't matter?