Skip to content

Ask to open file#15024

Merged
hius07 merged 8 commits into
koreader:masterfrom
hius07:ask-to-open-file
Feb 23, 2026
Merged

Ask to open file#15024
hius07 merged 8 commits into
koreader:masterfrom
hius07:ask-to-open-file

Conversation

@hius07

@hius07 hius07 commented Feb 22, 2026

Copy link
Copy Markdown
Member
  1. Show both MuPDF and CRE in "Open with" dialog for unsupported files.
    Closes FR: txt.zip/txtz improvements #14620
1
  1. Ask to open file: to prevent long opening on accidental taps.
2
3

This change is Reviewable

table.insert(radio_buttons, genRadioButton(provider.provider))
end
else
local provider = DocumentRegistry:getFallbackProvider()

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.

Should that function be removed if you're just doing it here? (Or conversely, adapted to do what you're changing it to do here.)

@hius07 hius07 Feb 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is also used in DocumentRegistry:getProvider().

return self:getFallbackProvider()

And the fallback provider is the first provider that supports txt:

function DocumentRegistry:getFallbackProvider()
for _, provider in ipairs(self.providers) do
if provider.extension == "txt" then
return provider.provider
end
end
end

I think it makes no sense, but it was added long ago for some reasons.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

By the way, DocumentRegistry:getProviders() doesn't use it and returns nil for unsupported files.

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.

Looks like it was added by you in #11056? :p

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've just separated the method, not changing the idea.

@Frenzie Frenzie added this to the 2026.02 milestone Feb 22, 2026
separator = true,
},
{
text = _("Ask to open file"),

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.

this sound a bit unnatural, "ask to open files" sounds proper or "ask before opening files"

I also feel this would go better with the "show... " boxes at the top


if not no_dialog and G_reader_settings:isTrue("file_ask_to_open") then
UIManager:show(ConfirmBox:new{
text = _("Open file?") .. "\n\n" .. BD.filename(file:match("([^/]+)$")),

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.

this is also incredibly robotic and not a proper sentence, "Do you want to open this file?"

@hius07 hius07 merged commit 7af08d4 into koreader:master Feb 23, 2026
4 checks passed
@hius07 hius07 deleted the ask-to-open-file branch February 23, 2026 17:57

if not no_dialog and G_reader_settings:isTrue("file_ask_to_open") then
UIManager:show(ConfirmBox:new{
text = _("Open this file?") .. "\n\n" .. BD.filename(file:match("([^/]+)$")),

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.

This is still not a proper sentence, but convincing you of that seems impossible

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thank you for your recommendations.

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.

You might find it annoying, but there is a reason rules and semantics exist. People won’t have the code beside them to decipher what a particular setting does; it is your responsibility to present that information clearly and correctly. In this case, we have a yes/no question, so it should be constructed properly. You already spent time writing the code, spent an extra ten minutes making sure the one and only thing users have to understand you code makes sense.

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.

Iirc "delete/open this file" is the phrasing recommended both Google and Apple for this kind of purpose. (But Apple seems to have reorganized and rewritten theirs, so I can't find it anymore.)

Microsoft/Windows goes for the longer sentences instead.

We traditionally tend to go with the longer ones, but you can't disagree with Apple and Google that it's more verbose. :-)

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.

In this case, we have a yes/no question

Mind, we're explicitly against Yes/OK as the answer. ;-)

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.

The difference here is, this isn't a subitem in the long-press menu (where it would be acceptable), is a direct question in a confirm widget. "would you like to open this file" is a correct sentence, we are not short of space, we're not messaging our mate.

Mind, we're explicitly against Yes/OK as the answer. ;-)

Very funny, but you know that is not what i'm saying.

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
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: txt.zip/txtz improvements

3 participants