Skip to content

Conversation

@Skalakid
Copy link
Contributor

@Skalakid Skalakid commented Jul 4, 2025

This PR updates the codeblock parsing rule so it:

  1. Doesn't accept any text before the code block opening syntax
  2. Require a new line after the code block opening syntax and before the closing syntax

We introduce this change to make parsing rules when typing markdown text inside the Live Markdown input more similar to the result that the user can see after sending the message in E/App. The above changes allow us to create a more intuitive and logical way to use code blocks when typing. Simplifying the parsing rules will also protect us from many complex and hard styling possibilities in the future.

This PR is part of code fence and inline code block refactor in the react-native-live-markdown library - PR

Fixed Issues

Expensify/App#39518

Tests

  1. Verify if the following scenarios aren't parsed as code fences:
```codeblock```
```test\ntest```
test ```\ntest\n```
  1. Verify if only the following messages are parsed as code fences:
```\ntest\n```
```\ntest\n```test
test\n```\ntest\n```test
test\n```\ntest\n```\ntest

QA

Verify new parsing rules as specified in the Test section

Copy link

@jmusial jmusial left a comment

Choose a reason for hiding this comment

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

LGTM

codeFenceExample = '~Test1 ```\ncode\n``` Test2~';
expect(parser.replace(codeFenceExample)).toBe('~Test1 <pre>code<br /></pre> Test2~');
expect(parser.replace(codeFenceExample, {shouldKeepRawInput: true})).toBe('~Test1 <pre>\ncode\n</pre> Test2~');
codeFenceExample = '~Test1\n```\ncode\n```\nTest2~';
Copy link

Choose a reason for hiding this comment

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

NAB, or part of this issue, but IMO the tests would benefit from making

Test code fencing with ExpensiMark syntax outside

a describe and having test cases as test with separate descriptions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think it applies to all other markdown tests. They are heavily mixed, so reorganizing them into describe sections would be beneficial

@dangrous dangrous requested review from a team and removed request for a team July 8, 2025 16:32
@melvin-bot melvin-bot bot requested review from chiragsalian and removed request for a team July 8, 2025 16:33
@chiragsalian chiragsalian merged commit bb824b2 into Expensify:main Jul 8, 2025
6 checks passed
@os-botify
Copy link
Contributor

os-botify bot commented Jul 8, 2025

🚀 Published to npm in 2.0.144 🎉

@jjcoffee
Copy link
Contributor

jjcoffee commented Jul 16, 2025

@Skalakid We have a version bump PR in App that includes this change. Is it safe to do now or should we wait until you've completed your project?

@Skalakid
Copy link
Contributor Author

@jjcoffee Yeah, it should be safe to bump it inside E/App now. It will just change codeblock parsing rules :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants