Translator: translate current page #10438

hius07 · 2023-05-14T16:37:28Z

Available via the menu and with a gesture.
Forces book language (if available) as a source language.
Shows main translation only, without source text, alternative translations and definitions.
Just a page to read.
Closes #10199.

This change is

poire-z · 2023-05-14T16:57:00Z

frontend/ui/translator.lua

-    if NetworkMgr:willRerunWhenOnline(function() self:showTranslation(text, target_lang, source_lang, from_highlight, page, index) end) then
+    if NetworkMgr:willRerunWhenOnline(function()
+            self:showTranslation(text, target_lang, source_lang, from_highlight, page, index, full_page) end) then
        return
    end


Not sure it's more readable than before :)
May be go a bit more proper:

if NetworkMgr:willRerunWhenOnline(function() self:showTranslation(text, target_lang, source_lang, from_highlight, page, index, full_page) end) then return end

poire-z · 2023-05-14T17:02:20Z

frontend/ui/translator.lua

+        if not full_page then
+            table.insert(output, "▣ " .. table.concat(source, " "))
+        end
+        text_main = (full_page and "" or "● ") .. table.concat(translated, " ")


(When naming this new parameter "full_page", did you think about translating the current full page - or showing the result translation full page ? I would have named it "no_alt_translation" or "simple_translation" as you're just ignoring all the other translations/definitions. But may be this ambivalent "full_page", because it is ambivalent, is fine.)

I think this snippet would read better if you kept the 2 ways separate, rather than (full_page and "" or "● ") ..:

if full_page then text_main = table.concat(translated, " ") else table.insert(output, "▣ " .. table.concat(source, " ")) text_main = "● " .. table.concat(translated, " ") end

poire-z · 2023-05-14T17:04:37Z

frontend/ui/translator.lua

+function Translator:onTranslateCurrentPage()
+    local ui = require("apps/reader/readerui").instance
+    if not ui or not ui.document then return end
+    local x0, y0, x1, y1, page
+    if ui.rolling then


It feels all this "reader" business should better be done in your added:

function ReaderHighlight:onTranslateCurrentPage() Translator:onTranslateCurrentPage() end

It is still ReaderHighlight:onTranslateCurrentPage that will always be called, isn't it?

Sure, it must be done in ReaderHighlight.

poire-z · 2023-05-14T17:09:01Z

frontend/ui/elements/reader_menu_order.lua

@@ -202,6 +202,7 @@ local order = {
        "wikipedia_history",
        "wikipedia_settings",
        "----------------------------",
+        "translate_page",


This will add a 11th item in that menu (10th for me with Vocabulary builder disabled), so making it 2 pages :/
No real better idea. In the 2nd menu with a page icon? so-so.
May be the dict & wiki lookup history should be moved into their respecting ... settings, or be in the own submenu?

12 (incl. calibre search).
Maybe move dict, wiki and translation settings into Settings submenu?

Yep, might be a good idea, now that "Translation" has one "action" item in there.

hius07 · 2023-05-15T04:52:57Z

poire-z · 2023-05-15T07:52:20Z

frontend/apps/reader/modules/readerhighlight.lua

+    -- main menu Search
+    menu_items.search_settings = { -- submenu with Dict, Wiki, Translation settings
+        text = _("Settings"),
+    }


May be this one should better be in readerdictionary, which is the one that brings into it the most subitems and the first in that search menu? (ReaderHighlight feels a bit unrelated).

What about wordings inconsistency Dictionary lookup history vs Wikipedia history?

Dunno when/where this difference comes from :/ @Frenzie will tell us if/how we should fix that.

I don't know where it came from, but I think "Dictionary history" might be a bit ambiguous? So in that sense I'd prefer Wikipedia lookup history if you feel it lacks balance, but I think it's fine as is.

poire-z · 2023-05-15T07:53:53Z

frontend/dispatcher.lua

@@ -340,6 +342,8 @@ local dispatcher_menu_order = {
    "book_description",
    "book_cover",

+    "translate_page",


(OK about having it here, can't find a better place in the Reader> actions menu.)

poire-z · 2023-05-15T11:58:15Z

frontend/ui/translator.lua

@@ -526,6 +557,7 @@ function Translator:_showTranslation(text, target_lang, source_lang, from_highli
    end
    local output = {}
    local text_main = ""
+    local not_full_page = not full_page


This feels ugly and prone to typo :)
And it means "detailed".
So, may be name it everywhere "detailed", and if the default should be true:
if detailed == nil then detailed = true end
and pass false when you don't want it.

poire-z · 2023-05-16T07:28:33Z

frontend/ui/translator.lua

    end)
 end

-function Translator:_showTranslation(text, target_lang, source_lang, from_highlight, page, index)
+function Translator:_showTranslation(text, target_lang, source_lang, from_highlight, page, index, translate_page)


Just some thoughts (not insisting too much, but just in case you'd agree :):
translate_page still feels a bit odd.
Translator can be seen as some utility tool, and shouldn't really be that linked to the idea of translating a book page or a fragment of a book content. It could be used to translate a Book description, or some Wikipedia result content.
So, ideally, the params shouldn't indicate what's the source (with some inner logic to decide what's expected as the result), but directly what's expected. So, "detailed" vs "simple".
We do pass from_highlight/page/index, so yes, there is a bit of outer context, but that's just for adding buttons.
(Isn't from_highlight a synonym of translate_fragment, ie. it's not there when you translate a page? Or can we don't have it even with fragments?)

"detailed" vs "simple"

Yes, I thought about these wordings, but translate_page mode also gets source lang from the book and shows bigger window.

Isn't from_highlight a synonym of translate_fragment, ie. it's not there when you translate a page? Or can we don't have it even with fragments?

We can translate_fragment from the pen_button in DictQuickLookup,

but translate_page mode also gets source lang from the book

Shouldn't it be readerhighlight that should get the source lang from the self.ui it knows and pass it to Translator:showTranslation(), instead of passing nil?

and shows bigger window

This could be implied when using "simple", or passed as another parameter (ie. large_window).

Fix translation for books without "language" in properties (it is "", Translator expects nil). Regression after #10438.

poire-z · 2023-05-20T10:24:42Z

Looks like "[x] Auto detect source language" has no longer the intended effect.
On a French book, highlighting a german sentence, and translate'ing, I get:
Calling https://translate.googleapis.com/translate_a/single?sl=fr&oe=UTF-8&hl=en&ssel=0&tsel=0&dt=t&dt=at&dt=md&client=gtx&tl=fr&ie=UTF-8&otf=1&q=the-words-in-german
and I get the title "Translation from French".
Previously (on an older appimage), I got "Translation from German".

Frenzie · 2023-06-07T13:56:21Z

Sounds like Google autodetect got worse? ;-P

hius07 added 6 commits May 14, 2023 19:31

credocument: do not draw selection in getTextFromPositions()

dd8ff27

filemanagerbookinfo: do not draw selection in getTextFromPositions()

102c022

readerhighlight: add TranslateCurrentPage

cd123fc

reader_menu_order: add TranslateCurrentPage

c611cbc

dispatcher: add TranslateCurrentPage

994d177

translator: add TranslateCurrentPage

852de03

poire-z assigned poire-z and unassigned poire-z May 14, 2023

poire-z reviewed May 14, 2023

View reviewed changes

hius07 added 3 commits May 15, 2023 07:49

Update reader_menu_order.lua

5eef542

Translator: move onTranslateCurrentPage() to ReaderHighlight

72dc206

ReaderHighlight: add onTranslateCurrentPage()

cbadaa1

poire-z reviewed May 15, 2023

View reviewed changes

hius07 added 4 commits May 15, 2023 13:59

readerdictionary: search_settings

d3c09e6

readerhighlight: fix translate page in reflow mode

387e8c3

Translator: optimize

bc09776

nit

fd16269

poire-z reviewed May 15, 2023

View reviewed changes

hius07 added 3 commits May 16, 2023 09:21

Update filemanager_menu_order.lua

6a48677

Translator: optimize

32221fc

Update translator.lua

fc62f55

poire-z reviewed May 16, 2023

View reviewed changes

hius07 added 3 commits May 16, 2023 14:50

Translator: detailed_view

34dca5f

readerhighlight: get source_lang from document

26bca66

dictquicklookup: Translator detailed_view

f83626c

poire-z approved these changes May 16, 2023

View reviewed changes

hius07 merged commit 3dce412 into koreader:master May 17, 2023

hius07 added this to the 2023.05 milestone May 17, 2023

hius07 deleted the translate-page branch May 17, 2023 04:34

hius07 mentioned this pull request May 17, 2023

ReaderHighlight: fix translation #10451

Merged

Frenzie pushed a commit that referenced this pull request May 18, 2023

readerhighlight: fix translate (#10451)

5f59ee6

Fix translation for books without "language" in properties (it is "", Translator expects nil). Regression after #10438.

hius07 mentioned this pull request May 20, 2023

ReaderHighlight: fix translator auto lang for fragment #10469

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Translator: translate current page #10438

Translator: translate current page #10438

hius07 commented May 14, 2023 •

edited

Loading

poire-z May 14, 2023

poire-z May 14, 2023

poire-z May 15, 2023

poire-z May 14, 2023

hius07 May 14, 2023

poire-z May 14, 2023

hius07 May 14, 2023

poire-z May 14, 2023

hius07 commented May 15, 2023

poire-z May 15, 2023

hius07 May 15, 2023 •

edited

Loading

poire-z May 15, 2023

Frenzie May 15, 2023 •

edited

Loading

poire-z May 15, 2023

poire-z May 15, 2023

poire-z May 16, 2023

hius07 May 16, 2023

poire-z May 16, 2023

poire-z commented May 20, 2023

Frenzie commented Jun 7, 2023

Translator: translate current page #10438

Translator: translate current page #10438

Conversation

hius07 commented May 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hius07 commented May 15, 2023

Choose a reason for hiding this comment

hius07 May 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Frenzie May 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poire-z commented May 20, 2023

Frenzie commented Jun 7, 2023

hius07 commented May 14, 2023 •

edited

Loading

hius07 May 15, 2023 •

edited

Loading

Frenzie May 15, 2023 •

edited

Loading