From 8d5ef57c6ff5af1edfb6a1573165d40c93f44fe1 Mon Sep 17 00:00:00 2001 From: Takeshi KOMIYA Date: Sun, 24 Mar 2019 16:35:20 +0900 Subject: [PATCH] ``todo`` directive now supports ``:name:`` option --- CHANGES | 1 + sphinx/ext/todo.py | 43 ++++++++++-------------------------------- tests/test_ext_todo.py | 7 ++++--- 3 files changed, 15 insertions(+), 36 deletions(-) diff --git a/CHANGES b/CHANGES index 33ee29596..58a42e64f 100644 --- a/CHANGES +++ b/CHANGES @@ -51,6 +51,7 @@ Features added * Add a helper method ``SphinxDirective.set_source_info()`` * #6180: Support ``--keep-going`` with BuildDoc setup command * ``math`` directive now supports ``:class:`` option +* todo: ``todo`` directive now supports ``:name:`` option Bugs fixed ---------- diff --git a/sphinx/ext/todo.py b/sphinx/ext/todo.py index 3939f8ff8..1922bb49c 100644 --- a/sphinx/ext/todo.py +++ b/sphinx/ext/todo.py @@ -22,6 +22,7 @@ from sphinx.errors import NoUri from sphinx.locale import _, __ from sphinx.util import logging from sphinx.util.docutils import SphinxDirective +from sphinx.util.nodes import make_refnode from sphinx.util.texescape import tex_escape_map if False: @@ -55,6 +56,7 @@ class Todo(BaseAdmonition, SphinxDirective): final_argument_whitespace = False option_spec = { 'class': directives.class_option, + 'name': directives.unchanged, } def run(self): @@ -67,13 +69,10 @@ class Todo(BaseAdmonition, SphinxDirective): return [todo] elif isinstance(todo, todo_node): todo.insert(0, nodes.title(text=_('Todo'))) + self.add_name(todo) self.set_source_info(todo) - - targetid = 'index-%s' % self.env.new_serialno('index') - # Stash the target to be retrieved later in latex_visit_todo_node. - todo['targetref'] = '%s:%s' % (self.env.docname, targetid) - targetnode = nodes.target('', '', ids=[targetid]) - return [targetnode, todo] + self.state.document.note_explicit_target(todo) + return [todo] else: raise RuntimeError # never reached here @@ -89,20 +88,14 @@ def process_todos(app, doctree): for node in doctree.traverse(todo_node): app.emit('todo-defined', node) - try: - targetnode = node.parent[node.parent.index(node) - 1] - if not isinstance(targetnode, nodes.target): - raise IndexError - except IndexError: - targetnode = None newnode = node.deepcopy() - del newnode['ids'] + newnode['ids'] = [] env.todo_all_todos.append({ # type: ignore 'docname': env.docname, 'source': node.source or env.doc2path(env.docname), 'lineno': node.line, 'todo': newnode, - 'target': targetnode, + 'target': node['ids'][0], }) if env.config.todo_emit_warnings: @@ -167,27 +160,16 @@ def process_todo_nodes(app, doctree, fromdocname): para += nodes.Text(desc1, desc1) # Create a reference - newnode = nodes.reference('', '', internal=True) innernode = nodes.emphasis(_('original entry'), _('original entry')) try: - newnode['refuri'] = app.builder.get_relative_uri( - fromdocname, todo_info['docname']) - if 'refid' in todo_info['target']: - newnode['refuri'] += '#' + todo_info['target']['refid'] - else: - newnode['refuri'] += '#' + todo_info['target']['ids'][0] + para += make_refnode(app.builder, fromdocname, todo_info['docname'], + todo_info['target'], innernode) except NoUri: # ignore if no URI can be determined, e.g. for LaTeX output pass - newnode.append(innernode) - para += newnode para += nodes.Text(desc2, desc2) todo_entry = todo_info['todo'] - # Remove targetref from the (copied) node to avoid emitting a - # duplicate label of the original entry when we walk this node. - if 'targetref' in todo_entry: - del todo_entry['targetref'] # (Recursively) resolve references in the todo content env.resolve_references(todo_entry, todo_info['docname'], @@ -230,12 +212,7 @@ def depart_todo_node(self, node): def latex_visit_todo_node(self, node): # type: (LaTeXTranslator, todo_node) -> None self.body.append('\n\\begin{sphinxadmonition}{note}{') - # If this is the original todo node, emit a label that will be referenced by - # a hyperref in the todolist. - target = node.get('targetref') - if target is not None: - self.body.append('\\label{%s}' % target) - + self.body.append(self.hypertarget_to(node)) title_node = cast(nodes.title, node[0]) self.body.append('%s:}' % title_node.astext().translate(tex_escape_map)) node.pop(0) diff --git a/tests/test_ext_todo.py b/tests/test_ext_todo.py index b53ac3f72..1ce030208 100644 --- a/tests/test_ext_todo.py +++ b/tests/test_ext_todo.py @@ -107,14 +107,15 @@ def test_todo_valid_link(app, status, warning): # Look for the link to foo. Note that there are two of them because the # source document uses todolist twice. We could equally well look for links # to bar. - link = (r'\{\\hyperref\[\\detokenize\{(.*?foo.*?)}]\{\\sphinxcrossref{' + link = (r'{\\hyperref\[\\detokenize{(.*?foo.*?)}]{\\sphinxcrossref{' r'\\sphinxstyleemphasis{original entry}}}}') m = re.findall(link, content) assert len(m) == 4 target = m[0] # Look for the targets of this link. - labels = [m for m in re.findall(r'\\label\{([^}]*)}', content) if m == target] + labels = re.findall(r'\\label{\\detokenize{([^}]*)}}', content) + matched = [l for l in labels if l == target] # If everything is correct we should have exactly one target. - assert len(labels) == 1 + assert len(matched) == 1