Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load DirectoryTree contents in a worker #2545

Merged
merged 37 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
30a20ac
Break iterdir out into a method of its own for easy testing
davep May 9, 2023
d673175
Experimenting with placing _load_directory in a worker
davep May 10, 2023
8b9a8e4
Simplify _load_directory
davep May 10, 2023
cd05d6c
Merge branch 'main' into directory-tree-work-in-worker
davep May 10, 2023
3997187
WiP: Throttle back the number of concurrent loads of a DirectoryTree
davep May 10, 2023
05dc877
Check for the worker being cancelled
davep May 11, 2023
df0f73b
Remove debug logging
davep May 11, 2023
4ead43c
Set up the job tracking before setting the path
davep May 11, 2023
ce7a78d
Have the reset method take part in background loading
davep May 11, 2023
791f2ea
Ensure we don't create a job for a node that's already being loaded
davep May 11, 2023
08246d8
Don't post the finished message if we've been cancelled
davep May 11, 2023
9ae8e47
Add a method for cancelling all of the load jobs
davep May 11, 2023
82a0817
Cancel any loads when resetting the tree
davep May 11, 2023
05eeaa7
Tidy up _load_directory
davep May 11, 2023
9b41b74
Remove the artificial slowdown
davep May 11, 2023
c45126b
Update the ChangeLog
davep May 11, 2023
4d225b8
Correct a comment typo
davep May 11, 2023
5f839da
Merge branch 'main' into directory-tree-work-in-worker
davep May 11, 2023
804d85a
Merge branch 'main' into directory-tree-work-in-worker
davep May 16, 2023
926c0a2
Reset all DirectoryTree worker changes
davep May 16, 2023
58f0d11
Change to a single loader thread with a queue
davep May 16, 2023
e69e57d
Remove unused import
davep May 16, 2023
80d00ce
Logging and experimenting for Windows
davep May 17, 2023
6876a04
More Windows thread oddness experimenting
davep May 17, 2023
64d9c60
Revert experimental code
davep May 17, 2023
82924c2
Make the main load worker into a asyncio task
davep May 17, 2023
a42250d
async Queue get blocks when empty, so don't handle empty exception
davep May 17, 2023
ecde90b
Remove unused import
davep May 17, 2023
26e6dbb
Swap to a dual-working approach
davep May 17, 2023
c04bbd1
Ensure the loader kicks off when starting up with . as the directory
davep May 17, 2023
dadd7c0
Guard against PermissionError
davep May 17, 2023
3f64728
Merge branch 'main' into directory-tree-work-in-worker
davep May 17, 2023
2a91e13
Mark each load task as done when it's done
davep May 17, 2023
86bee6c
Rename _to_load to _load_queue
davep May 17, 2023
522d56c
Be more optimistic about when the node content is loaded
davep May 17, 2023
e381c26
Create a single method for adding a node to the load queue
davep May 17, 2023
abbffbf
Code tidy
davep May 17, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Changed

- App `title` and `sub_title` attributes can be set to any type https://github.com/Textualize/textual/issues/2521
- `DirectoryTree` now loads directory contents in a worker https://github.com/Textualize/textual/issues/2456
- Only a single error will be written by default, unless in dev mode ("debug" in App.features) https://github.com/Textualize/textual/issues/2480
- Using `Widget.move_child` where the target and the child being moved are the same is now a no-op https://github.com/Textualize/textual/issues/1743
- Calling `dismiss` on a screen that is not at the top of the stack now raises an exception https://github.com/Textualize/textual/issues/2575
Expand Down
137 changes: 117 additions & 20 deletions src/textual/widgets/_directory_tree.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
from __future__ import annotations

from asyncio import Queue
from dataclasses import dataclass
from pathlib import Path
from typing import ClassVar, Iterable
from typing import ClassVar, Iterable, Iterator

from rich.style import Style
from rich.text import Text, TextType

from ..events import Mount
from .. import work
from ..message import Message
from ..reactive import var
from ..worker import Worker, WorkerCancelled, WorkerFailed, get_current_worker
from ._tree import TOGGLE_STYLE, Tree, TreeNode


Expand Down Expand Up @@ -90,7 +92,7 @@ def control(self) -> DirectoryTree:
"""
return self.tree

path: var[str | Path] = var["str | Path"](Path("."), init=False)
path: var[str | Path] = var["str | Path"](Path("."), init=False, always_update=True)
"""The path that is the root of the directory tree.

Note:
Expand All @@ -116,6 +118,7 @@ def __init__(
classes: A space-separated list of classes, or None for no classes.
disabled: Whether the directory tree is disabled or not.
"""
self._load_queue: Queue[TreeNode[DirEntry]] = Queue()
super().__init__(
str(path),
data=DirEntry(Path(path)),
Expand All @@ -126,10 +129,26 @@ def __init__(
)
self.path = path

def _add_to_load_queue(self, node: TreeNode[DirEntry]) -> None:
"""Add the given node to the load queue.

Args:
node: The node to add to the load queue.
"""
assert node.data is not None
node.data.loaded = True
self._load_queue.put_nowait(node)

def reload(self) -> None:
"""Reload the `DirectoryTree` contents."""
self.reset(str(self.path), DirEntry(Path(self.path)))
self._load_directory(self.root)
# Orphan the old queue...
self._load_queue = Queue()
# ...and replace the old load with a new one.
self._loader()
# We have a fresh queue, we have a fresh loader, get the fresh root
# loading up.
self._add_to_load_queue(self.root)

def validate_path(self, path: str | Path) -> Path:
"""Ensure that the path is of the `Path` type.
Expand Down Expand Up @@ -229,37 +248,115 @@ def filter_paths(self, paths: Iterable[Path]) -> Iterable[Path]:
"""
return paths

def _load_directory(self, node: TreeNode[DirEntry]) -> None:
"""Load the directory contents for a given node.
@staticmethod
def _safe_is_dir(path: Path) -> bool:
"""Safely check if a path is a directory.

Args:
node: The node to load the directory contents for.
path: The path to check.

Returns:
`True` if the path is for a directory, `False` if not.
"""
assert node.data is not None
node.data.loaded = True
directory = sorted(
self.filter_paths(node.data.path.iterdir()),
key=lambda path: (not path.is_dir(), path.name.lower()),
)
for path in directory:
try:
return path.is_dir()
except PermissionError:
# We may or may not have been looking at a directory, but we
# don't have the rights or permissions to even know that. Best
# we can do, short of letting the error blow up, is assume it's
# not a directory. A possible improvement in here could be to
# have a third state which is "unknown", and reflect that in the
# tree.
return False

def _populate_node(self, node: TreeNode[DirEntry], content: Iterable[Path]) -> None:
"""Populate the given tree node with the given directory content.

Args:
node: The Tree node to populate.
content: The collection of `Path` objects to populate the node with.
"""
for path in content:
node.add(
path.name,
data=DirEntry(path),
allow_expand=path.is_dir(),
allow_expand=self._safe_is_dir(path),
)
node.expand()

def _on_mount(self, _: Mount) -> None:
self._load_directory(self.root)
def _directory_content(self, location: Path, worker: Worker) -> Iterator[Path]:
"""Load the content of a given directory.

Args:
location: The location to load from.
worker: The worker that the loading is taking place in.

Yields:
Path: A entry within the location.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Path: A entry within the location.
Path: An entry within the location.

"""
try:
for entry in location.iterdir():
if worker.is_cancelled:
break
yield entry
except PermissionError:
pass

@work
def _load_directory(self, node: TreeNode[DirEntry]) -> list[Path]:
"""Load the directory contents for a given node.

Args:
node: The node to load the directory contents for.

Returns:
The list of entries within the directory associated with the node.
"""
assert node.data is not None
return sorted(
self.filter_paths(
self._directory_content(node.data.path, get_current_worker())
),
key=lambda path: (not self._safe_is_dir(path), path.name.lower()),
)

@work(exclusive=True)
async def _loader(self) -> None:
"""Background loading queue processor."""
worker = get_current_worker()
while not worker.is_cancelled:
# Get the next node that needs loading off the queue. Note that
# this blocks if the queue is empty.
node = await self._load_queue.get()
content: list[Path] = []
try:
# Spin up a short-lived thread that will load the content of
# the directory associated with that node.
content = await self._load_directory(node).wait()
except WorkerCancelled:
# The worker was cancelled, that would suggest we're all
# done here and we should get out of the loader in general.
break
except WorkerFailed:
# This particular worker failed to start. We don't know the
# reason so let's no-op that (for now anyway).
pass
else:
# We're still here and we have directory content, get it into
# the tree.
if content:
self._populate_node(node, content)
# Mark this iteration as done.
self._load_queue.task_done()

def _on_tree_node_expanded(self, event: Tree.NodeExpanded) -> None:
event.stop()
dir_entry = event.node.data
if dir_entry is None:
return
if dir_entry.path.is_dir():
if self._safe_is_dir(dir_entry.path):
if not dir_entry.loaded:
self._load_directory(event.node)
self._add_to_load_queue(event.node)
else:
self.post_message(self.FileSelected(self, event.node, dir_entry.path))

Expand All @@ -268,5 +365,5 @@ def _on_tree_node_selected(self, event: Tree.NodeSelected) -> None:
dir_entry = event.node.data
if dir_entry is None:
return
if not dir_entry.path.is_dir():
if not self._safe_is_dir(dir_entry.path):
self.post_message(self.FileSelected(self, event.node, dir_entry.path))