Repository: Textualize/textual
Pull request: Set classes
Adds the ability to change / clear classes.

- Adds a setter for classes, which may be intuitive to some.
- Also adds explicit `set_classes`

Fixes https://github.com/Textualize/textual/issues/1081

Problem statement:
Add a method of either setting classes to a specific collection, or clearing all classes
I've run into one situation where I wanted to be able to clear all of the classes on a `Widget` and set them to a very specific selection -- no toggle or the like. Perhaps it's worth considering adding a mechanism for doing this? Either "set to this very specific collection", or "clear all classes".

Hints from issue discussion:
By way of comparison, jQuery's `removeClass` function has (among other signatures) a zero-argument signature that will remove all classes present on matched elements.

However: I can see one potential downside, in terms of the code ecosystem that could build around a feature like this: when every method call to add/remove classes _has_ to provide class name, then it may be easier to track down the callsite that is adding/removing a particular CSS class.

(I don't know how to answer whether that's a realistic problem, other than to suggest parsing known `textual`-based applications to see whether they're implementing workarounds like looping over the CSS classes to remove them from an element.  a significantly larger task..)
Suggest `replace_classes` which takes class names as positional args. Without any args it is equievelent to clearing the classes. With args, it will set the classes to just those specified.
Actually, how about adding a setter for `classes` ?

Commit message: test bad identifiers

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 10de689901..55845259e1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Added Screen.ModalScreen which prevents App from handling bindings. https://github.com/Textualize/textual/pull/2139
 - Added TEXTUAL_LOG env var which should be a path that Textual will write verbose logs to (textual devtools is generally preferred) https://github.com/Textualize/textual/pull/2148
 - Added textual.logging.TextualHandler logging handler
+- Added Query.set_classes DOMNode.set_classes and `classes` setter for Widget https://github.com/Textualize/textual/issues/1081
 
 ## [0.16.0] - 2023-03-22
 
diff --git a/src/textual/css/query.py b/src/textual/css/query.py
index 73e09c61ba..98e0063a06 100644
--- a/src/textual/css/query.py
+++ b/src/textual/css/query.py
@@ -16,7 +16,7 @@
 
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Generic, Iterator, TypeVar, cast, overload
+from typing import TYPE_CHECKING, Generic, Iterable, Iterator, TypeVar, cast, overload
 
 import rich.repr
 
@@ -330,6 +330,25 @@ def set_class(self, add: bool, *class_names: str) -> DOMQuery[QueryType]:
             node.set_class(add, *class_names)
         return self
 
+    def set_classes(self, classes: str | Iterable[str]) -> DOMQuery[QueryType]:
+        """Set the classes on nodes to exactly the given set.
+
+        Args:
+            classes: A string of space separated classes, or an iterable of class names.
+
+        Returns:
+            Self.
+        """
+
+        if isinstance(classes, str):
+            for node in self:
+                node.set_classes(classes)
+        else:
+            class_names = list(classes)
+            for node in self:
+                node.set_classes(class_names)
+        return self
+
     def add_class(self, *class_names: str) -> DOMQuery[QueryType]:
         """Add the given class name(s) to nodes."""
         for node in self:
diff --git a/src/textual/dom.py b/src/textual/dom.py
index cb9881f653..df6e444acd 100644
--- a/src/textual/dom.py
+++ b/src/textual/dom.py
@@ -83,6 +83,26 @@ class NoScreen(DOMError):
     pass
 
 
+class _ClassesDescriptor:
+    """A descriptor to manage the `classes` property."""
+
+    def __get__(
+        self, obj: DOMNode, objtype: type[DOMNode] | None = None
+    ) -> frozenset[str]:
+        """A frozenset of the current classes on the widget."""
+        return frozenset(obj._classes)
+
+    def __set__(self, obj: DOMNode, classes: str | Iterable[str]) -> None:
+        """Replaces classes entirely."""
+        if isinstance(classes, str):
+            class_names = set(classes.split())
+        else:
+            class_names = set(classes)
+        check_identifiers("class name", *class_names)
+        obj._classes = class_names
+        obj._update_styles()
+
+
 @rich.repr.auto
 class DOMNode(MessagePump):
     """The base class for object that can be in the Textual DOM (App and Widget)"""
@@ -428,10 +448,7 @@ def css_identifier_styled(self) -> Text:
             tokens.append(f"[name={self.name}]", style="underline")
         return tokens
 
-    @property
-    def classes(self) -> frozenset[str]:
-        """A frozenset of the current classes set on the widget."""
-        return frozenset(self._classes)
+    classes = _ClassesDescriptor()
 
     @property
     def pseudo_classes(self) -> frozenset[str]:
@@ -888,6 +905,15 @@ def set_class(self, add: bool, *class_names: str) -> None:
         else:
             self.remove_class(*class_names)
 
+    def set_classes(self, classes: str | Iterable[str]) -> None:
+        """Replace all classes.
+
+        Args:
+            A string contain space separated classes, or an iterable of class names.
+
+        """
+        self.classes = classes
+
     def _update_styles(self) -> None:
         """Request an update of this node's styles.
 
diff --git a/tests/test_dom.py b/tests/test_dom.py
index b78a958c98..1b354067ce 100644
--- a/tests/test_dom.py
+++ b/tests/test_dom.py
@@ -45,6 +45,39 @@ def test_validate():
         node.toggle_class("1")
 
 
+def test_classes_setter():
+    node = DOMNode(classes="foo bar")
+    assert node.classes == frozenset({"foo", "bar"})
+    node.classes = "baz egg"
+    assert node.classes == frozenset({"baz", "egg"})
+    node.classes = ""
+    assert node.classes == frozenset({})
+
+
+def test_classes_setter_iterable():
+    node = DOMNode(classes="foo bar")
+    assert node.classes == frozenset({"foo", "bar"})
+    node.classes = "baz", "egg"
+    assert node.classes == frozenset({"baz", "egg"})
+    node.classes = []
+    assert node.classes == frozenset({})
+
+
+def test_classes_set_classes():
+    node = DOMNode(classes="foo bar")
+    assert node.classes == frozenset({"foo", "bar"})
+    node.set_classes("baz egg")
+    assert node.classes == frozenset({"baz", "egg"})
+    node.set_classes([])
+    assert node.classes == frozenset({})
+    node.set_classes(["paul"])
+    assert node.classes == frozenset({"paul"})
+    with pytest.raises(BadIdentifier):
+        node.classes = "foo 25"
+    with pytest.raises(BadIdentifier):
+        node.classes = ["foo", "25"]
+
+
 def test_inherited_bindings():
     """Test if binding merging is done correctly when (not) inheriting bindings."""
 
diff --git a/tests/test_query.py b/tests/test_query.py
index c58ffd575f..60dced9716 100644
--- a/tests/test_query.py
+++ b/tests/test_query.py
@@ -152,6 +152,25 @@ class ClassTest(Widget):
     # Now, let's check there are *no* children with the test class.
     assert len(app.query(".test")) == 0
 
+    # Add classes via set_classes
+    app.query(ClassTest).set_classes("foo bar")
+    assert (len(app.query(".foo"))) == CHILD_COUNT
+    assert (len(app.query(".bar"))) == CHILD_COUNT
+
+    # Reset classes
+    app.query(ClassTest).set_classes("")
+    assert (len(app.query(".foo"))) == 0
+    assert (len(app.query(".bar"))) == 0
+
+    # Repeat, to check setting empty iterable
+    app.query(ClassTest).set_classes("foo bar")
+    assert (len(app.query(".foo"))) == CHILD_COUNT
+    assert (len(app.query(".bar"))) == CHILD_COUNT
+
+    app.query(ClassTest).set_classes([])
+    assert (len(app.query(".foo"))) == 0
+    assert (len(app.query(".bar"))) == 0
+
     # Add the test class to everything and then check again.
     app.query(ClassTest).add_class("test")
     assert len(app.query(".test")) == CHILD_COUNT


Code review comments:
diff_hunk: @@ -888,6 +905,15 @@ def set_class(self, add: bool, *class_names: str) -> None:
         else:
             self.remove_class(*class_names)
 
+    def set_classes(self, classes: str | Iterable[str]) -> None:
path: src/textual/dom.py
text: @user1:
Perhaps this should return `self` so folk can chain calls?

@author:
It probably should. We don't have that on the other classes either.