From b226b5399f9907e87df2384b64f1fabd7cc98831 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Mon, 13 May 2024 03:01:23 +0200 Subject: [PATCH 1/9] =?UTF-8?q?=F0=9F=91=8C=20Improve=20footnote=20warning?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add warnings (with source mapping) for unused footnote definitions and footnote references that have no definition. --- docs/syntax/typography.md | 8 --- myst_parser/mdit_to_docutils/base.py | 69 ++++++++++++------- myst_parser/parsers/mdit.py | 5 +- myst_parser/warnings_.py | 2 + pyproject.toml | 2 +- .../fixtures/reporter_warnings.md | 18 ++++- .../sourcedirs/footnotes/footnote_md.md | 2 - 7 files changed, 66 insertions(+), 40 deletions(-) diff --git a/docs/syntax/typography.md b/docs/syntax/typography.md index 66eb28ea..87962805 100644 --- a/docs/syntax/typography.md +++ b/docs/syntax/typography.md @@ -295,13 +295,5 @@ that are not separated by a blank line This is not part of the footnote. ::: -````{important} -Although footnote references can be used just fine within directives, e.g.[^myref], -it is recommended that footnote definitions are not set within directives, -unless they will only be referenced within that same directive: - -This is because, they may not be available to reference in text outside that particular directive. -```` - By default, a transition line (with a `footnotes` class) will be placed before any footnotes. This can be turned off by adding `myst_footnote_transition = False` to the config file. diff --git a/myst_parser/mdit_to_docutils/base.py b/myst_parser/mdit_to_docutils/base.py index e604449b..f0a4cdf0 100644 --- a/myst_parser/mdit_to_docutils/base.py +++ b/myst_parser/mdit_to_docutils/base.py @@ -7,7 +7,6 @@ import os import posixpath import re -from collections import OrderedDict from contextlib import contextmanager, suppress from datetime import date, datetime from types import ModuleType @@ -203,13 +202,15 @@ def _render_tokens(self, tokens: list[Token]) -> None: node_tree = SyntaxTreeNode(tokens) # move footnote definitions to env - self.md_env.setdefault("foot_refs", {}) + self.md_env.setdefault("footnote_definitions", {}) for node in node_tree.walk(include_self=True): new_children = [] for child in node.children: if child.type == "footnote_reference": label = child.meta["label"] - self.md_env["foot_refs"].setdefault(label, []).append(child) + self.md_env["footnote_definitions"].setdefault(label, []).append( + child + ) else: new_children.append(child) @@ -279,30 +280,46 @@ def _render_finalise(self) -> None: # since references within directives/roles will have been added after # those from the initial markdown parse # instead we gather them from a walk of the created document - foot_refs = OrderedDict() + foot_refs: dict[str, list[nodes.footnote_reference]] = {} for refnode in findall(self.document)(nodes.footnote_reference): - if refnode["refname"] not in foot_refs: - foot_refs[refnode["refname"]] = True - + foot_refs.setdefault(refnode["refname"], []).append(refnode) if foot_refs and self.md_config.footnote_transition: self.current_node.append(nodes.transition(classes=["footnotes"])) - for footref in foot_refs: - foot_ref_tokens = self.md_env["foot_refs"].get(footref, []) - if len(foot_ref_tokens) > 1: - self.create_warning( - f"Multiple footnote definitions found for label: '{footref}'", - MystWarnings.MD_FOOTNOTE_DUPE, - append_to=self.current_node, - ) - - if len(foot_ref_tokens) < 1: - self.create_warning( - f"No footnote definitions found for label: '{footref}'", - MystWarnings.MD_FOOTNOTE_MISSING, - append_to=self.current_node, - ) + for foot_label, foot_ref_nodes in foot_refs.items(): + foot_def_tokens = self.md_env["footnote_definitions"].get(foot_label, []) + if len(foot_def_tokens) < 1: + for node in foot_ref_nodes: + self.create_warning( + f"No footnote definition found for label: '{foot_label}'", + MystWarnings.MD_FOOTNOTE_MISSING, + line=node.line, + append_to=self.current_node, + ) + # lets remove the footnote references, so that docutils does not produce any more warnings + # we need to replace them with an element though, so that ids can be moved over (otherwise docutils excepts) + if node.get("auto"): + self.document.autofootnote_refs.remove(node) + node.replace_self(nodes.inline(text=f"[^{node['refname']}]")) else: - self.render_footnote_reference(foot_ref_tokens[0]) + # render the first one, create a warning for any duplicates + self.render_footnote_reference(foot_def_tokens[0]) + for foot_def_token in foot_def_tokens[1:]: + self.create_warning( + f"Duplicate footnote definition found for label: '{foot_label}'", + MystWarnings.MD_FOOTNOTE_DUPE, + line=token_line(foot_def_token), + append_to=self.current_node, + ) + # finally lets warn about any unused footnotes definitions + for foot_label, foot_def_tokens in self.md_env["footnote_definitions"].items(): + if foot_label not in foot_refs: + for foot_def_token in foot_def_tokens: + self.create_warning( + f"Footnote definition not referenced: '{foot_label}'", + MystWarnings.MD_FOOTNOTE_UNUSED, + line=token_line(foot_def_token), + append_to=self.current_node, + ) # Add the wordcount, generated by the ``mdit_py_plugins.wordcount_plugin``. wordcount_metadata = self.md_env.get("wordcount", {}) @@ -1480,9 +1497,11 @@ def render_footnote_ref(self, token: SyntaxTreeNode) -> None: refnode = nodes.footnote_reference(f"[^{target}]") self.add_line_and_source_path(refnode, token) if not target.isdigit(): + # an auto-numbered footnote, similar to rST ``[#label]_`` refnode["auto"] = 1 self.document.note_autofootnote_ref(refnode) else: + # a manually numbered footnote, similar to rST ``[1]_`` refnode += nodes.Text(target) refnode["refname"] = target @@ -1491,17 +1510,21 @@ def render_footnote_ref(self, token: SyntaxTreeNode) -> None: self.current_node.append(refnode) def render_footnote_reference(self, token: SyntaxTreeNode) -> None: + """Despite the name, this is actually a footnote definition, e.g. `[^a]: ...`""" target = token.meta["label"] footnote = nodes.footnote() self.add_line_and_source_path(footnote, token) footnote["names"].append(target) if not target.isdigit(): + # an auto-numbered footnote, similar to rST ``.. [#label]`` footnote["auto"] = 1 self.document.note_autofootnote(footnote) else: + # a manually numbered footnote, similar to rST ``.. [1]`` footnote += nodes.label("", target) self.document.note_footnote(footnote) + self.document.note_explicit_target(footnote, footnote) with self.current_node_context(footnote, append=True): self.render_children(token) diff --git a/myst_parser/parsers/mdit.py b/myst_parser/parsers/mdit.py index b9dda5b1..ac18a985 100644 --- a/myst_parser/parsers/mdit.py +++ b/myst_parser/parsers/mdit.py @@ -61,11 +61,8 @@ def create_md_parser( .use(front_matter_plugin) .use(myst_block_plugin) .use(myst_role_plugin) - .use(footnote_plugin) + .use(footnote_plugin, inline=False, move_to_end=False, always_match_refs=True) .use(wordcount_plugin, per_minute=config.words_per_minute) - .disable("footnote_inline") - # disable this for now, because it need a new implementation in the renderer - .disable("footnote_tail") ) typographer = False diff --git a/myst_parser/warnings_.py b/myst_parser/warnings_.py index 9dcde95c..e23b122d 100644 --- a/myst_parser/warnings_.py +++ b/myst_parser/warnings_.py @@ -27,6 +27,8 @@ class MystWarnings(Enum): """Duplicate Markdown footnote definition.""" MD_FOOTNOTE_MISSING = "footnote" # noqa: PIE796 """Missing Markdown footnote definition.""" + MD_FOOTNOTE_UNUSED = "footnote" # noqa: PIE796 + """Unused Markdown footnote definition.""" MD_HEADING_NON_CONSECUTIVE = "header" """Non-consecutive heading levels.""" diff --git a/pyproject.toml b/pyproject.toml index 65c66548..bcdf786b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,7 +38,7 @@ dependencies = [ "docutils>=0.18,<0.22", "jinja2", # required for substitutions, but let sphinx choose version "markdown-it-py~=3.0", - "mdit-py-plugins~=0.4", + "mdit-py-plugins~=0.4,>=0.4.1", "pyyaml", "sphinx>=6,<8", ] diff --git a/tests/test_renderers/fixtures/reporter_warnings.md b/tests/test_renderers/fixtures/reporter_warnings.md index 4ff162c6..2c1dd5a7 100644 --- a/tests/test_renderers/fixtures/reporter_warnings.md +++ b/tests/test_renderers/fixtures/reporter_warnings.md @@ -110,14 +110,28 @@ Non-consecutive headings: :2: (WARNING/2) Non-consecutive header level increase; H1 to H3 [myst.header] . -multiple footnote definitions +footnote reference with no definition +. +[^a] +. +:1: (WARNING/2) No footnote definition found for label: 'a' [myst.footnote] +. + +footnote definition with no reference +. +[^a]: definition +. +:1: (WARNING/2) Footnote definition not referenced: 'a' [myst.footnote] +. + +duplicate footnote definition . [^a] [^a]: definition 1 [^a]: definition 2 . -:: (WARNING/2) Multiple footnote definitions found for label: 'a' [myst.footnote] +:4: (WARNING/2) Duplicate footnote definition found for label: 'a' [myst.footnote] . Warnings in eval-rst diff --git a/tests/test_sphinx/sourcedirs/footnotes/footnote_md.md b/tests/test_sphinx/sourcedirs/footnotes/footnote_md.md index ca46d0e8..28872a23 100644 --- a/tests/test_sphinx/sourcedirs/footnotes/footnote_md.md +++ b/tests/test_sphinx/sourcedirs/footnotes/footnote_md.md @@ -22,8 +22,6 @@ [^123]: multiple references footnote -[^x]: an unreferenced footnote - [^e] > - [^e]: footnote definition in a block element From 37c5261511eeef73e69a7e3684dbf33c2ae489a9 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Mon, 13 May 2024 04:10:19 +0200 Subject: [PATCH 2/9] test and fix issue with footnote reference translations --- myst_parser/mdit_to_docutils/base.py | 8 +++ .../gettext/fr/LC_MESSAGES/index.po | 12 ++++ tests/test_sphinx/sourcedirs/gettext/index.md | 6 ++ .../test_sphinx_builds/test_gettext.pot | 12 ++++ .../test_gettext_additional_targets.pot | 12 ++++ .../test_sphinx_builds/test_gettext_html.html | 56 +++++++++++++++++++ .../test_gettext_html.resolved.xml | 18 ++++++ .../test_sphinx_builds/test_gettext_html.xml | 18 ++++++ 8 files changed, 142 insertions(+) diff --git a/myst_parser/mdit_to_docutils/base.py b/myst_parser/mdit_to_docutils/base.py index f0a4cdf0..0d6b2b1f 100644 --- a/myst_parser/mdit_to_docutils/base.py +++ b/myst_parser/mdit_to_docutils/base.py @@ -288,6 +288,14 @@ def _render_finalise(self) -> None: for foot_label, foot_ref_nodes in foot_refs.items(): foot_def_tokens = self.md_env["footnote_definitions"].get(foot_label, []) if len(foot_def_tokens) < 1: + if ( + self.document.current_source + and self.document.current_source.endswith("") + ): + # TODO this is a bit of a hack for now, to detect if we are parsing a translation snippet + # in which case we won't have the definition loaded and should not warn/remove + # I think in the future we should look to move this footnote logic to a transform + continue for node in foot_ref_nodes: self.create_warning( f"No footnote definition found for label: '{foot_label}'", diff --git a/tests/test_sphinx/sourcedirs/gettext/fr/LC_MESSAGES/index.po b/tests/test_sphinx/sourcedirs/gettext/fr/LC_MESSAGES/index.po index 43e53202..f536c53c 100644 --- a/tests/test_sphinx/sourcedirs/gettext/fr/LC_MESSAGES/index.po +++ b/tests/test_sphinx/sourcedirs/gettext/fr/LC_MESSAGES/index.po @@ -125,3 +125,15 @@ msgid ".. image:: fun-fish.png\n" " :alt: Fun Fish 3" msgstr ".. image:: poisson-amusant.png\n" " :alt: Poisson amusant 3" + +#: ../../index.md:65 +msgid "footnote references [^1] [^a]" +msgstr "références aux notes de bas de page [^1] [^a]" + +#: ../../index.md:67 +msgid "footnote 1" +msgstr "note de bas de page 1" + +#: ../../index.md:69 +msgid "footnote a" +msgstr "note de bas de page a" diff --git a/tests/test_sphinx/sourcedirs/gettext/index.md b/tests/test_sphinx/sourcedirs/gettext/index.md index a721944b..66ade85f 100644 --- a/tests/test_sphinx/sourcedirs/gettext/index.md +++ b/tests/test_sphinx/sourcedirs/gettext/index.md @@ -61,3 +61,9 @@ doctest block ```{figure} fun-fish.png :alt: Fun Fish 3 ``` + +footnote references [^1] [^a] + +[^1]: footnote 1 + +[^a]: footnote a diff --git a/tests/test_sphinx/test_sphinx_builds/test_gettext.pot b/tests/test_sphinx/test_sphinx_builds/test_gettext.pot index 933b8b6a..1e725505 100644 --- a/tests/test_sphinx/test_sphinx_builds/test_gettext.pot +++ b/tests/test_sphinx/test_sphinx_builds/test_gettext.pot @@ -79,3 +79,15 @@ msgstr "" #: ../../index.md:61 msgid "Fun Fish 3" msgstr "" + +#: ../../index.md:65 +msgid "footnote references [^1] [^a]" +msgstr "" + +#: ../../index.md:67 +msgid "footnote 1" +msgstr "" + +#: ../../index.md:69 +msgid "footnote a" +msgstr "" diff --git a/tests/test_sphinx/test_sphinx_builds/test_gettext_additional_targets.pot b/tests/test_sphinx/test_sphinx_builds/test_gettext_additional_targets.pot index 11c51d4e..632aabdd 100644 --- a/tests/test_sphinx/test_sphinx_builds/test_gettext_additional_targets.pot +++ b/tests/test_sphinx/test_sphinx_builds/test_gettext_additional_targets.pot @@ -127,3 +127,15 @@ msgstr "" #: ../../index.md:61 msgid "Fun Fish 3" msgstr "" + +#: ../../index.md:65 +msgid "footnote references [^1] [^a]" +msgstr "" + +#: ../../index.md:67 +msgid "footnote 1" +msgstr "" + +#: ../../index.md:69 +msgid "footnote a" +msgstr "" diff --git a/tests/test_sphinx/test_sphinx_builds/test_gettext_html.html b/tests/test_sphinx/test_sphinx_builds/test_gettext_html.html index 469e1882..c9af4740 100644 --- a/tests/test_sphinx/test_sphinx_builds/test_gettext_html.html +++ b/tests/test_sphinx/test_sphinx_builds/test_gettext_html.html @@ -156,6 +156,62 @@

Poisson amusant 3
+

+ références aux notes de bas de page + + + [ + + 1 + + ] + + + + + [ + + 2 + + ] + + +

+
+ diff --git a/tests/test_sphinx/test_sphinx_builds/test_gettext_html.resolved.xml b/tests/test_sphinx/test_sphinx_builds/test_gettext_html.resolved.xml index 231ca337..6845a932 100644 --- a/tests/test_sphinx/test_sphinx_builds/test_gettext_html.resolved.xml +++ b/tests/test_sphinx/test_sphinx_builds/test_gettext_html.resolved.xml @@ -91,3 +91,21 @@ Poisson amusant 2
Poisson amusant 3 + + références aux notes de bas de page + + 1 + + + 2 + + +