Skip to content

Draw no ArrowTree connector or bar towards empty nested subtrees - #104

Merged
pazz merged 1 commit into
mainfrom
fix-arrow-trailing-empty-subtree
Oct 25, 2026
Merged

pazz merged 1 commit into
mainfrom
fix-arrow-trailing-empty-subtree

Conversation

@pazz

@pazz pazz commented Oct 25, 2026

Copy link
Copy Markdown
Owner

In a NestedTree, an empty subtree takes no line (#77). ArrowTree decorates with its own tree's positions, though, and asked that tree for the next sibling, so a node followed only by an empty subtree was drawn with a T-shaped connector, and its descendants with a vertical bar, leading to nothing:

before      after
R           R
├─➤a        └─➤a
│  └─➤a1       └─➤a1

ArrowTree now asks _has_next_sibling, which skips siblings that are trees holding nothing but empty trees, at all four places that decide between ├ and └ or draw a vertical bar (first lines, spacers and continuation lines). Trees without nested empty subtrees are unaffected.

The emptiness check is a loop with an explicit stack, so deeply nested empty trees do not hit the recursion limit.

Tests: test_arrows_skip_empty_subtrees covers the connector, the bar on a grandchild's line and a continuation line, with an empty subtree and a subtree holding only an empty subtree as trailing siblings. It fails without the fix. test_arrows_skip_deeply_nested_empty_subtree decorates a node followed by an empty tree wrapped 1500 levels deep (decoration only, since NestedTree navigation still recurses once per nesting level, a known limit).

Not covered: CollapsibleArrowTree and CollapsibleIndentedTree still draw a collapse icon on a node whose only children are empty subtrees, since collapsible asks the wrapped tree whether the node is a leaf.

Review findings and resolution

review-correctness

  • Recursion per nesting level in the emptiness check: fixed, it is now iterative, with a 1500-level regression test.
  • Quadratic work when a node with many lines below it is followed by a long run of empty subtrees: left, with a ponytail: comment. Each check walks the run of empties, so the cost is lines times empties, but only in that shape; NestedTree sibling moves already walk such runs, and the walker's line cache keeps a line from being decorated again until a refresh. A cache in ArrowTree would go stale when the tree changes and has no hook to clear it.
  • Reuse NestedTree's navigation instead: left, ArrowTree is wrapped by the NestedTree and cannot see it.
  • Other decorators were not checked: IndentedTree draws nothing per sibling; the collapse icon case is listed under "Not covered".

review-design

  • ArrowTree over a decorated tree is unverified: checked, ArrowTree(CollapsibleTree(...)) inside a NestedTree renders └─➤a, since DecoratedTree.__getitem__ returns the wrapped entry. No test added, as decorators do not stack (Document that decorators do not stack and CollapseMixin needs a wrapper #98).
  • Point _is_empty_tree at NestedTree._sanitize_position: done in its docstring.
  • A private helper in decoration.py that knows about nested trees: kept, as the reviewer recommends over a public predicate on Tree.

🤖 Generated with Claude Code

In a NestedTree an empty subtree takes no line, but ArrowTree asked its
own tree for the next sibling and so drew a T-shaped connector and a
vertical bar towards it when it was the last entry. ArrowTree now skips
siblings that are trees holding nothing but empty trees.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@pazz
pazz merged commit 092f2c3 into main Oct 25, 2026
4 checks passed
@pazz
pazz deleted the fix-arrow-trailing-empty-subtree branch October 25, 2026 04:16
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