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.

Sign up for GitHub

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

Use Regexp.exec instead of replace for side-effects #13

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Use Regexp.exec instead of replace for side-effects #13

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
86ba463
Use `Regexp.exec` instead of `replace`
jridgewell Aug 1, 2022
bdb0931
Merge branch 'main' into exec-noo-replace
steven-tey Aug 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 14 additions & 16 deletions lib/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,22 @@ export function postScanner(teamsAndKeywords: TeamAndKeywords) {
const text = combineText(post); // combine text from post's title, text, and url
const teamsInterestedInThisPost = new Set() as Set<string>; // set of team IDs that are interested in this post

text.replace(scanner, (_, ...terms) => {
let match;
while ((match = scanner.exec(text))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be an equality == comparison?

Copy link

@omkarkulkarni2704 omkarkulkarni2704 Dec 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

No, scanner.exec(text) checks null. So == is not needed.

for (let i = 0; i < keywordArray.length; i++) {
if (terms[i] !== undefined) {
if (match[i + 1] !== undefined) {
// if the keyword is found in the text
const teamsSubscribedToThisKeyword = keywordMapping.get(
keywordArray[i]
);
teamsSubscribedToThisKeyword!.forEach((teamId) => {
// using ! here because we know teamsSubscribedToThisKeyword is always defined
// using ! here because we know teamsSubscribedToThisKeyword is always defined
for (const teamId of teamsSubscribedToThisKeyword!) {
teamsInterestedInThisPost.add(teamId); // add team ID to set of teams that are interested in this post (if not already in set)
});
}
break;
}
}
return ""; // replace all instances of keywords with empty string (just for the replace function, we're not actually interested in the text here)
});
}

return teamsInterestedInThisPost; // return set of team IDs that are interested in this post
};
Expand Down Expand Up @@ -95,23 +95,21 @@ export function regexOperations(post: any, keywords: string[]) {
? mrkdwn(decode(post?.text || "")).text
: "";

// We use String.replace here so that we can know which capture group is
// actually matched, so that we can extract the appropriate keyword.
combineText(post).replace(termsRegex, (_, ...terms: string[]) => {
const combined = combineText(post);

let match;
while ((match = termsRegex.exec(combined))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this an equals sign = or an equality == comparison?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Equals. In this situation, the extra parens are meant to indicate that this is an intentional assignment. See https://eslint.org/docs/latest/rules/no-cond-assign

// In order to preserve the case-sensitivity of the keywords, we do a bit of meta-programming.
// We generated N regex capture groups, and we want to see if any of them matched. The index
// of the capture group matches the index of the keyword, so we can then decorate the actual
// text in the post, and know which keyword matched.
for (let i = 0; i < keywords.length; i++) {
if (terms[i] !== undefined) {
if (match[i + 1] !== undefined) {
mentionedTerms.add(keywords[i]);
break;
}
}

// We don't actually care about the replaced text, we're just using this
// for the side-effects.
return "";
});
}

// This regex searches for our keywords, while also matching a few extra
// patterns that we want to prevent a keyword match during. Eg, "Vercel" may
Expand Down