-
Notifications
You must be signed in to change notification settings - Fork 22
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
Make phrasal matching case-insensitive #304
Comments
Actually it could be backported to 1.4 easily too. There will probably be documentation updates to do as well, though. |
We introduced case-sensitivity on purpose in version 1.1, so that people could search for proper names: "Phrasal searches are now case-sensitive, meaning that you can use a ‘phrasal search’ to search for proper names, by putting quotation marks around them and making the first letter upper-case." I'd forgotten the history of this, but it's laid out in issue #104. That ticket ended with a comment from @joeytakeda:
And now we have one project which does. However, one project may not be enough to justify a new config item. |
We decided to do an experimental implementation in a branch as follows:
|
This work will happen in the iss304-make-phrasal-search-case-sensitivity branch. (Screwed up the name, but meh.) |
Basic working implementation now in iss304-make-phrasal-search-case-sensitivity branch. What remains to be done is tracking the checkbox's state in search history URLs. @joeytakeda I don't know if you were convinced of the usefulness of this, so you might want to try it out. One thing that's bugging me right now is that the auto-complete suggestions from the browser appear right where the checkbox and its label will appear and disappear, which means you don't necessarily see it right away when it appears. |
Added the URL tracking bit. This is now functionally complete, but without any tests as yet. |
@joeytakeda If you try this out and you think it's a good idea, I'll add tests and documentation before creating a PR. I purposely used the CSS visibility property rather than display because any end-user who doesn't want it to appear can keep it hidden with display: none. |
The Moses project team have requested that phrasal matching be made case-insensitive. I believe this could be done with a single change to what is currently line 2032 of StaticSearch.js:
becomes
I think this is a good idea, and I see no reason not to do it, although it may involve some fallout for tests, which might perhaps retrieve more hits for specific test phrases. @joeytakeda Do you agree we should just implement this for version 2.0 assuming there are no gotchas when we try it?
The text was updated successfully, but these errors were encountered: