Merge pull request #4978 from Textualize/rich-log-width-fix

RichLog fixes

CHANGELOG.md

@@ -15,11 +15,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Changed
 
 - Input validation for integers no longer accepts scientific notation like '1.5e2'; must be castable to int. https://github.com/Textualize/textual/pull/4784
+- Some fixes in `RichLog` result in slightly different semantics, see docstrings for details https://github.com/Textualize/textual/pull/4978
 
 ### Fixed
 
 - Input validation of floats no longer accepts NaN (not a number). https://github.com/Textualize/textual/pull/4784
 - Fixed issues with screenshots by simplifying segments only for snapshot tests https://github.com/Textualize/textual/issues/4929
+- Fixed `RichLog.write` not respecting `width` parameter https://github.com/Textualize/textual/pull/4978
+- Fixed `RichLog` writing at wrong width when `write` occurs before width is known (e.g. in `compose` or `on_mount`) https://github.com/Textualize/textual/pull/4978
+- Fixed `RichLog.write` incorrectly shrinking width to `RichLog.min_width` when `shrink=True` (now shrinks to fit content area instead) https://github.com/Textualize/textual/pull/4978
 
 ## [0.79.1] - 2024-08-31
 

src/textual/widgets/_rich_log.py

@@ -2,7 +2,8 @@
 
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Optional, cast
+from collections import deque
+from typing import TYPE_CHECKING, NamedTuple, Optional, cast
 
 from rich.console import RenderableType
 from rich.highlighter import Highlighter, ReprHighlighter
@@ -12,8 +13,10 @@
 from rich.segment import Segment
 from rich.text import Text
 
+from textual.events import Resize
+
 from ..cache import LRUCache
-from ..geometry import Region, Size
+from ..geometry import Size
 from ..reactive import var
 from ..scroll_view import ScrollView
 from ..strip import Strip
@@ -22,8 +25,28 @@
     from typing_extensions import Self
 
 
+class DeferredRender(NamedTuple):
+    """A renderable which is awaiting rendering.
+    This may happen if a `write` occurs before the width is known.
+
+    The arguments are the same as for `RichLog.write`, as this just
+    represents a deferred call to that method.
+    """
+
+    content: RenderableType | object
+    """The content to render."""
+    width: int | None = None
+    """The width to render or `None` to use optimal width."""
+    expand: bool = False
+    """Enable expand to widget width, or `False` to use `width`."""
+    shrink: bool = True
+    """Enable shrinking of content to fit width."""
+    scroll_end: bool | None = None
+    """Enable automatic scroll to end, or `None` to use `self.auto_scroll`."""
+
+
 class RichLog(ScrollView, can_focus=True):
-    """A widget for logging text."""
+    """A widget for logging Rich renderables and text."""
 
     DEFAULT_CSS = """
     RichLog{
@@ -54,13 +77,15 @@ def __init__(
         classes: str | None = None,
         disabled: bool = False,
     ) -> None:
-        """Create a RichLog widget.
+        """Create a `RichLog` widget.
 
         Args:
             max_lines: Maximum number of lines in the log or `None` for no maximum.
-            min_width: Minimum width of renderables.
+            min_width: Width to use for calls to `write` with no specified `width`.
             wrap: Enable word wrapping (default is off).
-            highlight: Automatically highlight content.
+            highlight: Automatically highlight content. By default, the `ReprHighlighter` is used.
+                To customize highlighting, set `highlight=True` and then set the `highlighter`
+                attribute to an instance of `Highlighter`.
             markup: Apply Rich console markup.
             auto_scroll: Enable automatic scrolling to end.
             name: The name of the text log.
@@ -73,9 +98,11 @@ def __init__(
         """Maximum number of lines in the log or `None` for no maximum."""
         self._start_line: int = 0
         self.lines: list[Strip] = []
+        """The lines currently visible in the log."""
         self._line_cache: LRUCache[tuple[int, int, int, int], Strip]
         self._line_cache = LRUCache(1024)
-        self.max_width: int = 0
+        self._deferred_renders: deque[DeferredRender] = deque()
+        """Queue of deferred renderables to be rendered."""
         self.min_width = min_width
         """Minimum width of renderables."""
         self.wrap = wrap
@@ -89,14 +116,24 @@ def __init__(
         self.highlighter: Highlighter = ReprHighlighter()
         """Rich Highlighter used to highlight content when highlight is True"""
 
-        self._last_container_width: int = min_width
-        """Record the last width we rendered content at."""
+        self._widest_line_width = 0
+        """The width of the widest line currently in the log."""
+
+        self._size_known = False
+        """Flag which is set to True when the size of the RichLog is known,
+        indicating we can proceed with rendering deferred writes."""
 
     def notify_style_update(self) -> None:
         self._line_cache.clear()
 
-    def on_resize(self) -> None:
-        self._last_container_width = self.scrollable_content_region.width
+    def on_resize(self, event: Resize) -> None:
+        if event.size.width and not self._size_known:
+            # This size is known for the first time.
+            self._size_known = True
+            deferred_renders = self._deferred_renders
+            while deferred_renders:
+                deferred_render = deferred_renders.popleft()
+                self.write(*deferred_render)
 
     def _make_renderable(self, content: RenderableType | object) -> RenderableType:
         """Make content renderable.
@@ -134,60 +171,80 @@ def write(
         shrink: bool = True,
         scroll_end: bool | None = None,
     ) -> Self:
-        """Write text or a rich renderable.
+        """Write a string or a Rich renderable to the bottom of the log.
+
+        Notes:
+            The rendering of content will be deferred until the size of the `RichLog` is known.
+            This means if you call `write` in `compose` or `on_mount`, the content will not be
+            rendered immediately.
 
         Args:
-            content: Rich renderable (or text).
-            width: Width to render or `None` to use optimal width.
-            expand: Enable expand to widget width, or `False` to use `width`.
-            shrink: Enable shrinking of content to fit width.
+            content: Rich renderable (or a string).
+            width: Width to render, or `None` to use `RichLog.min_width`.
+                If specified, `expand` and `shrink` will be ignored.
+            expand: Permit expanding of content to the width of the content region of the RichLog.
+                If `width` is specified, then `expand` will be ignored.
+            shrink: Permit shrinking of content to fit within the content region of the RichLog.
+                If `width` is specified, then `shrink` will be ignored.
             scroll_end: Enable automatic scroll to end, or `None` to use `self.auto_scroll`.
 
         Returns:
             The `RichLog` instance.
         """
+        if not self._size_known:
+            # We don't know the size yet, so we'll need to render this later.
+            # We defer ALL writes until the size is known, to ensure ordering is preserved.
+            if isinstance(content, Text):
+                content = content.copy()
+            self._deferred_renders.append(
+                DeferredRender(content, width, expand, shrink, scroll_end)
+            )
+            return self
 
+        renderable = self._make_renderable(content)
         auto_scroll = self.auto_scroll if scroll_end is None else scroll_end
 
         console = self.app.console
         render_options = console.options
 
-        renderable = self._make_renderable(content)
-
         if isinstance(renderable, Text) and not self.wrap:
             render_options = render_options.update(overflow="ignore", no_wrap=True)
 
-        render_width = measure_renderables(
-            console, render_options, [renderable]
-        ).maximum
+        if width is not None:
+            # Use the width specified by the caller.
+            # We ignore `expand` and `shrink` when a width is specified.
+            # This also overrides `min_width` set on the RichLog.
+            render_width = width
+        else:
+            # Compute the width based on available information.
+            renderable_width = measure_renderables(
+                console, render_options, [renderable]
+            ).maximum
 
-        container_width = (
-            self.scrollable_content_region.width if width is None else width
-        )
+            render_width = renderable_width
+            scrollable_content_width = self.scrollable_content_region.width
 
-        # Use the container_width if it's available, otherwise use the last available width.
-        container_width = (
-            container_width if container_width else self._last_container_width
-        )
+            if expand and renderable_width < scrollable_content_width:
+                # Expand the renderable to the width of the scrollable content region.
+                render_width = max(renderable_width, scrollable_content_width)
 
-        if expand and render_width < container_width:
-            render_width = container_width
-        if shrink and render_width > container_width:
-            render_width = container_width
+            if shrink and renderable_width > scrollable_content_width:
+                # Shrink the renderable down to fit within the scrollable content region.
+                render_width = min(renderable_width, scrollable_content_width)
 
-        render_width = max(render_width, self.min_width)
+            # The user has not supplied a width, so make sure min_width is respected.
+            render_width = max(render_width, self.min_width)
 
-        segments = self.app.console.render(
-            renderable, render_options.update_width(render_width)
-        )
+        render_options = render_options.update_width(render_width)
+
+        # Render into (possibly) wrapped lines.
+        segments = self.app.console.render(renderable, render_options)
         lines = list(Segment.split_lines(segments))
+
         if not lines:
+            self._widest_line_width = max(render_width, self._widest_line_width)
             self.lines.append(Strip.blank(render_width))
         else:
-            self.max_width = max(
-                self.max_width,
-                max(sum([segment.cell_length for segment in _line]) for _line in lines),
-            )
             strips = Strip.from_lines(lines)
             for strip in strips:
                 strip.adjust_cell_length(render_width)
@@ -197,7 +254,19 @@ def write(
                 self._start_line += len(self.lines) - self.max_lines
                 self.refresh()
                 self.lines = self.lines[-self.max_lines :]
-        self.virtual_size = Size(self.max_width, len(self.lines))
+
+            # Compute the width after wrapping and trimming
+            # TODO - this is wrong because if we trim a long line, the max width
+            #  could decrease, but we don't look at which lines were trimmed here.
+            self._widest_line_width = max(
+                self._widest_line_width,
+                max(sum([segment.cell_length for segment in _line]) for _line in lines),
+            )
+
+        # Update the virtual size - the width may have changed after adding
+        # the new line(s), and the height will definitely have changed.
+        self.virtual_size = Size(self._widest_line_width, len(self.lines))
+
         if auto_scroll:
             self.scroll_end(animate=False)
 
@@ -212,34 +281,25 @@ def clear(self) -> Self:
         self.lines.clear()
         self._line_cache.clear()
         self._start_line = 0
-        self.max_width = 0
-        self.virtual_size = Size(self.max_width, len(self.lines))
+        self._widest_line_width = 0
+        self._deferred_renders.clear()
+        self.virtual_size = Size(0, len(self.lines))
         self.refresh()
         return self
 
     def render_line(self, y: int) -> Strip:
         scroll_x, scroll_y = self.scroll_offset
-        line = self._render_line(scroll_y + y, scroll_x, self.size.width)
+        line = self._render_line(
+            scroll_y + y, scroll_x, self.scrollable_content_region.width
+        )
         strip = line.apply_style(self.rich_style)
         return strip
 
-    def render_lines(self, crop: Region) -> list[Strip]:
-        """Render the widget in to lines.
-
-        Args:
-            crop: Region within visible area to.
-
-        Returns:
-            A list of list of segments.
-        """
-        lines = self._styles_cache.render_widget(self, crop)
-        return lines
-
     def _render_line(self, y: int, scroll_x: int, width: int) -> Strip:
         if y >= len(self.lines):
             return Strip.blank(width, self.rich_style)
 
-        key = (y + self._start_line, scroll_x, width, self.max_width)
+        key = (y + self._start_line, scroll_x, width, self._widest_line_width)
         if key in self._line_cache:
             return self._line_cache[key]