`todo directive now supports :name:` option

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
 ----------

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)
-
+            self.state.document.note_explicit_target(todo)
-            targetid = 'index-%s' % self.env.new_serialno('index')
+            return [todo]
-            # 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]
         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(
+                para += make_refnode(app.builder, fromdocname, todo_info['docname'],
-                    fromdocname, todo_info['docname'])
+                                     todo_info['target'], innernode)
-                if 'refid' in todo_info['target']:
-                    newnode['refuri'] += '#' + todo_info['target']['refid']
-                else:
-                    newnode['refuri'] += '#' + todo_info['target']['ids'][0]
             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
+    self.body.append(self.hypertarget_to(node))
-    # a hyperref in the todolist.
-    target = node.get('targetref')
-    if target is not None:
-        self.body.append('\\label{%s}' % target)
-
     title_node = cast(nodes.title, node[0])
     self.body.append('%s:}' % title_node.astext().translate(tex_escape_map))
     node.pop(0)

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