Skip to content

fix: fix YOUTUBE_REGEX_STRING being too greedy#1171

Merged
B4nan merged 1 commit intomasterfrom
fix-youtube-regex
Sep 10, 2021
Merged

fix: fix YOUTUBE_REGEX_STRING being too greedy#1171
B4nan merged 1 commit intomasterfrom
fix-youtube-regex

Conversation

@B4nan
Copy link
Copy Markdown
Member

@B4nan B4nan commented Sep 10, 2021

The .* in the regexp caused everything in between two youtube urls to be matched as the url, resulting in issues when there were more than one youtube url.

Closes #881

@B4nan B4nan requested a review from szmarczak September 10, 2021 10:55
Comment thread src/utils_social.js
const FACEBOOK_REGEX_STRING = `(?<!\\w)(?:http(?:s)?:\\/\\/)?(?:www.)?(?:facebook.com|fb.com)\\/(?!(?:${FACEBOOK_RESERVED_PATHS})(?:[\\'\\"\\?\\.\\/]|$))(profile\\.php\\?id\\=[0-9]{3,20}|(?!profile\\.php)[a-z0-9\\.]{5,51})(?![a-z0-9\\.])(?:/)?`;
// eslint-disable-next-line max-len, quotes
const YOUTUBE_REGEX_STRING = '(?:https?:\\/\\/)?(?:youtu\\.be\\/|(?:www\\.|m\\.)?youtube\\.com\\/(?:watch|v|embed)(?:\\.php)?(?:\\?.*v=|\\/))([a-zA-Z0-9\\-_]+)';
const YOUTUBE_REGEX_STRING = '(?:https?:\\/\\/)?(?:youtu\\.be\\/|(?:www\\.|m\\.)?youtube\\.com\\/(?:watch|v|embed)(?:\\.php)?(?:\\?[^ ]*v=|\\/))([a-zA-Z0-9\\-_]+)';
Copy link
Copy Markdown
Contributor

@szmarczak szmarczak Sep 10, 2021

Choose a reason for hiding this comment

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

(?:.*?)

maybe? Anyway the [^ ]* looks pretty as well and might be better solution than the lazy above :D

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't like regular expressions at all, I wonder if we can use something else 🤔

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

without reinventing the regexes ourselves, i am not sure :]

i'll keep [^ ]*, it feels more readable, but .*? is also nice, not sure if i ever used that myself

@B4nan B4nan merged commit 6c21c4a into master Sep 10, 2021
@B4nan B4nan deleted the fix-youtube-regex branch September 10, 2021 13:11
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.

Youtube regex seems overly greedy

2 participants