https://bugzilla.wikimedia.org/show_bug.cgi?id=69540
--- Comment #9 from Jackmcbarn <[email protected]> --- (In reply to Marius Hoch from comment #7) > Addressed the concerns raised by Brad (changes still in gerrit as of now). > > > (In reply to Brad Jorsch from comment #2) > > Reviewing revision 1441b9ab: > > > > Capiunto.php line 30: > > > # XXX: Rather use ScribuntoExternalLibraryPaths ? > > > > ScribuntoExternalLibraryPaths is for pure-Lua libraries that are loaded > > using require() from a module that wants them. > > > > ScribuntoExternalLibraries is for libraries that are loaded by default. A > > library that is implemented with callbacks into PHP code currently needs to > > be done using this hook, as the require() code path doesn't allow for > > setting things up. > > > > Since this does appear to be a pure-Lua library that isn't of general > > utility, ScribuntoExternalLibraryPaths probably would be a good way to go. > > > > As we now call out to PHP in order to add the RL module, this is no longer > be possible for mw.capiunto.Infobox (see below). But I've made > mw.capiunto.Infobox._render a library which can be dynamically loaded, when > needed. I wonder if this tradeoff was worth it. Maybe we should have $wgScribuntoAllowedResourceLoaderModules[] that extensions can add to, and have a Lua method to pull in any listed there. I think we'd be better off doing that if it meant keeping this a pure-Lua library only loaded on demand. > > > includes/lua/Infobox.lua line 129: > > > data = preprocess( data ), > > > > I see you're preprocessing a lot of the fields being passed in. Note that > > when the fields are coming in from frame.args, they're already preprocessed > > and shouldn't need to be preprocessed again; 'data' here seems particularly > > likely to be in that situation. > > > > I don't see a way around doing preprocess on everything right now... the > problem is that we will get data from various sources and during some > initial tests it turned out that preprocessing is something people just > assume to be done. What we *could* do is add specific methods (or flags) > that bypass (or enable?) preprocessing, but I'm not a huge fan of that. I can't think of any use case at all where preprocessing like we are now is a good thing. In practice, all data from Lua came from the parser anyway, where we'd see it already preprocessed. I'd favor removing the method altogether. > > > includes/lua/Infobox.lua line 236: > > > mw.capiunto = mw.capiunto or {} > > > > The evolving style would point to using mw.ext.Capiunto. > > > I'm not a fan of that as it seems overcomplicated. If you insist, I can > change it, though. I'd like to see this change as well. I noticed this myself. -- You are receiving this mail because: You are on the CC list for the bug.
_______________________________________________ Wikidata-bugs mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
