Repeat less in toolset declarations

[erikrose]

Maybe stick [references to] the toolset functions within an outer function and return locals() from it, rather than writing out a big dict like https://github.com/peter17/mediawiki-parser/commit/19d8ad32e577ab2c9a35b65a1349133ff33f3b61#L0L26.

[peter17]

I would be glad not to have to copy/paste again and again those function names :-)

I'm not so familiar with how to deal with locals(). Can you suggest some code to do this, please?

While we are speaking about the postprocessors (html, text and raw): shouldn't they be classes and inherit from a mother class? And preprocessor as well? At the beginning, I created them as scripts to be able to load the render_* functions with an import, but now I think they should adopt a better structure.

[erikrose]

I assigned it to myself, but you can grab it if it saves you work. Basically,

def toolset(): def some_tool(): ...

def some_other_tool():
        ...

return locals()

[peter17]

So simple! Thanks! I can take this issue since I'm currently working on the concerned files.

What about a class structure for the postprocessors?

[erikrose]

I like the idea of putting the toolset function in a class rather than in a dict. It's little less weird, and, as you say, it lets us use inheritance (though there isn't much shared functionality right now). We can put a __getitem__ method on the superclass which essentially delegates to getattr(self, whatever) so we don't have to touch the generator again.

[peter17]

I have been trying to implement toolsets as classes for 2 days with no success. My problem is that the functions of the toolsets are really used as "global" functions. To put them in a class, I see 3 options:

• implement them as classic "members", which means defining function(self, node) and having an instance of the toolset, but I have to pass this instance to different Pijnu functions; otherwise, I get "TypeError: unbound method join() must be called with DefaultToolset instance as first argument (got Node instance instead)"; this makes quite a lot of changes in Pijnu and I don't really know how to easily have a toolset instance that would inherit from the 3 toolsets we have (our own in html/text/raw/preprocessor, the one in the .pijnu file and the default one from Pijnu)
• implement them as static methods, but this requires to list them in the class with "function = staticmethod(function)" or put @staticmethod before each function; in this case we pass a reference to the class itself and not to an instance of it, but I still have problems when I want to temporarily change a method (used in preprocessor.py); quite weird
• define some kind of "all static" class using a metaclass; I made some attempts but I also meets some problems like in the 2 previous cases

Finally, what we want is having something simple and easily define the allowed_tags and allowed_entities in the postprocessors. Maybe the current state of the code is enough for that. I will keep the class idea in mind in case I find a more efficient solution that what I tried; of course, please don't hesitate to suggest another approach, I'd be happy to try it!

[erikrose]

Your first alternative is the right one; the piece you're missing is to pass not SomeToolset.function but rather SomeToolset().function, so that function is bound to an instance and automatically gets the self arg. However, we don't really need the class-based approach working for the first iteration. If people want to specify a different set of allowed tags, they can pass in a custom toolset function that closes over the set of tags they want to use. We can come back to this later.