Merge pull request #3244 from davep/markdown-anchor-jump

Fix Markdown anchor load crash, and support going to an heading on document load

CHANGELOG.md

@@ -23,12 +23,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 - Fixed a crash when removing an option from an `OptionList` while the mouse is hovering over the last option https://github.com/Textualize/textual/issues/3270
+- Fixed a crash in `MarkdownViewer` when clicking on a link that contains an anchor https://github.com/Textualize/textual/issues/3094
 
 ### Changed
 
 - Widget.notify and App.notify are now thread-safe https://github.com/Textualize/textual/pull/3275
 - Breaking change: Widget.notify and App.notify now return None https://github.com/Textualize/textual/pull/3275
 - App.unnotify is now private (renamed to App._unnotify) https://github.com/Textualize/textual/pull/3275
+- `Markdown.load` will now attempt to scroll to a related heading if an anchor is provided https://github.com/Textualize/textual/pull/3244
 
 ## [0.36.0] - 2023-09-05
 

examples/markdown.py

@@ -1,4 +1,5 @@
 from pathlib import Path
+from sys import argv
 
 from textual.app import App, ComposeResult
 from textual.reactive import var
@@ -44,4 +45,6 @@ class MarkdownApp(App):
 
 if __name__ == "__main__":
     app = MarkdownApp()
+    if len(argv) > 1 and Path(argv[1]).exists():
+        app.path = Path(argv[1])
     app.run()

src/textual/widgets/_markdown.py

@@ -12,6 +12,7 @@ from rich.table import Table
 from rich.text import Text
 from typing_extensions import TypeAlias
 
+from .._slug import TrackedSlugs
 from ..app import ComposeResult
 from ..containers import Horizontal, Vertical, VerticalScroll
 from ..events import Mount
@@ -50,6 +51,10 @@ class Navigator:
         Returns:
             New location.
         """
+        location, anchor = Markdown.sanitize_location(str(path))
+        if location == Path(".") and anchor:
+            current_file, _ = Markdown.sanitize_location(str(self.location))
+            path = f"{current_file}#{anchor}"
         new_path = self.location.parent / Path(path)
         self.stack = self.stack[: self.index + 1]
         new_path = new_path.absolute()
@@ -564,6 +569,7 @@ class Markdown(Widget):
         super().__init__(name=name, id=id, classes=classes)
         self._markdown = markdown
         self._parser_factory = parser_factory
+        self._table_of_contents: TableOfContentsType | None = None
 
     class TableOfContentsUpdated(Message):
         """The table of contents was updated."""
@@ -628,6 +634,42 @@ class Markdown(Widget):
         if self._markdown is not None:
             self.update(self._markdown)
 
+    @staticmethod
+    def sanitize_location(location: str) -> tuple[Path, str]:
+        """Given a location, break out the path and any anchor.
+
+        Args:
+            location: The location to sanitize.
+
+        Returns:
+            A tuple of the path to the location cleaned of any anchor, plus
+            the anchor (or an empty string if none was found).
+        """
+        location, _, anchor = location.partition("#")
+        return Path(location), anchor
+
+    def goto_anchor(self, anchor: str) -> None:
+        """Try and find the given anchor in the current document.
+
+        Args:
+            anchor: The anchor to try and find.
+
+        Note:
+            The anchor is found by looking at all of the headings in the
+            document and finding the first one whose slug matches the
+            anchor.
+
+            Note that the slugging method used is similar to that found on
+            GitHub.
+        """
+        if not self._table_of_contents or not isinstance(self.parent, Widget):
+            return
+        unique = TrackedSlugs()
+        for _, title, header_id in self._table_of_contents:
+            if unique.slug(title) == anchor:
+                self.parent.scroll_to_widget(self.query_one(f"#{header_id}"), top=True)
+                return
+
     async def load(self, path: Path) -> None:
         """Load a new Markdown document.
 
@@ -641,7 +683,10 @@ class Markdown(Widget):
             The exceptions that can be raised by this method are all of
             those that can be raised by calling [`Path.read_text`][pathlib.Path.read_text].
         """
+        path, anchor = self.sanitize_location(str(path))
         await self.update(path.read_text(encoding="utf-8"))
+        if anchor:
+            self.goto_anchor(anchor)
 
     def unhandled_token(self, token: Token) -> MarkdownBlock | None:
         """Process an unhandled token.
@@ -672,7 +717,7 @@ class Markdown(Widget):
         )
 
         block_id: int = 0
-        table_of_contents: TableOfContentsType = []
+        self._table_of_contents = []
 
         for token in parser.parse(markdown):
             if token.type == "heading_open":
@@ -721,7 +766,7 @@ class Markdown(Widget):
                 if token.type == "heading_close":
                     heading = block._text.plain
                     level = int(token.tag[1:])
-                    table_of_contents.append((level, heading, block.id))
+                    self._table_of_contents.append((level, heading, block.id))
                 if stack:
                     stack[-1]._blocks.append(block)
                 else:
@@ -801,7 +846,9 @@ class Markdown(Widget):
                 if external is not None:
                     (stack[-1]._blocks if stack else output).append(external)
 
-        self.post_message(Markdown.TableOfContentsUpdated(self, table_of_contents))
+        self.post_message(
+            Markdown.TableOfContentsUpdated(self, self._table_of_contents)
+        )
         with self.app.batch_update():
             self.query("MarkdownBlock").remove()
             return self.mount_all(output)
@@ -952,7 +999,13 @@ class MarkdownViewer(VerticalScroll, can_focus=True, can_focus_children=True):
 
     async def go(self, location: str | PurePath) -> None:
         """Navigate to a new document path."""
-        await self.document.load(self.navigator.go(location))
+        path, anchor = self.document.sanitize_location(str(location))
+        if path == Path(".") and anchor:
+            # We've been asked to go to an anchor but with no file specified.
+            self.document.goto_anchor(anchor)
+        else:
+            # We've been asked to go to a file, optionally with an anchor.
+            await self.document.load(self.navigator.go(location))
 
     async def back(self) -> None:
         """Go back one level in the history."""

tests/test_markdownviewer.py

@@ -0,0 +1,64 @@
+from pathlib import Path
+
+import pytest
+
+from textual.app import App, ComposeResult
+from textual.geometry import Offset
+from textual.widgets import Markdown, MarkdownViewer
+
+TEST_MARKDOWN = """\
+* [First]({{file}}#first)
+* [Second](#second)
+
+# First
+
+The first.
+
+# Second
+
+The second.
+"""
+
+
+class MarkdownFileViewerApp(App[None]):
+    def __init__(self, markdown_file: Path) -> None:
+        super().__init__()
+        self.markdown_file = markdown_file
+        markdown_file.write_text(TEST_MARKDOWN.replace("{{file}}", markdown_file.name))
+
+    def compose(self) -> ComposeResult:
+        yield MarkdownViewer()
+
+    async def on_mount(self) -> None:
+        self.query_one(MarkdownViewer).show_table_of_contents = False
+        await self.query_one(MarkdownViewer).go(self.markdown_file)
+
+
+@pytest.mark.parametrize("link", [0, 1])
+async def test_markdown_file_viewer_anchor_link(tmp_path, link: int) -> None:
+    """Test https://github.com/Textualize/textual/issues/3094"""
+    async with MarkdownFileViewerApp(Path(tmp_path) / "test.md").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))
+
+
+class MarkdownStringViewerApp(App[None]):
+    def compose(self) -> ComposeResult:
+        yield MarkdownViewer(TEST_MARKDOWN.replace("{{file}}", ""))
+
+    async def on_mount(self) -> None:
+        self.query_one(MarkdownViewer).show_table_of_contents = False
+
+
+@pytest.mark.parametrize("link", [0, 1])
+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:
+        # 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))