Correct Frontlight status on suspend when screensaver mode is 'Leave …#5928
Correct Frontlight status on suspend when screensaver mode is 'Leave …#5928Frenzie merged 4 commits intokoreader:masterfrom clarkspark:screensaver_landscape
Conversation
|
I'd add a ref to your own issue in the comment above that already does the same for #4098, but that's just me ;). |
|
Sorry I'm not too familiar with the workings of github, I thought adding the #5290 would do that. Or do you mean adding a short synopsis of the issue here? |
|
No, I meant in the actual code ;). |
|
👍, lgtm, thanks ;). |
|
Thanks for the feedback, and the great software. |
frontend/device/generic/device.lua
Outdated
| if G_reader_settings:readSetting("screensaver_type") ~= "message" then | ||
| -- ... except when we just show an InfoMessage or when the screensaver | ||
| -- is disabled, as it plays badly with Landscape mode (c.f., #4098 and #5290) | ||
| if (G_reader_settings:readSetting("screensaver_type") ~= "message" and G_reader_settings:readSetting("screensaver_type") ~= "disable") then |
There was a problem hiding this comment.
No real need for parens, as it's a single and.
(When I see parens, I expect more twisted conditions - my expectations would be deceived here :)
There was a problem hiding this comment.
I can remove the parens. Are overly-long lines frowned upon? Like maybe instead break the if across two lines, in which case I'd need the parens, right?
There was a problem hiding this comment.
Yep, 145 chars long is a bit too long (120 is ok) - so you can split it on 2 lines.
And still no need for parens :) look a few lines below for a multilines and and around to get a feel for the Lua style and simple needs.
There was a problem hiding this comment.
You could also do
local screensaver_type = G_reader_etc_long_long()
if screensaver_type ~= blah and screensaver_type ~= blahblah then -- much shorter and more legibleThere was a problem hiding this comment.
@Frenzie: not a big deal I can make that change but if its alright I'd prefer just to keep it consistent with with the code around it (~12 lines below with the multi-line if using the same function)
There was a problem hiding this comment.
Quite possibly because it's from me (I like long lines & parentheses, and I cannot lie 🎵).
There was a problem hiding this comment.
I went with shorter lines for the whole if branch (even for un-effected code), let me know if that's OK.
There was a problem hiding this comment.
@Frenzie: I went with a variable to replace the long function name. Thanks for the feedback.
koreader#5928) Also use a var in place of a long function name
Link to the right issue ;). Typo dates back to the original commit, way back in koreader#5928
Link to the right issue ;). Typo dates back to the original commit, way back in #5928
Address #5920 to correct Frontlight status on suspend when screensaver mode is 'Leave screen as it is' in landscape mode.
This change is