Skip to content

UIManager: get rid of self._running and self._run_forever#9669

Merged
3 commits merged into
masterfrom
unknown repository
Oct 27, 2022
Merged

UIManager: get rid of self._running and self._run_forever#9669
3 commits merged into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Oct 22, 2022

Copy link
Copy Markdown

@NiLuJe Is this what you wished in #9657?


This change is Reviewable

Comment thread frontend/ui/uimanager.lua Outdated
Device:saveSettings()
Device:powerOff()
self:quit(Device:isKobo() and 88)
self:quit(Device:isKobo() and 88 or nil)

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.

Oh, right, good point, devcaps eval to true or false, not true or nil ;).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In this special case I would prefer to write:

if Device:isKobo() then
    self:quit(88)
else
    self.quit()
end

@NiLuJe NiLuJe 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.

Yup, :lgtm:, thanks!

Comment thread frontend/ui/uimanager.lua

-- Disable automatic UIManager quit; for testing purposes
function UIManager:setRunForeverMode()
self._gated_quit = function() return false end

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.

I don't recall if the testsuite ever unloads UIManager, so, we may need a way to restore the standard wrapper, too?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The test suite does a package.unloadAll() on most (all?) setup functions. On my laptop and CircleCi finish without error.
So the answer is probably: No, we don't need to have a restore functionallity.
(But I will implement these three lines for completeness.)

@ghost ghost added this to the 2022.11 milestone Oct 23, 2022
@ghost

ghost commented Oct 27, 2022

Copy link
Copy Markdown
Author

Ready to merge?

@NiLuJe NiLuJe 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.

Goddamit GH review UI. Forgot to hit the big green button yet again. -_-".

Comment thread frontend/ui/uimanager.lua
@ghost ghost merged commit 31b16ba into koreader:master Oct 27, 2022
@ghost ghost deleted the optimizeUIManager branch October 27, 2022 20:08
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Dec 11, 2022
@NiLuJe NiLuJe mentioned this pull request Dec 11, 2022
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Dec 12, 2022
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Dec 12, 2022
NiLuJe added a commit that referenced this pull request Dec 12, 2022
0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
This pull request was closed.
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.

2 participants