Merge pull request #4850 from Textualize/multi-bindings

WIP Multiple bindings

CHANGELOG.md

@@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
-## Unreleased
+## [0.76.0]
 
 ### Changed
 
@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 - Input cursor blink effect will now restart correctly when any action is performed on the input https://github.com/Textualize/textual/pull/4773
+- Fixed bindings on same key not updating description https://github.com/Textualize/textual/pull/4850
 
 ### Added
 
@@ -2274,6 +2275,7 @@ https://textual.textualize.io/blog/2022/11/08/version-040/#version-040
 - New handler system for messages that doesn't require inheritance
 - Improved traceback handling
 
+[0.76.0]: https://github.com/Textualize/textual/compare/v0.75.1...v0.76.0
 [0.75.1]: https://github.com/Textualize/textual/compare/v0.75.0...v0.75.1
 [0.75.0]: https://github.com/Textualize/textual/compare/v0.74.0...v0.75.0
 [0.74.0]: https://github.com/Textualize/textual/compare/v0.73.0...v0.74.0

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "textual"
-version = "0.75.1"
+version = "0.76.0"
 homepage = "https://github.com/Textualize/textual"
 repository = "https://github.com/Textualize/textual"
 documentation = "https://textual.textualize.io/"

src/textual/app.py

@@ -80,7 +80,7 @@
 from .actions import ActionParseResult, SkipAction
 from .await_complete import AwaitComplete
 from .await_remove import AwaitRemove
-from .binding import Binding, BindingType, _Bindings
+from .binding import Binding, BindingsMap, BindingType
 from .command import CommandPalette, Provider
 from .css.errors import StylesheetError
 from .css.query import NoMatches
@@ -3000,14 +3000,14 @@ def bell(self) -> None:
             self._driver.write("\07")
 
     @property
-    def _binding_chain(self) -> list[tuple[DOMNode, _Bindings]]:
+    def _binding_chain(self) -> list[tuple[DOMNode, BindingsMap]]:
         """Get a chain of nodes and bindings to consider.
 
         If no widget is focused, returns the bindings from both the screen and the app level bindings.
         Otherwise, combines all the bindings from the currently focused node up the DOM to the root App.
         """
         focused = self.focused
-        namespace_bindings: list[tuple[DOMNode, _Bindings]]
+        namespace_bindings: list[tuple[DOMNode, BindingsMap]]
 
         if focused is None:
             namespace_bindings = [
@@ -3048,10 +3048,11 @@ async def _check_bindings(self, key: str, priority: bool = False) -> bool:
             if priority
             else self.screen._modal_binding_chain
         ):
-            binding = bindings.keys.get(key)
-            if binding is not None and binding.priority == priority:
-                if await self.run_action(binding.action, namespace):
-                    return True
+            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):
+                        return True
         return False
 
     async def on_event(self, event: events.Event) -> None:

src/textual/binding.py

@@ -8,7 +8,7 @@
 from __future__ import annotations
 
 from dataclasses import dataclass
-from typing import TYPE_CHECKING, Iterable, NamedTuple
+from typing import TYPE_CHECKING, Iterable, Iterator, NamedTuple
 
 import rich.repr
 
@@ -64,7 +64,7 @@ class ActiveBinding(NamedTuple):
 
 
 @rich.repr.auto
-class _Bindings:
+class BindingsMap:
     """Manage a set of bindings."""
 
     def __init__(
@@ -83,14 +83,16 @@ def __init__(
         """
 
         def make_bindings(bindings: Iterable[BindingType]) -> Iterable[Binding]:
+            bindings = list(bindings)
             for binding in bindings:
                 # If it's a tuple of length 3, convert into a Binding first
                 if isinstance(binding, tuple):
                     if len(binding) not in (2, 3):
                         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
@@ -112,44 +114,72 @@ def make_bindings(bindings: Iterable[BindingType]) -> Iterable[Binding]:
                         priority=binding.priority,
                     )
 
-        self.keys: dict[str, Binding] = (
-            {binding.key: binding for binding in make_bindings(bindings)}
-            if bindings
-            else {}
+        self.key_to_bindings: dict[str, list[Binding]] = {}
+        for binding in make_bindings(bindings or {}):
+            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.key_to_bindings.items()
+                for binding in bindings
+            ]
         )
 
-    def copy(self) -> _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.key_to_bindings = keys
+        return bindings
+
+    def copy(self) -> BindingsMap:
         """Return a copy of this instance.
 
         Return:
             New bindings object.
         """
-        copy = _Bindings()
-        copy.keys = self.keys.copy()
+        copy = BindingsMap()
+        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[_Bindings]) -> _Bindings:
-        """Merge a bindings. Subsequent bound keys override initial keys.
+    def merge(cls, bindings: Iterable[BindingsMap]) -> BindingsMap:
+        """Merge a bindings.
 
         Args:
             bindings: A number of bindings.
 
         Returns:
-            New bindings.
+            New `BindingsMap`.
         """
-        keys: dict[str, Binding] = {}
+        keys: dict[str, list[Binding]] = {}
         for _bindings in bindings:
-            keys.update(_bindings.keys)
-        return _Bindings(keys.values())
+            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 binding in self.keys.values() if binding.show]
+        keys = [
+            binding
+            for bindings in self.key_to_bindings.values()
+            for binding in bindings
+            if binding.show
+        ]
         return keys
 
     def bind(
@@ -173,17 +203,19 @@ def bind(
         """
         all_keys = [key.strip() for key in keys.split(",")]
         for key in all_keys:
-            self.keys[key] = Binding(
-                key,
-                action,
-                description,
-                show=bool(description and show),
-                key_display=key_display,
-                priority=priority,
+            self.key_to_bindings.setdefault(key, []).append(
+                Binding(
+                    key,
+                    action,
+                    description,
+                    show=bool(description and show),
+                    key_display=key_display,
+                    priority=priority,
+                )
             )
 
-    def get_key(self, key: str) -> Binding:
-        """Get a binding if it exists.
+    def get_bindings_for_key(self, key: str) -> list[Binding]:
+        """Get a list of bindings for a given key.
 
         Args:
             key: Key to look up.
@@ -192,9 +224,9 @@ def get_key(self, key: str) -> Binding:
             NoBinding: If the binding does not exist.
 
         Returns:
-            A binding object for the key,
+            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

@@ -32,7 +32,7 @@
 from ._context import NoActiveAppError, active_message_pump
 from ._node_list import NodeList
 from ._types import WatchCallbackType
-from .binding import Binding, BindingType, _Bindings
+from .binding import Binding, BindingsMap, BindingType
 from .color import BLACK, WHITE, Color
 from .css._error_tools import friendly_list
 from .css.constants import VALID_DISPLAY, VALID_VISIBILITY
@@ -158,7 +158,7 @@ class DOMNode(MessagePump):
     _css_type_name: str = ""
 
     # Generated list of bindings
-    _merged_bindings: ClassVar[_Bindings | None] = None
+    _merged_bindings: ClassVar[BindingsMap | None] = None
 
     _reactives: ClassVar[dict[str, Reactive]]
 
@@ -197,7 +197,7 @@ def __init__(
         self._auto_refresh_timer: Timer | None = None
         self._css_types = {cls.__name__ for cls in self._css_bases(self.__class__)}
         self._bindings = (
-            _Bindings()
+            BindingsMap()
             if self._merged_bindings is None
             else self._merged_bindings.copy()
         )
@@ -590,27 +590,30 @@ def _css_bases(cls, base: Type[DOMNode]) -> Sequence[Type[DOMNode]]:
         return classes
 
     @classmethod
-    def _merge_bindings(cls) -> _Bindings:
+    def _merge_bindings(cls) -> BindingsMap:
         """Merge bindings from base classes.
 
         Returns:
             Merged bindings.
         """
-        bindings: list[_Bindings] = []
+        bindings: list[BindingsMap] = []
 
         for base in reversed(cls.__mro__):
             if issubclass(base, DOMNode):
                 if not base._inherit_bindings:
                     bindings.clear()
                 bindings.append(
-                    _Bindings(
+                    BindingsMap(
                         base.__dict__.get("BINDINGS", []),
                     )
                 )
-        keys: dict[str, Binding] = {}
+        keys: dict[str, list[Binding]] = {}
         for bindings_ in bindings:
-            keys.update(bindings_.keys)
-        return _Bindings(keys.values())
+            for key, key_bindings in bindings_.key_to_bindings.items():
+                keys[key] = key_bindings
+
+        new_bindings = BindingsMap().from_keys(keys)
+        return new_bindings
 
     def _post_register(self, app: App) -> None:
         """Called when the widget is registered

src/textual/screen.py

@@ -36,7 +36,7 @@
 from ._path import CSSPathType, _css_path_type_as_list, _make_path_object_relative
 from ._types import CallbackType
 from .await_complete import AwaitComplete
-from .binding import ActiveBinding, Binding, _Bindings
+from .binding import ActiveBinding, Binding, BindingsMap
 from .css.match import match
 from .css.parse import parse_selectors
 from .css.query import NoMatches, QueryType
@@ -289,12 +289,12 @@ def refresh_bindings(self) -> None:
         self.check_idle()
 
     @property
-    def _binding_chain(self) -> list[tuple[DOMNode, _Bindings]]:
+    def _binding_chain(self) -> list[tuple[DOMNode, BindingsMap]]:
         """Binding chain from this screen."""
         focused = self.focused
         if focused is not None and focused.loading:
             focused = None
-        namespace_bindings: list[tuple[DOMNode, _Bindings]]
+        namespace_bindings: list[tuple[DOMNode, BindingsMap]]
 
         if focused is None:
             namespace_bindings = [
@@ -309,7 +309,7 @@ def _binding_chain(self) -> list[tuple[DOMNode, _Bindings]]:
         return namespace_bindings
 
     @property
-    def _modal_binding_chain(self) -> list[tuple[DOMNode, _Bindings]]:
+    def _modal_binding_chain(self) -> list[tuple[DOMNode, BindingsMap]]:
         """The binding chain, ignoring everything before the last modal."""
         binding_chain = self._binding_chain
         for index, (node, _bindings) in enumerate(binding_chain, 1):
@@ -327,25 +327,31 @@ 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.keys.items():
+            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
+                enabled = bool(action_state)
                 if existing_key_and_binding := bindings_map.get(key):
-                    _, existing_binding, _ = existing_key_and_binding
-                    if binding.priority and not existing_binding.priority:
-                        bindings_map[key] = ActiveBinding(
-                            namespace, binding, bool(action_state)
-                        )
+                    # This key has already been bound
+                    # Replace priority bindings
+                    if (
+                        binding.priority
+                        and not existing_key_and_binding.binding.priority
+                    ):
+                        bindings_map[key] = ActiveBinding(namespace, binding, enabled)
                 else:
-                    bindings_map[key] = ActiveBinding(
-                        namespace, binding, bool(action_state)
-                    )
+                    # New binding
+                    bindings_map[key] = ActiveBinding(namespace, binding, enabled)
 
         return bindings_map
 

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))