comments and naming

Author: willmcgugan

src/textual/app.py

@@ -3048,7 +3048,7 @@ async def _check_bindings(self, key: str, priority: bool = False) -> bool:
             if priority
             else self.screen._modal_binding_chain
         ):
-            key_bindings = bindings.keys.get(key, ())
+            key_bindings = bindings.key_to_bindings.get(key, ())
             for binding in key_bindings:
                 if binding.priority == priority:
                     if await self.run_action(binding.action, namespace):

src/textual/binding.py

@@ -92,7 +92,8 @@ def make_bindings(bindings: Iterable[BindingType]) -> Iterable[Binding]:
                         raise BindingError(
                             f"BINDINGS must contain a tuple of two or three strings, not {binding!r}"
                         )
-                    binding = Binding(*binding)
+                    # `binding` is a tuple of 2 or 3 values at this point
+                    binding = Binding(*binding)  # type: ignore[reportArgumentType]
 
                 # At this point we have a Binding instance, but the key may
                 # be a list of keys, so now we unroll that single Binding
@@ -114,23 +115,32 @@ def make_bindings(bindings: Iterable[BindingType]) -> Iterable[Binding]:
                         priority=binding.priority,
                     )
 
-        self.keys: dict[str, list[Binding]] = {}
+        self.key_to_bindings: dict[str, list[Binding]] = {}
         for binding in make_bindings(bindings or {}):
-            self.keys.setdefault(binding.key, []).append(binding)
+            self.key_to_bindings.setdefault(binding.key, []).append(binding)
 
     def __iter__(self) -> Iterator[tuple[str, Binding]]:
+        """Iterating produces a sequence of (KEY, BINDING) tuples."""
         return iter(
             [
                 (key, binding)
-                for key, bindings in self.keys.items()
+                for key, bindings in self.key_to_bindings.items()
                 for binding in bindings
             ]
         )
 
     @classmethod
     def from_keys(cls, keys: dict[str, list[Binding]]) -> BindingsMap:
+        """Construct a BindingsMap from a dict of keys and bindings.
+
+        Args:
+            keys: A dict that maps a key on to a list of `Binding` objects.
+
+        Returns:
+            New `BindingsMap`
+        """
         bindings = cls()
-        bindings.keys = keys
+        bindings.key_to_bindings = keys
         return bindings
 
     def copy(self) -> BindingsMap:
@@ -140,35 +150,34 @@ def copy(self) -> BindingsMap:
             New bindings object.
         """
         copy = BindingsMap()
-        copy.keys = self.keys.copy()
+        copy.key_to_bindings = self.key_to_bindings.copy()
         return copy
 
     def __rich_repr__(self) -> rich.repr.Result:
-        yield self.keys
+        yield self.key_to_bindings
 
     @classmethod
     def merge(cls, bindings: Iterable[BindingsMap]) -> BindingsMap:
-        """Merge a bindings. Subsequent bound keys override initial keys.
+        """Merge a bindings.
 
         Args:
             bindings: A number of bindings.
 
         Returns:
-            New bindings.
+            New `BindingsMap`.
         """
         keys: dict[str, list[Binding]] = {}
         for _bindings in bindings:
-            for key, key_bindings in _bindings.keys.items():
+            for key, key_bindings in _bindings.key_to_bindings.items():
                 keys.setdefault(key, []).extend(key_bindings)
-
         return BindingsMap.from_keys(keys)
 
     @property
     def shown_keys(self) -> list[Binding]:
         """A list of bindings for shown keys."""
         keys = [
             binding
-            for bindings in self.keys.values()
+            for bindings in self.key_to_bindings.values()
             for binding in bindings
             if binding.show
         ]
@@ -195,7 +204,7 @@ def bind(
         """
         all_keys = [key.strip() for key in keys.split(",")]
         for key in all_keys:
-            self.keys.setdefault(key, []).append(
+            self.key_to_bindings.setdefault(key, []).append(
                 Binding(
                     key,
                     action,
@@ -219,6 +228,6 @@ def get_bindings_for_key(self, key: str) -> list[Binding]:
             A list of bindings associated with the key.
         """
         try:
-            return self.keys[key]
+            return self.key_to_bindings[key]
         except KeyError:
             raise NoBinding(f"No binding for {key}") from None

src/textual/dom.py

@@ -609,7 +609,7 @@ def _merge_bindings(cls) -> BindingsMap:
                 )
         keys: dict[str, list[Binding]] = {}
         for bindings_ in bindings:
-            for key, key_bindings in bindings_.keys.items():
+            for key, key_bindings in bindings_.key_to_bindings.items():
                 keys[key] = key_bindings
 
         new_bindings = BindingsMap().from_keys(keys)

src/textual/screen.py

@@ -327,22 +327,29 @@ def active_bindings(self) -> dict[str, ActiveBinding]:
         This property may be used to inspect current bindings.
 
         Returns:
-            A map of keys to a tuple containing (namespace, binding, enabled boolean).
+            A map of keys to a tuple containing (NAMESPACE, BINDING, ENABLED).
         """
 
         bindings_map: dict[str, ActiveBinding] = {}
         for namespace, bindings in self._modal_binding_chain:
             for key, binding in bindings:
+                # This will call the nodes `check_action` method.
                 action_state = self.app._check_action_state(binding.action, namespace)
                 if action_state is False:
+                    # An action_state of False indicates the action is disabled and not shown
+                    # Note that None has a different meaning, which is why there is an `is False`
+                    # rather than a truthy check.
                     continue
                 if existing_key_and_binding := bindings_map.get(key):
+                    # This key has already been bound
                     _, existing_binding, _ = existing_key_and_binding
+                    # Replace priority bindings
                     if binding.priority and not existing_binding.priority:
                         bindings_map[key] = ActiveBinding(
                             namespace, binding, bool(action_state)
                         )
                 else:
+                    # New binding
                     bindings_map[key] = ActiveBinding(
                         namespace, binding, bool(action_state)
                     )

src/textual/widgets/_footer.py

@@ -164,9 +164,8 @@ def compose(self) -> ComposeResult:
             for (_, binding, enabled) in self.screen.active_bindings.values()
             if binding.show
         ]
-        action_to_bindings: defaultdict[str, list[tuple[Binding, bool]]] = defaultdict(
-            list
-        )
+        action_to_bindings: defaultdict[str, list[tuple[Binding, bool]]]
+        action_to_bindings = defaultdict(list)
         for binding, enabled in bindings:
             action_to_bindings[binding.action].append((binding, enabled))
 

tests/test_binding.py

@@ -45,15 +45,17 @@ def test_bindings_get_key_spaced_list(more_bindings):
 def test_bindings_merge_simple(bindings):
     left = BindingsMap([BINDING1])
     right = BindingsMap([BINDING2])
-    assert BindingsMap.merge([left, right]).keys == bindings.keys
+    assert BindingsMap.merge([left, right]).key_to_bindings == bindings.keys
 
 
 def test_bindings_merge_overlap():
     left = BindingsMap([BINDING1])
     another_binding = Binding(
         "a", action="another_action", description="another_description"
     )
-    assert BindingsMap.merge([left, BindingsMap([another_binding])]).keys == {
+    assert BindingsMap.merge(
+        [left, BindingsMap([another_binding])]
+    ).key_to_bindings == {
         "a": [
             Binding("a", action="action1", description="description1"),
             another_binding,

tests/test_binding_inheritance.py

@@ -39,7 +39,7 @@ class NoBindings(App[None]):
 async def test_just_app_no_bindings() -> None:
     """An app with no bindings should have no bindings, other than the app's hard-coded ones."""
     async with NoBindings().run_test() as pilot:
-        assert list(pilot.app._bindings.keys.keys()) == [
+        assert list(pilot.app._bindings.key_to_bindings.keys()) == [
             "ctrl+c",
             "ctrl+backslash",
         ]
@@ -64,7 +64,7 @@ class AlphaBinding(App[None]):
 async def test_just_app_alpha_binding() -> None:
     """An app with a single binding should have just the one binding."""
     async with AlphaBinding().run_test() as pilot:
-        assert sorted(pilot.app._bindings.keys.keys()) == sorted(
+        assert sorted(pilot.app._bindings.key_to_bindings.keys()) == sorted(
             ["ctrl+c", "ctrl+backslash", "a"]
         )
         assert pilot.app._bindings.get_bindings_for_key("ctrl+c")[0].priority is True
@@ -88,7 +88,7 @@ class LowAlphaBinding(App[None]):
 async def test_just_app_low_priority_alpha_binding() -> None:
     """An app with a single low-priority binding should have just the one binding."""
     async with LowAlphaBinding().run_test() as pilot:
-        assert sorted(pilot.app._bindings.keys.keys()) == sorted(
+        assert sorted(pilot.app._bindings.key_to_bindings.keys()) == sorted(
             ["ctrl+c", "ctrl+backslash", "a"]
         )
         assert pilot.app._bindings.get_bindings_for_key("ctrl+c")[0].priority is True

tests/test_dom.py

@@ -97,19 +97,19 @@ class E(D):
         BINDINGS = [("e", "e", "e")]
 
     a = A()
-    assert list(a._bindings.keys.keys()) == ["a"]
+    assert list(a._bindings.key_to_bindings.keys()) == ["a"]
 
     b = B()
-    assert list(b._bindings.keys.keys()) == ["a", "b"]
+    assert list(b._bindings.key_to_bindings.keys()) == ["a", "b"]
 
     c = C()
-    assert list(c._bindings.keys.keys()) == ["c"]
+    assert list(c._bindings.key_to_bindings.keys()) == ["c"]
 
     d = D()
-    assert not list(d._bindings.keys.keys())
+    assert not list(d._bindings.key_to_bindings.keys())
 
     e = E()
-    assert list(e._bindings.keys.keys()) == ["e"]
+    assert list(e._bindings.key_to_bindings.keys()) == ["e"]
 
 
 def test__get_default_css():