[ReaderDictionary] Enable dictionary prioritisation via new menu by Commodore64user · Pull Request #13768 · koreader/koreader

Commodore64user · 2025-05-10T22:55:32Z

what's new

This pull request enhances the ReaderDictionary module by introducing functionality to manage dictionary preferences on a per-book basis. It allows users to prioritise dictionaries and disable specific dictionaries for individual books.

Added a new showPreferredDictsDialog method: This dialog allows users to prioritise active dictionaries and toggle their enabled/disabled state for the current book. Disabled dictionaries are visually marked and placed at the bottom of the list.
Updated addToMainMenu method: Introduced a new menu item, "Set dictionary priority for this book," which opens the dictionary preferences dialog. This menu item is conditionally displayed based on the document state and the number of active dictionaries.
Filtered dictionaries during lookup: Modified the stardictLookup method to exclude dictionaries disabled for the current book before performing a lookup.
Persisted per-book settings: Updated onReadSettings and onSaveSettings methods to load and save the list of disabled dictionaries for each book.
Added the ButtonDialog dependency to support the new preferences dialog.

screenshots

Expand to see some footage

related issues

This change is

poire-z · 2025-05-11T08:50:14Z

    end

+    -- Before starting the search, remove any dictionaries that are disabled for *this* book.
+    if dict_names and self.book_disabled_dicts then


and self.book_disabled_dicts > 1 ? as you make it a dict and not a nil in onReadSettings (or make it a nil there, I may prefer that if there's no reason to keep a {]).

changing to only readSetting("...") in ReadSettings then

poire-z · 2025-05-11T08:52:57Z

+    local buttons = {}
+    local disabled_buttons = {}  -- store disabled dict buttons separately
+
+    local function makeButtonEntry(dict, is_disabled)


May read better with a is_enabled where it is called?

poire-z · 2025-05-11T08:55:23Z

Mentionning that (not sure I would enjoy the adding kludge, but...) since you were recently into footer presets, you could in the same way make dictioneries presets, just swapping the managed list of enabled dicts? Would solve more issues and would be less cumbersome to have to do the re-org on each new book of some kind.

jonnyl2 · 2025-05-11T09:05:12Z

It should have a one-tap option to clear the entire prioritization. How about adding a Reset button on the bottom of the info-text (on long-press)?

Commodore64user

since you were recently into footer presets

you too? i can't deal with this one-hit-wonder nonsense anymore... listen i hear you, i am happy to look into it, but i would add them on-top as opposed to instead-of, (i still want to assing priority on NT, and this is the only sane way of doing it) because...

would be less cumbersome to have to do the re-org on each new book of some kind

that is kind of the point for me, some dicts are better aimed towards american spelling and others towards UK spelling, i actually want to be able to have that level of ~~nuisance~~ nuance.

poire-z · 2025-05-11T12:11:07Z

but i would add them on-top as opposed to instead-of, (i still want to assing priority on NT, and this is the only sane way of doing it)

Oh, right, so this PR is obviously fine.

i am happy to look into it

Well, then no real need :) Take some vacations !

Commodore64user · 2025-05-11T12:56:23Z

but i would add them on-top as opposed to instead-of, (i still want to assing priority on NT, and this is the only sane way of doing it)

Oh, right, so this PR is obviously fine.

i am happy to look into it

Well, then no real need :) Take some vacations !

I am not entirely sure how to interpret this, it feels like you're telling me off, are you? I will have a nap though

Commodore64user · 2025-05-12T10:10:41Z

okay @poire-z you win, I have a version of dict-presets however. I was thinking is best to extract the common preset functionality into a shared module. I was thinking presets.lua, but where should it go?

ui/elements/presets.lua? maybe @Frenzie could help too (it's mostly the bulky menu stuff)

Frenzie · 2025-05-12T11:57:45Z

What's common preset functionality precisely?

Commodore64user · 2025-05-12T12:09:23Z

the menu creation stuff (about 100 lines)... and the function for dispatcher. Everything else is custom (i.e save, load and build)

Frenzie · 2025-05-12T12:45:57Z

And preset means more than just the dictionary?

Commodore64user · 2025-05-12T13:05:29Z

No, just a list of enabled dictionaries at time of creation. We don't store anything else, an anything added after (new dicts) is treated as if it had disabled originally. There's even a fancy check for deleted ones.

Makes sense for presets like: english, french, spanish, etc

Edit: if you tell me where to place the file, i can push it and then we can see it

Frenzie · 2025-05-12T13:15:31Z

That sounds like ui/elements/dictionary_presets_menu_table.lua or similar then.

mergen3107 · 2025-05-12T14:26:01Z

This feature is quite nice!
I remember on forum about Fora Dictionary for Android, they have those color coded "Profiles" for various goals to use dictionaries with. Like, orthographic dictionaries only, or phonetic dictionaries only - for professionals who need those super specific dictionaries

Commodore64user · 2025-05-12T14:27:23Z

@Frenzie there is a draft open now, status bar presets would use presets_menu as well of course.

poire-z · 2025-05-12T18:33:01Z

Edit: if you tell me where to place the file, i can push it and then we can see it

That sounds like ui/elements/dictionary_presets_menu_table.lua or similar then.

You put it in frontend/ui/elements/presets_menu.lua in #13774.
I think frontend/ui/elements/ currently contains only "final" menu items, "elements" - which your presets_menu isn't.
I would put it into /frontend/ui/ where other "UI utils" are (ie. event, trapper, menusorter...)

I would even name it just "presets.lua", who know what other common non-menu-related generic stuff you may end up putting into it.

Commodore64user · 2025-05-12T19:54:52Z

Got it, now if someone could review this, it'd be great as this will conflict with #13774 so would be nice to get it out of the way soon-ish.

Frenzie · 2025-05-13T16:25:15Z

+    if is_docless then -- remove the "Set dictionary priority for this book" entry
+        table.remove(menu_items.dictionary_settings.sub_item_table, 2)
+    end


Don't we usually do this the other way around?

Would you prefer it the other way around? I just didn't want to break the order.

Edit: we rarely (a few cases still though) do it this way.

Well, generally speaking yes. Creating a whole table, then removing and garbage collecting it seems pretty silly at best.

I'll change it, but wouldn't the collector still run anyway?

I'm not entirely sure what you're asking. It won't or rather can't run for something you didn't create in the first place. ;-)

I was only suggesting that (in the words of Taylor Swift), "collector gonna collect" regardless... it won't just be coming for these 8 bytes, it will come for all bytes equally ;)

in Pratchett-ian words: LORD, WHAT CAN THE HARVEST HOPE FOR, IF NOT FOR THE CARE OF THE REAPER MAN?

Frenzie · 2025-05-15T19:38:54Z

    else
        self.disable_fuzzy_search = G_reader_settings:isTrue("disable_fuzzy_search")
    end
+    -- Add thw disabled-dictionaries list for this book


Frenzie

I guess it's okay, but dictionaries are mainly @poire-z's baby.

poire-z · 2025-05-16T20:31:58Z

        self.disable_fuzzy_search = G_reader_settings:isTrue("disable_fuzzy_search")
    end
+    -- Add the disabled-dictionaries list for this book
+    self.disabled_dicts_per_book = config:readSetting("disabled_dicts_per_book") or {}


The name of this variable and setting sounds strange to me: it's about the current book, but it's named "per_book" and is read from the current book settings: it should just not have any reference to "book" (we don't name our setting margin_for_book, style_tweaks_per_book).
In case you really want this, doesn't "xyz_for_book" sounds better for the current book than "xyz_per_book" ?
(And the comment says enough if one is in doubt - but why the word "add" ?.)

Because i have been doing this late at night, and should probably proof read it when 100% alert ;), i’ll change it

I get the same feeling with _by_book ...
It's saved in the book metadata.epub.lua - so there's no need to have _by/for/per_book in here.
I understand that may be (?) in the code, because there are global settings (?) and local book settings that it may be helping to carry the fact in the variable name - but is it really?

This is about the code, someone reading this in 2 years time needs to know, this disabled is different from the global one. You could not even remember what you wrote in a comment yesterday ;), do you really expect to remember which is the global later on?

Hm? Globals are G. The distinction normally only matters when initializing (1. use document settings 2. use global 3. use defaults)

document_disabled_dicts?

Sounds better.
But just disabled_dicts or dicts_disabled in metadata.epub.lua sounds even better :)
But you could name the variable doc_dicts_disabled or doc_disabled_dicts if you think it helps readability.

poire-z · 2025-05-16T20:43:08Z

+            text = _("Set dictionary priority for this book"),
+            help_text = _("This feature enables you to specify dictionary priorities on a per-book basis. Results from higher-priority dictionaries will be displayed first when looking up words. Only dictionaries that are currently active can be selected and prioritized."),
+            enabled_func = function()
+                -- we allow to use preferred dictionaries even if no dictionaries are enabled globally (see self:updateSdcvDictNamesOptions)


Strange what this comment says ? If no dict are enabled, why would some of none be used ?!

just playing by your rules here, you tell me why that is a thing… ;)

koreader/frontend/apps/reader/modules/readerdictionary.lua

Lines 199 to 201 in 68a9a8d

-- First, insert any preferred dicts, even if globally disabled

-- (this might allow enabling a dict only for a specific book,

-- while keeping it disabled for all others)

Oh, didn't remember that.
Does it still make sense ? (Feels like it allowed some workaround for what your PR is doing as its main feature ?) Fine either way.

I don't think it does but, I will let you come to that conclusion and change the world later on. ;)

poire-z · 2025-05-17T11:47:27Z

    if self.ui.doc_settings then
-        self.ui.doc_settings:saveSetting("preferred_dictionaries", self.preferred_dictionaries)
+        self.ui.doc_settings:saveSetting("preferred_dictionaries", next(self.preferred_dictionaries) and self.preferred_dictionaries or nil)
+        self.ui.doc_settings:saveSetting("doc_disabled_dicts", next(self.doc_disabled_dicts) and self.doc_disabled_dicts or nil)


Again, in metadata.epub.lua, all the settings are related to this document.
All of them could be prefixed with doc_ - but none is ! (except doc_pages and doc_path).
So, please save it just as disabled_dicts - and you can still have it as self.doc_disabled_dicts in this module if you really want it for reading clarity.

oh my bad, I see what you mean now... sorry!

Commodore64user · 2025-05-20T21:45:03Z

anything else here?, I would really like to rebase presets with this on. Also, I think I have been acting in good faith so, I am not entirely sure how to react to certain actions (pun intended) that are occurring at the moment.

Frenzie · 2025-05-21T17:15:17Z

@Commodore64user I didn't merge it this morning because your reply made me think there was still an ongoing discussion and I didn't notice the approval right above it. In this specific context "ping" or "bump" is fine.

…eader#13768)

poire-z reviewed May 11, 2025

View reviewed changes

Commodore64user commented May 11, 2025

View reviewed changes

Commodore64user mentioned this pull request May 13, 2025

[Presets] add new presets.lua module and dictionary presets #13774

Merged

Frenzie reviewed May 13, 2025

View reviewed changes

Commodore64user force-pushed the dict-fave branch from 0c383b7 to a1283ed Compare May 14, 2025 01:17

Commodore64user requested review from Frenzie and poire-z May 15, 2025 15:53

Commodore64user added 2 commits May 15, 2025 20:26

add dictionary priority menu, first commit

378e437

add a reset all button

e1f5513

Commodore64user force-pushed the dict-fave branch from a1283ed to ce7eebb Compare May 15, 2025 19:27

Frenzie reviewed May 15, 2025

View reviewed changes

Frenzie approved these changes May 15, 2025

View reviewed changes

poire-z reviewed May 16, 2025

View reviewed changes

Commodore64user force-pushed the dict-fave branch 2 times, most recently from 6c564b9 to 89ce3e8 Compare May 17, 2025 10:05

poire-z reviewed May 17, 2025

View reviewed changes

handel some edge cases and rename var to doc_disabled_dicts

0ff617f

Commodore64user force-pushed the dict-fave branch from 39e0957 to 0ff617f Compare May 17, 2025 11:54

poire-z approved these changes May 17, 2025

View reviewed changes

Frenzie added this to the 2025.05 milestone May 21, 2025

Frenzie merged commit 2512f06 into koreader:master May 21, 2025

Commodore64user deleted the dict-fave branch May 21, 2025 22:35

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026

[ReaderDictionary] Enable dictionary prioritisation via new menu (kor…

26b39a9

…eader#13768)

	-- First, insert any preferred dicts, even if globally disabled
	-- (this might allow enabling a dict only for a specific book,
	-- while keeping it disabled for all others)

Uh oh!

Uh oh!

Conversation

Commodore64user commented May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

what's new

screenshots

related issues

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poire-z commented May 11, 2025

Uh oh!

jonnyl2 commented May 11, 2025

Uh oh!

Commodore64user left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poire-z commented May 11, 2025

Uh oh!

Commodore64user commented May 11, 2025

Uh oh!

Commodore64user commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Frenzie commented May 12, 2025

Uh oh!

Commodore64user commented May 12, 2025

Uh oh!

Frenzie commented May 12, 2025

Uh oh!

Commodore64user commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Frenzie commented May 12, 2025

Uh oh!

mergen3107 commented May 12, 2025

Uh oh!

Commodore64user commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

poire-z commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Commodore64user commented May 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Commodore64user May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Frenzie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Commodore64user May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Commodore64user commented May 10, 2025 •

edited

Loading

Commodore64user left a comment •

edited

Loading

Commodore64user commented May 12, 2025 •

edited

Loading

Commodore64user commented May 12, 2025 •

edited

Loading

Commodore64user commented May 12, 2025 •

edited

Loading

poire-z commented May 12, 2025 •

edited

Loading

Commodore64user May 13, 2025 •

edited

Loading

Commodore64user May 16, 2025 •

edited

Loading