Closed rkfg closed 2 years ago
Thanks for the good work ! I did not review all yet, but I encountered a panic when I changed Menu to Routing. (I did not reproduce on Master)
lntop -c lntop.toml
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0xfd3d50]
goroutine 8 [running]:
github.com/awesome-gocui/gocui.(*View).SetOrigin(...)
/home/edouard/go/pkg/mod/github.com/awesome-gocui/gocui@v1.1.0/view.go:286
github.com/edouardparis/lntop/ui/views.(*Routing).SetOrigin(0xc0003f4070, 0x0, 0x0, 0x1, 0x7)
/home/edouard/code/edouardparis/lntop/ui/views/routing.go:121 +0x90
github.com/edouardparis/lntop/ui/views.(*Routing).Set(0xc0003f4070, 0xc00024a4b0, 0xb, 0x6, 0x4d, 0x53, 0xc0002ac360, 0xc000880060)
/home/edouard/code/edouardparis/lntop/ui/views/routing.go:219 +0x77e
github.com/edouardparis/lntop/ui.(*controller).OnEnter(0xc000886020, 0xc00024a4b0, 0xc000a05080, 0x0, 0x0)
/home/edouard/code/edouardparis/lntop/ui/controller.go:292 +0x2d8
github.com/awesome-gocui/gocui.(*Gui).execKeybinding(0xc00024a4b0, 0xc000a05080, 0xc000913560, 0x2, 0x3, 0x3)
/home/edouard/go/pkg/mod/github.com/awesome-gocui/gocui@v1.1.0/gui.go:947 +0x65
github.com/awesome-gocui/gocui.(*Gui).execKeybindings(0xc00024a4b0, 0xc000a05080, 0xc00002ddc0, 0xc00002dd82, 0xc0002a94b8, 0xc00002dd82)
/home/edouard/go/pkg/mod/github.com/awesome-gocui/gocui@v1.1.0/gui.go:935 +0x1dd
github.com/awesome-gocui/gocui.(*Gui).onKey(0xc00024a4b0, 0xc00002ddc0, 0xc0005307f0, 0xc00066a500)
/home/edouard/go/pkg/mod/github.com/awesome-gocui/gocui@v1.1.0/gui.go:882 +0x170
github.com/awesome-gocui/gocui.(*Gui).handleEvent(0xc00024a4b0, 0xc00002ddc0, 0x0, 0x0)
/home/edouard/go/pkg/mod/github.com/awesome-gocui/gocui@v1.1.0/gui.go:554 +0x46
github.com/awesome-gocui/gocui.(*Gui).MainLoop(0xc00024a4b0, 0x13e7520, 0xc000886020)
/home/edouard/go/pkg/mod/github.com/awesome-gocui/gocui@v1.1.0/gui.go:504 +0x24b
github.com/edouardparis/lntop/ui.Run(0x151a2b0, 0xc0000460d8, 0xc0003f18e0, 0xc0000450e0, 0x0, 0x0)
/home/edouard/code/edouardparis/lntop/ui/ui.go:36 +0x20d
github.com/edouardparis/lntop/cli.run.func1(0x151a2b0, 0xc0000460d8, 0xc0003f18e0, 0xc0000450e0, 0xc00040a870)
/home/edouard/code/edouardparis/lntop/cli/cli.go:66 +0x67
created by github.com/edouardparis/lntop/cli.run
/home/edouard/code/edouardparis/lntop/cli/cli.go:65 +0x15a
Right, it happens if the routing view is empty. My node gets routing attempts every few seconds so I think it was never the case but I can reproduce if I switch immediately after start. I think the same would happen with the channels view if it's empty. The new column views need to be initialized, it's also a good opportunity to move some properties initialization there instead of doing it on every render.
Should work now!
Thanks it is working better, but there is still a minor problem:
Menu > ROUTING > TRANSACTION closes lntop with an error: gocui.ErrUnknownView
My guess: every occurrence of if err != gocui.ErrUnknownView
must be changed for if !errors.Is(err, gocui.ErrUnknownView) {
Hmm, that's odd. I don't quite understand what "Menu > ROUTING > TRANSACTION" means, ROUTING
and TRANSAC
are different views. Do you open routing first and then go to transactions so it crashes? Does it print a stack trace when it closes? Kinda weird because I've never seen anything like this before.
Sorry, I meant: I click on m
to open Menu, then I move cursor on ROUTING
, press enter
without quitting the menu then I move cursor on TRANSACTIONS
, press enter
and it crashes without a Backtrace. In the log I read
2022-10-11T08:32:26.732+0200 DEBUG cli/cli.go:68 ui {"error": "unknown view"}
2022-10-11T08:32:26.732+0200 DEBUG pubsub/pubsub.go:181 Received signal, gracefully stopping {"logger": "pubsub"}
2022-10-11T08:32:26.732+0200 DEBUG lnd/lnd.go:165 stopping subscribe channels: context canceled {"network": "lnd", "name": "lnd"}
2022-10-11T08:32:26.732+0200 DEBUG lnd/lnd.go:246 stopping subscribe routing events: context canceled {"network": "lnd", "name": "lnd"}
2022-10-11T08:32:26.732+0200 DEBUG lnd/lnd.go:101 stopping subscribe invoice: context canceled {"network": "lnd", "name": "lnd"}
2022-10-11T08:32:26.732+0200 DEBUG lnd/lnd.go:209 stopping subscribe graph: context canceled {"network": "lnd", "name": "lnd"}
2022-10-11T08:32:26.732+0200 DEBUG lnd/lnd.go:133 stopping subscribe transactions: context canceled {"network": "lnd", "name": "lnd"}
I can't reproduce it. Does it happen with your mainnet node or some Polar setup? I created an empty network in Polar with just one bitcoind and one lnd, connected to it and I can switch between the views normally. They're all empty of course, but the program doesn't exit. Maybe your working tree isn't clean or something else is wrong locally? Incomplete checkout or such.
I am using a simple Polar setup with the three different nodes. I found a fix, I believe the problem does not relate to this PR so I am happy to merge it !
Thanks for the great work @rkfg! Just a quick check-in - is hovering over rows supposed to look like this? It shows the text in a greyish color.
Yeah, it is like this for me too. Should it be black? I think it's not hard to fix, will take a look later. And hey, thanks for the tip!
Imo yes, it was black before and the contrast is better I think.
Yeah, agree. For some reason it looks like this in the new library unless gocui.AttrDim
is also specified. #97 should fix this.
This is quite a big patch, it fixes #89. Updating to the supported and more recent gocui fork (awesome-gocui) allows to print emojis and wide characters better and also offers true color terminal support (that we don't use but it's still nice to have). Some highlights and explanations:
.Clear()
is replaced with.Rewind()
;awesome-gocui
resets the cursor position on.Clear()
unlike the old library and it causes the cursor to lock up. We can store and restore these coords but it's easier to use rewind instead. The only downside is that it doesn't actually clear the buffer but we don't even need it because all columns refresh the whole row anyway withPrintf
.Printf
if wide characters are in play. If we compensate them for the rows (channels) that have them it all looks good, but if we scroll to the right and the alias column becomes invisible all rows lose proper alignment because there are no more wide characters on screen but we still artificially remove spaces for them. Individual view buffers solve this easily and the underlying library do these calculations for us depending on what is actually on screen..SetView
now has a mandatory argumentoverlaps
which is used for drawing borders correctly. We don't use borders so it's just 0 everywhere.cursorCompat
function is used to restore the old library behavior regarding cursor positioning and updating origin if it goes outside the visible space;awesome-gocui
doesn't return an error in this case so I do it myself. Maybe it's better to rewrite this code (incursor.go
) to do it better but for now I decided to use this adapter instead.ToggleView
typo is corrected, it didn't matter before the library update but now that we don't clear the views with cursor it caused visual bugs.Overall, despite the number of changes the UI looks and feels exactly the same except it now works properly with wide characters, and as we know node operators love them so it's important!