Escape markup in markdown headings. (#3697)

* Escape markup in markdown headings. The markup would already be 'escaped' (ignored, really) in the markdown document itself, but it would be processed when building the table of contents because of the way the widget 'Tree' internally processes labels. This was changed, so that we create our own 'Text' instances for the labels, which means we get to avoid markup processing. Related issue: #3689. * Update CHANGELOG.md Co-authored-by: Dave Pearson <[email protected]> * Optimisation. Related review comment: #3697 * Optimisation. Related review comment: #3697 (comment) * Update snapshot. --------- Co-authored-by: Dave Pearson <[email protected]>
Textualize · Nov 20, 2023 · ef1aebd · ef1aebd
1 parent 018479e
commit ef1aebd
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 73 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - inline CSS error reporting will report widget/class variable where the CSS was read from https://github.com/Textualize/textual/pull/3582
 - Breaking change: Setting `Select.value` to `None` no longer clears the selection (See `Select.BLANK` and `Select.clear`) https://github.com/Textualize/textual/pull/3614
 - Breaking change: `Button` no longer inherits from `Static`, now it inherits directly from `Widget` https://github.com/Textualize/textual/issues/3603
+- Rich markup in markdown headings is now escaped when building the TOC https://github.com/Textualize/textual/issues/3689
 
 
 ## [0.41.0] - 2023-10-31

diff --git a/src/textual/widgets/_markdown.py b/src/textual/widgets/_markdown.py
@@ -937,7 +937,8 @@ def set_table_of_contents(self, table_of_contents: TableOfContentsType) -> None:
                     node.allow_expand = True
                 else:
                     node = node.add(NUMERALS[level], expand=True)
-            node.add_leaf(f"[dim]{NUMERALS[level]}[/] {name}", {"block_id": block_id})
+            node_label = Text.assemble((f"{NUMERALS[level]} ", "dim"), name)
+            node.add_leaf(node_label, {"block_id": block_id})
 
     async def _on_tree_node_selected(self, message: Tree.NodeSelected) -> None:
         node_data = message.node.data

diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr
diff --git a/tests/test_markdownviewer.py b/tests/test_markdownviewer.py
@@ -1,10 +1,12 @@
 from pathlib import Path
 
 import pytest
+from rich.text import Text
 
+import textual.widgets._markdown as MD
 from textual.app import App, ComposeResult
 from textual.geometry import Offset
-from textual.widgets import Markdown, MarkdownViewer
+from textual.widgets import Markdown, MarkdownViewer, Tree
 
 TEST_MARKDOWN = """\
 * [First]({{file}}#first)
@@ -45,8 +47,12 @@ async def test_markdown_file_viewer_anchor_link(tmp_path, link: int) -> None:
 
 
 class MarkdownStringViewerApp(App[None]):
+    def __init__(self, markdown_string: str) -> None:
+        self.markdown_string = markdown_string
+        super().__init__()
+
     def compose(self) -> ComposeResult:
-        yield MarkdownViewer(TEST_MARKDOWN.replace("{{file}}", ""))
+        yield MarkdownViewer(self.markdown_string)
 
     async def on_mount(self) -> None:
         self.query_one(MarkdownViewer).show_table_of_contents = False
@@ -57,8 +63,27 @@ async def test_markdown_string_viewer_anchor_link(link: int) -> None:
     """Test https://github.com/Textualize/textual/issues/3094
 
     Also https://github.com/Textualize/textual/pull/3244#issuecomment-1710278718."""
-    async with MarkdownStringViewerApp().run_test() as pilot:
+    async with MarkdownStringViewerApp(
+        TEST_MARKDOWN.replace("{{file}}", "")
+    ).run_test() as pilot:
         # There's not really anything to test *for* here, but the lack of an
         # exception is the win (before the fix this is testing it would have
         # been FileNotFoundError).
         await pilot.click(Markdown, Offset(2, link))
+
+
+@pytest.mark.parametrize("text", ["Hey [[/test]]", "[i]Hey there[/i]"])
+async def test_headings_that_look_like_they_contain_markup(text: str) -> None:
+    """Regression test for https://github.com/Textualize/textual/issues/3689.
+
+    Things that look like markup are escaped in markdown headings in the table of contents.
+    """
+
+    document = f"# {text}"
+    async with MarkdownStringViewerApp(document).run_test() as pilot:
+        assert pilot.app.query_one(MD.MarkdownH1)._text == Text(text)
+        toc_tree = pilot.app.query_one(MD.MarkdownTableOfContents).query_one(Tree)
+        # The toc label looks like "I {text}" but the I is styled so we drop it.
+        toc_label = toc_tree.root.children[0].label
+        _, text_label = toc_label.divide([2])
+        assert text_label == Text(text)