-
-
Notifications
You must be signed in to change notification settings - Fork 241
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
feat: allow regular expressions to be passed to search #204
feat: allow regular expressions to be passed to search #204
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #204 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 14 14
Lines 273 310 +37
Branches 30 30
=========================================
+ Hits 273 310 +37 ☔ View full report in Codecov by Sentry. |
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.
Looks great to me, nicely done! 🔥
* Search for emojis containing the provided name or pattern in their name. | ||
*/ | ||
export const search = (keyword: RegExp | string) => { | ||
assert.any([is.default.string, is.default.regExp], keyword) |
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.
[Non-actionable] Hmm, it is unfortunate to have to use the .default
before every property of is
. But we're on an older version of @sindresorhus/is
that doesn't set up ESM defaults properly yet. I think this is reasonable for now, and a followup issue could be to bump @sindresorhus/is
to latest.
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.
I created an issue #205.
What does [Non-actionable] mean?
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.
Oh I mean I'm not asking you to take any action in this PR. I'm not requesting changes, just noting something for the future.
We can ignore the |
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.
Thanks for the PR @ikelax! And thanks @JoshuaKGoldberg for the review
PR Checklist
status: accepting prs
Overview
search
acceptsRegExp
objects as an argument. They are evaluated for each emoji.💖