copt deduplicate: embedded_css, embedded_fonts

[hius07]

Main discussion in #10763.

---

This change is 

[poire-z]

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 !? :)

[hius07]

I postponed it, want to understand why they are similar

koreader/frontend/apps/reader/modules/readertypeset.lua

Lines 284 to 287 in 9b40bb1

	function ReaderTypeset:toggleEmbeddedFonts(toggle)
	self.ui.document:setEmbeddedFonts(toggle)
	self.ui:handleEvent(Event:new("UpdatePos"))
	end

koreader/frontend/apps/reader/modules/readertypeset.lua

Lines 360 to 363 in 9b40bb1

	function ReaderTypeset:toggleImageScaling(toggle)
	self.ui.document:setImageScaling(toggle)
	self.ui:handleEvent(Event:new("UpdatePos"))
	end

but are called from onReadSettings differently

koreader/frontend/apps/reader/modules/readertypeset.lua

Line 38 in 9b40bb1

self.ui.document:setEmbeddedFonts(0)

koreader/frontend/apps/reader/modules/readertypeset.lua

Line 95 in 9b40bb1

self:toggleImageScaling(self.configurable.smooth_scaling == 1)

ie, is this UpdatePos really needed for smooth_scaling and nightmode_images.

[poire-z]

but are called from onReadSettings differently

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 :)

ie, is this UpdatePos really needed for smooth_scaling and nightmode_images.

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.

[hius07]

but we will need a redraw

I'm talking about onReadSettings only.
Maybe it's better to call UpdatePos once in the end, instead of calling it after reading each setting.

[poire-z]

Maybe it's better to call UpdatePos once in the end, instead of calling it after reading each setting.

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

	function ReaderRolling:onUpdatePos(force)
	if self.batched_update_count > 0 then
	return
	end
	if self.ui.postReaderCallback ~= nil then -- ReaderUI:init() not yet done
	-- Don't schedule any updatePos as long as ReaderUI:init() is
	-- not finished (one will be called in the ui.postReaderCallback
	-- we have set above) to avoid multiple refreshes.
	return true
	end

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 :)

[poire-z]

"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.

[poire-z]

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 ?

[Frenzie]

(and may be only user ? :)

I do like the idea for some use cases but I admit I don't actually use it. :-)

[yparitcher]

Currently i set both eitherway (for example: font_size & copt_font_size) one does the actual change and one gets it to show up in the bottom menu.