copt deduplicate: embedded_css, embedded_fonts #10876
Conversation
|
I think you earlier posted a comment (that you removed?) about inlining the :toggleXyz() in the :onToggleXyz as they were used once ? Why the change of mind (or me imagining things !? :) |
|
I postponed it, want to understand why they are similar koreader/frontend/apps/reader/modules/readertypeset.lua Lines 284 to 287 in 9b40bb1 koreader/frontend/apps/reader/modules/readertypeset.lua Lines 360 to 363 in 9b40bb1 but are called from onReadSettings differentlyie, is this UpdatePos really needed for smooth_scaling and nightmode_images.
|
There may be no real answer or logic - depends on the people who implemented each over the years :/ and their preference between shortcuts or always using a single method :) (I'm not sure in which camp you are :)
Possibly not, but we will need a redraw. Usually, UpdatePos is dumb/clever enough to ask crengine to compute its hashes to see if a rerendering is needed, and tell frontend that - and UpdatePos should do the right thing in both cases. |
I'm talking about |
It shouldn't be needed in the onReadSettings() phase - and it is actually a no-op: koreader/frontend/apps/reader/modules/readerrolling.lua Lines 1026 to 1035 in f46f341 So, I guess that if you see it called, it's because the same individual methods used for setting settings after init are called while in onReadSettings - and it shouldn't hurt. So, it comes down to the 2 camps I mentionned above :) |
| -- default to enable embedded fonts | ||
| -- As this is new, call it only when embedded_fonts are explicitely disabled | ||
| -- self.ui.document:setEmbeddedFonts(self.embedded_fonts and 1 or 0) | ||
| if not self.embedded_fonts then | ||
| if self.configurable.embedded_fonts == 0 then | ||
| self.ui.document:setEmbeddedFonts(0) | ||
| end |
There was a problem hiding this comment.
"At this is new..." is my writting from 2017 :) and I was probably very cautious at the time.
It feels now that you don't need that if, and you could just set it always, even if it's 1.
|
Should probably not block this work, but pinging @yparitcher the author (and may be only user ? :) of the docsettingtweak.koplugin plugin: can users be affected with these deduplications, if they use the removed version of the setting - and how likely they would be to use the removed or the kept setting names. Or are all the settings @hius07 is playing with not really candidates for user per-directory customisation ? |
I do like the idea for some use cases but I admit I don't actually use it. :-) |
|
Currently i set both eitherway (for example: |
Main discussion in #10763.
This change is