https://bugzilla.wikimedia.org/show_bug.cgi?id=69540
Brad Jorsch <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] --- Comment #2 from Brad Jorsch <[email protected]> --- 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. Capiunto.hooks.php lines 54-55: > $extraLibraries['mw.capiunto.Infobox'] = '\Capiunto\LuaLibrary'; > $extraLibraries['mw.capiunto.Infobox._render'] = '\Capiunto\LuaLibrary'; This probably isn't right, it'll instantiate and call \Capiunto\LuaLibrary::register() twice. Either the the classes should be different for each, or just one entry should be made in $extraLibraries. Capiunto.hooks.php line 70: > // @FIXME: Find a way to do this conditionally from Lua Ideally you'd probably want to do addModules on the ParserOutput during the parse. If you were already using PHP callbacks that'd be easy, but providing a way for a pure-Lua library to do it without also allowing modules to load any random gadget could be tricky. includes/lua/Infobox.lua line 28: > error( name .. ' must be either of type string, number or nil' ) You'll most likely want to be specifying the 'level' parameter to your error() calls,[1] so that the error is reported at the location of the external caller rather than "Infobox.lua at line 28". The same goes for the other error() calls in your code. [1]: http://www.lua.org/manual/5.1/manual.html#pdf-error 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. includes/lua/Infobox.lua line 236: > mw.capiunto = mw.capiunto or {} The evolving style would point to using mw.ext.Capiunto. includes/lua/InfoboxRender.lua line 28: > :css( 'border-spacing', '3px' ) I have to wonder why this isn't in the stylesheet. Same for most of the other :css() calls in here (lines 40, 67, 95, 107, 162, 189). -- 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
