Fix regression from 2.6.1: ListBox used for tree implementation.#829
Merged
penguinolog merged 1 commit intourwid:masterfrom Feb 21, 2024
Merged
Conversation
penguinolog
commented
Feb 21, 2024
|
|
||
| widget, next_next_pos = self._body.get_next(next_pos) | ||
| if next_pos == next_next_pos: | ||
| raise ListBoxError( |
Collaborator
Author
There was a problem hiding this comment.
cause infinite cycle (technically should be unreachable)
penguinolog
commented
Feb 21, 2024
| next_pos not in rendered_positions, | ||
| ) | ||
| ): | ||
| if widget.rows((maxcol,), False): |
Collaborator
Author
There was a problem hiding this comment.
if we are here - calculate_visible is broken (main reason for this check)
Pull Request Test Coverage Report for Build 7990924110Details
💛 - Coveralls |
penguinolog
commented
Feb 21, 2024
| " 1: child_1 ", | ||
| " 2: child_2 ", | ||
| " 3: child_3 ", | ||
| " ", |
Collaborator
Author
There was a problem hiding this comment.
no next widgets, indexes are not int (main issue in regression)
penguinolog
commented
Feb 21, 2024
| def mouse_event( | ||
| self, | ||
| size, | ||
| size: tuple[int] | tuple[()], |
Collaborator
Author
There was a problem hiding this comment.
Padding is FLOW/FIXED in this case
penguinolog
commented
Feb 21, 2024
Collaborator
Author
There was a problem hiding this comment.
looks like we had 0 tests for trees
penguinolog
commented
Feb 21, 2024
| def load_child_keys(self): | ||
| def load_child_keys(self) -> Sequence[Hashable]: | ||
| """Provide ParentNode with an ordered list of child keys (virtual function)""" | ||
| raise TreeWidgetError("virtual function. Implement in subclass") |
Collaborator
Author
There was a problem hiding this comment.
IMHO, we should use NotImplementedError in all cases
penguinolog
commented
Feb 21, 2024
| widget = self.get_inner_widget() | ||
| if not self.is_leaf: | ||
| widget = urwid.Columns( | ||
| [("fixed", 1, [self.unexpanded_icon, self.expanded_icon][self.expanded]), widget], |
Collaborator
Author
There was a problem hiding this comment.
deprecated, backward compatible GIVEN width
* Fix TreeWalker support in render * Cover issue with test * Add initial test for: * * `TreeNode` * * `ParentNode` * * `TreeWalker` * * `TreeListBox` * Add basic type annotations for tree widgets Fix: urwid#828
penguinolog
force-pushed
the
fix_tree_listbox_render_empty_elements
branch
from
103ca87 to
cc6d341
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TreeNodeParentNodeTreeWalkerTreeListBoxFix: #828
Checklist
masterorpython-dual-supportbranchtoxsuccessfully in local environment