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

Fix regexpIsVideoPost #73

Merged
merged 3 commits into from
Sep 3, 2017
Merged

Fix regexpIsVideoPost #73

merged 3 commits into from
Sep 3, 2017

Conversation

acochrane
Copy link
Contributor

The previous regular expression statement was difficult to read and would not catch "me/videos" links.
Now it is more readable and catches any instance of "/videos".

@huandu
Copy link
Owner

huandu commented Aug 15, 2017

The regexp should be \/videos$. The \ and $ are required. Please update your PR. Thanks.

@acochrane
Copy link
Contributor Author

I had to escape the leading backslash, but I'm not sure if this is acceptable, being a neophyte of regular expressions.
It still works in my use case.

What do you think?

@huandu
Copy link
Owner

huandu commented Aug 22, 2017

Your PR should work fine. It's better to use `` to quote a regular expression so that you can write `\/videos$`. Please update your PR again. Thanks.

@huandu huandu merged commit e104ea5 into huandu:master Sep 3, 2017
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.

2 participants