Fix false positives and false negatives for when to populate file name

When opening the Open dialog, it shouldn't populate the file name input,
but when clicking a file, it should fill in the clicked file's name.

The `_expanding_directory_tree` flag gave false positives:
- When opening a file in a directory the program doesn't have permissions to via the CLI,
  such as with `python -m src.textual_paint.paint /root/nonexistent`,
  opening the Open dialog, and clicking a file, it wasn't populating the filename input,
  because the flag was never cleared if it couldn't find (access) the
  directories and file matching any of the path parts, working down the directory structure.
  
  This was broken in [b3ca55a3b1] "Use new callback to remove race condition"
  but before, it just cleared the flag after a delay, so it wasn't a good solution,

It also gave false negatives:
- When opening the Open dialog, it was populating the name input field
  with the file selected programmatically with `expand_to_path`,
  instead of leaving it blank.
  
  The automatic expansion was broken in [e9755637d6] "Update Textual to 0.26.0"
  due to changes 0.25.0, since directory contents are now loaded in a worker.
  This flag's behavior was broken since rewriting the code to handle asynchronous directory loading in
  [4e1f11ab23] "Fix expanding directory tree to current folder, in file dialogs"

An alternative I considered was to create a new message such as `EnhancedDirectoryTree.NodeHighlighted`, but let's call it `EnhancedNodeHighlighted`, then to have `on_tree_node_highlighted` in `EnhancedDirectoryTree` which sends `EnhancedNodeHighlighted` always with `from_expand_to_path=False`, and then do:

    | # Suppress EnhancedNodeHighlighted with from_expand_to_path=False
    | with self.prevent(self.EnhancedNodeHighlighted):
    | 	self.select_node(node)
    | # Send it with the flag set
    | self.post_message(self.EnhancedNodeHighlighted(..., from_expand_to_path=True))

The solution I settled on was to add an `on_tree_node_highlighted` method to my `EnhancedDirectoryTree` for the purpose of clearing the flag, with still a further delay so that the app's `on_tree_node_highlighted` can use the flag before its cleared.

(I left `_expanding_directory_tree` in at this commit for comparison of the behavior. For a fair estimate of the complexity of this change, include the following cleanup in the diff.)
This commit is contained in:
Isaiah Odhner 2023-07-14 19:50:39 -04:00
parent 1b8b07def9
commit 54f29eabf1
2 changed files with 64 additions and 2 deletions

View File

@ -2,25 +2,77 @@ from pathlib import Path
from typing import Callable, Iterable
from rich.text import TextType
from textual.widgets import DirectoryTree
from textual.reactive import var
from textual.widgets import DirectoryTree, Tree
from textual.widgets._tree import TreeNode
from textual.widgets._directory_tree import DirEntry
class EnhancedDirectoryTree(DirectoryTree):
node_highlighted_by_expand_to_path = var(False)
"""Whether a NodeHighlighted event was triggered by expand_to_path.
(An alternative would be to create a new message type wrapping `NodeHighlighted`,
which includes a flag.)
(Also, this could be a simple attribute, but I didn't want to make an `__init__`
method for this one thing.)
"""
def filter_paths(self, paths: Iterable[Path]) -> Iterable[Path]:
return [path for path in paths if not (path.name.startswith(".") or path.name.endswith("~") or path.name.startswith("~"))]
def _go_to_node(self, node: TreeNode[DirEntry], callback: Callable[[], None]) -> None:
"""Scroll to the node, and select it."""
def _go_to_node_now():
print("set flag")
self.node_highlighted_by_expand_to_path = True
self.select_node(node)
# def clear_flag() -> None:
# print("clear flag")
# self.node_highlighted_by_expand_to_path = False
# self.call_later(clear_flag) # too early!
# self.set_timer(0.01, clear_flag) # unreliable
# self.call_after_refresh(clear_flag) # unreliable
# The above is unreliable because NodeHighlighted is
# sent by watch_cursor_line, so it may go:
# * set flag
# * (add clear_flag callback to queue)
# * watch_cursor_line
# * (adds NodeHighlighted to queue)
# * clear flag
# * on_tree_node_highlighted
#
# So instead, listen for NodeHighlighted,
# and then clear the flag.
region = self._get_label_region(node._line) # type: ignore
assert region, "Node not found in tree"
self.scroll_to_region(region, animate=False, top=True)
callback()
# Is there a way to avoid this delay?
self.set_timer(0.01, _go_to_node_now)
def on_tree_node_highlighted(self, event: Tree.NodeHighlighted[DirEntry]) -> None:
"""Called when a node is highlighted in the DirectoryTree.
This handler is used to clear the flag set by expand_to_path.
See _go_to_node for more details.
"""
print("EnhancedDirectoryTree.on_tree_node_highlighted (queue clearing flag)")
def clear_flag() -> None:
print("clear flag")
self.node_highlighted_by_expand_to_path = False
# Now that we've received the NodeHighlighted event,
# we just need to wait for subclasses/parent widgets to handle it,
# so this can be tighter the earliest-calling method I tried above.
# self.call_next(clear_flag) # too early!
# Even though it's the same event, bubbling, the `call_next`
# callback actually comes before the event finishes bubbling.
# self.call_later(clear_flag) # too early!
self.call_after_refresh(clear_flag) # finally reliable # type: ignore
def _expand_matching_child(self, node: TreeNode[DirEntry], remaining_parts: tuple[str], callback: Callable[[], None]) -> None:
"""Hooks into DirectoryTree's add method, and expands the child node matching the next path part, recursively.

View File

@ -114,7 +114,17 @@ class FileDialogWindow(DialogWindow):
assert event.node.parent.data
self._directory_tree_selected_path = str(event.node.parent.data.path)
name = os.path.basename(event.node.data.path)
if not self._expanding_directory_tree:
assert isinstance(event.control, EnhancedDirectoryTree)
print(
"self._expanding_directory_tree",
self._expanding_directory_tree,
"event.control.node_highlighted_by_expand_to_path",
event.control.node_highlighted_by_expand_to_path
)
# if not self._expanding_directory_tree:
if not event.control.node_highlighted_by_expand_to_path:
# TODO: handle NoMatches if dialog is opened and closed immediately
# such as by spamming Ctrl+O
self.query_one("FileDialogWindow .filename_input", Input).value = name
else:
self._directory_tree_selected_path = None