-
Notifications
You must be signed in to change notification settings - Fork 136
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
Unify the blockquote parsing logic for messages and rawInput #820
Unify the blockquote parsing logic for messages and rawInput #820
Conversation
17c5562
to
a1511ab
Compare
commenting for the assignment. |
Hmm for some reason I can't add him tot he issue even if he commented |
@Skalakid I am trying to link this commit in E/App but facing some issue while running the app. Wondering how you connected and tested in E/App? |
@Pujan92 expensify-common commits can't be linked to the E/App, but you can add my changes by running |
@Skalakid Isn't the message here should be the same as of live markdown input(Applied a space between 2 blockquotes)? Screen.Recording.2024-12-05.at.15.55.53.mov |
@Pujan92 Oh, I forgot to tell you one more detail to set up the testing environment, sorry 😅 You must also apply these ExpensiMark changes inside the Live Markdown Library. To do that, you have to:
And now you will have new parsing logic applied both in the NewDot and Live Markdown libraries :D |
Also, it's worth mentioning here that new rules for the blockquote has been discussed here and now this "> > >" won't be parsed as a nested blocquote |
I updated the PR description with new rules explanation |
@Pujan92 I can't assign you for review to this PR but let me know when you have tested it and you can just leave a LGTM and I will merge it! |
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.
LGTM, Thanks @Skalakid!
This PR changes and unifies the blockquote parsing logic for the NewDot messages and the Live Markdown Input. Currently, the parsing result is different when the
shouldKeepRawInput
variable istrue
. Thanks to the changes in this PR, the results are consistent (visually). The only difference is that the tags are being grouped whenshouldKeepRawInput=false
.Example:
NewDot sent message
shouldKeepRawInput=false
Live Markdown Input
shouldKeepRawInput=true
Also, it's worth mentioning that new blocquote parsing rules has been discussed here and now the following cases are parsed:
Fixed Issues
$ Expensify/App#45154
$ Expensify/App#47951
Tests
QA
Same as Tests