Skip to content

Conversation

@javiereguiluz
Copy link
Member

This fixes #734.

The search engine is now more strict because it's case sensitive (if the post title is Lorém ipsum, lorém and Lorem gives no results; you must search for Lorém). I'd say it's OK because we're not building a real search engine. Thoughts?

@jlagneau
Copy link

jlagneau commented Dec 25, 2017

Thanks for this.

Case insensitive searches works great with just your changes in sanitizeSearchQuery. It just won't work for uppercase non-ASCII characters in some DBMS like sqlite but it works out of the box with MySQL for example.

E.g:
Title contains LorÉM
With sqlite, querying for LORÉM works, while lorém and lorem won't.
With mysql, querying for LORÉM, lorém or lorem works.

If sqlite is mandatory, I'd say keep it like this as there's no simple way to change that ( http://sqlite.org/faq.html#q18 ). Maybe add a message in the search page to inform the user that searches are case sensitive for the UX?

@jkufner
Copy link
Contributor

jkufner commented Dec 25, 2017

It is better to leave worrying about case sensitivity to database. Depending on configured collation it can compare strings according to local conventions. MySQL deals with this issue well, Sqlite not so much (at least in the default configuration) and it is ok ­— we should not expect much from the little Sqllite.

I think the strtolower is an ugly hack to fix incompetent database. I would not expect such a thing in example application.

@javiereguiluz
Copy link
Member Author

@jlagneau I prefer to not add this warning message yet because that requires a new content to translate in all languages and because by default we use SQLite and non-accented characters, so this always work as expected for most users.

@jkufner I know that your comments were well intended, but I'd like to add a minor clarification about this --> "... to fix incompetent database" We, the Symfony project, don't believe that SQLite is incompetent and we respect SQLite profoundly, as we do with all the other databases.

@javiereguiluz javiereguiluz merged commit 0e1c9d3 into symfony:master Dec 27, 2017
javiereguiluz added a commit that referenced this pull request Dec 27, 2017
…uiluz)

This PR was merged into the master branch.

Discussion
----------

Allow to search blog posts with unicode characters

This fixes #734.

The search engine is now more strict because it's case sensitive (if the post title is `Lorém ipsum`, `lorém` and `Lorem` gives no results; you must search for `Lorém`). I'd say it's OK because we're not building a real search engine. Thoughts?

Commits
-------

0e1c9d3 Allow to search blog posts with unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support accented chars in search

3 participants