Skip to content

Updated link markdown regex#4461

Merged
acywatson merged 3 commits intofacebook:mainfrom
ronaldlangeveld:fixed-link-regex
Aug 10, 2023
Merged

Updated link markdown regex#4461
acywatson merged 3 commits intofacebook:mainfrom
ronaldlangeveld:fixed-link-regex

Conversation

@ronaldlangeveld
Copy link
Copy Markdown
Contributor

@ronaldlangeveld ronaldlangeveld commented May 5, 2023 •
edited
Loading

No issue

The current regExp is causing non-link Markdown syntax to be parsed as a link.

Example, the markdown syntax for an image would be ![alt text](https://example.com/example.jpeg would be detected as link markdown and render it as a link, eg !<a href"">alt text</a>.
It completely overrides my custom transformers that wants to parse markdown to a custom image node.

This fix updates the regex to not render a link if ! is in front of the link markdown syntax, allowing the custom transformer to initiate if available.

@ronaldlangeveld
Updated link markdown regex
a66a3ae 
no issue

The current `regExp` is causing non-link Markdown syntax to be parsed as a link.
Example, the markdown syntax for an image would be `![alt text](https://example.com/example.jpeg`
would be detected as link markdown.
This fix updates the regex to not render a link if ! is infront of the
link markdown syntax.
@ronaldlangeveld
Added testing for the updated regex.
3bbb5e5
@vercel
Copy link
Copy Markdown

vercel bot commented May 5, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 6, 2023 8:55am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 6, 2023 8:55am

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented May 5, 2023

Hi @ronaldlangeveld!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@ronaldlangeveld ronaldlangeveld marked this pull request as ready for review May 5, 2023 14:45
@ronaldlangeveld
Copy link
Copy Markdown
Contributor Author

ronaldlangeveld commented May 5, 2023

CLA signed, should probably just rerun the check. 😊

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 5, 2023
@ronaldlangeveld ronaldlangeveld requested a review from fantactuka May 9, 2023 12:09
fantactuka
fantactuka approved these changes May 19, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@fantactuka fantactuka left a comment

Choose a reason for hiding this comment

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

LGTM, let's wait for CI to pass and can merge!

@ronaldlangeveld
Copy link
Copy Markdown
Contributor Author

ronaldlangeveld commented May 19, 2023

LGTM, let's wait for CI to pass and can merge!

Awesome thank you!

@zurfyx
Copy link
Copy Markdown
Member

zurfyx commented May 19, 2023

For some reason the Mac variants are taking forever to complete. We can retry later but let's hold on merging for now in case there's a real issue caused by this PR.

@ronaldlangeveld
Copy link
Copy Markdown
Contributor Author

ronaldlangeveld commented May 20, 2023

Will test locally too and see if the same happens. 😬

@ronaldlangeveld
Copy link
Copy Markdown
Contributor Author

ronaldlangeveld commented May 23, 2023 •
edited
Loading

Sorry, I'm struggling a bit reproducing the failing CI tests... I think it could've been something flaky. Can we rerun the test on CI?

@fantactuka fantactuka mentioned this pull request May 25, 2023
Open
@ronaldlangeveld
Copy link
Copy Markdown
Contributor Author

ronaldlangeveld commented Jun 13, 2023 •
edited
Loading

Having a tough time getting tests to run reliably locally, not sure what I'm missing to get it to run as expected, see discussion board post #4591

@ronaldlangeveld
Copy link
Copy Markdown
Contributor Author

ronaldlangeveld commented Aug 10, 2023

@fantactuka could you maybe help me look into why that 1 test is failing? Maybe we can merge main into this and check if tests behave better? Would be great to have this one merged in.

@acywatson
Copy link
Copy Markdown
Contributor

acywatson commented Aug 10, 2023

Yea I'm pretty confident this is fine - sorry for the delay

@acywatson acywatson merged commit c8f8366 into facebook:main Aug 10, 2023
zurfyx added a commit that referenced this pull request Aug 16, 2023
@zurfyx
Revert "Updated link markdown regex (#4461)"
254773c 
This reverts commit c8f8366.
zurfyx added a commit that referenced this pull request Aug 17, 2023
@zurfyx
Revert "Updated link markdown regex (#4461)" (#4896)
7f62dec
@acywatson acywatson mentioned this pull request Sep 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fantactuka fantactuka fantactuka approved these changes

@zurfyx zurfyx Awaiting requested review from zurfyx zurfyx is a code owner

@acywatson acywatson Awaiting requested review from acywatson acywatson is a code owner

@tylerjbainbridge tylerjbainbridge Awaiting requested review from tylerjbainbridge

@thegreatercurve thegreatercurve Awaiting requested review from thegreatercurve

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ronaldlangeveld @facebook-github-bot @zurfyx @acywatson @fantactuka