fix: Support Unicode characters in keywords check #41
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Thank you for submitting this @yehorkardash, @steven10a and I are taking a look now! |
There was a problem hiding this comment.
@yehorkardash Thank you for this PR! Could you do two things for us to get it ready to be merged? Otherwise I can implement them myself in a separate PR
- I believe this change would result in missing keywords that end or start with non-word characters. Can you test this with
@foomatches inexample@foo.comand#fooinexample#fooand add them to the unit tests? keywordsCheckis not async. Can you remove theasyncandawaitfrom the tests as commented
| it('ignores text without the configured keywords', () => { | ||
| const result = keywordsCheck( | ||
| it('ignores text without the configured keywords', async () => { | ||
| const result = await keywordsCheck( |
There was a problem hiding this comment.
keywordsCheck is not async can you remove async and await here and in all the tests added below.
|
Hey @gabor-openai, @steven10a, thanks for review and picking this up! I'll be able to get to update it only later this week, so I think you can close this PR and continue forward with your modified PR) |
* Support Unicode characters in keywords check Thank you @yehorkardash for finding this and submitting a fix in PR #41
|
Thank you so much for this @yehorkardash! We implemented a similar fix in #46 based on your finding and PR. |
3 participants
\bin regex doesn't work so well with Unicode characters. Replaced it with\pto be compatible with Unicode characters.