Don't auto-add inline links to ref section & rm if empty, per #2130

[CAM-Gerlach]

First step to implementing #2130 , as agreed with @gvanrossum and the PEP editor team.

When building, don't add redundant footnotes and references entries for URLs that are already directly linked inline. This avoids an unnecessary, potentially confusing footnote for each link, and taking up additional space in the references section for no real benefit, plus simplifies the build code and should improve build time, especially for Sphinx. Furthermore, if the references section is empty (not including now-invisible link targets), remove it, as we did before (but in a more robust manner). This allows past and future PEPs to still use inline references with targets stored in the references section, while avoiding showing a now-empty references section.

These are both implemented for Sphinx and the legacy builder, and I visually inspected a variety of PEPs with various cases with both builders to ensure the desired results were achieved, and there were no obvious side effects from this change.

Following merging this PR, following the plan outlined in #2130 , I'll proceed with one updating the meta-PEP docs in PEP 0, PEP 1 and PEP 12 to reflect the revised policy of using standard reST links (inline or with separate targets) rather than the references section, and follow that with a PR updating the relative handful of references in the other active meta-PEPs, for consistency.

[gvanrossum]

I trust you.

[AA-Turner]

Suggested change

	for i, section in enumerate(reversed(doc)):
	if not isinstance(section, nodes.section):
	continue
	title_words = section[0].astext().lower().split()
	if "references" in title_words:
	reference_section = section
	break
	for i, section in enumerate(reversed(self.document[0])):
	if not isinstance(section, nodes.section):
	continue
	title_words = section[0].astext().lower().split()
	if "references" in title_words:
	# Remove references section if there are no displayed
	# footnotes (it only has title & link target nodes)
	if all(isinstance(ref_node, (nodes.title, nodes.target)) for ref_node in section):
	section.parent.remove(section)
	break

Only commenting on the Sphinx version but I think this can be far simpler -- use the suggested in the loop itself, and then remove the if reference_section stanza and the entire _cleanup_callback -- as the pending was only needed due to the TargetNotes transform AFAIK.

Tested (briefly) locally and identical output versus your patch.

A

[AA-Turner]

Tried to add the full diff in the collapsible bit but it wasn't having it -- hence long text below.
A

diff --git a/pep_sphinx_extensions/pep_processor/transforms/pep_footer.py b/pep_sphinx_extensions/pep_processor/transforms/pep_footer.py
index 5fa1b3844..966a6aacb 100644
--- a/pep_sphinx_extensions/pep_processor/transforms/pep_footer.py
+++ b/pep_sphinx_extensions/pep_processor/transforms/pep_footer.py
@@ -4,8 +4,6 @@ import subprocess
 
 from docutils import nodes
 from docutils import transforms
-from docutils.transforms import misc
-from docutils.transforms import references
 
 from pep_sphinx_extensions import config
 
@@ -13,14 +11,9 @@ from pep_sphinx_extensions import config
 class PEPFooter(transforms.Transform):
     """Footer transforms for PEPs.
 
-     - Appends external links to footnotes.
+     - Removes the References section if it is empty when rendered.
      - Creates a link to the (GitHub) source text.
 
-    TargetNotes:
-        Locate the `References` section, insert a placeholder at the end
-        for an external target footnote insertion transform, and schedule
-        the transform to run immediately.
-
     Source Link:
         Create the link to the source file from the document source path,
         and append the text to the end of the document.
@@ -35,43 +28,17 @@ class PEPFooter(transforms.Transform):
         if not pep_source_path.match("pep-*"):
             return  # not a PEP file, exit early
 
-        doc = self.document[0]
-        reference_section = copyright_section = None
-
         # Iterate through sections from the end of the document
-        num_sections = len(doc)
-        for i, section in enumerate(reversed(doc)):
+        for i, section in enumerate(reversed(self.document[0])):
             if not isinstance(section, nodes.section):
                 continue
             title_words = section[0].astext().lower().split()
             if "references" in title_words:
-                reference_section = section
+                # Remove references section if there are no displayed
+                # footnotes (it only has title & link target nodes)
+                if all(isinstance(ref_node, (nodes.title, nodes.target)) for ref_node in section):
+                    section.parent.remove(section)
                 break
-            elif "copyright" in title_words:
-                copyright_section = num_sections - i - 1
-
-        # Add a references section if we didn't find one
-        if not reference_section:
-            reference_section = nodes.section()
-            reference_section += nodes.title("", "References")
-            self.document.set_id(reference_section)
-            if copyright_section:
-                # Put the new "References" section before "Copyright":
-                doc.insert(copyright_section, reference_section)
-            else:
-                # Put the new "References" section at end of doc:
-                doc.append(reference_section)
-
-        # Add and schedule execution of the TargetNotes transform
-        pending = nodes.pending(references.TargetNotes)
-        reference_section.append(pending)
-        self.document.note_pending(pending, priority=0)
-
-        # If there are no references after TargetNotes has finished, remove the
-        # references section
-        pending = nodes.pending(misc.CallBack, details={"callback": _cleanup_callback})
-        reference_section.append(pending)
-        self.document.note_pending(pending, priority=1)
 
         # Add link to source text and last modified date
         if pep_source_path.stem != "pep-0000":
@@ -79,16 +46,6 @@ class PEPFooter(transforms.Transform):
             self.document += _add_commit_history_info(pep_source_path)
 
 
-def _cleanup_callback(pending: nodes.pending) -> None:
-    """Remove an empty "References" section.
-
-    Called after the `references.TargetNotes` transform is complete.
-
-    """
-    if len(pending.parent) == 2:  # <title> and <pending>
-        pending.parent.parent.remove(pending.parent)
-
-
 def _add_source_link(pep_source_path: Path) -> nodes.paragraph:
     """Add link to source text on VCS (GitHub)"""
     source_link = config.pep_vcs_url + pep_source_path.name

[CAM-Gerlach]

Thanks, great catch—I wasn't 100% sure if I could drop the deferred callback. I applied the above manually to both the Sphinx and legacy build systems, and also dropped the no longer used enumerate and index variables and fixed the inconsistent quote chars. I tested it myself locally on both systems and a handful of different PEPs (some with different numbers of hardcoded and link references, and some with a condition of both) and everything rendered identical to before.

[CAM-Gerlach]

Hey @JelleZijlstra or others, are there any blockers to merging this? This would make my PEP 639 over five full pages shorter (since it has around 100 links) and it seems we all agree on it, and then I can proceed with Part 2, a PR updating PEP 1 and PEP 12 , etc., to reflect this.

This also unblocks/avoids merge conflicting with @AA-Turner 's important work to implement PEP 676, which among other things actually opens up a much better approach to addressing your concerns about PEP 639's excessive length, drastically reducing it (on top of cutting down verbosity and specific portions that can be elided, which I'm working on with @pradyunsg ) by moving around two thirds of the PEP text (which is appendices, documentation of rejected ideas and ancillary material) to separate supporting files in the pep-0639 subdirectory, without loosing it completely.

Thanks!

[AA-Turner]

& gone in https://python.github.io/peps/pep-0639!

[CAM-Gerlach]

Thanks so much @JelleZijlstra , @gvanrossum and @AA-Turner !

[gvanrossum]

Thank you! Have you been promoted to PEP editor yet?

[CAM-Gerlach]

Hahaha @gvanrossum , I'm still at "PEP intern" status 😆 But you never know, maybe someday, given that I sure am a lot better at nitpicking proofreading, rewriting copyediting, and pedantry formatting English technical documents than I am at programming, even in my favorite British-comedy-troupe-themed language created by some Dutch guy...

[CAM-Gerlach]

FYI @AA-Turner , looks like our 5788d8a resulted in my multiple-layers-of-indirection links in PEP 639 resolving to anchors on the page rather than the links themselves with the legacy build system (things work fine with your new build system, and my previous commit also works). Not sure how I missed this in testing, sorry...I must not have carefully checked a range of links on that specific PR on the old build system for the final version of that commit, despite having done so for the previous one. Seems your trust was misplaced after all, @gvanrossum 😢

I suspect it has to do with the delayed callback to delete the references section being necessary so that the links get resolved before the section is deleted and their original targets removed. I'll test and confirm and have a followup PR shortly to fix that.

[gvanrossum]

Don’t worry CAM, I’ve done worse. :-)

[CAM-Gerlach]

Yeah...after all, you were the one responsible for adding math.tau to the standard library 😆