-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Awaiting Payment] [$250] Single backtick code block is rendered within triple backtick code block when there is emoji #49276
Comments
Triggered auto assignment to @trjExpensify ( |
We think that this bug might be related to Live Markdown |
@trjExpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
Job added to Upwork: https://www.upwork.com/jobs/~021836063922739129987 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Ollyws ( |
Hello, there is a solution to your problem. You can find emojis using UTF-8 encoding and replace them with an empty string using the replace method. |
📣 @Irbis007! 📣
|
Contributor details Hello, there is a solution to your problem. You can find emojis using UTF-8 encoding and replace them with an empty string using the replace method. |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
Edited by proposal-police: This proposal was edited at 2024-09-22 06:03:09 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.The single backtick code block will not be rendered within the triple backtick code block when there is an emoji What is the root cause of that problem?The problem is in the ExpensiMark file of expensify-common package, the regex in formatting rule of inlineCodeBlock is slightly incorrect. The previous regex failed due to overly greedy matching, inconsistent handling of nested backticks (single inside triple), and not accounting for emojis properly. What changes do you think we should make in order to solve the problem?We need to update the rule with the updated regex and replacement
What alternative solutions did you explore? (Optional)Working solutionScreen.Recording.2024-09-22.at.11.22.30.AM.1.mov |
@abhinaybathina Thanks for the proposal but won't your solution disable markdown emojis completely? |
Hi @Ollyws, no it won't disable. The rule is just to format the emoji to wrap within tags. But because of this rule, it's |
That rule was intentionally added in Expensify/expensify-common#669, I don't think it's a good idea to disable it as it will cause the original issue to re-appear. |
I couldn't see any changes related to this rule in Expensify/expensify-common#669. Please correct me if I am wrong |
in Expensify/expensify-common@7d6659c, I don't know why it doesn't appear in 'Files Changed'...strange. |
Thanks for sharing, I'll check and let you know |
ProposalPlease re-state the problem that we are trying to solve in this issue.Single backtick code block is rendered within triple backtick code block when followed by emoji What is the root cause of that problem?Currently, the regex for inline code blocks rendered within triple backtick code blocks in ExpensifyMark incorrectly matches cases where a single backtick code block is followed by an emoji, as in: ``` This is wrongly converted to: <pre><code>diamond</code><emoji>💍</emoji></pre> It should be converted into: <pre>`diamond`<emoji>💍</emoji></pre> The issue occurs because the inline code block is followed by the "<" symbol from the What changes do you think we should make in order to solve the problem?Modify this rule to prevent matching inline code blocks when they are followed by a "<" symbol, such as an emoji tag. should be updated to: /(\B|_|)`((?:`)*(?!`).*?[\S| |\u00A0]+?.*?(?<!`)(?:`)*)`(\B|_|)(?!`|(?!<pre>)[^<]*(?:<(?!pre>)[^<]*)*<\/pre>|[^<]*<\/video>)/gm This would replace the What alternative solutions did you explore? (Optional)Result |
@ra-md Thanks for the proposal, but with your regex the Screen.Recording.2024-09-21.at.14.33.49.mov |
My proposal is based on the {
name: 'inlineCodeBlock',
// Use the url escaped version of a backtick (`) symbol. Mobile platforms do not support lookbehinds,
// so capture the first and third group and place them in the replacement.
// but we should not replace backtick symbols if they include <pre> tags between them.
regex: /(\B|_|)`((?:`)*)(?!`)(.*?\S+?.*?)(?<!`)((?:`)*)(`)(\B|_|)(?!`|[^<]*<\/pre>|[^<]*<\/video>)/gm,
replacement: '$1<code>$2$3$4</code>$6',
}, {
name: 'inlineCodeBlock',
// Use the url escaped version of a backtick (`) symbol. Mobile platforms do not support lookbehinds,
// so capture the first and third group and place them in the replacement.
// but we should not replace backtick symbols if they include <pre> tags between them.
// At least one non-whitespace character or a specific whitespace character (" " and "\u00A0")
// must be present inside the backticks.
regex: /(\B|_|)`((?:`)*(?!`).*?[\S| |\u00A0]+?.*?(?<!`)(?:`)*)`(\B|_|)(?!`|[^<]*<\/pre>|[^<]*<\/video>)/gm,
replacement: (_extras, _match, g1, g2, g3) => {
const g2Value = g2.trim() === '' ? g2.replaceAll(' ', ' ') : g2;
return `${g1}<code>${g2Value}</code>${g3}`;
},
}, Please modify the replacement function to match the one in v2.0.90. |
@Ollyws I've updated my proposal here #49276 (comment) |
@Ollyws Did you get a chance to look into my updated solution? Kindly let me know if that works! Thanks :) |
Still evaluating these, will post and update tomorrow. |
Triggered auto assignment to @thienlnam, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @ra-md 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@ra-md merged! |
@thienlnam This should be ready for payment. |
Yup, looks like this was deployed to prod on Oct 1st, 2024 - cc @trjExpensify Should be good to go |
Requested in ND. |
$250 approved for @Ollyws |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 9.0.35-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com
Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
The single backtick code block will not be rendered within the triple backtick code block when there is an emoji.
Actual Result:
The single backtick code block is rendered within the triple backtick code block when there is an emoji.
Workaround:
Unknown
Platforms:
Screenshots/Videos
Bug6604455_1726412999725.20240915_230535.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @trjExpensifyThe text was updated successfully, but these errors were encountered: