Skip to content

[ReaderDictionary] match words when they are Capitalised#13884

Merged
Frenzie merged 2 commits into
koreader:masterfrom
Commodore64user:lowercase
Jun 5, 2025
Merged

[ReaderDictionary] match words when they are Capitalised#13884
Frenzie merged 2 commits into
koreader:masterfrom
Commodore64user:lowercase

Conversation

@Commodore64user

@Commodore64user Commodore64user commented May 29, 2025

Copy link
Copy Markdown
Member

what's new

This pull request introduces a small enhancement to the ReaderDictionary module. The change improves the handling of word lookups by adding a lowercase version of a word to the search array if the word starts with a capital letter and fuzzy search is disabled.

why?

it does seem quite silly, that a user might not get any results due to punctuation, in the following example all "Count", "Back", "Your", "Wait", "Have", "Tonight", "Tomorrow" and "There" yield no results at all, event though the dictionary has entries for them (in lowercase)

related issues


This change is Reviewable

@Commodore64user

Copy link
Copy Markdown
Member Author

@poire-z this is 4 lines long, 50% of which are filler ;) say something

@poire-z

poire-z commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

Not competent enough to say if it's a good idea or not.
It feels this is supposed to be sdcv job - and not our job to give it 2 words and have it spend 100% more time doing the lookup.
Btw, you could check if word == word:lower() and don't add it if they are equal, to avoid the uneeded work and strip of duplicate resulsts if this happens.

@Frenzie

Frenzie commented Jun 2, 2025

Copy link
Copy Markdown
Member

It feels this is supposed to be sdcv job

Indeed. The problem (if it is one) is that there's full fuzzy or nothing but not something in between like "ignore capitalization and diacritics".

In this form it seems to somewhat sabotage the preferences of people who are against fuzzy search.

@jonnyl2

jonnyl2 commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

I agree with Frenzie here; it does seem to go a bit against the principle of exact search.

Btw, I did a quick test of the dictionary preset PR (#13774) and the 'One-time search with preset' feature is pretty cool.
That made me thinking, wouldn't it be awesome if the presets also allowed to select fuzzy search or non-fuzzy search as part of the setting? Then a fuzzy search preset could be selected in cases where a user who normally prefers exact searches wants to look up a word that is capitalized/conjugated.

And to access the presets from the highlight dialog, I would suggest that long-pressing Dictionary opens a dialog to select the list of dictionary presets for a one-time search.

@Commodore64user

Commodore64user commented Jun 2, 2025

Copy link
Copy Markdown
Member Author

Let's try to get some context around here, with the help of my friend Claude:

In a typical 100,000-word novel, roughly 8,000-12,000 words would be capitalised.
This breaks down approximately as:

* Proper nouns: 3,000-5,000 words (character names, place names, brand names, etc.)
* Sentence beginnings: 4,000-6,000 words (assuming average sentence length of 15-25 words)
* Dialogue and emphasis: 1,000-2,000 words (shouting, emphasis, chapter headings, etc.)

The percentage varies significantly based on genre - fantasy novels with lots of invented proper 
nouns will be higher, whilst literary fiction might be lower. Dialogue-heavy books also tend to have 
more capitalised words due to sentence fragments and exclamations.
This is roughly 8-12% of the total word count, which aligns with general English text patterns.

now our target is the last two bullets there, proper nouns is what Frenzie calls "sabotage the preferences of people who are against fuzzy search". I for one, have become very adamant of not using fuzzy searches as, and please forgive me here, I find it to not be fussy enough (I mean that in the sense of not being selective enough, "picky"). But back to the matter at hand.

It feels this is supposed to be sdcv job

I agree with this, however, sdcv seems to move at tectonic plate speeds, their last commit appears to have been made in Aug 2024 and in that year, it was the only commit committed. sorry. Here is a request for diacritic agnostic searches with a whopping zero replies Dushistov/sdcv#94, so we shouldn't rely on the goodness of their heart.

and not our job to give it 2 words and have it spend 100% more time doing the lookup.

I like to think of it as course correction, it's not really an interference, only making sure 5-8K potential capitalised words get a valid response. Also, we are not really increasing the search by 100%, sure we do if the word is capitalised but the majority of searches will continue to be of normal lower case words, as that makes up about 90% of a text, so although correct, in practise it will only add a bit of time in every 1 out 10 queries.

Btw, you could check if word == word:lower() and don't add it if they are equal

what? how could they be equal? genuinely asking, I mean we are checking for capital letters word:match("^%u") so they should be different, no?

it does seem to go a bit against the principle of exact search.

only it doesn't, or it does but only for those 3-5K proper nouns, but you still get the exact match though (if it exists), you just happen to get another one on top.

The problem (if it is one) is that there's full fuzzy or nothing but not something in between like "ignore capitalization and diacritics".

I was trying to avoid adding another menu entry, I still believe this is a cleaner approach but if that is a preferred solution, I wouldn't be against it. The diacritic though, I don't know about that part, it think that might be best left to a smart, intelligent frenchman.

Not competent enough to say if it's a good idea or not.

well, you need to become competent then, as it's your reader dictionary. ;)

@poire-z

poire-z commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

Btw, you could check if word == word:lower() and don't add it if they are equal

what? how could they be equal? genuinely asking, I mean we are checking for capital letters word:match("^%u") so they should be different, no?

Oh, right :) (It wasn't as explicite as my brain state needed, but you are right.)

as it's your reader dictionary. ;)

It's not mine - and I'm so used to fuzzy search, so I have no idea how non-fuzzy people work :)

@Commodore64user

Copy link
Copy Markdown
Member Author

Well you’re the gatekeeper ;)

@Frenzie Frenzie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Anyway, fine by me I suppose.

@Uukrull

Uukrull commented Jun 4, 2025

Copy link
Copy Markdown

The problem with word:lower() is that uppercase letters not available in the ASCII set (Ñ, for example) won't be converted.

Ñapa won't be converted to ñapa. That's why I used Utf8Proc.lowercase(word) in the patch I posted here

@Commodore64user

Copy link
Copy Markdown
Member Author

that is a valid point I hadn't consider.

@Frenzie

Frenzie commented Jun 4, 2025

Copy link
Copy Markdown
Member

Sidenote 'cause I had to leave home but yhe isUppercase function in that patch has the same issue. %l is just a-z

Edit:

> print(("n"):match("%l"))
n
> print(("N"):match("%l"))
nil
> print(("ñ"):match("%l"))
nil
> print(("é"):match("%l"))
nil

@Commodore64user

Copy link
Copy Markdown
Member Author

in the spirit of user patching, could I do something like this?

if not self.removed_by_patch and index == 1 then

if index == 1 then
if self.is_html then
self.definition = self.definition.."<br/>_______<br/>"
else
self.definition = self.definition.."\n_______\n"
end
self.definition = self.definition..T(_("(query : %1)"), self.word)
end

@Frenzie Frenzie merged commit c3352ee into koreader:master Jun 5, 2025
2 of 4 checks passed
@Frenzie Frenzie added this to the 2025.05 milestone Jun 5, 2025
@Frenzie

Frenzie commented Jun 5, 2025

Copy link
Copy Markdown
Member

That's probably better if you extract the entire thing into a method?

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.

FR: Dictionary: case-insensitive independent of fuzzy find Capitalized words can't be found in dictionaries without enabling fuzzy search

5 participants