awesome-gocui / gocui

Minimalist Go package aimed at creating Console User Interfaces.
BSD 3-Clause "New" or "Revised" License
350 stars 39 forks source link

fix view write not updating view #93

Closed mjarkk closed 3 years ago

mjarkk commented 3 years ago

fixes: #92

dankox commented 3 years ago

I was looking into it a bit and at first it didn't really make sense at all. Why would g.Update cause such a problem with print into the view. But after some testing and looking into @Rudi9719 code in his repo, I think the problem is somewhere else.

I was testing the original code with simple example and it works fine, you can try:

package main

import (
    "fmt"
    "log"
    "sync"
    "time"

    "github.com/awesome-gocui/gocui"
)

var (
    done = make(chan struct{})
    wg   sync.WaitGroup
)

func main() {
    g, err := gocui.NewGui(gocui.OutputNormal, true)
    if err != nil {
        log.Panicln(err)
    }
    defer g.Close()

    g.SetManagerFunc(layout)

    if err := keybindings(g); err != nil {
        log.Panicln(err)
    }

    printView(g)
    wg.Add(1)
    go textMessage(g)

    if err := g.MainLoop(); err != nil && err != gocui.ErrQuit {
        log.Panicln(err)
    }

    wg.Wait()
}

func layout(g *gocui.Gui) error {
    if v, err := g.SetView("myView", 2, 2, 50, 12, 0); err != nil {
        if err != gocui.ErrUnknownView {
            return err
        }
        v.Autoscroll = true
        // v.Wrap = true <- this is the problem
        fmt.Fprintln(v, "starting...")
    }
    return nil
}

func keybindings(g *gocui.Gui) error {
    if err := g.SetKeybinding("", gocui.KeyCtrlC, gocui.ModNone, quit); err != nil {
        return err
    }
    return nil
}

func quit(g *gocui.Gui, v *gocui.View) error {
    close(done)
    return gocui.ErrQuit
}

func textMessage(g *gocui.Gui) {
    defer wg.Done()

    for {
        select {
        case <-done:
            return
        case <-time.After(2000 * time.Millisecond):
            printView(g)
        }
    }
}

func printView(g *gocui.Gui) {
    g.Update(func(g *gocui.Gui) error {
        updatingView, err := g.View("myView")
        if err != nil {
            return err
        }
        fmt.Fprintf(updatingView, "%s\n", "test message: "+time.Now().String())
        return nil
    })
}

If you run the above code, the lines are print from the beginning in 2 seconds delays in between. Kinda simulate the problem.

However, I looked into his repo to see what else is there and I noticed that they use on all views Autoscroll and Wrap. So I tried with Autoscrol all good.
But Wrap is kinda different. If you set wrap, it will add one more line to the output and if the output should be scrolled, it might not (because of how the line count was counted).

In the sample above, just uncomment the Wrap line in layout function and you can see the difference where the lines are printed in 2 seconds interval until the end of view is hit and then a longer delay appears until the next line is printed (even though you can see it has the correct timestamp on it).
The reason is probably because the output is out of the view and is pushed in to the view after more lines are printed.

@Rudi9719 can you confirm the problem might be as described? With delay at the end of View container? Or does the problem appears also in the middle of the view, when it shouldn't scroll?

Rudi9719 commented 3 years ago

If I'm understanding you correctly, you are correct. It only happens at the bottom of the views and not in the middle.

mjarkk commented 3 years ago

@dankox I could recreate the bug described in #92 by modifying the hello.go example to include a 1 second timeout and then write to the view (in a new go routine), i would say that's a problem on our side

dankox commented 3 years ago

@mjarkk could you please show me the updated hello.go ? I'm kinda curious, because I just can't imagine how. On my tests it just works (unless wrap).

mjarkk commented 3 years ago
diff --git a/_examples/hello.go b/_examples/hello.go
index d485196..c9b8bc9 100644
--- a/_examples/hello.go
+++ b/_examples/hello.go
@@ -8,6 +8,7 @@ import (
        "errors"
        "fmt"
        "log"
+       "time"

        "github.com/awesome-gocui/gocui"
 )
@@ -41,7 +42,10 @@ func layout(g *gocui.Gui) error {
                        return err
                }

-               fmt.Fprintln(v, "Hello world!")
+               go func(v *gocui.View) {
+                       time.Sleep(time.Second)
+                       fmt.Fprintln(v, "Hello world!")
+               }(v)
        }

        return nil
dankox commented 3 years ago

@mjarkk yeah... that's a problem... I actually tried this one and I can see why the fix seems to work in this situation.

You can't really update or write into the View without using gocui.Update function if you are outside of the main loop (I think it's even mentioned in the documentation). If you do, it won't get updated until another "user event" will come. That's the reason why it works if you pass "user event" in the Write function, because the first update is in there already and the new event just triggers re-draw. Without it, it just waits for event to happen on line: https://github.com/awesome-gocui/gocui/blob/33004d6b9b372eea7f3edff8895d8566a6f5aaf4/gui.go#L489

You need to do the update in a way like this:

go func(g *gocui.Gui) {
        time.Sleep(time.Second)
    g.Update(func(g *gocui.Gui) error {
        fmt.Fprintln(v, "Hello world!")
        return nil
    })
}(v)

I hope it makes sense what I mean.

mjarkk commented 3 years ago

Good point.
I could imagine situations where you want to update a view outside of the main loop so maybe we should have a view force update function instead of this?

dankox commented 3 years ago

Well, there is gocui.Update and gocui.UpdateAsync for that specific reason which can update views as they push to the userEvents channel which is read inside of the main loop.

However, I think the issue is about the problem with Autoscroll not scrolling correctly to the end of the buffer when there are wrapped lines in the view.

mjarkk commented 3 years ago

Ah i see, dumb i looked over the update function. In that case i think we can close this PR.

However, I think the issue is about the problem with Autoscroll not scrolling correctly to the end of the buffer when there are wrapped lines in the view.

Welp then i have to look into that :)

Rudi9719 commented 3 years ago

This was closed but I don't think I saw the solution; should I be using a sleep to fix this? Or an UpdateAsync instead of the current Update? Should Fprintf be used in place of EditWrite? If I remember correctly, the EditWrite is used to ensure that text wraps properly inside of the view.

dankox commented 3 years ago

@Rudi9719 It was closed because it does not fix the issue. I think @mjarkk is looking into it. I guess the problem is as I described above, with the buffer lines and how the wrapping works in the new version. It doesn't count the lines correctly when they are wrap, so it won't autoscroll when needed.

What could help you is to turn off wrapping, but in such case I guess you won't see all the data in your views.

Another thing could be to try this patch in your fork of awesome-gocui:

diff --git a/view.go b/view.go
index 258d6c9..1f448a8 100644
--- a/view.go
+++ b/view.go
@@ -570,7 +570,7 @@ func (v *View) draw() error {
        linesToRender := v.viewLines()

        if v.Autoscroll && len(linesToRender) > maxY {
-               v.oy = len(v.lines) - maxY
+               v.oy = len(linesToRender) - maxY
        }

        newCache := []cellCache{}

I didn't fully tested it though. I have a hunch that this is the problem currently, but I wanted to look into it more when I will have time.

Rudi9719 commented 3 years ago

Sounds good, thank you! I'll keep watching the issue then (I didn't even realize the notification was in a pull request- my bad!)