Skip to content

Commit

Permalink
Merge pull request #3618 from davep/command-palette-worker-nuke
Browse files Browse the repository at this point in the history
Ensure that the command palette doesn't kill *all* workers when stoping command gathering
  • Loading branch information
davep authored Oct 31, 2023
2 parents efbb655 + 9cacf8c commit 665dca9
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Fixed `OptionList` event leakage from `CommandPalette` to `App`.
- Fixed crash in `LoadingIndicator` https://github.com/Textualize/textual/pull/3498
- Fixed crash when `Tabs` appeared as a descendant of `TabbedContent` in the DOM https://github.com/Textualize/textual/pull/3602
- Fixed the command palette cancelling other workers https://github.com/Textualize/textual/issues/3615

### Added

Expand Down
19 changes: 13 additions & 6 deletions src/textual/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ def _on_click(self, event: Click) -> None:
method of dismissing the palette.
"""
if self.get_widget_at(event.screen_x, event.screen_y)[0] is self:
self.workers.cancel_all()
self._cancel_gather_commands()
self.dismiss()

def on_mount(self, _: Mount) -> None:
Expand Down Expand Up @@ -774,7 +774,10 @@ def _refresh_command_list(
_NO_MATCHES: Final[str] = "--no-matches"
"""The ID to give the disabled option that shows there were no matches."""

@work(exclusive=True)
_GATHER_COMMANDS_GROUP: Final[str] = "--textual-command-palette-gather-commands"
"""The group name of the command gathering worker."""

@work(exclusive=True, group=_GATHER_COMMANDS_GROUP)
async def _gather_commands(self, search_value: str) -> None:
"""Gather up all of the commands that match the search value.
Expand Down Expand Up @@ -895,6 +898,10 @@ async def _gather_commands(self, search_value: str) -> None:
if command_list.option_count == 0 and not worker.is_cancelled:
self._start_no_matches_countdown()

def _cancel_gather_commands(self) -> None:
"""Cancel any operation that is gather commands."""
self.workers.cancel_group(self, self._GATHER_COMMANDS_GROUP)

@on(Input.Changed)
def _input(self, event: Input.Changed) -> None:
"""React to input in the command palette.
Expand All @@ -903,7 +910,7 @@ def _input(self, event: Input.Changed) -> None:
event: The input event.
"""
event.stop()
self.workers.cancel_all()
self._cancel_gather_commands()
self._stop_no_matches_countdown()

search_value = event.value.strip()
Expand All @@ -921,7 +928,7 @@ def _select_command(self, event: OptionList.OptionSelected) -> None:
event: The option selection event.
"""
event.stop()
self.workers.cancel_all()
self._cancel_gather_commands()
input = self.query_one(CommandInput)
with self.prevent(Input.Changed):
assert isinstance(event.option, Command)
Expand Down Expand Up @@ -958,7 +965,7 @@ def _select_or_command(
if self._selected_command is not None:
# ...we should return it to the parent screen and let it
# decide what to do with it (hopefully it'll run it).
self.workers.cancel_all()
self._cancel_gather_commands()
self.dismiss(self._selected_command.command)

@on(OptionList.OptionHighlighted)
Expand All @@ -971,7 +978,7 @@ def _action_escape(self) -> None:
if self._list_visible:
self._list_visible = False
else:
self.workers.cancel_all()
self._cancel_gather_commands()
self.dismiss()

def _action_command_list(self, action: str) -> None:
Expand Down
50 changes: 50 additions & 0 deletions tests/command_palette/test_worker_interference.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
"""Tests for https://github.com/Textualize/textual/issues/3615"""

from asyncio import sleep

from textual import work
from textual.app import App
from textual.command import Hit, Hits, Provider


class SimpleSource(Provider):
async def search(self, query: str) -> Hits:
def goes_nowhere_does_nothing() -> None:
pass

for _ in range(100):
yield Hit(1, query, goes_nowhere_does_nothing, query)


class CommandPaletteNoWorkerApp(App[None]):
COMMANDS = {SimpleSource}


async def test_no_command_palette_worker_droppings() -> None:
"""The command palette should not leave any workers behind.."""
async with CommandPaletteNoWorkerApp().run_test() as pilot:
assert len(pilot.app.workers) == 0
pilot.app.action_command_palette()
await pilot.press("a", "enter")
assert len(pilot.app.workers) == 0


class CommandPaletteWithWorkerApp(App[None]):
COMMANDS = {SimpleSource}

def on_mount(self) -> None:
self.innocent_worker()

@work
async def innocent_worker(self) -> None:
while True:
await sleep(1)


async def test_innocent_worker_is_untouched() -> None:
"""Using the command palette should not halt other workers."""
async with CommandPaletteWithWorkerApp().run_test() as pilot:
assert len(pilot.app.workers) > 0
pilot.app.action_command_palette()
await pilot.press("a", "enter")
assert len(pilot.app.workers) > 0

0 comments on commit 665dca9

Please sign in to comment.