feat: tree bubble - #893
Conversation
34a0a7b to
ba6f094
Compare
|
@andreynering I see 2.1 was released. Just checking, is this planned for 2.2? |
|
Anything else needed here @andreynering ? |
refactor test: add basic tests
Merge branch 'v2-exp' into dlvhdr/tree-bubble Merge branch 'v2-exp' into dlvhdr/tree-bubble fix: select keybind fix: nil edge cases fix: prevent extra help menu spacing and expose viewport scroll
ba6f094 to
8519a5d
Compare
| s.nodeFunc = func(_ Nodes, _ int) lipgloss.Style { | ||
| return s.NodeStyle | ||
| } | ||
|
|
There was a problem hiding this comment.
dead code due to SetStyles overriding either way? (unsure)
There was a problem hiding this comment.
this will be overwritten only when the user calls SetStyles with styles.NodeStyleFunc, which the default don't set.
|
|
||
| // Size returns the number of nodes in the tree. | ||
| // Note that if a child isn't open, its size is 1. | ||
| func (t *Node) Size() int { |
There was a problem hiding this comment.
Size() is len(AllNodes()), allocating per call. This combined with child.Size() per child in tree.go is O(n^2) per setAttributes
There was a problem hiding this comment.
how does setAttibutes get called in this flow?
This is basically recursive calls to tree.Children() to figure out how many children each node has. Since lipgloss implemented this where if a child is collapsed, the number of children is 1, I did the same.
I figured the user would already know how many total items he put in the tree but for scrolling purposes, and understanding the vertical size of the tree, I would expose it like lipgloss.
There was a problem hiding this comment.
sorry, I mixed up two things. no issue with the Size() API. my concern is only how often it's recomputed.
setAttributes runs in New, SetNodes and toggleNode, so on every expand/collapse, and Size()/AllNodes() is hit three more times per cursor move.
each node asks every child for its size and answering means walking that childs whole subtree, so nodes get recounted once per ancestor above them. cost grows with depth, not node count (which i mistakenly said). wide trees will be fine, but deep ones won't
andrinoff
left a comment
andrinoff
left a comment
There was a problem hiding this comment.
just a little more!
p.s. lint fails
| Select: key.NewBinding( | ||
| key.WithKeys("v"), | ||
| key.WithHelp("v", "select"), | ||
| ), |
There was a problem hiding this comment.
why is there a change in list?
| func (m Model) cursorView() string { | ||
| if m.cursorCharacter == "" { | ||
| return "" | ||
| } | ||
| cursor := strings.Split(strings.Repeat(" ", m.root.Size()), "") | ||
| cursor[m.yOffset] = m.cursorCharacter | ||
| return m.styles.CursorStyle.Render(lipgloss.JoinVertical(lipgloss.Left, cursor...)) | ||
| } |
There was a problem hiding this comment.
draws one cursor row per node, but the tree renders one row per line. those are the same number only if every node is exactly one line tall.
| // SetNodes sets the tree to the given root node. | ||
| func (m *Model) SetNodes(t *Node) { | ||
| m.root = t | ||
| m.root.value = t.GivenValue() |
There was a problem hiding this comment.
not nil guarded. the same thing in AllNodes (:582)
|
|
||
| // Size returns the number of nodes in the tree. | ||
| // Note that if a child isn't open, its size is 1. | ||
| func (t *Node) Size() int { |
There was a problem hiding this comment.
sorry, I mixed up two things. no issue with the Size() API. my concern is only how often it's recomputed.
setAttributes runs in New, SetNodes and toggleNode, so on every expand/collapse, and Size()/AllNodes() is hit three more times per cursor move.
each node asks every child for its size and answering means walking that childs whole subtree, so nodes get recounted once per ancestor above them. cost grows with depth, not node count (which i mistakenly said). wide trees will be fine, but deep ones won't
| m.setAttributes() | ||
| m.updateStyles() | ||
| m.updateViewport(0) | ||
| m.setRootStyles(m.styles) |
There was a problem hiding this comment.
(nit) styles are installed after the viewport content is built, so we will have 1 unstyled frame. reordering will fix this!
4 participants
CONTRIBUTING.md.