View Single Post
Old 05-14-2017, 05:06 PM   #196
chaley
Grand Sorcerer
chaley ought to be getting tired of karma fortunes by now.chaley ought to be getting tired of karma fortunes by now.chaley ought to be getting tired of karma fortunes by now.chaley ought to be getting tired of karma fortunes by now.chaley ought to be getting tired of karma fortunes by now.chaley ought to be getting tired of karma fortunes by now.chaley ought to be getting tired of karma fortunes by now.chaley ought to be getting tired of karma fortunes by now.chaley ought to be getting tired of karma fortunes by now.chaley ought to be getting tired of karma fortunes by now.chaley ought to be getting tired of karma fortunes by now.
 
Posts: 12,480
Karma: 8025702
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
Quote:
Originally Posted by kovidgoyal View Post
@chaley: From looking at the code, the best approach seems to be to allow TemplateFormatter to take an argument formatter_funcs which defaults to None. When None, it uses the global formatter_funcs, otherwise it uses the local one. Then have each db object keep its own copy of the formatter_funcs() with its funcs loaded. In get_metadata()/proxy_metadata() it will pass in a formatter to the Metadata() object that uses this copy instead of the gloabl formatter_funcs()

I could be way off base, however.
I am worried that this won't be reliable in the face of creating formatters on the fly, something that happens. There is also the fact that the current global deals with functions from multiple libraries, both when adding and deleting, and I think it gets it wrong if there are function name conflicts.

But that aside, your suggestion is where I was leaning, although if we go this far then we might want to get rid of the global. I am thinking the following might work:
a) we add a positional parameter to TemplateFormatter.
b) the db layer keeps the functions for that db, loading them when the library is opened (as it does today)
c) the function editing layer manipulates the db copy instead of the global list.
d) the db copy is serialized to the db when closed
e) the global function management stuff is removed
then function state becomes private to the db (library). Anyone who creates a formatter would need to provide the dict of functions. This implementation would permit defining functions using a non-GUI API, which might be interesting to do some day.

The biggest problem with this idea is that it is an incompatible change that would break plugins that use TemplateFormatter (are there any of these?). Making the argument named instead of positional deals with this. Perhaps the compromise is if the named argument is None then the functions are loaded from the current GUI db. But that leads to the problem where we don't have a current GUI so don't have access to the db, which will happen in helper functions. This pushes for leaving the global as a fallback or disabling custom template functions in plugin invocations of TemplateFormatter with no GUI. This might be acceptable given that the number of such invocations is probably zero.

A second incompatibility is really a bug. If two libraries have functions with the same name then the second one wins. Which one is "second" depends on dict iter order, which means it is "random". Removing that ambiguity might change behavior, but I think it changes it for the better.
chaley is offline   Reply With Quote