search

tk0miya / docutils-stubs

The Unlicense
9 stars 5 forks source link

add 'Admonition' visitors #43

Closed danieleades closed 3 years ago

danieleades commented 3 years ago

huh. this is harder to create typings for than i realised.

I'm trying to add typings for the visit_admonition and depart_admonition methods on NodeVisitor, however several derived classes redefine the signature of the methods.

that causes mypy to complain (quite rightly). not sure of the best way to resolve.

tk0miya commented 3 years ago

Hmm? AFAIK, the NodeVisitor does not have visit_admonition and depart_admonition. Where do they come from?

danieleades commented 3 years ago

Hmm? AFAIK, the NodeVisitor does not have visit_admonition and depart_admonition. Where do they come from?

ah my bad, it may be only the GenericNodeVisitor and SparseNodeVisitor classes that have this set.

def _add_node_class_names(names):
    """Save typing with dynamic assignments:"""
    for _name in names:
        setattr(GenericNodeVisitor, "visit_" + _name, _call_default_visit)
        setattr(GenericNodeVisitor, "depart_" + _name, _call_default_departure)
        setattr(SparseNodeVisitor, 'visit_' + _name, _nop)
        setattr(SparseNodeVisitor, 'depart_' + _name, _nop)

_add_node_class_names(node_class_names)

for some context, i'm trying to add typings to the 'todo' sphinx extension example

there's this bit of code-

from docutils import nodes
from docutils.parsers.rst import Directive

from sphinx.locale import _
from sphinx.util.docutils import SphinxDirective

class todo(nodes.Admonition, nodes.Element):
    pass

class todolist(nodes.General, nodes.Element):
    pass

def visit_todo_node(self, node):
    self.visit_admonition(node)

def depart_todo_node(self, node):
    self.depart_admonition(node)

i'm trying to find a reasonable type to use for the self parameter of the node visitors. I thought the base class (NodeVisitor) would be the best option, but perhaps not.

tk0miya commented 3 years ago

ah my bad, it may be only the GenericNodeVisitor and SparseNodeVisitor classes that have this set.

I understand. It's a reasonable change. +1 for adding them all (not only visit_admonition, but also other nodes) to GenericNodeVisitor and SparseNodeVisitor.

Please check the result of CI. A warning is emitted for your change:

docutils-stubs/writers/_html_base.pyi:9:1: F401 'typing.Optional' imported but unused

danieleades commented 3 years ago

closing in favour of #45