From caa43f09e96cfcace441b263058d4590ae03c075 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 10 Jul 2025 11:59:54 +0100 Subject: [PATCH 1/6] Optimize query_one --- src/textual/css/parse.py | 23 ++++++++++++++++++++++- src/textual/dom.py | 28 ++++++++++++++++++++++++---- src/textual/walk.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 5 deletions(-) diff --git a/src/textual/css/parse.py b/src/textual/css/parse.py index 61b759253b..edf783a097 100644 --- a/src/textual/css/parse.py +++ b/src/textual/css/parse.py @@ -1,6 +1,7 @@ from __future__ import annotations import dataclasses +import re from functools import lru_cache from typing import Iterable, Iterator, NoReturn @@ -16,7 +17,13 @@ SelectorType, ) from textual.css.styles import Styles -from textual.css.tokenize import Token, tokenize, tokenize_declarations, tokenize_values +from textual.css.tokenize import ( + IDENTIFIER, + Token, + tokenize, + tokenize_declarations, + tokenize_values, +) from textual.css.tokenizer import ReferencedBy, UnexpectedEnd from textual.css.types import CSSLocation, Specificity3 from textual.suggestions import get_suggestion @@ -33,6 +40,20 @@ "nested": (SelectorType.NESTED, (0, 0, 0)), } +RE_IDENTIFIER = re.compile(r"\#" + IDENTIFIER) + + +def is_id_selector(selector: str) -> bool: + """Is the selector an ID selector, i.e. "#foo"? + + Args: + selector: A CSS selector. + + Returns: + `True` if the selector is a simple ID selector, otherwise `False`. + """ + return RE_IDENTIFIER.fullmatch(selector) is not None + def _add_specificity( specificity1: Specificity3, specificity2: Specificity3 diff --git a/src/textual/dom.py b/src/textual/dom.py index 6be6c331ca..4d32357371 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -40,7 +40,7 @@ from textual.css.constants import VALID_DISPLAY, VALID_VISIBILITY from textual.css.errors import DeclarationError, StyleValueError from textual.css.match import match -from textual.css.parse import parse_declarations, parse_selectors +from textual.css.parse import is_id_selector, parse_declarations, parse_selectors from textual.css.query import InvalidQueryFormat, NoMatches, TooManyMatches, WrongType from textual.css.styles import RenderStyles, Styles from textual.css.tokenize import IDENTIFIER @@ -49,7 +49,7 @@ from textual.reactive import Reactive, ReactiveError, _Mutated, _watch from textual.style import Style as VisualStyle from textual.timer import Timer -from textual.walk import walk_breadth_first, walk_depth_first +from textual.walk import walk_breadth_first, walk_breadth_search_id, walk_depth_first from textual.worker_manager import WorkerManager if TYPE_CHECKING: @@ -1466,6 +1466,26 @@ def query_one( else: query_selector = selector.__name__ + if is_id_selector(query_selector): + cache_key = (base_node._nodes._updates, query_selector, expect_type) + cached_result = base_node._query_one_cache.get(cache_key) + if cached_result is not None: + return cached_result + if ( + node := walk_breadth_search_id( + base_node, query_selector[1:], with_root=False + ) + ) is not None: + if expect_type is not None and not isinstance(node, expect_type): + raise WrongType( + f"Node matching {query_selector!r} is the wrong type; expected type {expect_type.__name__!r}, found {node}" + ) + base_node._query_one_cache[cache_key] = node + return node + raise NoMatches( + f"No nodes match {query_selector!r} on {base_node!r} {list(base_node._nodes)}" + ) + try: selector_set = parse_selectors(query_selector) except TokenError: @@ -1492,7 +1512,7 @@ def query_one( base_node._query_one_cache[cache_key] = node return node - raise NoMatches(f"No nodes match {selector!r} on {base_node!r}") + raise NoMatches(f"No nodes match {query_selector!r} on {base_node!r}") if TYPE_CHECKING: @@ -1572,7 +1592,7 @@ def query_exactly_one( base_node._query_one_cache[cache_key] = node return node - raise NoMatches(f"No nodes match {selector!r} on {base_node!r}") + raise NoMatches(f"No nodes match {query_selector!r} on {base_node!r}") if TYPE_CHECKING: diff --git a/src/textual/walk.py b/src/textual/walk.py index dcda856e49..9869d775b7 100644 --- a/src/textual/walk.py +++ b/src/textual/walk.py @@ -137,3 +137,33 @@ def walk_breadth_first( if isinstance(node, check_type): yield node extend(node._nodes) + + +def walk_breadth_search_id( + root: DOMNode, node_id: str, *, with_root: bool = True +) -> DOMNode | None: + """Special case to walk breadth first searching for a node with a given id. + + This is more efficient that [walk_breadth_first][textual.walk.walk_breadth_first] for this special case, as it can use an index. + + Args: + root: The root node (starting point). + node_id: Node id to search for. + with_root: Consider the root node? If the root has the node id, then return it. + + Returns: + A DOMNode if a node was found, otherwise `None`. + """ + + if with_root and root.id == node_id: + return root + + queue: deque[DOMNode] = deque() + queue.append(root) + + while queue: + node = queue.popleft() + if (found_node := node._nodes._get_by_id(node_id)) is not None: + return found_node + queue.extend(node._nodes) + return None From 56ef7a0af07ff7d11a1d83fe2266bf64f28a9677 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 10 Jul 2025 12:03:42 +0100 Subject: [PATCH 2/6] small cache --- src/textual/css/parse.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/textual/css/parse.py b/src/textual/css/parse.py index edf783a097..7479d8461f 100644 --- a/src/textual/css/parse.py +++ b/src/textual/css/parse.py @@ -43,6 +43,7 @@ RE_IDENTIFIER = re.compile(r"\#" + IDENTIFIER) +@lru_cache(maxsize=128) def is_id_selector(selector: str) -> bool: """Is the selector an ID selector, i.e. "#foo"? From 20881edf79d009ef1db968e601db9a33457b10f9 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 10 Jul 2025 12:12:55 +0100 Subject: [PATCH 3/6] test --- src/textual/css/parse.py | 6 +++--- tests/css/test_parse.py | 24 +++++++++++++++++++++++- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/textual/css/parse.py b/src/textual/css/parse.py index 7479d8461f..6eac29bc36 100644 --- a/src/textual/css/parse.py +++ b/src/textual/css/parse.py @@ -40,12 +40,12 @@ "nested": (SelectorType.NESTED, (0, 0, 0)), } -RE_IDENTIFIER = re.compile(r"\#" + IDENTIFIER) +RE_ID_SELECTOR = re.compile(r"\#" + IDENTIFIER) @lru_cache(maxsize=128) def is_id_selector(selector: str) -> bool: - """Is the selector an ID selector, i.e. "#foo"? + """Is the selector an single ID selector, i.e. "#foo"? Args: selector: A CSS selector. @@ -53,7 +53,7 @@ def is_id_selector(selector: str) -> bool: Returns: `True` if the selector is a simple ID selector, otherwise `False`. """ - return RE_IDENTIFIER.fullmatch(selector) is not None + return RE_ID_SELECTOR.fullmatch(selector) is not None def _add_specificity( diff --git a/tests/css/test_parse.py b/tests/css/test_parse.py index febe4f06bb..a2151eee3b 100644 --- a/tests/css/test_parse.py +++ b/tests/css/test_parse.py @@ -4,7 +4,7 @@ from textual.color import Color from textual.css.errors import UnresolvedVariableError -from textual.css.parse import substitute_references +from textual.css.parse import is_id_selector, substitute_references from textual.css.scalar import Scalar, Unit from textual.css.stylesheet import Stylesheet, StylesheetParseError from textual.css.tokenize import tokenize @@ -1272,3 +1272,25 @@ def test_parse_bad_pseudo_selector_with_suggestion(): stylesheet.parse() assert error.value.start == (2, 7) + + +@pytest.mark.parametrize( + "selector, expected", + [ + # True cases + ("#foo", True), + ("#bar", True), + ("#f", True), + # False cases + ("#", False), + ("Foo", False), + (".foo", False), + ("#5foo", False), + ("#foo .bar", False), + ("#foo>.bar", False), + ("#foo.bar", False), + ], +) +def test_is_id_selector(selector: str, expected: bool): + """Test is_id_selector is working as expected""" + assert is_id_selector(selector) is expected From bcd1c0908e4d038c845e240a393e4654035ce836 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 10 Jul 2025 12:18:27 +0100 Subject: [PATCH 4/6] remove debug --- src/textual/dom.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/textual/dom.py b/src/textual/dom.py index 4d32357371..f7f854efde 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -1482,9 +1482,7 @@ def query_one( ) base_node._query_one_cache[cache_key] = node return node - raise NoMatches( - f"No nodes match {query_selector!r} on {base_node!r} {list(base_node._nodes)}" - ) + raise NoMatches(f"No nodes match {query_selector!r} on {base_node!r}") try: selector_set = parse_selectors(query_selector) From db979f5b64beee0ef26d21217e933b1b1d72ec27 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 10 Jul 2025 12:20:38 +0100 Subject: [PATCH 5/6] tests --- src/textual/css/parse.py | 2 +- tests/css/test_parse.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/textual/css/parse.py b/src/textual/css/parse.py index 6eac29bc36..422900ca1e 100644 --- a/src/textual/css/parse.py +++ b/src/textual/css/parse.py @@ -45,7 +45,7 @@ @lru_cache(maxsize=128) def is_id_selector(selector: str) -> bool: - """Is the selector an single ID selector, i.e. "#foo"? + """Is the selector a single ID selector, i.e. "#foo"? Args: selector: A CSS selector. diff --git a/tests/css/test_parse.py b/tests/css/test_parse.py index a2151eee3b..bb18c44d5a 100644 --- a/tests/css/test_parse.py +++ b/tests/css/test_parse.py @@ -1289,8 +1289,10 @@ def test_parse_bad_pseudo_selector_with_suggestion(): ("#foo .bar", False), ("#foo>.bar", False), ("#foo.bar", False), + (".foo #foo", False), + ("#foo #bar", False), ], ) -def test_is_id_selector(selector: str, expected: bool): - """Test is_id_selector is working as expected""" +def test_is_id_selector(selector: str, expected: bool) -> None: + """Test is_id_selector is working as expected.""" assert is_id_selector(selector) is expected From 72099601fd1905a6e1677625671213288fa121ca Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 10 Jul 2025 15:03:21 +0100 Subject: [PATCH 6/6] tweaks --- src/textual/css/parse.py | 2 +- src/textual/walk.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/textual/css/parse.py b/src/textual/css/parse.py index 422900ca1e..e030cc4aca 100644 --- a/src/textual/css/parse.py +++ b/src/textual/css/parse.py @@ -40,7 +40,7 @@ "nested": (SelectorType.NESTED, (0, 0, 0)), } -RE_ID_SELECTOR = re.compile(r"\#" + IDENTIFIER) +RE_ID_SELECTOR = re.compile("#" + IDENTIFIER) @lru_cache(maxsize=128) diff --git a/src/textual/walk.py b/src/textual/walk.py index 9869d775b7..43b8d4b6e4 100644 --- a/src/textual/walk.py +++ b/src/textual/walk.py @@ -144,7 +144,7 @@ def walk_breadth_search_id( ) -> DOMNode | None: """Special case to walk breadth first searching for a node with a given id. - This is more efficient that [walk_breadth_first][textual.walk.walk_breadth_first] for this special case, as it can use an index. + This is more efficient than [walk_breadth_first][textual.walk.walk_breadth_first] for this special case, as it can use an index. Args: root: The root node (starting point).