diff --git a/CHANGELOG.md b/CHANGELOG.md index ffaf36b14..6888cd36b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix crashes that can happen with `inspect` when docstrings contain some special control codes https://github.com/Textualize/rich/pull/2294 - Fix edges used in first row of tables when `show_header=False` https://github.com/Textualize/rich/pull/2330 - Fix interaction between `Capture` contexts and `Console(record=True)` https://github.com/Textualize/rich/pull/2343 +- Fixed hash issue in Styles class https://github.com/Textualize/rich/pull/2346 ## [12.4.4] - 2022-05-24 diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 5562f14ef..79c03094b 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -12,6 +12,7 @@ The following people have contributed to the development of Rich: - [Pete Davison](https://github.com/pd93) - [James Estevez](https://github.com/jstvz) - [Oleksis Fraga](https://github.com/oleksis) +- [Andy Gimblett](https://github.com/gimbo) - [Michał Górny](https://github.com/mgorny) - [Leron Gray](https://github.com/daddycocoaman) - [Kenneth Hoste](https://github.com/boegel) diff --git a/rich/_lru_cache.py b/rich/_lru_cache.py index a22789435..46e775f09 100644 --- a/rich/_lru_cache.py +++ b/rich/_lru_cache.py @@ -1,38 +1,116 @@ -from typing import Dict, Generic, TypeVar, TYPE_CHECKING -import sys +from threading import Lock +from typing import Dict, Generic, List, Optional, TypeVar, Union, overload CacheKey = TypeVar("CacheKey") CacheValue = TypeVar("CacheValue") +DefaultValue = TypeVar("DefaultValue") -if sys.version_info < (3, 9): - from typing_extensions import OrderedDict -else: - from collections import OrderedDict - -class LRUCache(OrderedDict[CacheKey, CacheValue]): +class LRUCache(Generic[CacheKey, CacheValue]): """ A dictionary-like container that stores a given maximum items. If an additional item is added when the LRUCache is full, the least recently used key is discarded to make room for the new item. + The implementation is similar to functools.lru_cache, which uses a linked + list to keep track of the most recently used items. + + Each entry is stored as [PREV, NEXT, KEY, VALUE] where PREV is a reference + to the previous entry, and NEXT is a reference to the next value. + """ - def __init__(self, cache_size: int) -> None: - self.cache_size = cache_size + def __init__(self, maxsize: int) -> None: + self.maxsize = maxsize + self.cache: Dict[CacheKey, List[object]] = {} + self.full = False + self.root: List[object] = [] + self._lock = Lock() super().__init__() - def __setitem__(self, key: CacheKey, value: CacheValue) -> None: - """Store a new views, potentially discarding an old value.""" - if key not in self: - if len(self) >= self.cache_size: - self.popitem(last=False) - super().__setitem__(key, value) + def __len__(self) -> int: + return len(self.cache) + + def set(self, key: CacheKey, value: CacheValue) -> None: + """Set a value. + + Args: + key (CacheKey): Key. + value (CacheValue): Value. + """ + with self._lock: + link = self.cache.get(key) + if link is None: + root = self.root + if not root: + self.root[:] = [self.root, self.root, key, value] + else: + self.root = [root[0], root, key, value] + root[0][1] = self.root # type: ignore[index] + root[0] = self.root + self.cache[key] = self.root + + if self.full or len(self.cache) > self.maxsize: + self.full = True + root = self.root + last = root[0] + last[0][1] = root # type: ignore[index] + root[0] = last[0] # type: ignore[index] + del self.cache[last[2]] # type: ignore[index] + + __setitem__ = set + + @overload + def get(self, key: CacheKey) -> Optional[CacheValue]: + ... + + @overload + def get( + self, key: CacheKey, default: DefaultValue + ) -> Union[CacheValue, DefaultValue]: + ... + + def get( + self, key: CacheKey, default: Optional[DefaultValue] = None + ) -> Union[CacheValue, Optional[DefaultValue]]: + """Get a value from the cache, or return a default if the key is not present. + + Args: + key (CacheKey): Key + default (Optional[DefaultValue], optional): Default to return if key is not present. Defaults to None. + + Returns: + Union[CacheValue, Optional[DefaultValue]]: Either the value or a default. + """ + link = self.cache.get(key) + if link is None: + return default + if link is not self.root: + with self._lock: + link[0][1] = link[1] # type: ignore[index] + link[1][0] = link[0] # type: ignore[index] + root = self.root + link[0] = root[0] + link[1] = root + root[0][1] = link # type: ignore[index] + root[0] = link + self.root = link + return link[3] # type: ignore[return-value] def __getitem__(self, key: CacheKey) -> CacheValue: - """Gets the item, but also makes it most recent.""" - value: CacheValue = super().__getitem__(key) - super().__delitem__(key) - super().__setitem__(key, value) - return value + link = self.cache[key] + if link is not self.root: + with self._lock: + link[0][1] = link[1] # type: ignore[index] + link[1][0] = link[0] # type: ignore[index] + root = self.root + link[0] = root[0] + link[1] = root + root[0][1] = link # type: ignore[index] + root[0] = link + self.root = link + return link[3] # type: ignore[return-value] + + def __contains__(self, key: CacheKey) -> bool: + return key in self.cache diff --git a/rich/cells.py b/rich/cells.py index 834c37103..3278b6fa1 100644 --- a/rich/cells.py +++ b/rich/cells.py @@ -1,6 +1,6 @@ import re from functools import lru_cache -from typing import Dict, List +from typing import List from ._cell_widths import CELL_WIDTHS from ._lru_cache import LRUCache @@ -9,7 +9,7 @@ _is_single_cell_widths = re.compile("^[\u0020-\u006f\u00a0\u02ff\u0370-\u0482]*$").match -def cell_len(text: str, _cache: Dict[str, int] = LRUCache(1024 * 4)) -> int: +def cell_len(text: str, _cache: LRUCache[str, int] = LRUCache(1024 * 4)) -> int: """Get the number of cells required to display text. Args: @@ -80,7 +80,7 @@ def set_cell_size(text: str, total: int) -> str: return text + " " * (total - size) return text[:total] - if not total: + if total <= 0: return "" cell_size = cell_len(text) if cell_size == total: diff --git a/rich/rule.py b/rich/rule.py index d14394f25..f1159524b 100644 --- a/rich/rule.py +++ b/rich/rule.py @@ -4,6 +4,7 @@ from .cells import cell_len, set_cell_size from .console import Console, ConsoleOptions, RenderResult from .jupyter import JupyterMixin +from .measure import Measurement from .style import Style from .text import Text @@ -62,10 +63,7 @@ def __rich_console__( chars_len = cell_len(characters) if not self.title: - rule_text = Text(characters * ((width // chars_len) + 1), self.style) - rule_text.truncate(width) - rule_text.plain = set_cell_size(rule_text.plain, width) - yield rule_text + yield self._rule_line(chars_len, width) return if isinstance(self.title, Text): @@ -75,10 +73,16 @@ def __rich_console__( title_text.plain = title_text.plain.replace("\n", " ") title_text.expand_tabs() - rule_text = Text(end=self.end) + required_space = 4 if self.align == "center" else 2 + truncate_width = max(0, width - required_space) + if not truncate_width: + yield self._rule_line(chars_len, width) + return + + rule_text = Text(end=self.end) if self.align == "center": - title_text.truncate(width - 4, overflow="ellipsis") + title_text.truncate(truncate_width, overflow="ellipsis") side_width = (width - cell_len(title_text.plain)) // 2 left = Text(characters * (side_width // chars_len + 1)) left.truncate(side_width - 1) @@ -89,12 +93,12 @@ def __rich_console__( rule_text.append(title_text) rule_text.append(" " + right.plain, self.style) elif self.align == "left": - title_text.truncate(width - 2, overflow="ellipsis") + title_text.truncate(truncate_width, overflow="ellipsis") rule_text.append(title_text) rule_text.append(" ") rule_text.append(characters * (width - rule_text.cell_len), self.style) elif self.align == "right": - title_text.truncate(width - 2, overflow="ellipsis") + title_text.truncate(truncate_width, overflow="ellipsis") rule_text.append(characters * (width - title_text.cell_len - 1), self.style) rule_text.append(" ") rule_text.append(title_text) @@ -102,14 +106,29 @@ def __rich_console__( rule_text.plain = set_cell_size(rule_text.plain, width) yield rule_text + def _rule_line(self, chars_len: int, width: int) -> Text: + rule_text = Text(self.characters * ((width // chars_len) + 1), self.style) + rule_text.truncate(width) + rule_text.plain = set_cell_size(rule_text.plain, width) + return rule_text + + def __rich_measure__( + self, console: Console, options: ConsoleOptions + ) -> Measurement: + return Measurement(1, 1) + if __name__ == "__main__": # pragma: no cover - from rich.console import Console import sys + from rich.console import Console + try: text = sys.argv[1] except IndexError: text = "Hello, World" console = Console() console.print(Rule(title=text)) + + console = Console() + console.print(Rule("foo"), width=4) diff --git a/rich/style.py b/rich/style.py index 17b1ace89..c4b432829 100644 --- a/rich/style.py +++ b/rich/style.py @@ -2,9 +2,10 @@ from functools import lru_cache from marshal import dumps, loads from random import randint -from typing import Any, Dict, Iterable, List, Optional, Type, Union, cast +from typing import Any, Dict, Iterable, List, Optional, Tuple, Type, Union, cast from . import errors +from ._lru_cache import LRUCache from .color import Color, ColorParseError, ColorSystem, blend_rgb from .repr import Result, rich_repr from .terminal_theme import DEFAULT_TERMINAL_THEME, TerminalTheme @@ -59,7 +60,7 @@ class Style: _bgcolor: Optional[Color] _attributes: int _set_attributes: int - _hash: int + _hash: Optional[int] _null: bool _meta: Optional[bytes] @@ -119,6 +120,9 @@ class Style: "o": "overline", } + # Caches results of Style.__add__ + _add_cache: LRUCache[Tuple["Style", Optional["Style"]], "Style"] = LRUCache(1024) + def __init__( self, *, @@ -190,16 +194,7 @@ def _make_color(color: Union[Color, str]) -> Color: self._link = link self._link_id = f"{randint(0, 999999)}" if link else "" self._meta = None if meta is None else dumps(meta) - self._hash = hash( - ( - self._color, - self._bgcolor, - self._attributes, - self._set_attributes, - link, - self._meta, - ) - ) + self._hash: Optional[int] = None self._null = not (self._set_attributes or color or bgcolor or link or meta) @classmethod @@ -227,17 +222,8 @@ def from_color( style._link = None style._link_id = "" style._meta = None - style._hash = hash( - ( - color, - bgcolor, - None, - None, - None, - None, - ) - ) style._null = not (color or bgcolor) + style._hash = None return style @classmethod @@ -257,16 +243,7 @@ def from_meta(cls, meta: Optional[Dict[str, Any]]) -> "Style": style._link = None style._link_id = "" style._meta = dumps(meta) - style._hash = hash( - ( - None, - None, - None, - None, - None, - style._meta, - ) - ) + style._hash = None style._null = not (meta) return style @@ -366,6 +343,7 @@ def _make_ansi_codes(self, color_system: ColorSystem) -> str: Returns: str: String containing codes. """ + if self._ansi is None: sgr: List[str] = [] append = sgr.append @@ -446,16 +424,26 @@ def __rich_repr__(self) -> Result: def __eq__(self, other: Any) -> bool: if not isinstance(other, Style): return NotImplemented - return ( - self._color == other._color - and self._bgcolor == other._bgcolor - and self._set_attributes == other._set_attributes - and self._attributes == other._attributes - and self._link == other._link - and self._meta == other._meta - ) + return self.__hash__() == other.__hash__() + + def __ne__(self, other: Any) -> bool: + if not isinstance(other, Style): + return NotImplemented + return self.__hash__() != other.__hash__() def __hash__(self) -> int: + if self._hash is not None: + return self._hash + self._hash = hash( + ( + self._color, + self._bgcolor, + self._attributes, + self._set_attributes, + self._link, + self._meta, + ) + ) return self._hash @property @@ -502,9 +490,9 @@ def without_color(self) -> "Style": style._set_attributes = self._set_attributes style._link = self._link style._link_id = f"{randint(0, 999999)}" if self._link else "" - style._hash = self._hash style._null = False style._meta = None + style._hash = None return style @classmethod @@ -677,7 +665,7 @@ def update_link(self, link: Optional[str] = None) -> "Style": style._set_attributes = self._set_attributes style._link = link style._link_id = f"{randint(0, 999999)}" if link else "" - style._hash = self._hash + style._hash = None style._null = False style._meta = self._meta return style @@ -700,7 +688,7 @@ def render( """ if not text or color_system is None: return text - attrs = self._make_ansi_codes(color_system) + attrs = self._ansi or self._make_ansi_codes(color_system) rendered = f"\x1b[{attrs}m{text}\x1b[0m" if attrs else text if self._link and not legacy_windows: rendered = ( @@ -721,6 +709,10 @@ def test(self, text: Optional[str] = None) -> None: sys.stdout.write(f"{self.render(text)}\n") def __add__(self, style: Optional["Style"]) -> "Style": + cache_key = (self, style) + cached_style = self._add_cache.get(cache_key) + if cached_style is not None: + return cached_style.copy() if cached_style.link else cached_style if not (isinstance(style, Style) or style is None): return NotImplemented if style is None or style._null: @@ -738,12 +730,13 @@ def __add__(self, style: Optional["Style"]) -> "Style": new_style._set_attributes = self._set_attributes | style._set_attributes new_style._link = style._link or self._link new_style._link_id = style._link_id or self._link_id - new_style._hash = style._hash new_style._null = style._null if self._meta and style._meta: new_style._meta = dumps({**self.meta, **style.meta}) else: new_style._meta = self._meta or style._meta + new_style._hash = None + self._add_cache[cache_key] = new_style return new_style diff --git a/tests/test_lrucache.py b/tests/test_lrucache.py index 3cb1e2981..76ba5a670 100644 --- a/tests/test_lrucache.py +++ b/tests/test_lrucache.py @@ -1,6 +1,5 @@ from __future__ import unicode_literals - from rich._lru_cache import LRUCache @@ -12,18 +11,49 @@ def test_lru_cache(): cache["bar"] = 2 cache["baz"] = 3 assert "foo" in cache + assert "bar" in cache + assert "baz" in cache # Cache size is 3, so the following should kick oldest one out cache["egg"] = 4 assert "foo" not in cache - assert "egg" in "egg" in cache + assert "egg" in cache # cache is now full # look up two keys cache["bar"] cache["baz"] + # Insert a new value + cache["eggegg"] = 5 + assert len(cache) == 3 + # Check it kicked out the 'oldest' key + assert "egg" not in cache + assert "eggegg" in cache + + +def test_lru_cache_get(): + cache = LRUCache(3) + + # insert some values + cache["foo"] = 1 + cache["bar"] = 2 + cache["baz"] = 3 + assert "foo" in cache + + # Cache size is 3, so the following should kick oldest one out + cache["egg"] = 4 + # assert len(cache) == 3 + assert cache.get("foo") is None + assert "egg" in cache + + # cache is now full + # look up two keys + cache.get("bar") + cache.get("baz") + # Insert a new value cache["eggegg"] = 5 # Check it kicked out the 'oldest' key assert "egg" not in cache + assert "eggegg" in cache diff --git a/tests/test_rule.py b/tests/test_rule.py index 571734e3e..398c75585 100644 --- a/tests/test_rule.py +++ b/tests/test_rule.py @@ -61,6 +61,47 @@ def test_rule_cjk(): assert console.file.getvalue() == expected +@pytest.mark.parametrize( + "align,outcome", + [ + ("center", "───\n"), + ("left", "… ─\n"), + ("right", "─ …\n"), + ], +) +def test_rule_not_enough_space_for_title_text(align, outcome): + console = Console(width=3, file=io.StringIO(), record=True) + console.rule("Hello!", align=align) + assert console.file.getvalue() == outcome + + +def test_rule_center_aligned_title_not_enough_space_for_rule(): + console = Console(width=4, file=io.StringIO(), record=True) + console.rule("ABCD") + assert console.file.getvalue() == "────\n" + + +@pytest.mark.parametrize("align", ["left", "right"]) +def test_rule_side_aligned_not_enough_space_for_rule(align): + console = Console(width=2, file=io.StringIO(), record=True) + console.rule("ABCD", align=align) + assert console.file.getvalue() == "──\n" + + +@pytest.mark.parametrize( + "align,outcome", + [ + ("center", "─ … ─\n"), + ("left", "AB… ─\n"), + ("right", "─ AB…\n"), + ], +) +def test_rule_just_enough_width_available_for_title(align, outcome): + console = Console(width=5, file=io.StringIO(), record=True) + console.rule("ABCD", align=align) + assert console.file.getvalue() == outcome + + def test_characters(): console = Console( width=16, diff --git a/tests/test_rule_in_table.py b/tests/test_rule_in_table.py new file mode 100644 index 000000000..bc911391a --- /dev/null +++ b/tests/test_rule_in_table.py @@ -0,0 +1,76 @@ +import io +from textwrap import dedent + +import pytest + +from rich import box +from rich.console import Console +from rich.rule import Rule +from rich.table import Table + + +@pytest.mark.parametrize("expand_kwarg", ({}, {"expand": False})) +def test_rule_in_unexpanded_table(expand_kwarg): + console = Console(width=32, file=io.StringIO(), legacy_windows=False, _environ={}) + table = Table(box=box.ASCII, show_header=False, **expand_kwarg) + table.add_column() + table.add_column() + table.add_row("COL1", "COL2") + table.add_row("COL1", Rule()) + table.add_row("COL1", "COL2") + console.print(table) + expected = dedent( + """\ + +-------------+ + | COL1 | COL2 | + | COL1 | ──── | + | COL1 | COL2 | + +-------------+ + """ + ) + result = console.file.getvalue() + assert result == expected + + +def test_rule_in_expanded_table(): + console = Console(width=32, file=io.StringIO(), legacy_windows=False, _environ={}) + table = Table(box=box.ASCII, expand=True, show_header=False) + table.add_column() + table.add_column() + table.add_row("COL1", "COL2") + table.add_row("COL1", Rule(style=None)) + table.add_row("COL1", "COL2") + console.print(table) + expected = dedent( + """\ + +------------------------------+ + | COL1 | COL2 | + | COL1 | ──────────── | + | COL1 | COL2 | + +------------------------------+ + """ + ) + result = console.file.getvalue() + assert result == expected + + +def test_rule_in_ratio_table(): + console = Console(width=32, file=io.StringIO(), legacy_windows=False, _environ={}) + table = Table(box=box.ASCII, expand=True, show_header=False) + table.add_column(ratio=1) + table.add_column() + table.add_row("COL1", "COL2") + table.add_row("COL1", Rule(style=None)) + table.add_row("COL1", "COL2") + console.print(table) + expected = dedent( + """\ + +------------------------------+ + | COL1 | COL2 | + | COL1 | ──── | + | COL1 | COL2 | + +------------------------------+ + """ + ) + result = console.file.getvalue() + assert result == expected diff --git a/tests/test_spinner.py b/tests/test_spinner.py index 98e3d9d66..efeeb7173 100644 --- a/tests/test_spinner.py +++ b/tests/test_spinner.py @@ -46,17 +46,15 @@ def get_time(): spinner = Spinner("dots") console.print(spinner) - spinner.update(text="Bar", style="green", speed=2) - time += 80 / 1000 - console.print(spinner) + rule = Rule("Bar") - spinner.update(text=Rule("Bar")) + spinner.update(text=rule) time += 80 / 1000 console.print(spinner) result = console.end_capture() print(repr(result)) - expected = f"⠋\n\x1b[32m⠙\x1b[0m Bar\n\x1b[32m⠸\x1b[0m \x1b[92m────── \x1b[0mBar\x1b[92m ───────\x1b[0m\n" + expected = "⠋\n⠙ \x1b[92m─\x1b[0m\n" assert result == expected