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

Prevent filtering out @ from search queries in Comb #4602

Merged
merged 5 commits into from
Oct 29, 2021
Merged

Prevent filtering out @ from search queries in Comb #4602

merged 5 commits into from
Oct 29, 2021

Conversation

jelleroorda
Copy link
Contributor

Since this causes searches for email addresses to fail; searching for john@doe.com would actually search for johndoe.com.

Fixes #4519

jelleroorda and others added 2 commits October 28, 2021 17:26
Since this causes searches for email addresses to fail; searching for "john@doe.com" would actually search for "johndoe.com".
@jasonvarga
Copy link
Member

I wonder why @s were being stripped out to begin with. Looks intentional.

@jelleroorda
Copy link
Contributor Author

jelleroorda commented Oct 29, 2021

I'm not 100% sure; the stripping in the regex is based on "excluding" when stripping. What I mean by that is that the regex is in the format [^ <all exclusions here>]. So you could be right where it was intentionally stripped, or, maybe nobody noticed any issues with it before and we "just forgot to add it to the exclusions".

Edit for clarity's sake: you could still totally be right though, I'm not sure myself.

@jasonvarga
Copy link
Member

Sorry, my mistake. I read the diff wrong. I thought you removed an @ from the regex. 🤣

@jasonvarga
Copy link
Member

Thank you! (For this and for all the recent PRs you've been pumping out)

@jasonvarga jasonvarga changed the title Don't filter out @ symbols when preformating. Prevent filtering out @ from search queries in Comb Oct 29, 2021
@jasonvarga jasonvarga merged commit 3f776f6 into statamic:3.2 Oct 29, 2021
@jelleroorda jelleroorda deleted the patch-2 branch October 29, 2021 17:02
@jelleroorda
Copy link
Contributor Author

No worries, I like the testing you did for the Comb now. I am on a 'small effort issues' crusade 🚒 to try and decrease the issues counter 😄

@jasonvarga
Copy link
Member

I see that. You're kicking ass.

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.

User search is broken
2 participants