rivo / tview

Terminal UI library with rich, interactive widgets — written in Golang
MIT License
10.33k stars 545 forks source link

What is the correct way to programmatically change CurrentNode in TreeView #1000

Closed jnathanh closed 1 week ago

jnathanh commented 1 week ago

I need to programmatically change the current node of a TreeView and am not able to find a recommended way of doing it. I got it to work by directly calling app.Draw(), but saw in the function comments that it can deadlock the application, so want to learn if there is a better way to get the same result.

My use-case is that I'm building a side-by-side differ with a TreeView on the left and right, and I want to set an addition line on the right tree as current node when the associated removal line on the left is selected as current node.

The example below shows the type of behavior I want with a single tree for simplicity.

package main

import (
    "time"

    "github.com/rivo/tview"
)

func main() {

    var children []*tview.TreeNode
    for _, s := range []string{"1", "2", "3", "4", "5", "6", "7", "8", "9", "10"} {
        children = append(children, tview.NewTreeNode(s))
    }

    root := tview.NewTreeNode("Numbers").
        SetChildren(children)

    tree := tview.NewTreeView().
        SetRoot(root).
        SetCurrentNode(root)

    app := tview.NewApplication()

    go func() {
        for _, c := range children {
            time.Sleep(time.Second / 2)
            tree.SetCurrentNode(c)
            // Draw is reported to be prone to deadlocks, is there a better way of triggering a redraw?
            app.Draw()
        }
    }()

    if err := app.SetRoot(tree, true).Run(); err != nil {
        panic(err)
    }
}
Macmod commented 1 week ago

According to https://github.com/rivo/tview/wiki/Concurrency, calling app.Draw() is always safe inside a goroutine. I think your implementation is fine?

jnathanh commented 1 week ago

Thanks @Macmod. I missed that link in the function comment.

I did a little more reading and sharing here in case its useful to others. I think the advice can be summed up as follows.

In a callback (main thread), use ForceDraw()

primitive.SetSomeCallback(func(){
    app.ForceDraw()
})

In a goroutine, always use Draw()

go func(){
    app.Draw()
}()

In the Concurrency section of the wiki it says:

Note that calling Application.Draw() is always safe.

Based on the Application.Draw() comment and docs.go, I think this is implying

...always safe [in a goroutine]

The deadlock warning in the function comment says:

It can actually deadlock your application if you call it from the main thread (e.g. in a callback function of a widget)

I think this is because Draw calls QueueUpdate, which is writing to a channel that is only emptied in the main thread. If the channel buffer is full, then QueueUpdate will block the main thread until the buffer is emptied, which never happens because the channel write blocks the next invocation of the channel read (and emptying of the buffer).

QueueUpdate

a.updates <- queuedUpdate{f: f, done: ch}

Run

case update := <-a.updates:

I also found the ForceDraw() function, which does not use QueueUpdate. Even though it says

It is always preferable to call Application.Draw instead.

again, I think it is implied to only mean "in a goroutine", because in the main thread ForceDraw would avoid deadlocks compared to Draw which could cause them.

Do these conclusions look correct?

I will go ahead and close the issue because I think my question was answered.