Skip to content

Shorten Tree's depth-first steps and SimpleTree's lookup helpers - #108

Merged
pazz merged 1 commit into
mainfrom
tree-shrink
Oct 25, 2026
Merged

pazz merged 1 commit into
mainfrom
tree-shrink

Conversation

@pazz

@pazz pazz commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Shortens urwidtrees/tree.py without changing behaviour; found by an audit for over-engineering.

  • Tree.next_position and prev_position return early instead of nesting, and prev_position returns parent_position(pos) directly instead of copying it through a None check.
  • SimpleTree's private helpers _get_substructure and _get_node read self._treelist instead of taking it as an argument that was always self._treelist, and _confirm_pos is one line.

Navigation reads the tree's data through _get_node, never through __getitem__, so a subclass that overrides __getitem__ (to wrap or hide widgets) navigates as before. test_getitem_override covers that; it fails if _confirm_pos goes through self[pos].

Review findings and resolution

review-correctness

  • _confirm_pos went through self[pos], so a SimpleTree subclass overriding __getitem__ changed navigation (raising or hiding positions): fixed, with a regression test.
  • The private _get_node/_get_substructure signatures change: left; they are private and unused elsewhere.
  • Checked without findings: the full suite on urwid 3.0.4 and 2.1.2; identical forward and reverse walks on a 1500-level chain and on 10000 children, with unchanged timing.

review-design

  • The same __getitem__ coupling, with a subclass that never stopped walking: fixed, with the test asked for.
  • parent_position and last_child_position keep the older style: left, to keep this change to what it states.

🤖 Generated with Claude Code

next_position and prev_position return early instead of nesting, and
prev_position returns the parent position directly. SimpleTree's helpers
read self._treelist instead of taking it as an argument. Navigation
still reads the data through _get_node, never through a subclass's
__getitem__; a test now covers that.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@pazz
pazz merged commit 68d8b5f into main Oct 25, 2026
4 checks passed
@pazz
pazz deleted the tree-shrink branch October 25, 2026 11:31
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