Textualize / textual

The lean application framework for Python. Build sophisticated user interfaces with a simple Python API. Run your apps in the terminal and a web browser.
https://textual.textualize.io/
MIT License
25.57k stars 786 forks source link

Markdown Widget crashes on close tag in heading #3689

Closed GuutBoy closed 11 months ago

GuutBoy commented 11 months ago

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```
TomJGooding commented 11 months ago

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.

rodrigogiraoserrao commented 11 months ago

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.

GuutBoy commented 11 months ago

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).

rodrigogiraoserrao commented 11 months ago

Thanks for the additional context, @GuutBoy. PR 3697 will take care of the crashing table of contents and when that's merged, it will close this issue.

Feel free to open a new issue if you'd like to request support for wikilink-style tags/links. (This is not an endorsement for such a feature request!)

TomJGooding commented 11 months ago

Thanks to the parser_factory parameter in the Markdown widget, you could potentially create a custom markdown-it-py parser to handle WikiLinks.

I've managed to hack together a quick example after a couple of hours looking into markdown-it-py plugins. This isn't a working example as I don't actually understand how WikiLinks should work, but hopefully gives an idea what is possible.

import re

from markdown_it import MarkdownIt
from markdown_it.rules_inline import StateInline
from textual import on
from textual.app import App, ComposeResult
from textual.widgets import Markdown

WIKILINK_PATTERN = re.compile(r"\[\[([^\[\]\|\:]+)\]\]")

def wikilinks_plugin(md: MarkdownIt) -> None:
    md.inline.ruler.before("link", "wikilink", _wikilink_rule)

def _wikilink_rule(state: StateInline, silent: bool) -> bool:
    start = state.pos
    match = WIKILINK_PATTERN.match(state.src[start:])
    if not match:
        return False
    href = match.group(1)

    if not silent:
        token = state.push("link_open", "a", 1)
        token.attrSet("href", href)

        token = state.push("text", "", 0)
        token.content = href

        token = state.push("link_close", "a", -1)

    state.pos = start + match.end()
    return True

def parser_factory() -> MarkdownIt:
    parser = MarkdownIt("gfm-like")
    parser.use(wikilinks_plugin)
    return parser

WIKILINK_MD = """\
# WikiLinks Example

This is an example of a [Markdown link](https://github.com) in some text.

This is an example of a [[WikiLink]] in some text.
"""

class WikiLinksMarkdownApp(App):
    def compose(self) -> ComposeResult:
        yield Markdown(
            markdown=WIKILINK_MD,
            parser_factory=parser_factory,
        )

    @on(Markdown.LinkClicked)
    def on_markdown_link_clicked(self, event: Markdown.LinkClicked) -> None:
        self.notify(event.href)

if __name__ == "__main__":
    app = WikiLinksMarkdownApp()
    app.run()
github-actions[bot] commented 11 months ago

Don't forget to star the repository!

Follow @textualizeio for Textual updates.