Exclude substitution definitions from the `gettext` builder (#9846)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>

CHANGES

@@ -48,6 +48,8 @@ Bugs fixed
 * #10614: Fixed a number of bugs in inheritance diagrams that resulted in
   missing or broken links.
   Patch by Albert Shih.
+* #9428: Exclude substitution definitions when running the ``gettext`` builder.
+  Patch by Alvin Wong.
 
 Testing
 -------

sphinx/builders/gettext.py

@@ -157,6 +157,8 @@ class I18nBuilder(Builder):
                 catalog.add(msg, node)
 
         for node, msg in extract_messages(doctree):
+            # Do not extract messages from within substitution definitions.
+            if not _is_node_in_substitution_definition(node):
                 catalog.add(msg, node)
 
         if 'index' in self.env.config.gettext_additional_targets:
@@ -217,6 +219,15 @@ def should_write(filepath: str, new_content: str) -> bool:
     return True
 
 
+def _is_node_in_substitution_definition(node: nodes.Node) -> bool:
+    """Check "node" to test if it is in a substitution definition."""
+    while node.parent:
+        if isinstance(node, nodes.substitution_definition):
+            return True
+        node = node.parent
+    return False
+
+
 class MessageCatalogBuilder(I18nBuilder):
     """
     Builds gettext-style message catalogs (.pot files).

sphinx/environment/collectors/asset.py

@@ -71,8 +71,11 @@ class ImageCollector(EnvironmentCollector):
 
                 # Update `node['uri']` to a relative path from srcdir
                 # from a relative path from current document.
+                original_uri = node['uri']
                 node['uri'], _ = app.env.relfn2path(imguri, docname)
                 candidates['*'] = node['uri']
+                if node['uri'] != original_uri:
+                    node['original_uri'] = original_uri
 
             # map image paths to unique image names (so that they can be put
             # into a single directory)

sphinx/util/nodes.py

@@ -257,7 +257,8 @@ def extract_messages(doctree: Element) -> Iterable[tuple[Element, str]]:
             if node.get('alt'):
                 yield node, node['alt']
             if node.get('translatable'):
-                msg = '.. image:: %s' % node['uri']
+                image_uri = node.get('original_uri', node['uri'])
+                msg = f'.. image:: {image_uri}'
             else:
                 msg = ''
         elif isinstance(node, nodes.meta):  # type: ignore

tests/roots/test-intl_substitution_definitions/conf.py

@@ -0,0 +1,13 @@
+exclude_patterns = ['_build']
+
+rst_prolog = """\
+.. |subst_prolog_1| replace:: prologue substitute text
+
+.. |subst_prolog_2| image:: /img.png
+"""
+
+rst_epilog = """\
+.. |subst_epilog_1| replace:: epilogue substitute text
+
+.. |subst_epilog_2| image:: /i18n.png
+"""

tests/roots/test-intl_substitution_definitions/index.rst

@@ -0,0 +1,10 @@
+CONTENTS
+========
+
+.. toctree::
+   :maxdepth: 2
+   :numbered:
+   :caption: Table of Contents
+
+   prolog_epilog_substitution
+   prolog_epilog_substitution_excluded

tests/roots/test-intl_substitution_definitions/prolog_epilog_substitution.rst

@@ -0,0 +1,12 @@
+:tocdepth: 2
+
+i18n with prologue and epilogue substitutions
+=============================================
+
+This is content that contains |subst_prolog_1|.
+
+Substituted image |subst_prolog_2| here.
+
+This is content that contains |subst_epilog_1|.
+
+Substituted image |subst_epilog_2| here.

tests/roots/test-intl_substitution_definitions/prolog_epilog_substitution_excluded.rst

@@ -0,0 +1,6 @@
+:tocdepth: 2
+
+i18n without prologue and epilogue substitutions
+================================================
+
+This is content that does not include prologue and epilogue substitutions.

tests/roots/test-intl_substitution_definitions/xx/LC_MESSAGES/prolog_epilog_substitution.po

@@ -0,0 +1,38 @@
+msgid ""
+msgstr ""
+"Project-Id-Version: sphinx tests\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 2021-07-21 12:00+0800\n"
+"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
+"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
+"Language-Team: LANGUAGE <LL@li.org>\n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=utf-8\n"
+"Content-Transfer-Encoding: 8bit\n"
+
+msgid "i18n with prologue and epilogue substitutions"
+msgstr "I18N WITH PROLOGUE AND EPILOGUE SUBSTITUTIONS"
+
+msgid "This is content that contains |subst_prolog_1|."
+msgstr "THIS IS CONTENT THAT CONTAINS |subst_prolog_1|."
+
+msgid "Substituted image |subst_prolog_2| here."
+msgstr "SUBSTITUTED IMAGE |subst_prolog_2| HERE."
+
+msgid "This is content that contains |subst_epilog_1|."
+msgstr "THIS IS CONTENT THAT CONTAINS |subst_epilog_1|."
+
+msgid "Substituted image |subst_epilog_2| here."
+msgstr "SUBSTITUTED IMAGE |subst_epilog_2| HERE."
+
+msgid "subst_prolog_2"
+msgstr "SUBST_PROLOG_2 TRANSLATED"
+
+msgid ".. image:: /img.png"
+msgstr ".. image:: /i18n.png"
+
+msgid "subst_epilog_2"
+msgstr "SUBST_EPILOG_2 TRANSLATED"
+
+msgid ".. image:: /i18n.png"
+msgstr ".. image:: /img.png"

tests/test_build_gettext.py

@@ -202,3 +202,69 @@ def test_build_single_pot(app):
          'msgid "Generated section".*'),
         result,
         flags=re.S)
+
+
+@pytest.mark.sphinx(
+    'gettext',
+    testroot='intl_substitution_definitions',
+    srcdir='gettext-subst',
+    confoverrides={'gettext_compact': False,
+                   'gettext_additional_targets': ['image']})
+def test_gettext_prolog_epilog_substitution(app):
+    app.builder.build_all()
+
+    _msgid_pattern = re.compile(r'msgid "(.*)"')
+
+    def msgid_getter(msgid):
+        if m := _msgid_pattern.search(msgid):
+            return m.groups()[0]
+        return None
+
+    assert (app.outdir / 'prolog_epilog_substitution.pot').is_file()
+    pot = (app.outdir / 'prolog_epilog_substitution.pot').read_text(encoding='utf8')
+    msg_ids = list(filter(None, map(msgid_getter, pot.splitlines())))
+    assert msg_ids == [
+        "i18n with prologue and epilogue substitutions",
+        "This is content that contains |subst_prolog_1|.",
+        "Substituted image |subst_prolog_2| here.",
+        "subst_prolog_2",
+        ".. image:: /img.png",
+        "This is content that contains |subst_epilog_1|.",
+        "Substituted image |subst_epilog_2| here.",
+        "subst_epilog_2",
+        ".. image:: /i18n.png",
+    ]
+
+
+@pytest.mark.sphinx(
+    'gettext',
+    testroot='intl_substitution_definitions',
+    srcdir='gettext-subst',
+    confoverrides={'gettext_compact': False,
+                   'gettext_additional_targets': ['image']})
+def test_gettext_prolog_epilog_substitution_excluded(app):
+    # regression test for #9428
+    app.builder.build_all()
+
+    _msgid_getter = re.compile(r'msgid "(.*)"').search
+
+    def msgid_getter(msgid):
+        m = _msgid_getter(msgid)
+        if m:
+            return m.groups()[0]
+        return None
+
+    assert (app.outdir / 'prolog_epilog_substitution_excluded.pot').is_file()
+    pot = (app.outdir / 'prolog_epilog_substitution_excluded.pot').read_text(encoding='utf8')
+    msgids = [_f for _f in map(msgid_getter, pot.splitlines()) if _f]
+
+    expected_msgids = [
+        "i18n without prologue and epilogue substitutions",
+        "This is content that does not include prologue and epilogue substitutions.",
+    ]
+    for expect in expected_msgids:
+        assert expect in msgids
+        msgids.remove(expect)
+
+    # unexpected msgid existent
+    assert msgids == []

tests/test_intl.py

@@ -1283,6 +1283,37 @@ def test_additional_targets_should_be_translated(app):
     assert_count(expected_expr, result, 1)
 
 
+@pytest.mark.sphinx(
+    'html',
+    testroot='intl_substitution_definitions',
+    confoverrides={
+        'language': 'xx', 'locale_dirs': ['.'],
+        'gettext_compact': False,
+        'gettext_additional_targets': [
+            'index',
+            'literal-block',
+            'doctest-block',
+            'raw',
+            'image',
+        ],
+    },
+)
+def test_additional_targets_should_be_translated_substitution_definitions(app):
+    app.builder.build_all()
+
+    # [prolog_epilog_substitution.txt]
+
+    result = (app.outdir / 'prolog_epilog_substitution.html').read_text(encoding='utf8')
+
+    # alt and src for image block should be translated
+    expected_expr = """<img alt="SUBST_PROLOG_2 TRANSLATED" src="_images/i18n.png" />"""
+    assert_count(expected_expr, result, 1)
+
+    # alt and src for image block should be translated
+    expected_expr = """<img alt="SUBST_EPILOG_2 TRANSLATED" src="_images/img.png" />"""
+    assert_count(expected_expr, result, 1)
+
+
 @sphinx_intl
 @pytest.mark.sphinx('text')
 @pytest.mark.test_params(shared_result='test_intl_basic')
@@ -1294,6 +1325,33 @@ def test_text_references(app, warning):
     assert_count(warning_expr, warnings, 0)
 
 
+@pytest.mark.sphinx(
+    'text',
+    testroot='intl_substitution_definitions',
+    confoverrides={
+        'language': 'xx', 'locale_dirs': ['.'],
+        'gettext_compact': False,
+    },
+)
+def test_text_prolog_epilog_substitution(app):
+    app.build()
+
+    result = (app.outdir / 'prolog_epilog_substitution.txt').read_text(encoding='utf8')
+
+    assert result == """\
+1. I18N WITH PROLOGUE AND EPILOGUE SUBSTITUTIONS
+************************************************
+
+THIS IS CONTENT THAT CONTAINS prologue substitute text.
+
+SUBSTITUTED IMAGE [image: SUBST_PROLOG_2 TRANSLATED][image] HERE.
+
+THIS IS CONTENT THAT CONTAINS epilogue substitute text.
+
+SUBSTITUTED IMAGE [image: SUBST_EPILOG_2 TRANSLATED][image] HERE.
+"""
+
+
 @pytest.mark.sphinx(
     'dummy', testroot='images',
     srcdir='test_intl_images',