lemon24
commented
3 years ago Case study: Mistune v2
markdown = mistune.create_markdown(plugins=['strikethrough'])
renderer = mistune.HTMLRenderer()
markdown = mistune.Markdown(renderer, plugins=[plugin_strikethrough])
markdown = mistune.create_markdown(plugins=[DirectiveToc(...)])The string version uses plugins from a hand-written dict of "built-in" plugins.
On the second strikethrough version, the renderer is required by the plugin (because it needs to do stuff to it); I assume create_markdown() adds it automatically.
Case study: Flask extensions
db = SQLAlchemy(app, ...)
db = SQLAlchemy(...)
db.init_app(app)
Unlike Mistune:
- the user initializes the extensions
- the init_app() version can be used to initialize multiple apps with the same extension; it is required that the extension does not store a reference to the app in this case
Case study: make_reader_from_config()
reader = make_reader_from_config(..., plugins=[
'reader._plugins.regex_mark_as_read:regex_mark_as_read',
reader._plugins.ua_fallback.init,
])Like Mistune, it takes either a string or a callable; unlike Mistune, the string is an import path, not a name.
It should be possible to change a make_reader() call to make_reader_from_config() without changing anything else.
If we use plugin names in make_reader(), we risk shadowing other packages; put another way, we need to be able to distinguish between plugin names and import paths from the start. Possible ways to deal with this:
- namespace built-in plugins:
reader.ua_fallback(anything starting withreader.is a built-in plugin)- don't like that it looks like an import path
- require import paths to have the
module:objectform- somewhat unfriendly, see the
regex_mark_as_readduplication above (OTOH, if the callable was calledinit, it'd be much shorter; newer plugins follow this convention; also, our current plugin-loading code requires this form) - alternatively, make
module:expand tomodule:init
- somewhat unfriendly, see the
- make built-in plugins start with a dot:
.ua_fallbackactually expands to the import pathreader.plugins.ua_fallback
It seems we have a trade-off between:
- making it easier for people to use built-in plugins:
ua_fallback,thirdparty:init - making it easier for people to use third-party plugins:
reader.ua_fallback,thirdparty(where the plugin loader looks forinitby convention) - getting both, but having built-in plugins be a bit confusing:
.ua_fallback,thirdparty
Update: A one-user focus group concluded import paths everywhere is better:
reader = make_reader('db.sqlite', plugins=[
'reader.plugins.ua_fallback',
'thirdparty', # implicit callable
'thirdparty:init', # explicit callable also OK
])This is more intuitive (users just have to learn one thing), and we don't have to explain why and how things are different.
Update #2: reader.ua_fallback is not much harder to support, and still has (most) the benefits above.
With #226, it's become obvious that some plugins should be enabled by default.
There's also reader._config.make_reader_from_config(); ideally, make_reader() should tend toward it.
Important: This is not to make public hooks etc. (#80); the only promise we make is: this is how you enable/disable built-in plugins, and these are the built-in plugins.
Additionally, it would probably nice to allow callable-style plugins (a la Mistune v2); here, the promise we make is: given this list of callables, we'll call them with the instantiated reader before returning it.
Candidates for the status of built-in: