diff --git a/sandbox/will/spacing.py b/sandbox/will/spacing.py index 184a53526..92e195eb1 100644 --- a/sandbox/will/spacing.py +++ b/sandbox/will/spacing.py @@ -9,7 +9,7 @@ class Clickable(Static): class SpacingApp(App): def compose(self): - yield Clickable() + yield Static(id="2332") app = SpacingApp(css_path="spacing.css") diff --git a/src/textual/css/tokenize.py b/src/textual/css/tokenize.py index 7cee24ce2..dbec369df 100644 --- a/src/textual/css/tokenize.py +++ b/src/textual/css/tokenize.py @@ -26,6 +26,8 @@ TOKEN = "[a-zA-Z][a-zA-Z0-9_-]*" STRING = r"\".*?\"" VARIABLE_REF = r"\$[a-zA-Z0-9_\-]+" +IDENTIFIER = r"[a-zA-Z_\-][a-zA-Z0-9_\-]*" + # Values permitted in variable and rule declarations. DECLARATION_VALUES = { "scalar": SCALAR, @@ -44,8 +46,8 @@ DECLARATION_VALUES = { expect_root_scope = Expect( whitespace=r"\s+", comment_start=COMMENT_START, - selector_start_id=r"\#[a-zA-Z_\-][a-zA-Z0-9_\-]*", - selector_start_class=r"\.[a-zA-Z_\-][a-zA-Z0-9_\-]*", + selector_start_id=r"\#" + IDENTIFIER, + selector_start_class=r"\." + IDENTIFIER, selector_start_universal=r"\*", selector_start=r"[a-zA-Z_\-]+", variable_name=rf"{VARIABLE_REF}:", diff --git a/src/textual/dom.py b/src/textual/dom.py index ba53f0775..9b828c066 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -1,6 +1,7 @@ from __future__ import annotations from inspect import getfile +import re from typing import ( cast, ClassVar, @@ -27,6 +28,7 @@ from .css.constants import VALID_DISPLAY, VALID_VISIBILITY from .css.errors import StyleValueError, DeclarationError from .css.parse import parse_declarations from .css.styles import Styles, RenderStyles +from .css.tokenize import IDENTIFIER from .css.query import NoMatchingNodesError from .message_pump import MessagePump from .timer import Timer @@ -38,6 +40,32 @@ if TYPE_CHECKING: from .widget import Widget +_re_identifier = re.compile(IDENTIFIER) + + +class BadIdentifier(Exception): + """raised by check_identifiers.""" + + +def check_identifiers(description: str, *names: str) -> None: + """Validate identifier and raise an error if it fails. + + Args: + description (str): Description of where identifier is used for error message. + names (list[str]): Identifiers to check. + + Returns: + bool: True if the name is valid. + """ + match = _re_identifier.match + for name in names: + if match(name) is None: + raise BadIdentifier( + f"{name!r} is an invalid {description}; " + "identifiers must contain only letters, numbers, underscores, or hyphens, and must not begin with a number." + ) + + class DOMError(Exception): pass @@ -75,9 +103,16 @@ class DOMNode(MessagePump): id: str | None = None, classes: str | None = None, ) -> None: + self._classes = set() self._name = name - self._id = id - self._classes: set[str] = set() if classes is None else set(classes.split()) + self._id = None + if id is not None: + self.id = id + + _classes = classes.split() if classes else [] + check_identifiers("class name", *_classes) + self._classes.update(_classes) + self.children = NodeList() self._css_styles: Styles = Styles(self) self._inline_styles: Styles = Styles(self) @@ -248,6 +283,8 @@ class DOMNode(MessagePump): ValueError: If the ID has already been set. """ + check_identifiers("id", new_id) + if self._id is not None: raise ValueError( f"Node 'id' attribute may not be changed once set (current id={self._id!r})" @@ -723,6 +760,7 @@ class DOMNode(MessagePump): *class_names (str): CSS class names to add. """ + check_identifiers("class name", *class_names) old_classes = self._classes.copy() self._classes.update(class_names) if old_classes == self._classes: @@ -739,6 +777,7 @@ class DOMNode(MessagePump): *class_names (str): CSS class names to remove. """ + check_identifiers("class name", *class_names) old_classes = self._classes.copy() self._classes.difference_update(class_names) if old_classes == self._classes: @@ -755,6 +794,7 @@ class DOMNode(MessagePump): *class_names (str): CSS class names to toggle. """ + check_identifiers("class name", *class_names) old_classes = self._classes.copy() self._classes.symmetric_difference_update(class_names) if old_classes == self._classes: diff --git a/tests/test_dom.py b/tests/test_dom.py index 1a597a383..a7ec46482 100644 --- a/tests/test_dom.py +++ b/tests/test_dom.py @@ -2,7 +2,7 @@ import pytest from textual.css.errors import StyleValueError from textual.css.query import NoMatchingNodesError -from textual.dom import DOMNode +from textual.dom import DOMNode, BadIdentifier def test_display_default(): @@ -55,3 +55,23 @@ def test_get_child_no_matching_child(parent): def test_get_child_only_immediate_descendents(parent): with pytest.raises(NoMatchingNodesError): parent.get_child(id="grandchild1") + + +def test_validate(): + with pytest.raises(BadIdentifier): + DOMNode(id="23") + with pytest.raises(BadIdentifier): + DOMNode(id=".3") + with pytest.raises(BadIdentifier): + DOMNode(classes="+2323") + with pytest.raises(BadIdentifier): + DOMNode(classes="foo 22") + + node = DOMNode() + node.add_class("foo") + with pytest.raises(BadIdentifier): + node.add_class("1") + with pytest.raises(BadIdentifier): + node.remove_class("1") + with pytest.raises(BadIdentifier): + node.toggle_class("1")