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

Add the typographer’s apostrophe (U+2019) to the set of punctuation characters #791

Merged

Conversation

bohning
Copy link
Collaborator

@bohning bohning commented Dec 21, 2023

Currently, the search field (a.k.a. jump to) does not allow entering this apostrophe because it is neither recognized as alphanumeric nor as punctuation character.

This PR adds the typographers apostrophe (’) to the list of punctuation characters to allow searching for songs that use this apostrophe (e.g. "Let’s Twist Again").

punctuation characters. Currently, the search field (a.k.a. jump to) does not allow entering this apostrophe because it is neither recognized as alphanumeric nor as punctuation character.
@DeinAlptraum
Copy link
Contributor

Wouldn't a better solution to the underlying problem be to instead let the search allow matching the straight apostrophe in the query with typographer's apostrophes in titles/artist names?

The majority of users wouldn't know how to type it anyway, and searching for songs with typographer's apostrohpes in their title is quite cumbersome this way. We would lose the ability to filter specifically for either apostrophe, but I don't think this has much of a use-case.

@bohning
Copy link
Collaborator Author

bohning commented Dec 21, 2023

Yes, I agree that the search query should be somewhat normalized, especially regarding unicode normalization (https://en.wikipedia.org/wiki/Unicode_equivalence#Normalization). This would include normalizing the search query and searchable strings also with respect to the apostrophes (so that either apostrophe in the search query will find either apostrophe). But this a much greater task. This PR just solves the problem that I couldn’t even type the typographer’s apostrophe in the search field (nothing would appear if I typed ’).

Copy link
Collaborator

@barbeque-squared barbeque-squared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unable to test this (I can't type it) and while I agree with the other comment that we should eventually do something about normalization (hello Scandinavian AE, hello German eszet, umlauts), I'll believe that this PR on its own is still better than how things were before.

If we want to do proper normalization, just compute it for each song at startup and use that for searching instead. And use the same function on the search input as well, then it just becomes string matching.

@barbeque-squared barbeque-squared merged commit 474b5b3 into master Dec 23, 2023
2 checks passed
@barbeque-squared barbeque-squared deleted the add_typographers_apostrophe_to_punctuation_chars branch December 23, 2023 18:37
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.

None yet

3 participants