Merge pull request #4153 from Smattr/1086bfa6-fa4c-485f-bd17-6aa1ca49f851

Fix #1020: ext.todo todolist not linking to the page in pdflatex

AUTHORS

@@ -29,6 +29,7 @@ Other contributors, listed alphabetically, are:
 * Kevin Dunn -- MathJax extension
 * Josip Dzolonga -- coverage builder
 * Buck Evan -- dummy builder
+* Matthew Fernandez -- todo extension fix
 * Hernan Grecco -- search improvements
 * Horst Gutmann -- internationalization support
 * Martin Hans -- autodoc improvements

CHANGES

@@ -33,6 +33,7 @@ Bugs fixed
 * #4063: Sphinx crashes when labeling directive ``.. todolist::``
 * #4134: [doc] :file:`docutils.conf` is not documented explicitly
 * #4169: Chinese language doesn't trigger Chinese search automatically
+* #1020: ext.todo todolist not linking to the page in pdflatex
 
 Testing
 --------

sphinx/ext/todo.py

@@ -69,6 +69,8 @@ class Todo(BaseAdmonition):
 
         env = self.state.document.settings.env
         targetid = 'index-%s' % env.new_serialno('index')
+        # Stash the target to be retrieved later in latex_visit_todo_node.
+        todo['targetref'] = '%s:%s' % (env.docname, targetid)
         targetnode = nodes.target('', '', ids=[targetid])
         return [targetnode, todo]
 
@@ -173,8 +175,12 @@ def process_todo_nodes(app, doctree, fromdocname):
             para += newnode
             para += nodes.Text(desc2, desc2)
 
-            # (Recursively) resolve references in the todo content
             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.
+            del todo_entry['targetref']
+
+            # (Recursively) resolve references in the todo content
             env.resolve_references(todo_entry, todo_info['docname'],
                                    app.builder)
 
@@ -216,7 +222,13 @@ def depart_todo_node(self, node):
 def latex_visit_todo_node(self, node):
     # type: (nodes.NodeVisitor, todo_node) -> None
     title = node.pop(0).astext().translate(tex_escape_map)
-    self.body.append(u'\n\\begin{sphinxadmonition}{note}{%s:}' % title)
+    self.body.append(u'\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(u'\\label{%s}' % target)
+    self.body.append('%s:}' % title)
 
 
 def latex_depart_todo_node(self, node):

tests/roots/test-ext-todo/conf.py

@@ -2,3 +2,8 @@
 
 extensions = ['sphinx.ext.todo']
 master_doc = 'index'
+
+latex_documents = [
+    (master_doc, 'TodoTests.tex', 'Todo Tests Documentation',
+     'Robin Banks', 'manual'),
+]

tests/test_ext_todo.py

@@ -84,3 +84,31 @@ def test_todo_not_included(app, status, warning):
     # check handled event
     assert len(todos) == 2
     assert set(todo[1].astext() for todo in todos) == set(['todo in foo', 'todo in bar'])
+
+@pytest.mark.sphinx('latex', testroot='ext-todo', freshenv=True,
+                    confoverrides={'todo_include_todos': True, 'todo_emit_warnings': True})
+def test_todo_valid_link(app, status, warning):
+    """
+    Test that the inserted "original entry" links for todo items have a target
+    that exists in the LaTeX output. The target was previously incorrectly
+    omitted (GitHub issue #1020).
+    """
+
+    # Ensure the LaTeX output is built.
+    app.builder.build_all()
+
+    content = (app.outdir / 'TodoTests.tex').text()
+
+    # Look for the link to foo. We could equally well look for the link to bar.
+    link = r'\{\\hyperref\[\\detokenize\{(.*?foo.*?)}]\{\\sphinxcrossref{' \
+        r'\\sphinxstyleemphasis{original entry}}}}'
+    m = re.findall(link, content)
+    assert len(m) == 1
+    target = m[0]
+
+    # Look for the targets of this link.
+    labels = [m for m in re.findall(r'\\label\{([^}]*)}', content)
+        if m == target]
+
+    # If everything is correct we should have exactly one target.
+    assert len(labels) == 1

tests/test_writer_latex.py

@@ -27,7 +27,7 @@ def test_rstdim_to_latexdim():
     assert rstdim_to_latexdim('30%') == '0.300\\linewidth'
     assert rstdim_to_latexdim('160') == '160\\sphinxpxdimen'
 
-    # flaot values
+    # float values
     assert rstdim_to_latexdim('160.0em') == '160.0em'
     assert rstdim_to_latexdim('.5em') == '.5em'