Repository: Textualize/textual
Pull request: 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.

Fixes #3689.

Problem statement:
Markdown Widget crashes on close tag in heading
The Markdown Widget crashes if there is a closing rich tag in a heading. I.e., if there is a string like `[/some_text]` in a markdown heading. This can become problematic when using Markdown extensions to WikiLinks and having something linke `[[/test.md]]` in a heading.

Note in regular text this issue does not seem to be present. I have only seen this for headings.

**Example**

Running this
```
from textual.app import App, ComposeResult
from textual.widgets import MarkdownViewer

EXAMPLE_MARKDOWN = """\
# Markdown [/test]
"""


class MarkdownExampleApp(App):
    def compose(self) -> ComposeResult:
        yield MarkdownViewer(EXAMPLE_MARKDOWN, show_table_of_contents=True)


if __name__ == "__main__":
    app = MarkdownExampleApp()
    app.run()
```
will produce this error:
```
python3 markdown.py
╭──────────────────────────────────────────────────────────────────────────────────────────── Traceback (most recent call last) ─────────────────────────────────────────────────────────────────────────────────────────────╮
│ /usr/local/lib/python3.11/site-packages/textual/widgets/_markdown.py:1056 in _on_markdown_table_of_contents_updated                                                                                                        │
│                                                                                                                                                                                                                            │
│   1053 │   ) -> None:                                                                           ╭────────────── locals ──────────────╮                                                                                     │
│   1054 │   │   self.query_one(                                                                  │ message = TableOfContentsUpdated() │                                                                                     │
│   1055 │   │   │   MarkdownTableOfContents                                                      │    self = MarkdownViewer()         │                                                                                     │
│ ❱ 1056 │   │   ).table_of_contents = message.table_of_contents                                  ╰────────────────────────────────────╯                                                                                     │
│   1057 │   │   message.stop()                                                                                                                                                                                              │
│   1058 │                                                                                                                                                                                                                   │
│   1059 │   def _on_markdown_table_of_contents_selected(                                                                                                                                                                    │
│                                                                                                                                                                                                                            │
│ /usr/local/lib/python3.11/site-packages/textual/widgets/_markdown.py:920 in watch_table_of_contents                                                                                                                        │
│                                                                                                                                                                                                                            │
│    917 │                                                                                        ╭──────────────────────── locals ─────────────────────────╮                                                                │
│    918 │   def watch_table_of_contents(self, table_of_contents: TableOfContentsType) -> None:   │              self = MarkdownTableOfContents()           │                                                                │
│    919 │   │   """Triggered when the table of contents changes."""                              │ table_of_contents = [(1, 'Markdown [/test]', 'block1')] │                                                                │
│ ❱  920 │   │   self.set_table_of_contents(table_of_contents)                                    ╰─────────────────────────────────────────────────────────╯                                                                │
│    921 │                                                                                                                                                                                                                   │
│    922 │   def set_table_of_contents(self, table_of_contents: TableOfContentsType) -> None:                                                                                                                                │
│    923 │   │   """Set the table of contents.                                                                                                                                                                               │
│                                                                                                                                                                                                                            │
│ /usr/local/lib/python3.11/site-packages/textual/widgets/_markdown.py:940 in set_table_of_contents                                                                                                                          │
│                                                                                                                                                                                                                            │
│    937 │   │   │   │   │   node.allow_expand = True                                             ╭──────────────────────── locals ─────────────────────────╮                                                                │
│    938 │   │   │   │   else:                                                                    │          block_id = 'block1'                            │                                                                │
│    939 │   │   │   │   │   node = node.add(NUMERALS[level], expand=True)                        │             level = 1                                   │                                                                │
│ ❱  940 │   │   │   node.add_leaf(f"[dim]{NUMERALS[level]}[/] {name}", {"block_id": block_id})   │              name = 'Markdown [/test]'                  │                                                                │
│    941 │                                                                                        │              node = TreeNode('TOC', None)               │                                                                │
│    942 │   async def _on_tree_node_selected(self, message: Tree.NodeSelected) -> None:          │              root = TreeNode('TOC', None)               │                                                                │
│    943 │   │   node_data = message.node.data                                                    │              self = MarkdownTableOfContents()           │                                                                │
│                                                                                                 │ table_of_contents = [(1, 'Markdown [/test]', 'block1')] │                                                                │
│                                                                                                 │              tree = Tree()                              │                                                                │
│                                                                                                 ╰─────────────────────────────────────────────────────────╯                                                                │
│                                                                                                                                                                                                                            │
│ /usr/local/lib/python3.11/site-packages/textual/widgets/_tree.py:352 in add_leaf                                                                                                                                           │
│                                                                                                                                                                                                                            │
│    349 │   │   Returns:                                                                         ╭─────────────── locals ───────────────╮                                                                                   │
│    350 │   │   │   New node.                                                                    │  data = {'block_id': 'block1'}       │                                                                                   │
│    351 │   │   """                                                                              │ label = '[dim]Ⅰ[/] Markdown [/test]' │                                                                                   │
│ ❱  352 │   │   node = self.add(label, data, expand=False, allow_expand=False)                   │  self = TreeNode('TOC', None)        │                                                                                   │
│    353 │   │   return node                                                                      ╰──────────────────────────────────────╯                                                                                   │
│    354 │                                                                                                                                                                                                                   │
│    355 │   class RemoveRootError(Exception):                                                                                                                                                                               │
│                                                                                                                                                                                                                            │
│ /usr/local/lib/python3.11/site-packages/textual/widgets/_tree.py:331 in add                                                                                                                                                │
│                                                                                                                                                                                                                            │
│    328 │   │   Returns:                                                                         ╭────────────────── locals ───────────────────╮                                                                            │
│    329 │   │   │   A new Tree node                                                              │ allow_expand = False                        │                                                                            │
│    330 │   │   """                                                                              │         data = {'block_id': 'block1'}       │                                                                            │
│ ❱  331 │   │   text_label = self._tree.process_label(label)                                     │       expand = False                        │                                                                            │
│    332 │   │   node = self._tree._add_node(self, text_label, data)                              │        label = '[dim]Ⅰ[/] Markdown [/test]' │                                                                            │
│    333 │   │   node._expanded = expand                                                          │         self = TreeNode('TOC', None)        │                                                                            │
│    334 │   │   node._allow_expand = allow_expand                                                ╰─────────────────────────────────────────────╯                                                                            │
│                                                                                                                                                                                                                            │
│ /usr/local/lib/python3.11/site-packages/textual/widgets/_tree.py:635 in process_label                                                                                                                                      │
│                                                                                                                                                                                                                            │
│    632 │   │   │   A Rich Text object.                                                          ╭─────────────── locals ───────────────╮                                                                                   │
│    633 │   │   """                                                                              │ label = '[dim]Ⅰ[/] Markdown [/test]' │                                                                                   │
│    634 │   │   if isinstance(label, str):                                                       │  self = Tree()                       │                                                                                   │
│ ❱  635 │   │   │   text_label = Text.from_markup(label)                                         ╰──────────────────────────────────────╯                                                                                   │
│    636 │   │   else:                                                                                                                                                                                                       │
│    637 │   │   │   text_label = label                                                                                                                                                                                      │
│    638 │   │   first_line = text_label.split()[0]                                                                                                                                                                          │
│                                                                                                                                                                                                                            │
│ /usr/local/lib/python3.11/site-packages/rich/text.py:286 in from_markup                                                                                                                                                    │
│                                                                                                                                                                                                                            │
│ /usr/local/lib/python3.11/site-packages/rich/markup.py:167 in render                                                                                                                                                       │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
MarkupError: closing tag '[/test]' at position 19 doesn't match any open tag```

Hints from issue discussion:
> Note in regular text this issue does not seem to be present. I have only seen this for headings.

Just to clarify, it looks like it isn't the `Markdown` widget itself that's crashing, but rather the `Tree` used for the table of contents, which is why this issues only occurs with headings.
Hey @GuutBoy thanks for the issue and @TomJGooding for the headstart on this.

@GuutBoy could you help me understand better how/why something like `[[/test.md]]` might end up at the end of a markdown heading?
And what would be your expected output?
Would the `[[/test.md]]` just disappear from the output markdown document?
I also don't think I followed your mention of wikilinks/markdown extensions.
Maybe the focus on wikilinks is not so important. The point is that any string like `[/test]` (i.e., something than resembles a unclosed Rich tag) in a Markdown heading will cause Markdown widget to crash. It does not matter if this string is in the beginning, end or anywhere else in the heading. (In fact as TomJGooding points out it seems to be the Tree widget that is really causing the issue).

I just mentioned wikilinks because this is how I noticed the issue because wikilinks are likely to trigger this issue. The use of wikilinks is a common extension to Markdown. WikiLinks is a different way to make links than the regular syntax for links in Markdown. A wikilink surrounds the link path in double brackets like so `[[link]]`. If the path of you link starts with a `/` you trigger this issue (something that would be common if you are linking to local files).  Ideally, wikilinks would be treated as regular Markdown links. But at least it would be good widgets did not just crash on these types of strings

Why would someone have wikilinks in a heading? Well, why not?   

Btw. I noticed that if the I make a regular markdown link with the text starting in a `/` the issue is not triggered. Like so `[/test](/test.md)`.

Commit message: 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.

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9d269fc123..ebff906cd1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - CSS error reporting will no longer provide links to the files in question https://github.com/Textualize/textual/pull/3582
 - 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
+- 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
index ac6de4f4bc..96b81cc6eb 100644
--- 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.from_markup(f"[dim]{NUMERALS[level]}[/] ") + Text(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/test_markdownviewer.py b/tests/test_markdownviewer.py
index 27ccf0da99..8d94d4b946 100644
--- 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)


Code review comments:
diff_hunk: @@ -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.from_markup(f"[dim]{NUMERALS[level]}[/] ") + Text(name)
path: src/textual/widgets/_markdown.py
text: @user1:
You could use Text.assemble here for greater efficiency.

@author:
Thanks. I never know what's faster. See aeee036.

@user1:
You can give the style as a string, i.e. "dim". Which should be faster because it will be cached rather than constructed every time.

@author:
Understood. 5ae7b27 should do that.