rivo / tview

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

Feature request: add title or text Spinner (example included) #225

Open dguendisch opened 5 years ago

dguendisch commented 5 years ago

Would be cool to have a title or text Spinner for e.g. a Box indicating that a (potentially long) operation is currently being executed. Maybe a more basic primitive would be even more suitable (e.g. to have Spinner in a status bar, etc).

2019-01-16_22-54-58

I currently get by myself with this simple function:

func main() {
    app := tview.NewApplication()
    box := tview.NewBox().SetBorder(true).SetTitle("Hello")

    spinTitle(app, box, "Hello", func() {
        //something long running
        time.Sleep(3 * time.Second)
    })

    if err := app.SetRoot(box, true).Run(); err != nil {
        log.Panic("Couldn't construct tview application")
    }
}

// spinTitle will execute action and prepend the box's title with a spinning character until action terminates
func spinTitle(app *tview.Application, box *tview.Box, title string, action func()) {
    done := make(chan bool)

    //action
    go func() {
        action()
    done <- true
        close(done)
    }()

// spinner
    go func() {
        spinners := []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"}
    var i int
    for {
        select {
        case _ = <-done:
            app.QueueUpdateDraw(func() {
            box.SetTitle(title)
        })
        return
        case <-time.After(100 * time.Millisecond):
        spin := i % len(spinners)
        app.QueueUpdateDraw(func() {
                box.SetTitle(spinners[spin] + title)
        })
        i++
            }
        }
    }()
}
Sacules commented 5 years ago

I'd be very interested in something like this :)

rivo commented 5 years ago

There was a similar request previously for a progress bar, see #83, but it sounds like this is not what you are looking for.

I agree this would be useful. I'm just not yet sure what this should look like. I don't think it should become part of the title as in your example. Maybe the bottom of a box frame could be used to display the status?

Any suggestions?

dguendisch commented 5 years ago

Yeah, it's just a busy indicator (e.g. doing a network request and waiting for the result), so a progress bar would be hard to "progress" and overkill in terms of screen size :) I saw that title spinner here and liked it pretty much, but I wouldn't mind where it is placed, but should be specific to some box (to indicate that this part of the screen is working hard / awaiting updates)

Your idea about the bottom box frame points me to another question (mostly about efficiently using the screen space): is it possible to customize that bottom part? E.g. I would love to be able to place some short status information there to avoid adding an extra line for this.

Sacules commented 5 years ago

I am using a flex with a TextField at the bottom as a status bar of sorts, and works fine by sending messages from other goroutines. The only gotcha is that it calls to Application.Draw directly instead of wrapping the messages with QueueUpdateDraw, since doing so blocks the screen when other queue events are present and draws everything at once, which can sometimes delete messages or make them appear later than they should have.

diamondburned commented 5 years ago

I've tried doing this, and it's a lot harder than it seems, going with the "proper implementation" of using a Primitive. The issue is, you can't manually call Draw() as the Primitive, which means you can't periodically redraw the Primitive with the proper loading animation. Maybe a Start(screen tcell.Screen) method would do, but I decided to stop.

mpictor commented 4 years ago

I believe when I've seen text spinners in the past, they've been at the end of a line of text.

That's how I implemented it on an LCD (not that you have much choice on a 2x20 char display...)

If curious, that code is here.

ghostsquad commented 4 years ago

I'm interested in the finer details of the concurrency issues that are involved here. I'm new to using this package and I'm starting down a similar path. Here's the relevant code bits I'm playing with (just as a test):

import (
   "time"
   "github.com/tj/go-spin"
)

func NewSpinner(spinFrames string, intervalMilliseconds int64) chan string {
    spinner := spin.New()
    spinner.Set(spinFrames)

    outChan := make(chan string)
    go func() {
        for {
            select {
            case <-outChan:
                return
            default:
                time.Sleep(time.Duration(intervalMilliseconds) * time.Millisecond)
                outChan<-spinner.Next()
            }
        }
    }()

    return outChan
}
m := tview.NewTextView()
spinnerChan := NewSpinner(spin.Box1, 200)
go func() {
    for {
        app.QueueUpdateDraw(func() {
            m.SetText(<-spinnerChan)
        })
    }
}()

@Sacules can you describe more about the problem you mentioned?

The only gotcha is that it calls to Application.Draw directly instead of wrapping the messages with QueueUpdateDraw, since doing so blocks the screen when other queue events are present and draws everything at once, which can sometimes delete messages or make them appear later than they should have.

@diamondburned can you go into more detail here?

The issue is, you can't manually call Draw() as the Primitive, which means you can't periodically redraw the Primitive with the proper loading animation. Maybe a Start(screen tcell.Screen) method would do, but I decided to stop.

diamondburned commented 4 years ago

The issue is precisely this:

m := tview.NewTextView()
spinnerChan := NewSpinner(spin.Box1, 200)
go func() {
    for {
        app.QueueUpdateDraw(func() {
            m.SetText(<-spinnerChan)
        })
    }
}()

In this code, the caller must explicitly set up the control flow of the spinner, which in your case, is a background loop to draw (which isn't implemented correctly.

A good hack around this would be to have Spinner take its own reference on draw:

type Spinner struct {
    tview.TextView // or anything
    loopOnce sync.Once
    stop     chan struct{}
}

func (s *Spinner) Draw(screen tcell.Screen) {   
    s.loopOnce.Do(func() {
        s.stop = make(chan struct{})

        go func() {
            ticker := time.NewTicker(time.Second) // any duration
            defer ticker.Stop()

            for {
                select {
                case <-s.stop:
                    return
                case <-ticker.C:
                    // Call self with screen.
                    s.Draw(screen)
                }
            }
        }()
    })

    // Draw the spinner
}
ghostsquad commented 4 years ago

Interesting. I'm not sure how a background loop to cause the app to redraw is a problem.

In your snippet, what happens when the terminal is resized? or some other event changes the layout?

diamondburned commented 4 years ago

I'm not sure how a background loop to cause the app to redraw is a problem.

It's a problem, because unlike other widgets which you add to a container and not have to worry about when it will be drawn, you have to explicitly state when this widget should be drawn.

In your snippet, what happens when the terminal is resized? or some other event changes the layout?

I don't see an issue here. The library already updates the widget layouts accordingly using other callbacks that are in tview.Primitive, and Draw only reads them.

ghostsquad commented 4 years ago

So what if you set the text, and use the OnChanged of textview to redraw?

ghostsquad commented 4 years ago

Oh because setText isn't thread safe

diamondburned commented 4 years ago

Nothing is thread-safe except for the Draw call, really, so my code wouldn't work either. There should be a way to trigger a draw in a thread-safe way otherwise.

Another option would probably be to make tview redraw even when it doesn't have to (or it doesn't know that it has to, since there's no explicit call), but this sounds wasteful.

ghostsquad commented 4 years ago

Ya, ok, so I think I understand now. Basically, the problem is that by forcing the application to redraw in a goroutine, and by not having much in the way of thread safety, it impedes the ability for other parts of the application to "setup" some set of changes that they want to display, without drawing until it's done it's operation. A redraw part way through that "setup" would definitely cause some weird behavior to occur.

So, I was thinking more on this, and had a thought experiment where, what if you didn't need to tell the whole application to redraw (except when the terminal was resized, or on other specific events). You only needed to tell specific sub-components to redraw. This is similar to like the virtual-dom used by React in browsers.

diamondburned commented 4 years ago

That's what my snippet above does. Except it's harder in a terminal environment. Technically, the code above has a possible race condition, for example when the primitive is asked to set a new size while it's being drawn inside that loop.

It's hard to make a comparison to browser DOM APIs, as they already abstracted all that for you. React's virtual DOM is to get the difference between DOM nodes; it doesn't differentiate pixels to draw on the screen. The browser does that for you.