Merge pull request #7230 from tk0miya/refactor_std_domain

std domain: Generate node_id for objects in the right way
sphinx-doc · Mar 6, 2020 · 5a848e3 · 5a848e3
2 parents 1224af7 + c0535d7
commit 5a848e3
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 39 deletions.
diff --git a/sphinx/domains/std.py b/sphinx/domains/std.py
@@ -214,16 +214,18 @@ def handle_signature(self, sig: str, signode: desc_signature) -> str:
     def add_target_and_index(self, firstname: str, sig: str, signode: desc_signature) -> None:
         currprogram = self.env.ref_context.get('std:program')
         for optname in signode.get('allnames', []):
-            targetname = optname.replace('/', '-')
-            if not targetname.startswith('-'):
-                targetname = '-arg-' + targetname
+            prefixes = ['cmdoption']
             if currprogram:
-                targetname = '-' + currprogram + targetname
-            targetname = 'cmdoption' + targetname
-            signode['names'].append(targetname)
+                prefixes.append(currprogram)
+            if not optname.startswith(('-', '/')):
+                prefixes.append('arg')
+            prefix = '-'.join(prefixes)
+            node_id = make_id(self.env, self.state.document, prefix, optname)
+            signode['ids'].append(node_id)
 
-        domain = cast(StandardDomain, self.env.get_domain('std'))
         self.state.document.note_explicit_target(signode)
+
+        domain = cast(StandardDomain, self.env.get_domain('std'))
         for optname in signode.get('allnames', []):
             domain.add_program_option(currprogram, optname,
                                       self.env.docname, signode['ids'][0])
@@ -487,28 +489,36 @@ def run(self) -> List[Node]:
             subnode = addnodes.production(rule)
             subnode['tokenname'] = name.strip()
             if subnode['tokenname']:
-                # nodes.make_id converts '_' to '-',
-                # so we can use '_' to delimit group from name,
-                # and make sure we don't clash with other IDs.
-                idname = 'grammar-token-%s_%s' \
-                         % (nodes.make_id(productionGroup), nodes.make_id(name))
-                if idname not in self.state.document.ids:
-                    subnode['ids'].append(idname)
-
-                idnameOld = nodes.make_id('grammar-token-' + name)
-                if idnameOld not in self.state.document.ids:
-                    subnode['ids'].append(idnameOld)
+                prefix = 'grammar-token-%s' % productionGroup
+                node_id = make_id(self.env, self.state.document, prefix, name)
+                subnode['ids'].append(node_id)
+
+                # Assign old styled node_id not to break old hyperlinks (if possible)
+                # Note: Will be removed in Sphinx-5.0 (RemovedInSphinx50Warning)
+                old_node_id = self.make_old_id(name)
+                if (old_node_id not in self.state.document.ids and
+                        old_node_id not in subnode['ids']):
+                    subnode['ids'].append(old_node_id)
+
                 self.state.document.note_implicit_target(subnode, subnode)
+
                 if len(productionGroup) != 0:
                     objName = "%s:%s" % (productionGroup, name)
                 else:
                     objName = name
-                domain.note_object(objtype='token', name=objName, labelid=idname,
-                                   location=node)
+                domain.note_object('token', objName, node_id, location=node)
             subnode.extend(token_xrefs(tokens, productionGroup))
             node.append(subnode)
         return [node]
 
+    def make_old_id(self, token: str) -> str:
+        """Generate old styled node_id for tokens.
+
+        .. note:: Old Styled node_id was used until Sphinx-3.0.
+                  This will be removed in Sphinx-5.0.
+        """
+        return nodes.make_id('grammar-token-' + token)
+
 
 class TokenXRefRole(XRefRole):
     def process_link(self, env: "BuildEnvironment", refnode: Element, has_explicit_title: bool,

diff --git a/tests/test_build_html.py b/tests/test_build_html.py
@@ -222,7 +222,7 @@ def test_html4_output(app, status, warning):
          "[@class='reference internal']/code/span[@class='pre']", 'HOME'),
         (".//a[@href='#with']"
          "[@class='reference internal']/code/span[@class='pre']", '^with$'),
-        (".//a[@href='#grammar-token-_try-stmt']"
+        (".//a[@href='#grammar-token-try-stmt']"
          "[@class='reference internal']/code/span", '^statement$'),
         (".//a[@href='#some-label'][@class='reference internal']/span", '^here$'),
         (".//a[@href='#some-label'][@class='reference internal']/span", '^there$'),
@@ -254,7 +254,7 @@ def test_html4_output(app, status, warning):
         (".//dl/dt[@id='term-boson']", 'boson'),
         # a production list
         (".//pre/strong", 'try_stmt'),
-        (".//pre/a[@href='#grammar-token-_try1-stmt']/code/span", 'try1_stmt'),
+        (".//pre/a[@href='#grammar-token-try1-stmt']/code/span", 'try1_stmt'),
         # tests for ``only`` directive
         (".//p", 'A global substitution.'),
         (".//p", 'In HTML.'),

diff --git a/tests/test_domain_std.py b/tests/test_domain_std.py
@@ -352,23 +352,23 @@ def test_productionlist(app, status, warning):
         linkText = span.text.strip()
         cases.append((text, link, linkText))
     assert cases == [
-        ('A', 'Bare.html#grammar-token-_a', 'A'),
-        ('B', 'Bare.html#grammar-token-_b', 'B'),
-        ('P1:A', 'P1.html#grammar-token-p1_a', 'P1:A'),
-        ('P1:B', 'P1.html#grammar-token-p1_b', 'P1:B'),
-        ('P2:A', 'P1.html#grammar-token-p1_a', 'P1:A'),
-        ('P2:B', 'P2.html#grammar-token-p2_b', 'P2:B'),
-        ('Explicit title A, plain', 'Bare.html#grammar-token-_a', 'MyTitle'),
-        ('Explicit title A, colon', 'Bare.html#grammar-token-_a', 'My:Title'),
-        ('Explicit title P1:A, plain', 'P1.html#grammar-token-p1_a', 'MyTitle'),
-        ('Explicit title P1:A, colon', 'P1.html#grammar-token-p1_a', 'My:Title'),
-        ('Tilde A', 'Bare.html#grammar-token-_a', 'A'),
-        ('Tilde P1:A', 'P1.html#grammar-token-p1_a', 'A'),
-        ('Tilde explicit title P1:A', 'P1.html#grammar-token-p1_a', '~MyTitle'),
-        ('Tilde, explicit title P1:A', 'P1.html#grammar-token-p1_a', 'MyTitle'),
-        ('Dup', 'Dup2.html#grammar-token-_dup', 'Dup'),
-        ('FirstLine', 'firstLineRule.html#grammar-token-_firstline', 'FirstLine'),
-        ('SecondLine', 'firstLineRule.html#grammar-token-_secondline', 'SecondLine'),
+        ('A', 'Bare.html#grammar-token-a', 'A'),
+        ('B', 'Bare.html#grammar-token-b', 'B'),
+        ('P1:A', 'P1.html#grammar-token-p1-a', 'P1:A'),
+        ('P1:B', 'P1.html#grammar-token-p1-b', 'P1:B'),
+        ('P2:A', 'P1.html#grammar-token-p1-a', 'P1:A'),
+        ('P2:B', 'P2.html#grammar-token-p2-b', 'P2:B'),
+        ('Explicit title A, plain', 'Bare.html#grammar-token-a', 'MyTitle'),
+        ('Explicit title A, colon', 'Bare.html#grammar-token-a', 'My:Title'),
+        ('Explicit title P1:A, plain', 'P1.html#grammar-token-p1-a', 'MyTitle'),
+        ('Explicit title P1:A, colon', 'P1.html#grammar-token-p1-a', 'My:Title'),
+        ('Tilde A', 'Bare.html#grammar-token-a', 'A'),
+        ('Tilde P1:A', 'P1.html#grammar-token-p1-a', 'A'),
+        ('Tilde explicit title P1:A', 'P1.html#grammar-token-p1-a', '~MyTitle'),
+        ('Tilde, explicit title P1:A', 'P1.html#grammar-token-p1-a', 'MyTitle'),
+        ('Dup', 'Dup2.html#grammar-token-dup', 'Dup'),
+        ('FirstLine', 'firstLineRule.html#grammar-token-firstline', 'FirstLine'),
+        ('SecondLine', 'firstLineRule.html#grammar-token-secondline', 'SecondLine'),
     ]
 
     text = (app.outdir / 'LineContinuation.html').read_text()