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

chore: update regex for first, middle and last names #105

Merged
merged 3 commits into from
Aug 19, 2021

Conversation

kleyow
Copy link
Contributor

@kleyow kleyow commented Aug 18, 2021

No description provided.

@kleyow kleyow marked this pull request as ready for review August 18, 2021 20:53
@lewisdaly
Copy link
Contributor

@kleyow Is there a ticket related to this PR? I think we need to be careful here - there was some discussion about the regex being defined in the API and we've run into problems in the past with different regex engines in the past.

@elnyry-sam-k thoughts?

@elnyry-sam-k elnyry-sam-k requested a review from mdebarros August 19, 2021 06:17
@elnyry-sam-k
Copy link
Member

elnyry-sam-k commented Aug 19, 2021

Thanks @lewisdaly for catching this, appreciate it!

This came out of a fix we recently made in ALS and other places where names are used; there was support for myanmar script missing in the current regex; I discussed this with Kevin following the fix we made here: mojaloop/project#2358. Miguel corrected the regex to use the correct implementation interpretation of \w now which actually implements the regex as intended in the original FSPIOP Spec

@mdebarros
Copy link
Member

mdebarros commented Aug 19, 2021

mojaloop/project#2358

We know the \w in Javascript land does not match the intended Mojaloop Interface Specification. API Snippets should be modified to implement a regex pattern that matches the intended Unicode groups.

@lewisdaly see the following comment: mojaloop/project#2358 (comment) specifically.

That describes how we came to this regex pattern, and as per @elnyry-sam-k's comment...I believe it is as close as one can get to the standard Unicode definition for \w, unless Javascript ECMAScript actually implements the latest Unicode standards for \w.

Here is some additional "historical" context around this, and how the original regex alternative pattern came about: mojaloop/project#1261.

Copy link
Member

@mdebarros mdebarros left a comment

Choose a reason for hiding this comment

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

This PR is +1 from me, but I would prefer to wait for @lewisdaly to confirm that he is happy with this change. @lewisdaly please confirm from your side.

mdebarros
mdebarros approved these changes Aug 19, 2021
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