Skip to content
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

Fixed characters support between [] for Hyperlinks #400

Closed

Conversation

mananjadhav
Copy link
Contributor

@mananjadhav mananjadhav commented Jul 27, 2021

Can someone please review it?

  • Updated the regex for handling characters between [] for hyperlinks

Fixed Issues

$ Fixes: Expensify/App#4229

Tests

  1. "Test markdown and url links with inconsistent starting and closing parens" covers testing of the given changes
  2. Tested against incorrect pairs, symbols, and special characters handling

QA

  1. What does QA need to do to validate your changes?
  2. What areas to they need to test for regressions?

@mananjadhav mananjadhav requested a review from a team as a code owner July 27, 2021 18:49
@github-actions
Copy link

github-actions bot commented Jul 27, 2021

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@MelvinBot MelvinBot requested review from stitesExpensify and removed request for a team July 27, 2021 18:49
@mananjadhav
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@AndrewGable AndrewGable self-requested a review July 27, 2021 19:19
@mananjadhav mananjadhav changed the title Fixed characters support between with [] for Hyperlinks Fixed characters support between [] for Hyperlinks Jul 27, 2021
@AndrewGable
Copy link
Contributor

I'll let @stitesExpensify do the final review and merge!

@parasharrajat
Copy link
Member

parasharrajat commented Jul 27, 2021

I think it is good to let QA know that we should try complex combinations for the markup. Just to make sure nothing is broken with existing rules.

@AndrewGable
Copy link
Contributor

Sounds great @parasharrajat - Let's do that on the https://github.com/Expensify/App PR since I am not sure they get a checklist for these changes.

@stitesExpensify
Copy link
Contributor

Hmm it looks like the commits are not PGP signed, so we can't merge this. @mananjadhav we're going to need you to check out step 8 here and then create a new PR with the same changes.

@mananjadhav
Copy link
Contributor Author

Noted. Closing and raising another PR.

@mananjadhav
Copy link
Contributor Author

mananjadhav commented Jul 27, 2021

#401 PR raised with signed commits.

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.

Hyperlink markdown doesn't work if the ] is preceded by an (
4 participants