diff --git a/CHANGELOG.md b/CHANGELOG.md index 908cbe1b2b..f1d794a8d7 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/pyproject.toml b/pyproject.toml index 55d12538f9..b63bcf0715 100644 --- a/pyproject.toml +++ b/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/" diff --git a/src/textual/app.py b/src/textual/app.py index 6e7c5bc556..501c4117e8 100644 --- a/src/textual/app.py +++ b/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: diff --git a/src/textual/binding.py b/src/textual/binding.py index 68f9bb6786..9a396478a4 100644 --- a/src/textual/binding.py +++ b/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,6 +83,7 @@ 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): @@ -90,7 +91,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 @@ -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 diff --git a/src/textual/dom.py b/src/textual/dom.py index f9d490f075..69a7313e40 100644 --- a/src/textual/dom.py +++ b/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 diff --git a/src/textual/screen.py b/src/textual/screen.py index 722e14719a..f03bbca593 100644 --- a/src/textual/screen.py +++ b/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 diff --git a/src/textual/widgets/_footer.py b/src/textual/widgets/_footer.py index 09a743ac48..0cd93ece70 100644 --- a/src/textual/widgets/_footer.py +++ b/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)) diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots/test_bind_override.svg b/tests/snapshot_tests/__snapshots__/test_snapshots/test_bind_override.svg new file mode 100644 index 0000000000..c91c2fe46c --- /dev/null +++ b/tests/snapshot_tests/__snapshots__/test_snapshots/test_bind_override.svg @@ -0,0 +1,156 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + BindApp + + + + + + + + + + ┌──────────────────────────────────────────────────────────────────────────────┐ +│MyWidget│ +││ +││ +└──────────────────────────────────────────────────────────────────────────────┘ +▊▔▔▔▔▔▔▔▔▎ +▊▎ +▊▁▁▁▁▁▁▁▁▎ + + + + + + + + + + + + + + + + SPACE Bell (Widget)  a widget  b widget  c app  + + + diff --git a/tests/snapshot_tests/snapshot_apps/bind_override.py b/tests/snapshot_tests/snapshot_apps/bind_override.py new file mode 100644 index 0000000000..07a991c112 --- /dev/null +++ b/tests/snapshot_tests/snapshot_apps/bind_override.py @@ -0,0 +1,41 @@ +from textual.app import App, ComposeResult +from textual.widget import Widget +from textual.widgets import Footer, Switch +from textual.binding import Binding + + +class MyWidget(Widget, can_focus=True): + BINDINGS = [ + Binding("space", "app.bell", "Bell (Widget)"), + Binding("a", "app.notify('a widget')", "widget"), + Binding("b", "app.notify('b widget')", "widget"), + ] + + DEFAULT_CSS = """ + MyWidget { + border: solid green; + height: 5; + } + """ + + +class BindApp(App): + BINDINGS = [ + Binding("space", "app.bell", "Bell (App)"), + Binding("c", "app.notify('c app')", "app"), + Binding("a", "app.notify('a app')", "app"), + Binding("b", "app.notify('b app')", "app"), + ] + + def compose(self) -> ComposeResult: + yield MyWidget() + yield Switch() + yield Footer() + + def action_notify(self, msg: str): + self.notify(msg) + + +if __name__ == "__main__": + app = BindApp() + app.run() diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index 895bd4a1bf..5126837653 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -1405,3 +1405,8 @@ def test_remove_tab_no_animation(snap_compare): def test_auto_height_scrollbar(snap_compare): """Regression test for https://github.com/Textualize/textual/issues/4778""" assert snap_compare(SNAPSHOT_APPS_DIR / "data_table_auto_height.py") + + +def test_bind_override(snap_compare): + """Regression test for https://github.com/Textualize/textual/issues/4755""" + assert snap_compare(SNAPSHOT_APPS_DIR / "bind_override.py") diff --git a/tests/test_binding.py b/tests/test_binding.py index 5c545d7a65..8ae7bb29ad 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -3,7 +3,13 @@ import pytest from textual.app import App -from textual.binding import Binding, BindingError, InvalidBinding, NoBinding, _Bindings +from textual.binding import ( + Binding, + BindingError, + BindingsMap, + InvalidBinding, + NoBinding, +) BINDING1 = Binding("a,b", action="action1", description="description1") BINDING2 = Binding("c", action="action2", description="description2") @@ -12,60 +18,67 @@ @pytest.fixture def bindings(): - yield _Bindings([BINDING1, BINDING2]) + yield BindingsMap([BINDING1, BINDING2]) @pytest.fixture def more_bindings(): - yield _Bindings([BINDING1, BINDING2, BINDING3]) + yield BindingsMap([BINDING1, BINDING2, BINDING3]) def test_bindings_get_key(bindings): - assert bindings.get_key("b") == Binding( - "b", action="action1", description="description1" - ) - assert bindings.get_key("c") == BINDING2 + assert bindings.get_bindings_for_key("b") == [ + Binding("b", action="action1", description="description1") + ] + assert bindings.get_bindings_for_key("c") == [BINDING2] with pytest.raises(NoBinding): - bindings.get_key("control+meta+alt+shift+super+hyper+t") + bindings.get_bindings_for_key("control+meta+alt+shift+super+hyper+t") def test_bindings_get_key_spaced_list(more_bindings): - assert more_bindings.get_key("d").action == more_bindings.get_key("e").action + assert ( + more_bindings.get_bindings_for_key("d")[0].action + == more_bindings.get_bindings_for_key("e")[0].action + ) def test_bindings_merge_simple(bindings): - left = _Bindings([BINDING1]) - right = _Bindings([BINDING2]) - assert _Bindings.merge([left, right]).keys == bindings.keys + left = BindingsMap([BINDING1]) + right = BindingsMap([BINDING2]) + assert BindingsMap.merge([left, right]).key_to_bindings == bindings.key_to_bindings def test_bindings_merge_overlap(): - left = _Bindings([BINDING1]) + left = BindingsMap([BINDING1]) another_binding = Binding( "a", action="another_action", description="another_description" ) - assert _Bindings.merge([left, _Bindings([another_binding])]).keys == { - "a": another_binding, - "b": Binding("b", action="action1", description="description1"), + assert BindingsMap.merge( + [left, BindingsMap([another_binding])] + ).key_to_bindings == { + "a": [ + Binding("a", action="action1", description="description1"), + another_binding, + ], + "b": [Binding("b", action="action1", description="description1")], } def test_bad_binding_tuple(): with pytest.raises(BindingError): - _ = _Bindings((("a",),)) + _ = BindingsMap((("a",),)) with pytest.raises(BindingError): - _ = _Bindings((("a", "action", "description", "too much"),)) + _ = BindingsMap((("a", "action", "description", "too much"),)) def test_binding_from_tuples(): - assert ( - _Bindings(((BINDING2.key, BINDING2.action, BINDING2.description),)).get_key("c") - == BINDING2 - ) + assert BindingsMap( + ((BINDING2.key, BINDING2.action, BINDING2.description),) + ).get_bindings_for_key("c") == [BINDING2] def test_shown(): - bindings = _Bindings( + bindings = BindingsMap( [ Binding( key, diff --git a/tests/test_binding_inheritance.py b/tests/test_binding_inheritance.py index 07c483bc65..dde97bb409 100644 --- a/tests/test_binding_inheritance.py +++ b/tests/test_binding_inheritance.py @@ -39,11 +39,11 @@ 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", ] - assert pilot.app._bindings.get_key("ctrl+c").priority is True + assert pilot.app._bindings.get_bindings_for_key("ctrl+c")[0].priority is True ############################################################################## @@ -64,11 +64,11 @@ 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_key("ctrl+c").priority is True - assert pilot.app._bindings.get_key("a").priority is True + assert pilot.app._bindings.get_bindings_for_key("ctrl+c")[0].priority is True + assert pilot.app._bindings.get_bindings_for_key("a")[0].priority is True ############################################################################## @@ -88,11 +88,11 @@ 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_key("ctrl+c").priority is True - assert pilot.app._bindings.get_key("a").priority is False + assert pilot.app._bindings.get_bindings_for_key("ctrl+c")[0].priority is True + assert pilot.app._bindings.get_bindings_for_key("a")[0].priority is False ############################################################################## @@ -121,7 +121,7 @@ def on_mount(self) -> None: async def test_app_screen_with_bindings() -> None: """Test a screen with a single key binding defined.""" async with AppWithScreenThatHasABinding().run_test() as pilot: - assert pilot.app.screen._bindings.get_key("a").priority is True + assert pilot.app.screen._bindings.get_bindings_for_key("a")[0].priority is True ############################################################################## @@ -150,7 +150,7 @@ def on_mount(self) -> None: async def test_app_screen_with_low_bindings() -> None: """Test a screen with a single low-priority key binding defined.""" async with AppWithScreenThatHasALowBinding().run_test() as pilot: - assert pilot.app.screen._bindings.get_key("a").priority is False + assert pilot.app.screen._bindings.get_bindings_for_key("a")[0].priority is False ############################################################################## diff --git a/tests/test_dom.py b/tests/test_dom.py index ea14a25bfe..89bbe53c43 100644 --- a/tests/test_dom.py +++ b/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():