Skip to content

Compute SimpleTree and TreeListWalker roots on access - #102

Merged
pazz merged 1 commit into
mainfrom
fix-stale-roots
Oct 25, 2026
Merged

pazz merged 1 commit into
mainfrom
fix-stale-roots

Conversation

@pazz

@pazz pazz commented Oct 25, 2026

Copy link
Copy Markdown
Owner

SimpleTree computed its root once, in the constructor. A tree built from an empty list therefore stayed empty after nodes were appended to that list: root stayed None and positions() yielded nothing. TreeListWalker also kept its own copy of the tree's root, and fixed its initial focus to the root at construction, so a TreeBox opened on an empty tree showed nothing even after the tree was filled and refreshed.

  • SimpleTree.root is now a property: (0,) while the list has nodes, None while it is empty. Assigning root still works and overrides this (assigning None restores the default), as DecoratedTree.root (Read DecoratedTree.root from the wrapped tree #76) forwards assignments to the wrapped tree and its test assigns a SimpleTree's root.
  • TreeListWalker.root reads the tree's root, and assigning it writes through to the tree, as DecoratedTree.root does since Read DecoratedTree.root from the wrapped tree #76. Before, assigning it changed only the walker's unused copy.
  • A walker created without a focus follows the tree's root until a focus is set.

Tests: test_root_follows_list (append to and clear the list) and test_follows_root_of_tree_filled_later (a TreeBox on an empty tree, filled and refreshed). Both fail without the fix.

Review findings and resolution

review-correctness

  • TreeListWalker.root became read-only: fixed, it now has a setter that writes through to the tree.
  • SimpleTree.root = None cannot force an empty root: left, as an empty root on a non-empty list has no use; the docstring now says that assigning None restores the default.
  • A walker without a focus follows a root that changes later: left, that is the intent of the change.
  • The commit message does not say that a TreeBox must be refreshed: left, refresh() is the existing contract for any change to the tree.

review-design

  • Use self.root in get_focus: fixed.
  • State the read-only walker root and the None assignment in the description: done above (the walker root is no longer read-only).
  • Nothing blocking.

🤖 Generated with Claude Code

SimpleTree copied its root at construction, so a tree built from an empty
list stayed empty after nodes were appended to that list, and
TreeListWalker kept its own copy of the tree's root and fixed its initial
focus to it. SimpleTree.root is now computed from the list unless a root
is assigned, TreeListWalker.root reads and writes the tree's, as
DecoratedTree.root does, and a walker without a focus follows the root.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@pazz
pazz merged commit 708e878 into main Oct 25, 2026
4 checks passed
@pazz
pazz deleted the fix-stale-roots branch October 25, 2026 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant