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 typo in escapeRegExp #3448

Merged
merged 1 commit into from
Oct 25, 2017
Merged

Fix typo in escapeRegExp #3448

merged 1 commit into from
Oct 25, 2017

Conversation

younesfkihi
Copy link
Contributor

No description provided.

@jdalton jdalton merged commit 102d9e3 into lodash:master Oct 25, 2017
@jdalton
Copy link
Member

jdalton commented Oct 25, 2017

Thank you @younesfkihi !

@falsyvalues
Copy link
Contributor

@jdalton I don't think this is correct \\ is an escape for \ character and we match against \ not \\.

@jdalton
Copy link
Member

jdalton commented Nov 7, 2017

@falsyvalues
The typo is that you can't write "\" in JS, because it's escaping the ".
So to make it more JS-y, they wanted it written as "\\".

@falsyvalues
Copy link
Contributor

@jdalton Still if you think about how regex work (and it is about regex) it's misleading and it's a comment which will be published on docs site. Anyway its up to you, just my few 💵 😄

@younesfkihi
Copy link
Contributor Author

@falsyvalues @jdalton I added the fix because the docs site shows "$", "", "." instead of "$", "\", ".". How can "\" be shown on the site without escaping \?

@jdalton
Copy link
Member

jdalton commented Nov 7, 2017

@falsyvalues Ah okay, I see your point. If you'd like to revert that's cool.

@lock
Copy link

lock bot commented Dec 26, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

3 participants